-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Remove aliases resolution limitations when security is enabled #31952
Changes from 5 commits
94a75b1
d8553ab
58dfae5
5e927d6
a6b5c78
85f6c11
b9925da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,3 +79,11 @@ the only behavior in 8.0.0, this parameter is deprecated in 7.0.0 for removal in | |
==== The deprecated stored script contexts have now been removed | ||
When putting stored scripts, support for storing them with the deprecated `template` context or without a context is | ||
now removed. Scripts must be stored using the `script` context as mentioned in the documentation. | ||
|
||
==== Get Aliases API limitations when xpack security is enabled removed | ||
|
||
When xpack security is enabled, the Get Aliases API behaviour and response | ||
code returned are now aligned to those of vanilla Elasticsearch. Previously | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion "vanilla" might be confusing, since X-Pack is included in Elasticsearch by default now. Maybe something like this might be clearer?: "The behavior and response codes of the get aliases API no longer vary depending on whether {security} is enabled. Previously..." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed, that sounds great, thank you. |
||
a 404 - NOT FOUND (IndexNotFoundException) could be returned in case the | ||
current user was not authorized for any alias. An empty response with status | ||
200 - OK is now returned instead. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,11 @@ public interface AliasesRequest extends IndicesRequest.Replaceable { | |
*/ | ||
String[] aliases(); | ||
|
||
/** | ||
* Returns the aliases as they were originally requested, before any potential name resolution | ||
*/ | ||
String[] getOriginalAliases(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you add javadocs for this method? |
||
|
||
/** | ||
* Replaces current aliases with the provided aliases. | ||
* | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -24,6 +24,7 @@ | |||
import com.carrotsearch.hppc.cursors.ObjectObjectCursor; | ||||
import org.apache.logging.log4j.Logger; | ||||
import org.apache.lucene.util.CollectionUtil; | ||||
import org.elasticsearch.action.AliasesRequest; | ||||
import org.elasticsearch.cluster.ClusterState; | ||||
import org.elasticsearch.cluster.ClusterState.FeatureAware; | ||||
import org.elasticsearch.cluster.Diff; | ||||
|
@@ -247,22 +248,54 @@ public SortedMap<String, AliasOrIndex> getAliasAndIndexLookup() { | |||
return aliasAndIndexLookup; | ||||
} | ||||
|
||||
/** | ||||
* Finds the specific index aliases that point to the specified concrete indices or match partially with the indices via wildcards. | ||||
* | ||||
* @param concreteIndices The concrete indexes the index aliases must point to order to be returned. | ||||
* @return a map of index to a list of alias metadata, the list corresponding to a concrete index will be empty if no aliases are | ||||
* present for that index | ||||
*/ | ||||
public ImmutableOpenMap<String, List<AliasMetaData>> findAllAliases(String[] concreteIndices) { | ||||
return findAliases(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, concreteIndices); | ||||
} | ||||
|
||||
/** | ||||
* Finds the specific index aliases that match with the specified aliases directly or partially via wildcards and | ||||
* that point to the specified concrete indices or match partially with the indices via wildcards. | ||||
* | ||||
* @param aliases The names of the index aliases to find | ||||
* @param aliasesRequest The request to find aliases for | ||||
* @param concreteIndices The concrete indexes the index aliases must point to order to be returned. | ||||
* @return a map of index to a list of alias metadata, the list corresponding to a concrete index will be empty if no aliases are | ||||
* present for that index | ||||
*/ | ||||
public ImmutableOpenMap<String, List<AliasMetaData>> findAliases(final AliasesRequest aliasesRequest, String[] concreteIndices) { | ||||
return findAliases(aliasesRequest.getOriginalAliases(), aliasesRequest.aliases(), concreteIndices); | ||||
} | ||||
|
||||
/** | ||||
* Finds the specific index aliases that match with the specified aliases directly or partially via wildcards and | ||||
* that point to the specified concrete indices or match partially with the indices via wildcards. | ||||
* | ||||
* @param aliases The aliases to look for | ||||
* @param originalAliases The original aliases that the user originally requested | ||||
* @param concreteIndices The concrete indexes the index aliases must point to order to be returned. | ||||
* @return a map of index to a list of alias metadata, the list corresponding to a concrete index will be empty if no aliases are | ||||
* present for that index | ||||
*/ | ||||
public ImmutableOpenMap<String, List<AliasMetaData>> findAliases(final String[] aliases, String[] concreteIndices) { | ||||
private ImmutableOpenMap<String, List<AliasMetaData>> findAliases(String[] originalAliases, String[] aliases, | ||||
String[] concreteIndices) { | ||||
assert aliases != null; | ||||
assert originalAliases != null; | ||||
assert concreteIndices != null; | ||||
if (concreteIndices.length == 0) { | ||||
return ImmutableOpenMap.of(); | ||||
} | ||||
|
||||
//if aliases were provided but they got replaced with empty aliases, return empty map | ||||
if (originalAliases.length > 0 && aliases.length == 0) { | ||||
return ImmutableOpenMap.of(); | ||||
} | ||||
|
||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @javanna I think this is wrong.
I think this should be: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ops, you raise a pretty bad issue. I think that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel my previous comment requires some clarifications. I am now certain that in 6.0 you are not allowed to create aliases starting with '-'. elasticsearch/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java Line 176 in 7bb79ed
(A test might though be nice in Given that reindex requires first creating the index, and that any index created in < 5.1, when '-' was a valid char in the name, requires reindexing in 6.x before upgrading to 7.x, I think it is safe to assume '-' cannot be part of alias names in 7.x. Therefore I will raise the PR soon! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree @albertzaharovits I think this simplifies things, I had not realized that we backported the breaking change to 5.x too. Thanks for checking. |
||||
boolean matchAllAliases = matchAllAliases(aliases); | ||||
ImmutableOpenMap.Builder<String, List<AliasMetaData>> mapBuilder = ImmutableOpenMap.builder(); | ||||
for (String index : concreteIndices) { | ||||
|
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.
I recommend using "{security}" instead of "xpack security", since the X-Pack terminology is changing in the near future.