Revisit multi-fields logic in ECS

Author: mrodm

internal/fields/mappings.go

@@ -315,7 +315,29 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini
 		return err
 	}
 
-	// Compare multifields
+	return compareMultiFieldsInECSSchema(currentPath, found, definition)
+}
+
+func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, actual map[string]any) error {
+	actualType := mappingParameter("type", actual)
+	if ecs.Type != actualType {
+		// exceptions related to numbers
+		if !isNumberTypeField(ecs.Type, actualType) {
+			return fmt.Errorf("actual mapping type (%s) does not match with ECS definition type: %s", actualType, ecs.Type)
+		} else {
+			logger.Debugf("Allowed number fields with different types (ECS %s - actual %s)", string(ecs.Type), string(actualType))
+		}
+	}
+
+	// Compare other parameters
+	metricType := mappingParameter("time_series_metric", actual)
+	if ecs.MetricType != metricType {
+		return fmt.Errorf("actual mapping \"time_series_metric\" (%s) does not match with ECS definition value: %s", metricType, ecs.MetricType)
+	}
+	return nil
+}
+
+func compareMultiFieldsInECSSchema(currentPath string, found *FieldDefinition, definition map[string]any) error {
 	var ecsMultiFields []FieldDefinition
 	// Filter multi-fields added by appendECSMappingMultifields
 	for _, f := range found.MultiFields {
@@ -342,21 +364,34 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini
 		return fmt.Errorf("invalid multi_field mapping %q: %w", currentPath, err)
 	}
 
+	missingMultiFields := []string{}
 	for key, value := range actualMultiFields {
 		multiFieldPath := currentMappingPath(currentPath, key)
 
 		def, ok := value.(map[string]any)
 		if !ok {
 			return fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath)
 		}
+		found := false
 		for _, ecsMultiField := range ecsMultiFields {
+			if ecsMultiField.Name != key {
+				continue
+			}
 			err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def)
 			if err == nil {
-				return nil
+				found = true
+				break
 			}
 		}
+		if !found {
+			missingMultiFields = append(missingMultiFields, key)
+		}
+	}
+	if len(missingMultiFields) > 0 {
+		return fmt.Errorf("missing definition for multi-fields in ECS: %q", strings.Join(missingMultiFields, ", "))
 	}
-	return fmt.Errorf("not found multi-field definition in ECS for %q", currentPath)
+
+	return nil
 
 	// for _, ecsMultiField := range ecsMultiFields {
 	// 	multiFieldPath := currentMappingPath(currentPath, ecsMultiField.Name)
@@ -379,25 +414,6 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini
 	// return nil
 }
 
-func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, actual map[string]any) error {
-	actualType := mappingParameter("type", actual)
-	if ecs.Type != actualType {
-		// exceptions related to numbers
-		if !isNumberTypeField(ecs.Type, actualType) {
-			return fmt.Errorf("actual mapping type (%s) does not match with ECS definition type: %s", actualType, ecs.Type)
-		} else {
-			logger.Debugf("Allowed number fields with different types (ECS %s - actual %s)", string(ecs.Type), string(actualType))
-		}
-	}
-
-	// Compare other parameters
-	metricType := mappingParameter("time_series_metric", actual)
-	if ecs.MetricType != metricType {
-		return fmt.Errorf("actual mapping \"time_series_metric\" (%s) does not match with ECS definition value: %s", metricType, ecs.MetricType)
-	}
-	return nil
-}
-
 // flattenMappings returns all the mapping definitions found at "path" flattened including
 // specific entries for multi fields too.
 func flattenMappings(path string, definition map[string]any) (map[string]any, error) {
@@ -670,7 +686,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi
 		return nil
 	}
 
-	logger.Debugf(">> No template matching for path: %q", currentPath)
+	// logger.Debugf(">> No template matching for path: %q", currentPath)
 	return fmt.Errorf("no template matching for path: %q", currentPath)
 }
 

internal/fields/mappings_test.go

@@ -99,6 +99,24 @@ func TestComparingMappings(t *testing.T) {
 				"foo": map[string]any{
 					"type": "keyword",
 				},
+				"user": map[string]any{
+					"type": "keyword",
+				},
+				"time": map[string]any{
+					"type": "keyword",
+					"fields": map[string]any{
+						"text": map[string]any{
+							"type": "match_only_text",
+						},
+						"other": map[string]any{
+							"type": "match_only_text",
+						},
+					},
+				},
+				// Should this fail since it has no multi-fields as in the ECS definition?
+				"name": map[string]any{
+					"type": "keyword",
+				},
 			},
 			schema: []FieldDefinition{
 				{
@@ -116,9 +134,35 @@ func TestComparingMappings(t *testing.T) {
 					Type:     "keyword",
 					External: "",
 				},
+				{
+					Name:     "time",
+					Type:     "keyword",
+					External: "ecs",
+					MultiFields: []FieldDefinition{
+						{
+							Name:     "text",
+							Type:     "match_only_text",
+							External: "ecs",
+						},
+					},
+				},
+				{
+					Name:     "name",
+					Type:     "keyword",
+					External: "ecs",
+					MultiFields: []FieldDefinition{
+						{
+							Name:     "text",
+							Type:     "match_only_text",
+							External: "ecs",
+						},
+					},
+				},
 			},
 			expectedErrors: []string{
 				`field "metrics" is undefined: actual mapping type (long) does not match with ECS definition type: keyword`,
+				`field "user" is undefined: missing definition for path (not in ECS)`,
+				`field "time" is undefined: missing definition for multi-fields in ECS: "other"`,
 			},
 		},
 		{
@@ -230,6 +274,14 @@ func TestComparingMappings(t *testing.T) {
 		{
 			title: "validate multifields failure",
 			preview: map[string]any{
+				"time": map[string]any{
+					"type": "keyword",
+					"fields": map[string]any{
+						"text": map[string]any{
+							"type": "match_only_text",
+						},
+					},
+				},
 				"foo": map[string]any{
 					"type": "keyword",
 					"fields": map[string]any{
@@ -255,6 +307,10 @@ func TestComparingMappings(t *testing.T) {
 				},
 			},
 			actual: map[string]any{
+				// Should this fail since it has no multi-fields as in the preview?
+				"time": map[string]any{
+					"type": "keyword",
+				},
 				"foo": map[string]any{
 					"type": "keyword",
 					"fields": map[string]any{