Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 32 commits into from
Jan 21, 2025
Merged

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

Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
2c3f76e
Add dynamic templates as parameter
mrodm Dec 12, 2024
41d7b88
Add validation for each dynamic template depending on the parameters
mrodm Dec 16, 2024
ff5298e
Fixes for dynamic templates
mrodm Dec 16, 2024
af04d66
Compare fields with dynamic templates
mrodm Dec 17, 2024
f21c712
Remove multi_fields from flattened fields
mrodm Dec 17, 2024
aa28db6
Test without filtering dynamic templates
mrodm Dec 17, 2024
3767111
Ensure properties subfields are validated accordingly
mrodm Dec 18, 2024
9fb80b4
Restore filtering and add tests
mrodm Dec 18, 2024
2ad28ac
Disable unmatch_mapping_type and match_mapping_type and continue look…
mrodm Dec 19, 2024
8bdad65
Merge upstream/main into validate-dynamic-mappings
mrodm Dec 19, 2024
151e59e
Refactors and remove log statements
mrodm Dec 19, 2024
d0ff6b9
Fix function naming
mrodm Dec 19, 2024
b474b4b
Add test with match_pattern regex
mrodm Dec 19, 2024
066b5a0
Add comment in tests
mrodm Dec 19, 2024
a7eb191
Remove loading schema from create validator for mappings method
mrodm Jan 7, 2025
7666ac2
Separate parsing and validation of dynamic templates
mrodm Jan 7, 2025
a718796
Support match_pattern for the other settings
mrodm Jan 7, 2025
0a5e775
fix test
mrodm Jan 8, 2025
131cc9c
Update tests for multi-fields
mrodm Jan 8, 2025
38db8df
Review multi-fields logic
mrodm Jan 8, 2025
9ff3d0c
Revisit multi-fields logic in ECS
mrodm Jan 8, 2025
5165612
Report all errors related to multi-fields comparing with ECS
mrodm Jan 8, 2025
ce557c9
Ignored validation of multi-fields with ECS
mrodm Jan 8, 2025
ebda859
Fix multi-field test
mrodm Jan 8, 2025
019ffd4
Rephrase errors
mrodm Jan 9, 2025
8f36c18
Validate fully dynamic objects (preview) with dynamic templates
mrodm Jan 9, 2025
d96ffbf
Rephrase debug message
mrodm Jan 9, 2025
dbca4fe
Add logging - to be removed
mrodm Jan 9, 2025
8fce0ec
Merge remote-tracking branch 'upstream/main' into validate-dynamic-ma…
mrodm Jan 14, 2025
c333707
Revisited log messages
mrodm Jan 20, 2025
0569cfb
Remove comments
mrodm Jan 20, 2025
1aea303
Remove more debug statements
mrodm Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Refactors and remove log statements
  • Loading branch information
@mrodm
mrodm committed Dec 19, 2024
commit 151e59e8a8d4b5b8a658095363bac7c57a1f9b08
130 changes: 63 additions & 67 deletions internal/fields/mappings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ func createValidatorForMappingsAndPackageRoot(fieldsParentDir string, finder pac
return v, nil
}

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

@mrodm mrodm Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@jsoriano jsoriano Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sounds good.


var fdm *DependencyManager
Expand Down Expand Up @@ -266,6 +267,15 @@ func currentMappingPath(path, key string) string {
return fmt.Sprintf("%s.%s", path, key)
}

func fieldNameFromPath(path string) string {
if !strings.Contains(path, ".") {
return path
}

elems := strings.Split(path, ".")
return elems[len(elems)-1]
}

func mappingParameter(field string, definition map[string]any) string {
fieldValue, ok := definition[field]
if !ok {
Expand Down Expand Up @@ -396,6 +406,7 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini
var ecsMultiFields []FieldDefinition
// Filter multi-fields added by appendECSMappingMultifields
for _, f := range found.MultiFields {
// TODO: Should we use another way to filter these fields?
if f.External != externalFieldAppendedTag {
ecsMultiFields = append(ecsMultiFields, f)
}
Expand Down Expand Up @@ -573,7 +584,6 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit
if err != nil {
errs = append(errs, fmt.Errorf("found invalid properties type in actual mappings for path %q: %w", path, err))
}
// logger.Debugf(">> Compare object properties (mappings) - length %d: %q", len(actualProperties), path)
compareErrors := v.compareMappings(path, false, previewProperties, actualProperties, dynamicTemplates)
errs = append(errs, compareErrors...)

Expand All @@ -597,17 +607,11 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit
if err != nil {
errs = append(errs, fmt.Errorf("found invalid multi_fields type in actual mappings for path %q: %w", path, err))
}
// if len(dynamicTemplates) == 0 {
// logger.Debugf(">>> Compare multi-fields %q", path)
// }
compareErrors := v.compareMappings(path, false, previewFields, actualFields, dynamicTemplates)
errs = append(errs, compareErrors...)
// not returning here to keep validating the other fields of this object if any
}

// if len(dynamicTemplates) == 0 {
// logger.Debugf(">>> Compare object properties %q", path)
// }
// Compare and validate the elements under "properties": objects or fields and its parameters
propertiesErrs := v.validateObjectProperties(path, false, containsMultifield, preview, actual, dynamicTemplates)
errs = append(errs, propertiesErrs...)
Expand Down Expand Up @@ -728,15 +732,6 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
return all, nil
}

fieldName := func(path string) string {
if !strings.Contains(path, ".") {
return path
}

elems := strings.Split(path, ".")
return elems[len(elems)-1]
}

stringMatchesPatterns := func(regexes []string, elem string) (bool, error) {
applies := false
for _, v := range regexes {
Expand Down Expand Up @@ -787,20 +782,11 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
rawContents = value
}

// Filter out dynamic templates created by elastic-package (import_mappings)
// or added automatically by ecs@mappings component template
if strings.HasPrefix(templateName, "_embedded_ecs-") {
continue
}
if strings.HasPrefix(templateName, "ecs_") {
continue
}
if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) {
if shouldSkipDynamicTemplate(templateName) {
continue
}

logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName)

// logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName)
contents, ok := rawContents.(map[string]any)
if !ok {
return fmt.Errorf("unexpected dynamic template format found for %q", templateName)
Expand Down Expand Up @@ -828,7 +814,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
// Do nothing
case "match":
name := fieldName(currentPath)
logger.Debugf("> Check match: %q (key %q)", currentPath, name)
// logger.Debugf("> Check match: %q (key %q)", currentPath, name)
values, err := parseSetting(value)
if err != nil {
logger.Warnf("failed to check match setting: %s", err)
Expand All @@ -850,12 +836,12 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
}

if !matches {
logger.Debugf(">> Issue: not matches")
// logger.Debugf(">> Issue: not matches")
matched = false
}
case "unmatch":
name := fieldName(currentPath)
logger.Debugf("> Check unmatch: %q (key %q)", currentPath, name)
// logger.Debugf("> Check unmatch: %q (key %q)", currentPath, name)
values, err := parseSetting(value)
if err != nil {
return fmt.Errorf("failed to check unmatch setting: %w", err)
Expand All @@ -876,11 +862,11 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
}

if matches {
logger.Debugf(">> Issue: matches")
// logger.Debugf(">> Issue: matches")
matched = false
}
case "path_match":
logger.Debugf("> Check path_match: %q", currentPath)
// logger.Debugf("> Check path_match: %q", currentPath)
values, err := parseSetting(value)
if err != nil {
return fmt.Errorf("failed to check path_match setting: %w", err)
Expand All @@ -890,11 +876,11 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err)
}
if !matches {
logger.Debugf(">> Issue: not matches")
// logger.Debugf(">> Issue: not matches")
matched = false
}
case "path_unmatch":
logger.Debugf("> Check path_unmatch: %q", currentPath)
// logger.Debugf("> Check path_unmatch: %q", currentPath)
values, err := parseSetting(value)
if err != nil {
return fmt.Errorf("failed to check path_unmatch setting: %w", err)
Expand All @@ -904,38 +890,38 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err)
}
if matches {
logger.Debugf(">> Issue: matches")
// logger.Debugf(">> Issue: matches")
matched = false
}
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
// }
// 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
// }
default:
return fmt.Errorf("unexpected setting found in dynamic template")
}
Expand All @@ -959,7 +945,8 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
logger.Debugf(">> Check parameters (%q): %q", templateName, currentPath)
errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}, true)
if errs != nil {
logger.Warnf("invalid mapping found for %q:\n%s", currentPath, errs.Unique())
// Look for another dynamic template
logger.Debugf("invalid mapping found for %q:\n%s", currentPath, errs.Unique())
continue
}

Expand All @@ -970,6 +957,21 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
return fmt.Errorf("no template matching for path: %q", currentPath)
}

func shouldSkipDynamicTemplate(templateName string) bool {
// Filter out dynamic templates created by elastic-package (import_mappings)
// or added automatically by ecs@mappings component template
if strings.HasPrefix(templateName, "_embedded_ecs-") {
return true
}
if strings.HasPrefix(templateName, "ecs_") {
return true
}
if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) {
return true
}
return false
}

// validateObjectMappingAndParameters validates the current object or field parameter (currentPath) comparing the values
// in the actual mapping with the values in the preview mapping.
func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actualValue any, currentPath string, dynamicTemplates []map[string]any, couldBeParametersDefinition bool) multierror.Error {
Expand All @@ -985,14 +987,8 @@ func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actu
if !ok {
errs = append(errs, fmt.Errorf("unexpected type in actual mappings for path: %q", currentPath))
}
// if len(dynamicTemplates) == 0 {
// logger.Debugf(">>> Compare mappings map[string]any %s - length %d", currentPath, len(actualField))
// }
errs = append(errs, v.compareMappings(currentPath, couldBeParametersDefinition, previewField, actualField, dynamicTemplates)...)
case any:
// if len(dynamicTemplates) == 0 {
// logger.Debugf(">>> Compare mappings any %s", currentPath)
// }
// Validate each setting/parameter of the mapping
// If a mapping exist in both preview and actual, they should be the same. But forcing to compare each parameter just in case
if previewValue == actualValue {
Expand Down