From d4a6f71c48ba7f7720f0b8ce8325eb7772eb98b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= <exekias@gmail.com>
Date: Fri, 1 Sep 2017 13:13:05 +0200
Subject: [PATCH] Combine `fields.yml` properties when they are defined in
 different sources

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/fields.go      | 27 ++++++++++++++----
 libbeat/template/fields_test.go | 50 +++++++++++++++++++++++++++++++++
 libbeat/template/template.go    |  5 +++-
 5 files changed, 80 insertions(+), 7 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/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..6425e444360 100644
--- a/libbeat/template/fields_test.go
+++ b/libbeat/template/fields_test.go
@@ -3,6 +3,7 @@ package template
 import (
 	"testing"
 
+	"github.com/elastic/beats/libbeat/common"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -57,3 +58,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