From 86c08f22e9faaab7410758010529bcc36fc1fcd0 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Thu, 12 May 2022 15:07:50 +0200 Subject: [PATCH] Don't import ECS fields of type group (#818) --- internal/fields/dependency_manager.go | 66 +++++++------ internal/fields/dependency_manager_test.go | 105 +++++++++++++++++++++ internal/fields/validate.go | 2 + internal/fields/validate_test.go | 9 ++ 4 files changed, 155 insertions(+), 27 deletions(-) diff --git a/internal/fields/dependency_manager.go b/internal/fields/dependency_manager.go index f43246f97..14e856497 100644 --- a/internal/fields/dependency_manager.go +++ b/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 { diff --git a/internal/fields/dependency_manager_test.go b/internal/fields/dependency_manager_test.go index 2b86de57d..52c42a6b3 100644 --- a/internal/fields/dependency_manager_test.go +++ b/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} diff --git a/internal/fields/validate.go b/internal/fields/validate.go index afed35405..3af27d5ac 100644 --- a/internal/fields/validate.go +++ b/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 } diff --git a/internal/fields/validate_test.go b/internal/fields/validate_test.go index 26b352f8a..048da75f1 100644 --- a/internal/fields/validate_test.go +++ b/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) {