Skip to content

Commit 9fb80b4

Browse files
mrodmmrodm
committed
Restore filtering and add tests
1 parent 3767111 commit 9fb80b4

File tree

2 files changed

+122
-38
lines changed

2 files changed

+122
-38
lines changed

‎internal/fields/mappings.go‎

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -401,15 +401,15 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini
401401
}
402402
}
403403

404-
if isMultiFields(definition) != (len(ecsMultiFields) > 0) {
405-
return fmt.Errorf("not matched definitions for multifields for %q: actual multi_fields in mappings %t - ECS multi_fields length %d", currentPath, isMultiFields(definition), len(ecsMultiFields))
406-
}
407-
408404
// if there are no multifieds in ECS, nothing to compare
409405
if len(ecsMultiFields) == 0 {
410406
return nil
411407
}
412408

409+
if isMultiFields(definition) != (len(ecsMultiFields) > 0) {
410+
return fmt.Errorf("not matched definitions for multifields for %q: actual multi_fields in mappings %t - ECS multi_fields length %d", currentPath, isMultiFields(definition), len(ecsMultiFields))
411+
}
412+
413413
actualMultiFields, err := getMappingDefinitionsField("fields", definition)
414414
if err != nil {
415415
return fmt.Errorf("invalid multi_field mapping %q: %w", currentPath, err)
@@ -573,6 +573,7 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit
573573
if err != nil {
574574
errs = append(errs, fmt.Errorf("found invalid properties type in actual mappings for path %q: %w", path, err))
575575
}
576+
// logger.Debugf(">> Compare object properties (mappings) - length %d: %q", len(actualProperties), path)
576577
compareErrors := v.compareMappings(path, false, previewProperties, actualProperties, dynamicTemplates)
577578
errs = append(errs, compareErrors...)
578579

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

608+
// if len(dynamicTemplates) == 0 {
609+
// logger.Debugf(">>> Compare object properties %q", path)
610+
// }
604611
// Compare and validate the elements under "properties": objects or fields and its parameters
605612
propertiesErrs := v.validateObjectProperties(path, false, containsMultifield, actual, preview, dynamicTemplates)
606613
errs = append(errs, propertiesErrs...)
@@ -677,9 +684,11 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil
677684
}
678685

679686
// validate whether or not the field has a corresponding dynamic template
680-
err := v.matchingWithDynamicTemplates(fieldPath, def, dynamicTemplates)
681-
if err == nil {
682-
continue
687+
if len(dynamicTemplates) > 0 {
688+
err := v.matchingWithDynamicTemplates(fieldPath, def, dynamicTemplates)
689+
if err == nil {
690+
continue
691+
}
683692
}
684693

685694
// validate whether or not all fields under this key are defined in ECS
@@ -780,15 +789,15 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
780789

781790
// Filter out dynamic templates created by elastic-package (import_mappings)
782791
// or added automatically by ecs@mappings component template
783-
// if strings.HasPrefix(templateName, "_embedded_ecs-") {
784-
// continue
785-
// }
786-
// if strings.HasPrefix(templateName, "ecs_") {
787-
// continue
788-
// }
789-
// if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) {
790-
// continue
791-
// }
792+
if strings.HasPrefix(templateName, "_embedded_ecs-") {
793+
continue
794+
}
795+
if strings.HasPrefix(templateName, "ecs_") {
796+
continue
797+
}
798+
if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) {
799+
continue
800+
}
792801

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

@@ -822,11 +831,12 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
822831
logger.Debugf("> Check match: %q (key %q)", currentPath, name)
823832
values, err := parseSetting(value)
824833
if err != nil {
834+
logger.Warnf("failed to check match setting: %s", err)
825835
return fmt.Errorf("failed to check match setting: %w", err)
826836
}
827-
if !slices.Contains(values, name) {
828-
matched = false
829-
break
837+
if slices.Contains(values, name) {
838+
logger.Warnf(">>>> no contained %s: %s", values, name)
839+
continue
830840
}
831841

832842
var matches bool
@@ -939,20 +949,20 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
939949
logger.Debugf("> Found dynamic template matched: %s", templateName)
940950
mappingParameter, ok := contents["mapping"]
941951
if !ok {
942-
logger.Warnf(">> Missing \"mapping\" field: %s", templateName)
943952
return fmt.Errorf("missing mapping parameter in %s", templateName)
944953
}
945954

946-
logger.Debugf(">> Check all the other parameters (%q): %q", templateName, currentPath)
955+
logger.Debugf(">> Check parameters (%q): %q", templateName, currentPath)
947956
errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}, true)
948957
if errs != nil {
958+
logger.Warnf("invalid mapping found for %q:\n%s", currentPath, errs.Unique())
949959
return fmt.Errorf("invalid mapping found for %q: %w", currentPath, errs.Unique())
950960
}
951961

952962
return nil
953963
}
954964

955-
logger.Debugf(">>> No template matching")
965+
logger.Debugf(">>> No template matching for path: %q", currentPath)
956966
return fmt.Errorf("no template matching for path: %q", currentPath)
957967
}
958968

@@ -971,14 +981,14 @@ func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actu
971981
if !ok {
972982
errs = append(errs, fmt.Errorf("unexpected type in actual mappings for path: %q", currentPath))
973983
}
974-
if len(dynamicTemplates) == 0 {
975-
logger.Debugf(">>> Compare mappings map[string]any %s", currentPath)
976-
}
984+
// if len(dynamicTemplates) == 0 {
985+
// logger.Debugf(">>> Compare mappings map[string]any %s - length %d", currentPath, len(actualField))
986+
// }
977987
errs = append(errs, v.compareMappings(currentPath, couldBeObjectDefinition, previewField, actualField, dynamicTemplates)...)
978988
case any:
979-
if len(dynamicTemplates) == 0 {
980-
logger.Debugf(">>> Compare mappings any %s", currentPath)
981-
}
989+
// if len(dynamicTemplates) == 0 {
990+
// logger.Debugf(">>> Compare mappings any %s", currentPath)
991+
// }
982992
// Validate each setting/parameter of the mapping
983993
// If a mapping exist in both preview and actual, they should be the same. But forcing to compare each parameter just in case
984994
if previewValue == actualValue {

‎internal/fields/mappings_test.go‎

Lines changed: 84 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@ import (
1616
func TestComparingMappings(t *testing.T) {
1717
defaultSpecVersion := "3.3.0"
1818
cases := []struct {
19-
title string
20-
preview map[string]any
21-
actual map[string]any
22-
schema []FieldDefinition
23-
spec string
24-
exceptionFields []string
25-
expectedErrors []string
19+
title string
20+
preview map[string]any
21+
actual map[string]any
22+
schema []FieldDefinition
23+
dynamicTemplates []map[string]any
24+
spec string
25+
exceptionFields []string
26+
expectedErrors []string
2627
}{
2728
{
2829
title: "same mappings",
@@ -706,12 +707,85 @@ func TestComparingMappings(t *testing.T) {
706707
`not found properties in preview mappings for path: "foo"`,
707708
},
708709
},
710+
{
711+
title: "fields matching dynamic templates",
712+
preview: map[string]any{},
713+
actual: map[string]any{
714+
"foo": map[string]any{
715+
"type": "keyword",
716+
},
717+
"foa": map[string]any{
718+
"type": "long",
719+
},
720+
"fob": map[string]any{
721+
"type": "double",
722+
"time_series_metric": "gauge",
723+
},
724+
"bar": map[string]any{
725+
"type": "text",
726+
"fields": map[string]any{
727+
"type": "keyword",
728+
},
729+
},
730+
"bar_double": map[string]any{
731+
"type": "double",
732+
},
733+
},
734+
dynamicTemplates: []map[string]any{
735+
{
736+
"fo*_keyword": map[string]any{
737+
"path_match": "fo*",
738+
"unmatch_mapping_type": []any{"long", "double"},
739+
"mapping": map[string]any{
740+
"type": "keyword",
741+
},
742+
},
743+
},
744+
{
745+
"fo*_number": map[string]any{
746+
"path_match": "fo*",
747+
"match_mapping_type": []any{"long", "double"},
748+
"mapping": map[string]any{
749+
"type": "double",
750+
"time_series_metric": "counter",
751+
},
752+
},
753+
},
754+
{
755+
"bar_match": map[string]any{
756+
"unmatch": []any{"foo", "foo42", "*42"},
757+
"match": []any{"*ar", "bar42"},
758+
"match_mapping_type": "text",
759+
"mapping": map[string]any{
760+
"type": "text",
761+
"fields": map[string]any{
762+
"type": "keyword",
763+
},
764+
},
765+
},
766+
},
767+
{
768+
"bar_star_double": map[string]any{
769+
"match": "*",
770+
"unmatch_mapping_type": []any{"text"},
771+
"mapping": map[string]any{
772+
"type": "double",
773+
},
774+
},
775+
},
776+
},
777+
exceptionFields: []string{},
778+
spec: "3.0.0",
779+
schema: []FieldDefinition{},
780+
expectedErrors: []string{
781+
// Should it be considered this error in "foa" "missing time_series_metric bar",
782+
`field "fob" is undefined: missing definition for path`,
783+
},
784+
},
709785
}
710786

711787
for _, c := range cases {
712788
t.Run(c.title, func(t *testing.T) {
713-
dynamicTemplates := []map[string]any{}
714-
715789
logger.EnableDebugMode()
716790
specVersion := defaultSpecVersion
717791
if c.spec != "" {
@@ -725,7 +799,7 @@ func TestComparingMappings(t *testing.T) {
725799
)
726800
require.NoError(t, err)
727801

728-
errs := v.compareMappings("", false, c.preview, c.actual, dynamicTemplates)
802+
errs := v.compareMappings("", false, c.preview, c.actual, c.dynamicTemplates)
729803
if len(c.expectedErrors) > 0 {
730804
assert.Len(t, errs, len(c.expectedErrors))
731805
for _, err := range errs {

0 commit comments

Comments
 (0)