From a96b02a6302654c31f328830a9ffae91ae8d8653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Fri, 1 Sep 2017 15:59:26 +0200 Subject: [PATCH] Combine `fields.yml` properties when they are defined in different sources (#5077) We define fields with the same root key across several files, this change ensures all of them are combined in the final index template. Fixes #5075 --- CHANGELOG.asciidoc | 1 + libbeat/template/field.go | 4 ++- libbeat/template/field_test.go | 3 +- libbeat/template/fields.go | 27 +++++++++++++---- libbeat/template/fields_test.go | 51 +++++++++++++++++++++++++++++++++ libbeat/template/template.go | 5 +++- 6 files changed, 83 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 3e71840750f..7cd8cf35cf0 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -34,6 +34,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di - Register kubernetes `field_format` matcher and remove logger in `Encode` API {pull}4888[4888] - Add support for `initContainers` in `add_kubernetes_metadata` processor. {issue}4825[4825] - Fix the `/usr/bin/beatname` script to accept `-d "*"` as a parameter. {issue}5040[5040] +- Combine `fields.yml` properties when they are defined in different sources. {issue}5075[5075] *Auditbeat* diff --git a/libbeat/template/field.go b/libbeat/template/field.go index 70967bc7540..a2a401ffe21 100644 --- a/libbeat/template/field.go +++ b/libbeat/template/field.go @@ -131,7 +131,9 @@ func (f *Field) text() common.MapStr { } if len(f.MultiFields) > 0 { - properties["fields"] = f.MultiFields.process("", f.esVersion) + fields := common.MapStr{} + f.MultiFields.process("", f.esVersion, fields) + properties["fields"] = fields } return properties diff --git a/libbeat/template/field_test.go b/libbeat/template/field_test.go index fe8a9a2728f..6a5e56e6fd8 100644 --- a/libbeat/template/field_test.go +++ b/libbeat/template/field_test.go @@ -3,9 +3,10 @@ package template import ( "testing" + "github.com/elastic/beats/libbeat/common" + "github.com/stretchr/testify/assert" - "github.com/elastic/beats/libbeat/common" "github.com/elastic/go-ucfg/yaml" ) diff --git a/libbeat/template/fields.go b/libbeat/template/fields.go index ae08718036a..937c0f6a398 100644 --- a/libbeat/template/fields.go +++ b/libbeat/template/fields.go @@ -1,6 +1,7 @@ package template import ( + "errors" "strings" "github.com/elastic/beats/libbeat/common" @@ -12,9 +13,7 @@ var ( type Fields []Field -func (f Fields) process(path string, esVersion common.Version) common.MapStr { - output := common.MapStr{} - +func (f Fields) process(path string, esVersion common.Version, output common.MapStr) error { for _, field := range f { var mapping common.MapStr @@ -54,7 +53,25 @@ func (f Fields) process(path string, esVersion common.Version) common.MapStr { if field.Dynamic.value != nil { mapping["dynamic"] = field.Dynamic.value } - mapping["properties"] = field.Fields.process(newPath, esVersion) + + // Combine properties with previous field definitions (if any) + properties := common.MapStr{} + key := generateKey(field.Name) + ".properties" + currentProperties, err := output.GetValue(key) + if err == nil { + var ok bool + properties, ok = currentProperties.(common.MapStr) + if !ok { + // This should never happen + return errors.New(key + " is expected to be a MapStr") + } + } + + if err := field.Fields.process(newPath, esVersion, properties); err != nil { + return err + } + mapping["properties"] = properties + default: mapping = field.other() } @@ -64,7 +81,7 @@ func (f Fields) process(path string, esVersion common.Version) common.MapStr { } } - return output + return nil } // HasKey checks if inside fields the given key exists diff --git a/libbeat/template/fields_test.go b/libbeat/template/fields_test.go index 27b45d7f47d..d39fa131afc 100644 --- a/libbeat/template/fields_test.go +++ b/libbeat/template/fields_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "github.com/elastic/beats/libbeat/common" ) func TestHasKey(t *testing.T) { @@ -57,3 +59,52 @@ func TestHasKey(t *testing.T) { assert.Equal(t, test.result, test.fields.HasKey(test.key)) } } + +func TestPropertiesCombine(t *testing.T) { + // Test common fields are combined even if they come from different objects + fields := Fields{ + Field{ + Name: "test", + Type: "group", + Fields: Fields{ + Field{ + Name: "one", + Type: "text", + }, + }, + }, + Field{ + Name: "test", + Type: "group", + Fields: Fields{ + Field{ + Name: "two", + Type: "text", + }, + }, + }, + } + + output := common.MapStr{} + version, err := common.NewVersion("6.0.0") + if err != nil { + t.Fatal(err) + } + + err = fields.process("", *version, output) + if err != nil { + t.Fatal(err) + } + + v1, err := output.GetValue("test.properties.one") + if err != nil { + t.Fatal(err) + } + v2, err := output.GetValue("test.properties.two") + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, v1, common.MapStr{"type": "text", "norms": false}) + assert.Equal(t, v2, common.MapStr{"type": "text", "norms": false}) +} diff --git a/libbeat/template/template.go b/libbeat/template/template.go index 97995d69a8e..646614f6fc3 100644 --- a/libbeat/template/template.go +++ b/libbeat/template/template.go @@ -99,7 +99,10 @@ func (t *Template) Load(file string) (common.MapStr, error) { } // Start processing at the root - properties := fields.process("", t.esVersion) + properties := common.MapStr{} + if err := fields.process("", t.esVersion, properties); err != nil { + return nil, err + } output := t.generate(properties, dynamicTemplates) return output, nil