Don't import ECS fields of type group (#818)

internal/fields/dependency_manager.go

@@ -163,33 +163,54 @@ func (dm *DependencyManager) injectFieldsWithRoot(root string, defs []common.Map
 			transformed["type"] = imported.Type
 			transformed.Delete("external")
 
-			updated = append(updated, transformed)
+			def = transformed
 			changed = true
-			continue
-		}
-
-		fields, _ := def.GetValue("fields")
-		if fields != nil {
-			fieldsMs, err := common.ToMapStrSlice(fields)
-			if err != nil {
-				return nil, false, errors.Wrap(err, "can't convert fields")
-			}
-			updatedFields, fieldsChanged, err := dm.injectFieldsWithRoot(fieldPath, fieldsMs)
-			if err != nil {
-				return nil, false, err
-			}
-
-			if fieldsChanged {
-				changed = true
+		} else {
+			fields, _ := def.GetValue("fields")
+			if fields != nil {
+				fieldsMs, err := common.ToMapStrSlice(fields)
+				if err != nil {
+					return nil, false, errors.Wrap(err, "can't convert fields")
+				}
+				updatedFields, fieldsChanged, err := dm.injectFieldsWithRoot(fieldPath, fieldsMs)
+				if err != nil {
+					return nil, false, err
+				}
+
+				if fieldsChanged {
+					changed = true
+				}
+
+				def.Put("fields", updatedFields)
 			}
+		}
 
-			def.Put("fields", updatedFields)
+		if skipField(def) {
+			continue
 		}
 		updated = append(updated, def)
 	}
 	return updated, changed, nil
 }
 
+// skipField decides if a field should be skipped and not injected in the built fields.
+func skipField(def common.MapStr) bool {
+	t, _ := def.GetValue("type")
+	if t == "group" {
+		fields, _ := def.GetValue("fields")
+		switch fields := fields.(type) {
+		case nil:
+			return true
+		case []interface{}:
+			return len(fields) == 0
+		case []common.MapStr:
+			return len(fields) == 0
+		}
+	}
+
+	return false
+}
+
 // ImportField method resolves dependency on a single external field using available schemas.
 func (dm *DependencyManager) ImportField(schemaName, fieldPath string) (FieldDefinition, error) {
 	if dm == nil {
@@ -241,15 +262,6 @@ func transformImportedField(fd FieldDefinition) common.MapStr {
 		m["doc_values"] = *fd.DocValues
 	}
 
-	if len(fd.Fields) > 0 {
-		var t []common.MapStr
-		for _, f := range fd.Fields {
-			i := transformImportedField(f)
-			t = append(t, i)
-		}
-		m.Put("fields", t)
-	}
-
 	if len(fd.MultiFields) > 0 {
 		var t []common.MapStr
 		for _, f := range fd.MultiFields {

internal/fields/dependency_manager_test.go

@@ -210,6 +210,94 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
 			},
 			valid: false,
 		},
+		{
+			title: "import nested fields",
+			defs: []common.MapStr{
+				{
+					"name":     "host.id",
+					"external": "test",
+				},
+				{
+					"name":     "host.hostname",
+					"external": "test",
+				},
+			},
+			result: []common.MapStr{
+				{
+					"name":        "host.id",
+					"description": "Unique host id",
+					"type":        "keyword",
+				},
+				{
+					"name":        "host.hostname",
+					"description": "Hostname of the host",
+					"type":        "keyword",
+				},
+			},
+			valid:   true,
+			changed: true,
+		},
+		{
+			title: "import nested definitions",
+			defs: []common.MapStr{
+				{
+					"name": "host",
+					"type": "group",
+					"fields": []interface{}{
+						common.MapStr{
+							"name":     "id",
+							"external": "test",
+						},
+						common.MapStr{
+							"name":     "hostname",
+							"external": "test",
+						},
+					},
+				},
+			},
+			result: []common.MapStr{
+				{
+					"name": "host",
+					"type": "group",
+					"fields": []common.MapStr{
+						{
+							"name":        "id",
+							"description": "Unique host id",
+							"type":        "keyword",
+						},
+						{
+							"name":        "hostname",
+							"description": "Hostname of the host",
+							"type":        "keyword",
+						},
+					},
+				},
+			},
+			valid:   true,
+			changed: true,
+		},
+		{
+			title: "keep group for docs but not for fields",
+			defs: []common.MapStr{
+				{
+					"name":     "host",
+					"external": "test",
+				},
+				{
+					"name":     "host.hostname",
+					"external": "test",
+				},
+			},
+			result: []common.MapStr{
+				{
+					"name":        "host.hostname",
+					"description": "Hostname of the host",
+					"type":        "keyword",
+				},
+			},
+			valid:   true,
+			changed: true,
+		},
 	}
 
 	indexFalse := false
@@ -253,6 +341,23 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
 			Pattern:     "^[A-F0-9]{2}(-[A-F0-9]{2}){5,}$",
 			Type:        "keyword",
 		},
+		{
+			Name:        "host",
+			Description: "A general computing instance",
+			Type:        "group",
+			Fields: []FieldDefinition{
+				{
+					Name:        "id",
+					Description: "Unique host id",
+					Type:        "keyword",
+				},
+				{
+					Name:        "hostname",
+					Description: "Hostname of the host",
+					Type:        "keyword",
+				},
+			},
+		},
 	}}
 	dm := &DependencyManager{schema: schema}
 

internal/fields/validate.go

@@ -416,6 +416,8 @@ func (v *Validator) parseElementValue(key string, definition FieldDefinition, va
 		}
 	case "float", "long", "double":
 		_, valid = val.(float64)
+	case "group":
+		return fmt.Errorf("field %q is a group of fields, it cannot store values", key)
 	default:
 		valid = true // all other types are considered valid not blocking validation
 	}

internal/fields/validate_test.go

@@ -228,6 +228,15 @@ func Test_parseElementValue(t *testing.T) {
 			},
 			fail: true,
 		},
+		// fields shouldn't be stored in groups
+		{
+			key:   "host",
+			value: "42",
+			definition: FieldDefinition{
+				Type: "group",
+			},
+			fail: true,
+		},
 	} {
 		v := Validator{disabledDependencyManagement: true}
 		t.Run(test.key, func(t *testing.T) {