ITV2: disallow duplicate dynamic templates#56291
Merged
andreidan merged 6 commits intoelastic:masterfrom May 12, 2020
Merged
Conversation
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.
Collaborator
|
Pinging @elastic/es-core-features (:Core/Features/Indices APIs) |
Member
|
@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. |
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 |
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.
Contributor
Author
|
@elasticmachine update branch |
dakrone
requested changes
May 7, 2020
Member
dakrone
left a comment
There was a problem hiding this comment.
I left a couple of comments similar to what we talked about regarding naming, thanks for doing this Andrei!
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java
Outdated
Show resolved
Hide resolved
dakrone
approved these changes
May 7, 2020
Member
dakrone
left a comment
There was a problem hiding this comment.
LGTM, thanks for working on this Andrei!
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 <andrei.dan@elastic.co>
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 <andrei.dan@elastic.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_templatesarray.Relates to #53101
Relates to #53326
Relates to #28988