Validate fields based on their mappings and the dynamic templates set #2285

mrodm · 2024-12-17T18:26:20Z

Relates #2207

Add support to validate mappings after ingesting documents based on dynamic templates.

It also updates how mappings not found in the preview are compared/validated with ECS fields.

Assumptions:

Filtered out dynamic templates created by import_mappings
Filtered out dynamic templates created by ecs@mappings component template
Filtered out the field definitions added via appendECSMappingMultifields function.
All the mappings present in the actual mappings (after ingesting documents) must fulfill one of these conditions to be valid:
- All the fields from the actual mapping must be present in the preview (not viceversa).
- All the fields from the actual mapping must be present in the dynamic template (not viceversa).
Multi-fields are just validated with the definitions in the preview (not in ECS or in dynamic templates).

Author's Checklist

Clean log/debug statements
Run tests on packages from integration repository

multi_fields are now compared directly with dynamic templates or ECS fields. It was required to know whether or not that field was a multi_field or not to apply some specific validations.

mrodm · 2024-12-19T09:34:48Z

test integrations

elastic-vault-github-plugin-prod · 2024-12-19T09:41:10Z

Created or updated PR in integrations repository to test this version. Check elastic/integrations#12164

…ing for other dynamic templates if parameters do not match

mrodm · 2024-12-19T11:24:26Z

test integrations

elastic-vault-github-plugin-prod · 2024-12-19T11:29:41Z

Created or updated PR in integrations repository to test this version. Check elastic/integrations#12164

mrodm · 2024-12-19T12:29:37Z

As a reference, this is the previous errors found without support of dynamic templates: #2214 (comment)

Looking at the errors in this build: https://buildkite.com/elastic/integrations/builds/19707

There are some packages that are failing due to missing parameters:

auditd_manager: auditd.data.a0, auditd.data.a1, auditd.data.a2 and auditd.data.a3
teleport: some examples: teleport.audit.account_id, teleport.audit.aws_host, teleport.audit.aws_region, teleport.audit.instance_id,...
docker: some examples: docker.cpu.core.0.norm.pct, docker.cpu.core.2.ticks, docker.cpu.core.3.pct, ...
- incorrect field definitions? https://github.com/elastic/integrations/blob/51460fb00b85968f3f987cf73683946f0d821d37/packages/docker/data_stream/cpu/fields/fields.yml#L81-L99

There are packages that fail due to different types comparing with ECS:

box_events: different type for threat.enrichments.indicator.first_seen and threat.enrichments.indicator.last_seen
claroty_ctd: different type for threat.indicator.modified_at
mimecast: different type for threat.indicator.first_seen and threat.indicator.modified_at
ti_anomali: different type for threat.indicator.modified_at

Packages with issues related to multi_fields:

fortinet_fortimail: missing multi_field (probably due to ecs@mappings component template)
cef: missing multi_field (probably due to ecs@mappings component template)

mrodm · 2024-12-19T16:52:04Z

internal/fields/validate.go

@@ -448,7 +450,7 @@ func appendECSMappingMultifields(schema []FieldDefinition, prefix string) []Fiel
 				{
 					Name:     "text",
 					Type:     "match_only_text",
-					External: "ecs",
+					External: externalFieldAppendedTag,


Workaround to be able to filter multifieds generated here during the validation.

Would it be a better approach for this ?

Do we have many failures if we don't do this? Maybe we can leave this for a separate change.

As talked offline, we decided to ignore the validation of these multi-fields.
Added a comment about the reasoning behind this in https://github.com/elastic/elastic-package/pull/2285/files#diff-67b718cebdeb840d99361c07166f75df079827a75df20850ef6cada861304521R318-R322

mrodm · 2024-12-19T16:53:25Z

internal/fields/mappings.go

+				var matches bool
+				if fullRegex {
+					matches, err = stringMatchesPatterns(values, name)
+				} else {
+					matches, err = stringMatchesWildcards(values, name)
+				}


Does match_pattern applies also for unmatch ?

According to this, maybe it just applies for match.
https://www.elastic.co/guide/en/elasticsearch/reference/current/dynamic-templates.html#match-unmatch

WDYT ?

Yeah, the docs seem to indicate that it only applies to match, but it would be weird that it is not possible to set this option for unmatch. Maybe match_pattern applies to both?

In any case, do we generate templates with these options?

Yeah, the docs seem to indicate that it only applies to match, but it would be weird that it is not possible to set this option for unmatch. Maybe match_pattern applies to both?

According to the tests it looks like it applies to both parameters
https://github.com/elastic/elasticsearch/blob/fe3a3cb70e3dd18b3e5ddaf6a63389b4cb16b776/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java#L2391-L2397

And also it looks like it could apply to path_match and path_unmatch, lookign at this test
https://github.com/elastic/elasticsearch/blob/75bf27c8b69b7c32fb200afd5a244e7b1f8aea32/x-pack/plugin/core/template-resources/src/main/resources/watch-history.json#L23

Currently, I think that math_pattern parameter is not used. Fleet does not generate that and currently the ECS component template does not use it neither.

mrodm · 2024-12-19T16:56:26Z

internal/fields/mappings.go

+	// TODO: Should we remove this code to load external and local fields?
 	fieldsDir := filepath.Join(fieldsParentDir, "fields")


This code to load the external and local fields maybe can be removed. That would mean that just the schema set (optionally) via options is taken into account.

WDYT?

Yes, sounds good.

internal/fields/mappings.go

mrodm · 2024-12-19T16:59:13Z

internal/fields/mappings.go

+			case "match_mapping_type", "unmatch_mapping_type":
+				// Do nothing
+				// These comparisons are done with the original data, and it's likely that the
+				// resulting mapping does not have the same type since it could change by the `mapping` field
+				// case "match_mapping_type":
+				// 	logger.Debugf("> Check match_mapping_type: %q (type %s)", currentPath, fieldType)
+				// 	values, err := parseSetting(value)
+				// 	if err != nil {
+				// 		return fmt.Errorf("failed to check match_mapping_type setting: %w", err)
+				// 	}
+				// 	logger.Debugf(">> Comparing to values: %s", values)
+				// 	if slices.Contains(values, "*") {
+				// 		continue
+				// 	}
+				// 	if !slices.Contains(values, fieldType) {
+				// 		logger.Debugf(">> Issue: not matches")
+				// 		matched = false
+				// 	}
+				// case "unmatch_mapping_type":
+				// 	logger.Debugf("> Check unmatch_mapping_type: %q (type %s)", currentPath, fieldType)
+				// 	values, err := parseSetting(value)
+				// 	if err != nil {
+				// 		return fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err)
+				// 	}
+				// 	logger.Debugf(">> Comparing to values: %s", values)
+				// 	if slices.Contains(values, fieldType) {
+				// 		logger.Debugf(">> Issue: matches")
+				// 		matched = false
+				// 	}


Looks like that these options would be better to skip them, since they would require to have the original type before the documents are stored into Elasticsearch.

If there is no objection, this whole case could be left as empty (with a comment).

Yes, I think we can ignore this, but we should still check that the mapping type in the template matches with the actual mapping.

Ok, then I'll remove the code and add a comment there about this.

Comparing and validating the type would be achieved via the validateObjectMappingAndParameters function once it is found a dynamic template that matches the given parameters.

jsoriano · 2024-12-19T18:03:20Z

internal/fields/mappings.go

+		return fmt.Errorf("missing type parameter for field: %q", currentPath)
+	}
+
+	for _, template := range dynamicTemplates {


Nit. This loop mixes parsing of the dynamic templates and its execution. Consider unmarshalling the dynamic templates first, so this loop becomes something like:

for _, template := dynamicTemplates { matches, err := template.Matches(currentPath, definition) if err != nil { return err } if matches { return nil } }

The parsing of the dynamic templates could be done once, in validateMappingsNotInPreview.

Separated that logic in two stages. First one parses the dynamic templates from map[string]any into a specific struct.

Once the dynamic templates are parsed, the required mappings are validated with those dynamic templates.

jsoriano · 2024-12-19T18:08:40Z

internal/fields/mappings.go

+				var matches bool
+				if fullRegex {
+					matches, err = stringMatchesPatterns(values, name)
+				} else {
+					matches, err = stringMatchesWildcards(values, name)
+				}


Yeah, the docs seem to indicate that it only applies to match, but it would be weird that it is not possible to set this option for unmatch. Maybe match_pattern applies to both?

In any case, do we generate templates with these options?

jsoriano · 2024-12-19T18:10:21Z

internal/fields/mappings.go

+			case "match_mapping_type", "unmatch_mapping_type":
+				// Do nothing
+				// These comparisons are done with the original data, and it's likely that the
+				// resulting mapping does not have the same type since it could change by the `mapping` field
+				// case "match_mapping_type":
+				// 	logger.Debugf("> Check match_mapping_type: %q (type %s)", currentPath, fieldType)
+				// 	values, err := parseSetting(value)
+				// 	if err != nil {
+				// 		return fmt.Errorf("failed to check match_mapping_type setting: %w", err)
+				// 	}
+				// 	logger.Debugf(">> Comparing to values: %s", values)
+				// 	if slices.Contains(values, "*") {
+				// 		continue
+				// 	}
+				// 	if !slices.Contains(values, fieldType) {
+				// 		logger.Debugf(">> Issue: not matches")
+				// 		matched = false
+				// 	}
+				// case "unmatch_mapping_type":
+				// 	logger.Debugf("> Check unmatch_mapping_type: %q (type %s)", currentPath, fieldType)
+				// 	values, err := parseSetting(value)
+				// 	if err != nil {
+				// 		return fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err)
+				// 	}
+				// 	logger.Debugf(">> Comparing to values: %s", values)
+				// 	if slices.Contains(values, fieldType) {
+				// 		logger.Debugf(">> Issue: matches")
+				// 		matched = false
+				// 	}


Yes, I think we can ignore this, but we should still check that the mapping type in the template matches with the actual mapping.

jsoriano · 2024-12-19T18:12:05Z

internal/fields/validate.go

@@ -448,7 +450,7 @@ func appendECSMappingMultifields(schema []FieldDefinition, prefix string) []Fiel
 				{
 					Name:     "text",
 					Type:     "match_only_text",
-					External: "ecs",
+					External: externalFieldAppendedTag,


Do we have many failures if we don't do this? Maybe we can leave this for a separate change.

mrodm · 2025-01-08T13:26:18Z

test integrations

elastic-vault-github-plugin-prod · 2025-01-08T13:32:14Z

Created or updated PR in integrations repository to test this version. Check elastic/integrations#12164

internal/fields/mappings.go

mrodm · 2025-01-08T18:18:00Z

internal/testrunner/runners/system/tester.go

@@ -1528,12 +1528,10 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re
 		logger.Warn("Validate mappings found (technical preview)")
 		exceptionFields := listExceptionFields(scenario.docs, fieldsValidator)

-		mappingsValidator, err := fields.CreateValidatorForMappings(r.dataStreamPath, r.esClient,


As the process to load the fields from the package and ECS has been removed, these parameters are not needed anymore.

The schema to be used to validate fields with ECS is the one set (optionally) via the method WithMappingValidatorFallbackSchema

mrodm · 2025-01-08T18:26:08Z

test integrations

elastic-vault-github-plugin-prod · 2025-01-08T18:31:53Z

Created or updated PR in integrations repository to test this version. Check elastic/integrations#12164

mrodm · 2025-01-09T12:29:25Z

test integrations

elastic-vault-github-plugin-prod · 2025-01-09T12:35:17Z

Created or updated PR in integrations repository to test this version. Check elastic/integrations#12164

mrodm · 2025-01-09T15:19:30Z

test integrations

elastic-vault-github-plugin-prod · 2025-01-09T15:26:43Z

Created or updated PR in integrations repository to test this version. Check elastic/integrations#12164

mrodm · 2025-01-09T17:02:56Z

Follow-up of the above comment #2285 (comment)

Summary of the errors found in the build: https://buildkite.com/elastic/integrations/builds/20226

There are some packages that are failing due to missing parameters/definitions:

auditd_manager: auditd.data.a0, auditd.data.a1, auditd.data.a2 and auditd.data.a3
teleport: some examples: teleport.audit.account_id, teleport.audit.aws_host, teleport.audit.aws_region, teleport.audit.instance_id,...
mongodb_atlas: missing definition for two fields mongodb_atlas.project.additional_info.hidden and mongodb_atlas.project.additional_info.is_mms_admin
- They do not match the dynamic template since they are boolean and not string.
crowdstrike: errors related to flattened type
- https://github.com/elastic/integrations/blob/80d18e9d05055786dbca2dcff1a9a8c0abb95fb3/packages/crowdstrike/data_stream/fdr/fields/fields.yml#L31
- this is the exact error: unexpected value found in mapping for field "crowdstrike.assessments.execution_blocking_custom_blocking_enabled_mac.type": preview mappings value ("flattened") different from the actual mappings value ("keyword")
- probably it does not match the dynamic template created:
```
    {
      "crowdstrike.assessments.*": {
        "path_match": "crowdstrike.assessments.*",
        "mapping": {
          "type": "flattened"
        },
        "match_mapping_type": "object"
      }
    }
```
sublime_security: field does not match the dynamic template sublime_security.email_message.headers.hops.fields.position
- https://github.com/elastic/integrations/blob/80d18e9d05055786dbca2dcff1a9a8c0abb95fb3/packages/sublime_security/data_stream/email_message/fields/fields.yml#L644-L646
- this is the exact error: unexpected value found in mapping for field "sublime_security.email_message.headers.hops.fields.position.type": preview mappings value ("keyword") different from the actual mappings value ("long")
- It does not match the dynamic template:
```
    "sublime_security.email_message.headers.hops.fields": {
        "path_match": "sublime_security.email_message.headers.hops.fields.*",
        "mapping": {
          "type": "keyword"
        },
        "match_mapping_type": "string"
      }
    }
```

There are packages that fail due to different types comparing with ECS:

box_events: different type for threat.enrichments.indicator.first_seen and threat.enrichments.indicator.last_seen
claroty_ctd: different type for threat.indicator.modified_at
mimecast: different type for threat.indicator.first_seen and threat.indicator.modified_at
ti_anomali: different type for threat.indicator.modified_at
cef: different type for url.original (keyword vs wildcard)

Packages that do not have any test failing now:

docker: mappings were fixed in Kibana since 8.12.0 [Fleet] Add support for additional types for dynamic mappings kibana#168842
fortinet_fortimail: multi-fields are ignored now

While inspecting the mappings and dynamic templates created for the docker package, I found some discrepancies in the mappings created
taking into account the definition in the package:

field definition:

    - name: core.*.pct
      type: scaled_float
      format: percent
      unit: percent
      metric_type: gauge
      description: |
        Percentage of CPU time in this core.

dynamic template created:

        {
          "docker.cpu.core.*.pct": {
            "path_match": "docker.cpu.core.*.pct",
            "match_mapping_type": "*",
            "mapping": {
              "scaling_factor": 1000,
              "type": "scaled_float"
            }
          }
        },

cc @jsoriano

mrodm · 2025-01-13T17:50:35Z

Among the errors mentioned above:

cef: different type for url.original (keyword vs wildcard)

One option to solve this error in cef package is to update the ECS mappings that elastic-package adds automatically via import_mappings setting.

Currently, the embedded dynamic template created by elastic-package when import_mappings is enabled does not take any effect (it does not match path_match):

        {
          "_embedded_ecs-url_original_to_multifield": {
            "path_match": "*.url.original",
            "mapping": {
              "fields": {
                "text": {
                  "type": "match_only_text"
                }
              },
              "type": "wildcard"
            }
          }
        },

This dynamic template could be updated as in the ecs@mappings component template (without unmatch_mapping_type parameter since it is not supported in 8.6.x):

        {
          "ecs_path_match_wildcard_and_match_only_text": {
            "path_match": [
              "*.body.content",
              "*url.full",
              "*url.original"
            ],
            "unmatch_mapping_type": "object",
            "mapping": {
              "fields": {
                "text": {
                  "type": "match_only_text"
                }
              },
              "type": "wildcard"
            }
          }
        },

The inconvenient is that it would be needed to do a new cef release in order to include these new dynamic templates, since they are included in the built package.

WDYT about updating the embedded dynamic templates (in a follow-up PR)? Should it be created a new "empty" release in cef (with the new embedded dynamic templates) if this change is applied? Or just wait for the next cef version whenever it is created? @jsoriano

EDIT:

Possible follow-up PR updating dynamic templates created when import_mappings is enabled Update dynamic template for url.original and url.full (import_mappings setting) #2337

jsoriano · 2025-01-13T21:51:01Z

cef: different type for url.original (keyword vs wildcard)

Why is this issue not detected with current validation? 🤔

WDYT about updating the embedded dynamic templates (in a follow-up PR)?

We probably need to do that in any case.

Should it be created a new "empty" release in cef (with the new embedded dynamic templates) if this change is applied? Or just wait for the next cef version whenever it is created?

We will need this to apply the new validation in integrations, we can wait till then.

mrodm · 2025-01-14T10:54:38Z

cef: different type for url.original (keyword vs wildcard)

Why is this issue not detected with current validation? 🤔

Looking at the current logic to validate the fields (without mappings), IIUC wildcards are not validated or at least considered as not blocking due to this switch case (parseSingleElementValue function):

elastic-package/internal/fields/validate.go

Lines 1249 to 1252 in a7d863a

    
           // All other types are considered valid not blocking validation. 
        
           default: 
        
           	return nil 
        
           }

WDYT about updating the embedded dynamic templates (in a follow-up PR)?

We probably need to do that in any case.

Should it be created a new "empty" release in cef (with the new embedded dynamic templates) if this change is applied? Or just wait for the next cef version whenever it is created?

We will need this to apply the new validation in integrations, we can wait till then.

As this requires the ECS templates file to be updated in elastic-package, I'll move forward with this PR to prepare it for review.

…ppings

elasticmachine · 2025-01-14T17:55:11Z

💚 Build Succeeded

Buildkite Build
Commit: 8fce0ec

History

💚 Build #4609 succeeded dbca4fe
💚 Build #4608 succeeded d96ffbf
💚 Build #4607 succeeded 8f36c18
💔 Build #4605 failed ebda859
💔 Build #4604 failed 9ff3d0c
💚 Build #4603 succeeded 0a5e775

cc @mrodm

mrodm · 2025-01-14T18:12:23Z

Created this PR elastic/integrations#12300 to start testing different approaches/workarounds that could be followed in the failing packages found in the integrations repository.

As a note, that integrations PR is just for testing purposes, and it is not going to be merged as it is.

mrodm added 5 commits December 16, 2024 19:15

Add dynamic templates as parameter

2c3f76e

Add validation for each dynamic template depending on the parameters

41d7b88

Fixes for dynamic templates

ff5298e

Compare fields with dynamic templates

af04d66

Remove multi_fields from flattened fields

f21c712

multi_fields are now compared directly with dynamic templates or ECS fields. It was required to know whether or not that field was a multi_field or not to apply some specific validations.

mrodm self-assigned this Dec 17, 2024

mrodm added 3 commits December 17, 2024 20:09

Test without filtering dynamic templates

aa28db6

Ensure properties subfields are validated accordingly

3767111

Restore filtering and add tests

9fb80b4

elastic-vault-github-plugin-prod bot mentioned this pull request Dec 19, 2024

Test elastic-package#2285 - DO NOT MERGE elastic/integrations#12164

Draft

Disable unmatch_mapping_type and match_mapping_type and continue look…

2ad28ac

…ing for other dynamic templates if parameters do not match

mrodm added 4 commits December 19, 2024 16:56

Merge upstream/main into validate-dynamic-mappings

8bdad65

Refactors and remove log statements

151e59e

Fix function naming

d0ff6b9

Add test with match_pattern regex

b474b4b

mrodm commented Dec 19, 2024

View reviewed changes

internal/fields/mappings.go Outdated Show resolved Hide resolved

mrodm commented Dec 19, 2024

View reviewed changes

mrodm requested a review from a team December 19, 2024 17:14

mrodm marked this pull request as ready for review December 19, 2024 17:14

mrodm changed the title ~~[WIP] Validate dynamic mappings~~ Validate fields based on their mappings and the dynamic templates set Dec 19, 2024

Add comment in tests

066b5a0

jsoriano reviewed Dec 19, 2024

View reviewed changes

mrodm added 3 commits January 8, 2025 12:52

Update tests for multi-fields

131cc9c

Review multi-fields logic

38db8df

Revisit multi-fields logic in ECS

9ff3d0c

mrodm added 3 commits January 8, 2025 18:38

Report all errors related to multi-fields comparing with ECS

5165612

Ignored validation of multi-fields with ECS

ce557c9

Fix multi-field test

ebda859

mrodm commented Jan 8, 2025

View reviewed changes

internal/fields/mappings.go Show resolved Hide resolved

mrodm commented Jan 8, 2025

View reviewed changes

mrodm added 2 commits January 9, 2025 12:16

Rephrase errors

019ffd4

Validate fully dynamic objects (preview) with dynamic templates

8f36c18

mrodm added 2 commits January 9, 2025 13:41

Rephrase debug message

d96ffbf

Add logging - to be removed

dbca4fe

mrodm requested a review from jsoriano January 9, 2025 17:02

mrodm mentioned this pull request Jan 13, 2025

Update dynamic template for url.original and url.full (import_mappings setting) #2337

Merged

2 tasks

Merge remote-tracking branch 'upstream/main' into validate-dynamic-ma…

8fce0ec

…ppings

mrodm mentioned this pull request Jan 16, 2025

Validate transforms mappings #2347

Draft

2 tasks

		// TODO: Should we remove this code to load external and local fields?
		fieldsDir := filepath.Join(fieldsParentDir, "fields")

Validate fields based on their mappings and the dynamic templates set #2285

Are you sure you want to change the base?

Validate fields based on their mappings and the dynamic templates set #2285

Conversation

mrodm commented Dec 17, 2024 • edited Loading

Author's Checklist

mrodm commented Dec 19, 2024

elastic-vault-github-plugin-prod bot commented Dec 19, 2024

mrodm commented Dec 19, 2024

elastic-vault-github-plugin-prod bot commented Dec 19, 2024

mrodm commented Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrodm commented Jan 8, 2025

elastic-vault-github-plugin-prod bot commented Jan 8, 2025

Choose a reason for hiding this comment

mrodm commented Jan 8, 2025

elastic-vault-github-plugin-prod bot commented Jan 8, 2025

mrodm commented Jan 9, 2025

elastic-vault-github-plugin-prod bot commented Jan 9, 2025

mrodm commented Jan 9, 2025

elastic-vault-github-plugin-prod bot commented Jan 9, 2025

mrodm commented Jan 9, 2025

mrodm commented Jan 13, 2025 • edited Loading

jsoriano commented Jan 13, 2025

mrodm commented Jan 14, 2025

elasticmachine commented Jan 14, 2025

💚 Build Succeeded

History

mrodm commented Jan 14, 2025

mrodm commented Dec 17, 2024 •

edited

Loading

mrodm commented Dec 19, 2024 •

edited

Loading

mrodm commented Jan 13, 2025 •

edited

Loading