From 7274f6e2e152928418a677fcb6b9c3550b8a6227 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 31 Jan 2024 15:51:39 +0100 Subject: [PATCH] [Fleet] Fix conflicting dynamic template mappings for intermediate objects (#175970) When there are multiple dynamic templates with the same name, we only use the first appearance. Add an exception for intermediate objects, so we use the last appearance. This way we avoid that less specific dynamic templates created for intermediate objects match before more specific dynamic templates defined in packages. This was a problem for example in the Prometheus package, where there are definitions for `prometheus.*.value` and `prometheus.*.histogram`. For both cases Fleet tries to create a dynamic template for the intermediate objects `prometheus.*`. If this dynamic template is inserted before the dynamic template for the histograms, it matches for them, causing mapping errors on ingestion, like the following one: ``` mapper [prometheus.queue_duration.histogram.values] cannot be changed from type [float] to [long] ``` With this change, when multiple definitions generate the same dynamic template for intermediate objects, only the last one ends up in the template. These intermediate objects were introduced in 8.12, after https://github.com/elastic/kibana/pull/169981. --- .../__snapshots__/template.test.ts.snap | 20 +++---- .../elasticsearch/template/template.test.ts | 57 +++++++++++++++++++ .../epm/elasticsearch/template/template.ts | 18 ++++-- 3 files changed, 80 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/__snapshots__/template.test.ts.snap b/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/__snapshots__/template.test.ts.snap index ea615318fe696..eba963feced11 100644 --- a/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/__snapshots__/template.test.ts.snap +++ b/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/__snapshots__/template.test.ts.snap @@ -93,16 +93,6 @@ exports[`EPM template tests loading cockroachdb_dynamic_templates.yml: cockroach "path_match": "cockroachdb.status.*.value" } }, - { - "cockroachdb.status.*": { - "mapping": { - "type": "object", - "dynamic": true - }, - "match_mapping_type": "object", - "path_match": "cockroachdb.status.*" - } - }, { "cockroachdb.status.*.counter": { "mapping": { @@ -129,6 +119,16 @@ exports[`EPM template tests loading cockroachdb_dynamic_templates.yml: cockroach "match_mapping_type": "*", "path_match": "cockroachdb.status.*.histogram" } + }, + { + "cockroachdb.status.*": { + "mapping": { + "type": "object", + "dynamic": true + }, + "match_mapping_type": "object", + "path_match": "cockroachdb.status.*" + } } ] } diff --git a/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.test.ts b/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.test.ts index 670ba36ed6a8d..0e354b40ec38c 100644 --- a/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.test.ts +++ b/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.test.ts @@ -1430,6 +1430,63 @@ describe('EPM template', () => { expect(mappings).toEqual(runtimeFieldMapping); }); + it('tests processing dynamic templates priority of intermediate objects', () => { + const textWithRuntimeFieldsLiteralYml = ` +- name: group.*.value + type: object + object_type: double + object_type_mapping_type: "*" + metric_type: gauge +- name: group.*.histogram + type: object + object_type: histogram + object_type_mapping_type: "*" +`; + const runtimeFieldMapping = { + properties: { + group: { + type: 'object', + dynamic: true, + }, + }, + dynamic_templates: [ + { + 'group.*.value': { + match_mapping_type: '*', + path_match: 'group.*.value', + mapping: { + time_series_metric: 'gauge', + type: 'double', + }, + }, + }, + { + 'group.*.histogram': { + path_match: 'group.*.histogram', + match_mapping_type: '*', + mapping: { + type: 'histogram', + }, + }, + }, + { + 'group.*': { + path_match: 'group.*', + match_mapping_type: 'object', + mapping: { + type: 'object', + dynamic: true, + }, + }, + }, + ], + }; + const fields: Field[] = safeLoad(textWithRuntimeFieldsLiteralYml); + const processedFields = processFields(fields); + const mappings = generateMappings(processedFields, true); + expect(mappings).toEqual(runtimeFieldMapping); + }); + it('tests unexpected type for field as dynamic template fails', () => { const textWithRuntimeFieldsLiteralYml = ` - name: labels.* diff --git a/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts b/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts index a60e9dc0c7ced..5d8156118c049 100644 --- a/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts +++ b/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts @@ -149,7 +149,7 @@ export function generateMappings( isIndexModeTimeSeries = false ): IndexTemplateMappings { const dynamicTemplates: Array> = []; - const dynamicTemplateNames = new Set(); + const dynamicTemplateNames: Record = {}; const runtimeFields: RuntimeFields = {}; const { properties } = _generateMappings( @@ -163,8 +163,16 @@ export function generateMappings( runtimeProperties?: Properties; }) => { const name = dynamicMapping.path; - if (dynamicTemplateNames.has(name)) { - return; + if (name in dynamicTemplateNames) { + if (name.includes('*') && dynamicMapping.properties?.type === 'object') { + // This is a conflicting intermediate object, use the last one so + // more specific templates are chosen before. + const index = dynamicTemplateNames[name]; + delete dynamicTemplateNames[name]; + dynamicTemplates.splice(index, 1); + } else { + return; + } } const dynamicTemplate: Properties = {}; @@ -182,8 +190,8 @@ export function generateMappings( dynamicTemplate.path_match = dynamicMapping.pathMatch; } - dynamicTemplateNames.add(name); - dynamicTemplates.push({ [dynamicMapping.path]: dynamicTemplate }); + const size = dynamicTemplates.push({ [name]: dynamicTemplate }); + dynamicTemplateNames[name] = size - 1; }, addRuntimeField: (runtimeField: { path: string; properties: Properties }) => { runtimeFields[`${runtimeField.path}`] = runtimeField.properties;