-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Disallow : in cluster and index/alias names #26247
Merged
Merged
Conversation
This file contains 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
We use `:` for cross-cluster search (eg `cluster:index`), therefore, we should not allow the ambiguity when allowing cluster or index names. Relates to elastic#23892
rjernst
approved these changes
Aug 16, 2017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dakrone
added a commit
that referenced
this pull request
Aug 17, 2017
We use `:` for cross-cluster search (eg `cluster:index`), therefore, we should not allow the ambiguity when allowing cluster or index names. Relates to #23892
dakrone
added a commit
that referenced
this pull request
Aug 17, 2017
We use `:` for cross-cluster search (eg `cluster:index`), therefore, we should not allow the ambiguity when allowing cluster or index names. Relates to #23892
clintongormley
added
:Distributed Indexing/Distributed
A catch all label for anything in the Distributed Area. Please avoid if you can.
and removed
:Cluster
labels
Feb 13, 2018
javanna
added a commit
that referenced
this pull request
Sep 25, 2018
We currently fallback to local indices whenever a remote cluster is not found, as there may still be indices / aliases with the same name. Such behaviour is lenient but needs to be kept for backwards compatibility. Clarified that in the code so we don't forget. Relates to #26247
javanna
added a commit
to javanna/elasticsearch
that referenced
this pull request
Sep 25, 2018
We currently fallback to local indices whenever a remote cluster is not found, as there may still be indices / aliases with the same name. Such behaviour is lenient but needs to be kept for backwards compatibility. Clarified that in the code so we don't forget. Relates to elastic#26247
kcm
pushed a commit
that referenced
this pull request
Oct 30, 2018
We currently fallback to local indices whenever a remote cluster is not found, as there may still be indices / aliases with the same name. Such behaviour is lenient but needs to be kept for backwards compatibility. Clarified that in the code so we don't forget. Relates to #26247
This was referenced Nov 29, 2018
javanna
added a commit
to javanna/elasticsearch
that referenced
this pull request
Jun 24, 2021
We support the cluster:index syntax in all the API that support cross-cluster calls. Those API will extract remote indices, properly resolve them, and resolve locally the local indices. API that don't support this syntax though end up attempting to resolve such indices locally, which in most cases leads to an index not found exception depending on how ignore_unavailable is configured for the API. The reason for treating these index names as local is that we used to support ':' in index names, but that is no longer supported since 7.x. That means that 7.x may still have indices with ':' in their names from 6.x, but 8.x won't. We can then switch 8.0 to throw a more specific error in place of the index not found, to signal that remote indices have been requested in the context of an API that does not support cross cluster calls. relates to elastic#26247
javanna
added a commit
that referenced
this pull request
Jun 29, 2021
We support the cluster:index syntax in all the API that support cross-cluster calls. Those API will extract remote indices, properly resolve them, and resolve locally the local indices. API that don't support this syntax though end up attempting to resolve such indices locally, which in most cases leads to an index not found exception depending on how ignore_unavailable is configured for the API. The reason for treating these index names as local is that we used to support ':' in index names, but that is no longer supported since 7.x. That means that 7.x may still have indices with ':' in their names from 6.x, but 8.x won't. We can then switch 8.0 to throw a more specific error in place of the index not found, to signal that remote indices have been requested in the context of an API that does not support cross cluster calls. relates to #26247
javanna
added a commit
to javanna/elasticsearch
that referenced
this pull request
Jun 30, 2021
We support the cluster:index syntax in all the API that support cross-cluster calls. Those API will extract remote indices, properly resolve them, and resolve locally the local indices. API that don't support this syntax though end up attempting to resolve such indices locally, which in most cases leads to an index not found exception depending on how ignore_unavailable is configured for the API. The reason for treating these index names as local is that we used to support ':' in index names, but that is no longer supported since 7.x. That means that 7.x may still have indices with ':' in their names from 6.x though. Silently failing makes it hard for users to know that they are even relying on a feature that is not supported, hence we'd like to start throwing error also in 7.x, similarly to what we did in elastic#74556. This commit introduces a check for remote indices that are locally resolved, which is an indication of cross cluster syntax used in API that don't support cross cluster calls. We then check if that index exists in the local cluster, and if so we proceed to resolve it as usual. If not, we throw a specific error that makes it clear to users that they are relying on cross cluster calls calling API that does not support them. relates to elastic#26247
javanna
added a commit
that referenced
this pull request
Jul 1, 2021
We support the cluster:index syntax in all the API that support cross-cluster calls. Those API will extract remote indices, properly resolve them, and resolve locally the local indices. API that don't support this syntax though end up attempting to resolve such indices locally, which in most cases leads to an index not found exception depending on how ignore_unavailable is configured for the API. The reason for treating these index names as local is that we used to support ':' in index names, but that is no longer supported since 7.x. That means that 7.x may still have indices with ':' in their names from 6.x though. Silently failing makes it hard for users to know that they are even relying on a feature that is not supported, hence we'd like to start throwing error also in 7.x, similarly to what we did in #74556. This commit introduces a check for remote indices that are locally resolved, which is an indication of cross cluster syntax used in API that don't support cross cluster calls. We then check if that index exists in the local cluster, and if so we proceed to resolve it as usual. If not, we throw a specific error that makes it clear to users that they are relying on cross cluster calls calling API that does not support them. relates to #26247
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
>breaking
:Distributed Indexing/Distributed
A catch all label for anything in the Distributed Area. Please avoid if you can.
v6.0.0-beta2
v7.0.0-beta1
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.
We use
:
for cross-cluster search (egcluster:index
), therefore, we shouldnot allow the ambiguity when allowing cluster or index names.
Relates to #23892
This is for master (7.0+) with the intent to change it to deprecation warning/logging when backporting to the 6.0 and 6.x branches.