libbeat/template: duplicate entries in fields.yml leads to repeated d…

…ynamic templates (elastic#23240) * libbeat/template: deduplicate dynamic templates In case of duplicate fields, do not generate duplicate dynamic_template items. We already deduplicate field mappings because we update a map. * libbeat/template: preserve dynamic template order (cherry picked from commit c0bfea4)
axw · Feb 6, 2021 · 66e66e0 · 66e66e0
1 parent 4b74e6d
commit 66e66e0
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 21 deletions.
diff --git a/libbeat/template/processor.go b/libbeat/template/processor.go
@@ -31,6 +31,11 @@ type Processor struct {
 	EsVersion       common.Version
 	Migration       bool
 	ElasticLicensed bool
+
+	// dynamicTemplatesMap records which dynamic templates have been added, to prevent duplicates.
+	dynamicTemplatesMap map[dynamicTemplateKey]common.MapStr
+	// dynamicTemplates records the dynamic templates in the order they were added.
+	dynamicTemplates []common.MapStr
 }
 
 var (
@@ -420,7 +425,7 @@ func (p *Processor) object(f *mapping.Field) common.MapStr {
 		if len(otParams) > 1 {
 			path = fmt.Sprintf("%s_%s", path, matchingType)
 		}
-		addDynamicTemplate(path, pathMatch, dynProperties, matchingType)
+		p.addDynamicTemplate(path, pathMatch, dynProperties, matchingType)
 	}
 
 	properties := getDefaultProperties(f)
@@ -436,17 +441,36 @@ func (p *Processor) object(f *mapping.Field) common.MapStr {
 	return properties
 }
 
-func addDynamicTemplate(path string, pathMatch string, properties common.MapStr, matchType string) {
-	template := common.MapStr{
+type dynamicTemplateKey struct {
+	path      string
+	pathMatch string
+	matchType string
+}
+
+func (p *Processor) addDynamicTemplate(path string, pathMatch string, properties common.MapStr, matchType string) {
+	key := dynamicTemplateKey{
+		path:      path,
+		pathMatch: pathMatch,
+		matchType: matchType,
+	}
+	if p.dynamicTemplatesMap == nil {
+		p.dynamicTemplatesMap = make(map[dynamicTemplateKey]common.MapStr)
+	} else {
+		if _, ok := p.dynamicTemplatesMap[key]; ok {
+			// Dynamic template already added.
+			return
+		}
+	}
+	dynamicTemplate := common.MapStr{
 		// Set the path of the field as name
 		path: common.MapStr{
 			"mapping":            properties,
 			"match_mapping_type": matchType,
 			"path_match":         pathMatch,
 		},
 	}
-
-	dynamicTemplates = append(dynamicTemplates, template)
+	p.dynamicTemplatesMap[key] = dynamicTemplate
+	p.dynamicTemplates = append(p.dynamicTemplates, dynamicTemplate)
 }
 
 func getDefaultProperties(f *mapping.Field) common.MapStr {

diff --git a/libbeat/template/processor_test.go b/libbeat/template/processor_test.go
@@ -317,7 +317,6 @@ func TestProcessor(t *testing.T) {
 }
 
 func TestDynamicTemplates(t *testing.T) {
-	p := &Processor{}
 	tests := []struct {
 		field    mapping.Field
 		expected []common.MapStr
@@ -493,9 +492,10 @@ func TestDynamicTemplates(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		dynamicTemplates = nil
+		p := &Processor{}
 		p.object(&test.field)
-		assert.Equal(t, test.expected, dynamicTemplates)
+		p.object(&test.field) // should not be added twice
+		assert.Equal(t, test.expected, p.dynamicTemplates)
 	}
 }
 

diff --git a/libbeat/template/template.go b/libbeat/template/template.go
@@ -37,9 +37,6 @@ var (
 	defaultNumberOfRoutingShards   = 30
 	defaultMaxDocvalueFieldsSearch = 200
 
-	// Array to store dynamicTemplate parts in
-	dynamicTemplates []common.MapStr
-
 	defaultFields []string
 )
 
@@ -147,7 +144,6 @@ func (t *Template) load(fields mapping.Fields) (common.MapStr, error) {
 	t.Lock()
 	defer t.Unlock()
 
-	dynamicTemplates = nil
 	defaultFields = nil
 
 	var err error
@@ -164,7 +160,8 @@ func (t *Template) load(fields mapping.Fields) (common.MapStr, error) {
 	if err := processor.Process(fields, nil, properties); err != nil {
 		return nil, err
 	}
-	output := t.Generate(properties, dynamicTemplates)
+
+	output := t.Generate(properties, processor.dynamicTemplates)
 
 	return output, nil
 }
@@ -255,17 +252,16 @@ func (t *Template) GetPattern() string {
 func (t *Template) Generate(properties common.MapStr, dynamicTemplates []common.MapStr) common.MapStr {
 	switch t.templateType {
 	case IndexTemplateLegacy:
-		return t.generateLegacy(properties)
+		return t.generateLegacy(properties, dynamicTemplates)
 	case IndexTemplateComponent:
-		return t.generateComponent(properties)
+		return t.generateComponent(properties, dynamicTemplates)
 	case IndexTemplateIndex:
-		return t.generateIndex(properties)
-	default:
+		return t.generateIndex(properties, dynamicTemplates)
 	}
 	return nil
 }
 
-func (t *Template) generateLegacy(properties common.MapStr) common.MapStr {
+func (t *Template) generateLegacy(properties common.MapStr, dynamicTemplates []common.MapStr) common.MapStr {
 	keyPattern, patterns := buildPatternSettings(t.esVersion, t.GetPattern())
 	return common.MapStr{
 		keyPattern: patterns,
@@ -284,7 +280,7 @@ func (t *Template) generateLegacy(properties common.MapStr) common.MapStr {
 	}
 }
 
-func (t *Template) generateComponent(properties common.MapStr) common.MapStr {
+func (t *Template) generateComponent(properties common.MapStr, dynamicTemplates []common.MapStr) common.MapStr {
 	return common.MapStr{
 		"template": common.MapStr{
 			"mappings": buildMappings(
@@ -302,8 +298,8 @@ func (t *Template) generateComponent(properties common.MapStr) common.MapStr {
 	}
 }
 
-func (t *Template) generateIndex(properties common.MapStr) common.MapStr {
-	tmpl := t.generateComponent(properties)
+func (t *Template) generateIndex(properties common.MapStr, dynamicTemplates []common.MapStr) common.MapStr {
+	tmpl := t.generateComponent(properties, dynamicTemplates)
 	tmpl["priority"] = t.priority
 	keyPattern, patterns := buildPatternSettings(t.esVersion, t.GetPattern())
 	tmpl[keyPattern] = patterns