elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.62k stars 8.22k forks source link

[Fleet] Transform mappings lost in shallow merge #175331

Closed chrisberkhout closed 8 months ago

chrisberkhout commented 9 months ago

Kibana version: v8.11.0+

Describe the bug:

A transform definition in an integration may have multiple files of field mappings, for example:

elasticsearch/transform/latest_ioc/fields/ecs.yml
elasticsearch/transform/latest_ioc/fields/ecs-extra.yml

Each file is processed and mappings are accumulated to produce the full set. However, mappings from files processed earlier will be overwritten if a later file defines a mapping for a field with the same prefix (e.g. threat.*. This is due to the shallow merge done here.

This differs from the behavior of mappings files for a data stream.

This merge logic may have the additional issue that it only processes data under the properties key, and not sibling keys such as dynamic_templates. I haven't verified this and it may be that it is correctly handled elsewhere.

Expected behavior:

A mapping for one field shouldn't overwrite the mapping for another field, even if they are defined in separate files and share a field prefix.

Any additional context:

The current merge logic was introduced in https://github.com/elastic/kibana/pull/168499.

This issue came up in https://github.com/elastic/integrations/pull/8920, where I initially had one file for overriding/extending ECS fields and another file for plain references to external ECS field definitions. To work around this issue I combined them into one file.

elasticmachine commented 9 months ago

Pinging @elastic/fleet (Team:Fleet)

nchaulet commented 8 months ago

Looking at the code it looks like we have two different approach to build mappings for template and datastreams;

For datastream, we process all fields field and then put the index template https://github.com/elastic/kibana/blob/main/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/install.ts#L512-L519

For transform we process the fields file one by one and put the new mappings.

I think we should fix this, and process the mappings for the transform the same way we do for datastream, I am trying to come with a PR for that.

chrisberkhout commented 8 months ago

Related? https://github.com/elastic/elastic-package/issues/1641

nchaulet commented 8 months ago

Related? https://github.com/elastic/elastic-package/issues/1641

@chrisberkhout I think this is a different issue, that will need to be solved in elastic-package