Restrict template name when putting index template#53970
Restrict template name when putting index template#53970spinscale wants to merge 1 commit intoelastic:masterfrom
Conversation
Storing an index template named `*` will result in the in ability to delete it. Also that naming is really confusing.
|
Pinging @elastic/es-search (:Search/Mapping) |
cbuescher
left a comment
There was a problem hiding this comment.
Hi @spinscale, I took a look and the asterisk behaviour looks really trappy indeed, especially for deletion.
I pretty much like the logic in MetaDataCreateIndexService.validateIndexOrAliasName
There seems to be naming conventions for template names already, (see MetadataIndexTemplateService#validate()), so I would probably look into adding an additional case there.
Unfortunately most of the rules checked there look pretty much undocumented, maybe you could also add a few lines to the docs in terms of documentation?
Another more general issue I'm curious about wrt to this PR: poking around I found that in the course of the work for "Index Templates v2" (#53101) we are adding other code paths to putting index templates. I wonder if validation of names is or will be also included there, currently it doesn't seem to be. Maybe @martijnvg or somebody else from @elastic/es-core-infra can comment on this aspect? Also they might have an opinion about the breaking nature of an additional restriction in naming rules.
|
Pinging @elastic/es-core-features (:Core/Features/Indices APIs) |
|
@andreidan thanks for correcting the assigned team, I would mostly like to get an opinion on where to best put the proposed validation (dissallow asterisks) on template names. Currently most of the restrictions are checked in |
Yes, re-using the logic would be best. I'll add this as a todo for the meta issue #53101 |
This commit changes the validation for V2 index and component templates to re-use the same validation that V1 templates used. This includes things like invalid template names, index patterns, etc. This also adds validation that template names do not contain `*`. Supercedes elastic#53970 Relates to elastic#53101
|
I opened #56170 to address this in a generic way for v1 and v2 templates. |
|
@spinscale with #56170 open which should adress your issue, I hope you don't mind if I close this. |
This commit changes the validation for V2 index and component templates to re-use the same validation that V1 templates used. This includes things like invalid template names, index patterns, etc. This also adds validation that template names do not contain `*` and index patterns do not contain `:` (index names can't contain this regardless). Supercedes #53970 Relates to #53101 Resolves #43737 Resolves #46045
This commit changes the validation for V2 index and component templates to re-use the same validation that V1 templates used. This includes things like invalid template names, index patterns, etc. This also adds validation that template names do not contain `*` and index patterns do not contain `:` (index names can't contain this regardless). Supercedes elastic#53970 Relates to elastic#53101 Resolves elastic#43737 Resolves elastic#46045
Storing an index template named
*will result in the in ability todelete it. Also that naming is really confusing.
Reviewers Note: I pretty much like the logic in
MetaDataCreateIndexService.validateIndexOrAliasNamethat we could probably use for templates as well. I just added the smallest fix to prevent the above behaviour.This change breaks BWC, but I still think it's worth getting in (in master), as this happens due to user mistakes. See https://discuss.elastic.co/t/template-with-asterisk-name/224452