Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ITV2: disallow duplicate dynamic templates #56291

Merged
merged 6 commits into from
May 12, 2020

Conversation

andreidan
Copy link
Contributor

Dynamic templates can contain multiple templates with the same name. This
commit changes the way V2 index templates mappings are resolved to add
deduplication of dynamic templates when resolving the mappings, by having
the last dynamic templates override the ones defined with the same name
earlier in the dynamic_templates array.

Relates to #53101
Relates to #53326
Relates to #28988

Dynamic templates can contain multiple templates with the same name. This
commit changes the way V2 index templates mappings are resolved to add
deduplication of dynamic templates when resolving the mappings, by having
the last dynamic templates override the ones defined with the same name
earlier in the `dynamic_templates` array.
@andreidan andreidan added :Data Management/Indices APIs APIs to create and manage indices and templates v8.0.0 v7.8.1 labels May 6, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Indices APIs)

@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label May 6, 2020
@andreidan andreidan requested a review from dakrone May 6, 2020 16:10
@martijnvg martijnvg mentioned this pull request May 6, 2020
39 tasks
@dakrone
Copy link
Member

dakrone commented May 6, 2020

@andreidan can you add a test that component templates merge their dynamic templates correctly? I think because we merge the XContent maps prior to the dedupe it is possible to end up with an invalid dynamic template if you have two component templates that each define a dynamic template with the same name but different configurations that, when merged, make the template invalid.

@andreidan
Copy link
Contributor Author

@dakrone thanks for the suggestion on the component templates merging. Great shout on testing that as you were right and we are merging the Maps so, even though we dedup the key names (ie. there will only be one dynamic_template with a particular name) the bodies are merged.

This filters duplicate dynamic templates when merging different component
templates that specify the same dynamic template and when merging the
mappings specified in the request with the ones in the index template.
@andreidan
Copy link
Contributor Author

@elasticmachine update branch

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a couple of comments similar to what we talked about regarding naming, thanks for doing this Andrei!

@andreidan andreidan requested a review from dakrone May 7, 2020 15:48
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for working on this Andrei!

@andreidan andreidan merged commit eb4a557 into elastic:master May 12, 2020
andreidan added a commit to andreidan/elasticsearch that referenced this pull request May 12, 2020
Dynamic templates can contain multiple templates with the same name. This
commit changes the way V2 index templates mappings are resolved to add
deduplication of dynamic templates when resolving the mappings, by having
the last dynamic templates override the ones defined with the same name
earlier in the `dynamic_templates` array.

This also filters duplicate dynamic templates when merging different component
templates that specify the same dynamic template and when merging the
mappings specified in the request with the ones in the index template.

(cherry picked from commit eb4a557)
Signed-off-by: Andrei Dan <[email protected]>
andreidan added a commit that referenced this pull request May 14, 2020
Dynamic templates can contain multiple templates with the same name. This
commit changes the way V2 index templates mappings are resolved to add
deduplication of dynamic templates when resolving the mappings, by having
the last dynamic templates override the ones defined with the same name
earlier in the `dynamic_templates` array.

This also filters duplicate dynamic templates when merging different component
templates that specify the same dynamic template and when merging the
mappings specified in the request with the ones in the index template.

(cherry picked from commit eb4a557)
Signed-off-by: Andrei Dan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team v7.8.1 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants