Skip to content

Commit 4761a1f

Browse files
martijnvgmartijnvg
authored
Do not return all indices if a specific alias is requested via get aliases api. (#29538)
If a get alias api call requests a specific alias pattern then indices not having any matching aliases should not be included in the response. This is a second attempt to fix this (first attempt was #28294). The reason that the first attempt was reverted is because when xpack security is enabled then index expression (like * or _all) are resolved prior to when a request is processed in the get aliases transport action, then `MetaData#findAliases` can't know whether requested all where requested since it was already expanded in concrete alias names. This change replaces aliases(...) replaceAliases(...) method on AliasesRequests class and leave the aliases(...) method on subclasses. So there is a distinction between when xpack security replaces aliases and a user setting aliases via the transport or high level http client. Closes #27763
1 parent a5f5ea8 commit 4761a1f

File tree

9 files changed

+141
-31
lines changed

9 files changed

+141
-31
lines changed

‎server/src/main/java/org/elasticsearch/action/AliasesRequest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,11 @@ public interface AliasesRequest extends IndicesRequest.Replaceable {
3333
String[] aliases();
3434

3535
/**
36-
* Sets the array of aliases that the action relates to
36+
* Replaces current aliases with the provided aliases.
37+
*
38+
* Sometimes aliases expressions need to be resolved to concrete aliases prior to executing the transport action.
3739
*/
38-
AliasesRequest aliases(String... aliases);
40+
void replaceAliases(String... aliases);
3941

4042
/**
4143
* Returns true if wildcards expressions among aliases should be resolved, false otherwise

‎server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,6 @@ public AliasActions index(String index) {
302302
/**
303303
* Aliases to use with this action.
304304
*/
305-
@Override
306305
public AliasActions aliases(String... aliases) {
307306
if (type == AliasActions.Type.REMOVE_INDEX) {
308307
throw new IllegalArgumentException("[aliases] is unsupported for [" + type + "]");
@@ -428,6 +427,11 @@ public String[] aliases() {
428427
return aliases;
429428
}
430429

430+
@Override
431+
public void replaceAliases(String... aliases) {
432+
this.aliases = aliases;
433+
}
434+
431435
@Override
432436
public boolean expandAliasesWildcards() {
433437
//remove operations support wildcards among aliases, add operations don't

‎server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.elasticsearch.action.admin.indices.alias.get;
2020

21+
import org.elasticsearch.Version;
2122
import org.elasticsearch.action.ActionRequestValidationException;
2223
import org.elasticsearch.action.AliasesRequest;
2324
import org.elasticsearch.action.support.IndicesOptions;
@@ -32,15 +33,12 @@ public class GetAliasesRequest extends MasterNodeReadRequest<GetAliasesRequest>
3233

3334
private String[] indices = Strings.EMPTY_ARRAY;
3435
private String[] aliases = Strings.EMPTY_ARRAY;
35-
3636
private IndicesOptions indicesOptions = IndicesOptions.strictExpand();
37+
private String[] originalAliases = Strings.EMPTY_ARRAY;
3738

38-
public GetAliasesRequest(String[] aliases) {
39+
public GetAliasesRequest(String... aliases) {
3940
this.aliases = aliases;
40-
}
41-
42-
public GetAliasesRequest(String alias) {
43-
this.aliases = new String[]{alias};
41+
this.originalAliases = aliases;
4442
}
4543

4644
public GetAliasesRequest() {
@@ -51,6 +49,9 @@ public GetAliasesRequest(StreamInput in) throws IOException {
5149
indices = in.readStringArray();
5250
aliases = in.readStringArray();
5351
indicesOptions = IndicesOptions.readIndicesOptions(in);
52+
if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
53+
originalAliases = in.readStringArray();
54+
}
5455
}
5556

5657
@Override
@@ -59,6 +60,9 @@ public void writeTo(StreamOutput out) throws IOException {
5960
out.writeStringArray(indices);
6061
out.writeStringArray(aliases);
6162
indicesOptions.writeIndicesOptions(out);
63+
if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
64+
out.writeStringArray(originalAliases);
65+
}
6266
}
6367

6468
@Override
@@ -67,9 +71,9 @@ public GetAliasesRequest indices(String... indices) {
6771
return this;
6872
}
6973

70-
@Override
7174
public GetAliasesRequest aliases(String... aliases) {
7275
this.aliases = aliases;
76+
this.originalAliases = aliases;
7377
return this;
7478
}
7579

@@ -88,6 +92,18 @@ public String[] aliases() {
8892
return aliases;
8993
}
9094

95+
@Override
96+
public void replaceAliases(String... aliases) {
97+
this.aliases = aliases;
98+
}
99+
100+
/**
101+
* Returns the aliases as was originally specified by the user
102+
*/
103+
public String[] getOriginalAliases() {
104+
return originalAliases;
105+
}
106+
91107
@Override
92108
public boolean expandAliasesWildcards() {
93109
return true;

‎server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.elasticsearch.threadpool.ThreadPool;
3434
import org.elasticsearch.transport.TransportService;
3535

36+
import java.util.Collections;
3637
import java.util.List;
3738

3839
public class TransportGetAliasesAction extends TransportMasterNodeReadAction<GetAliasesRequest, GetAliasesResponse> {
@@ -62,7 +63,24 @@ protected GetAliasesResponse newResponse() {
6263
@Override
6364
protected void masterOperation(GetAliasesRequest request, ClusterState state, ActionListener<GetAliasesResponse> listener) {
6465
String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(state, request);
65-
ImmutableOpenMap<String, List<AliasMetaData>> result = state.metaData().findAliases(request.aliases(), concreteIndices);
66-
listener.onResponse(new GetAliasesResponse(result));
66+
ImmutableOpenMap<String, List<AliasMetaData>> aliases = state.metaData().findAliases(request.aliases(), concreteIndices);
67+
listener.onResponse(new GetAliasesResponse(postProcess(request, concreteIndices, aliases)));
6768
}
69+
70+
/**
71+
* Fills alias result with empty entries for requested indices when no specific aliases were requested.
72+
*/
73+
static ImmutableOpenMap<String, List<AliasMetaData>> postProcess(GetAliasesRequest request, String[] concreteIndices,
74+
ImmutableOpenMap<String, List<AliasMetaData>> aliases) {
75+
boolean noAliasesSpecified = request.getOriginalAliases() == null || request.getOriginalAliases().length == 0;
76+
ImmutableOpenMap.Builder<String, List<AliasMetaData>> mapBuilder = ImmutableOpenMap.builder(aliases);
77+
for (String index : concreteIndices) {
78+
if (aliases.get(index) == null && noAliasesSpecified) {
79+
List<AliasMetaData> previous = mapBuilder.put(index, Collections.emptyList());
80+
assert previous == null;
81+
}
82+
}
83+
return mapBuilder.build();
84+
}
85+
6886
}

‎server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,7 @@ public ImmutableOpenMap<String, List<AliasMetaData>> findAliases(final String[]
265265

266266
boolean matchAllAliases = matchAllAliases(aliases);
267267
ImmutableOpenMap.Builder<String, List<AliasMetaData>> mapBuilder = ImmutableOpenMap.builder();
268-
Iterable<String> intersection = HppcMaps.intersection(ObjectHashSet.from(concreteIndices), indices.keys());
269-
for (String index : intersection) {
268+
for (String index : concreteIndices) {
270269
IndexMetaData indexMetaData = indices.get(index);
271270
List<AliasMetaData> filteredValues = new ArrayList<>();
272271
for (ObjectCursor<AliasMetaData> cursor : indexMetaData.getAliases().values()) {
@@ -276,11 +275,11 @@ public ImmutableOpenMap<String, List<AliasMetaData>> findAliases(final String[]
276275
}
277276
}
278277

279-
if (!filteredValues.isEmpty()) {
278+
if (filteredValues.isEmpty() == false) {
280279
// Make the list order deterministic
281280
CollectionUtil.timSort(filteredValues, Comparator.comparing(AliasMetaData::alias));
281+
mapBuilder.put(index, Collections.unmodifiableList(filteredValues));
282282
}
283-
mapBuilder.put(index, Collections.unmodifiableList(filteredValues));
284283
}
285284
return mapBuilder.build();
286285
}

‎server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetAliasesAction.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ public String getName() {
7777

7878
@Override
7979
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
80+
// The TransportGetAliasesAction was improved do the same post processing as is happening here.
81+
// We can't remove this logic yet to support mixed clusters. We should be able to remove this logic here
82+
// in when 8.0 becomes the new version in the master branch.
83+
8084
final boolean namesProvided = request.hasParam("name");
8185
final String[] aliases = request.paramAsStringArrayOrEmptyIfAll("name");
8286
final GetAliasesRequest getAliasesRequest = new GetAliasesRequest(aliases);

‎server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.elasticsearch.action.admin.indices.alias.get;
20+
21+
import org.elasticsearch.cluster.metadata.AliasMetaData;
22+
import org.elasticsearch.common.collect.ImmutableOpenMap;
23+
import org.elasticsearch.test.ESTestCase;
24+
25+
import java.util.Collections;
26+
import java.util.List;
27+
28+
import static org.hamcrest.Matchers.equalTo;
29+
30+
public class TransportGetAliasesActionTests extends ESTestCase {
31+
32+
public void testPostProcess() {
33+
GetAliasesRequest request = new GetAliasesRequest();
34+
ImmutableOpenMap<String, List<AliasMetaData>> aliases = ImmutableOpenMap.<String, List<AliasMetaData>>builder()
35+
.fPut("b", Collections.singletonList(new AliasMetaData.Builder("y").build()))
36+
.build();
37+
ImmutableOpenMap<String, List<AliasMetaData>> result =
38+
TransportGetAliasesAction.postProcess(request, new String[]{"a", "b", "c"}, aliases);
39+
assertThat(result.size(), equalTo(3));
40+
assertThat(result.get("a").size(), equalTo(0));
41+
assertThat(result.get("b").size(), equalTo(1));
42+
assertThat(result.get("c").size(), equalTo(0));
43+
44+
request = new GetAliasesRequest();
45+
request.replaceAliases("y", "z");
46+
aliases = ImmutableOpenMap.<String, List<AliasMetaData>>builder()
47+
.fPut("b", Collections.singletonList(new AliasMetaData.Builder("y").build()))
48+
.build();
49+
result = TransportGetAliasesAction.postProcess(request, new String[]{"a", "b", "c"}, aliases);
50+
assertThat(result.size(), equalTo(3));
51+
assertThat(result.get("a").size(), equalTo(0));
52+
assertThat(result.get("b").size(), equalTo(1));
53+
assertThat(result.get("c").size(), equalTo(0));
54+
55+
request = new GetAliasesRequest("y", "z");
56+
aliases = ImmutableOpenMap.<String, List<AliasMetaData>>builder()
57+
.fPut("b", Collections.singletonList(new AliasMetaData.Builder("y").build()))
58+
.build();
59+
result = TransportGetAliasesAction.postProcess(request, new String[]{"a", "b", "c"}, aliases);
60+
assertThat(result.size(), equalTo(1));
61+
assertThat(result.get("b").size(), equalTo(1));
62+
}
63+
64+
}

‎server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -570,24 +570,20 @@ public void testIndicesGetAliases() throws Exception {
570570
logger.info("--> getting alias1");
571571
GetAliasesResponse getResponse = admin().indices().prepareGetAliases("alias1").get();
572572
assertThat(getResponse, notNullValue());
573-
assertThat(getResponse.getAliases().size(), equalTo(5));
573+
assertThat(getResponse.getAliases().size(), equalTo(1));
574574
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(1));
575575
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
576576
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("alias1"));
577577
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
578578
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), nullValue());
579579
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), nullValue());
580-
assertTrue(getResponse.getAliases().get("test").isEmpty());
581-
assertTrue(getResponse.getAliases().get("test123").isEmpty());
582-
assertTrue(getResponse.getAliases().get("foobarbaz").isEmpty());
583-
assertTrue(getResponse.getAliases().get("bazbar").isEmpty());
584580
AliasesExistResponse existsResponse = admin().indices().prepareAliasesExist("alias1").get();
585581
assertThat(existsResponse.exists(), equalTo(true));
586582

587583
logger.info("--> getting all aliases that start with alias*");
588584
getResponse = admin().indices().prepareGetAliases("alias*").get();
589585
assertThat(getResponse, notNullValue());
590-
assertThat(getResponse.getAliases().size(), equalTo(5));
586+
assertThat(getResponse.getAliases().size(), equalTo(1));
591587
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(2));
592588
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
593589
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("alias1"));
@@ -599,10 +595,6 @@ public void testIndicesGetAliases() throws Exception {
599595
assertThat(getResponse.getAliases().get("foobar").get(1).getFilter(), nullValue());
600596
assertThat(getResponse.getAliases().get("foobar").get(1).getIndexRouting(), nullValue());
601597
assertThat(getResponse.getAliases().get("foobar").get(1).getSearchRouting(), nullValue());
602-
assertTrue(getResponse.getAliases().get("test").isEmpty());
603-
assertTrue(getResponse.getAliases().get("test123").isEmpty());
604-
assertTrue(getResponse.getAliases().get("foobarbaz").isEmpty());
605-
assertTrue(getResponse.getAliases().get("bazbar").isEmpty());
606598
existsResponse = admin().indices().prepareAliasesExist("alias*").get();
607599
assertThat(existsResponse.exists(), equalTo(true));
608600

@@ -687,13 +679,12 @@ public void testIndicesGetAliases() throws Exception {
687679
logger.info("--> getting f* for index *bar");
688680
getResponse = admin().indices().prepareGetAliases("f*").addIndices("*bar").get();
689681
assertThat(getResponse, notNullValue());
690-
assertThat(getResponse.getAliases().size(), equalTo(2));
682+
assertThat(getResponse.getAliases().size(), equalTo(1));
691683
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
692684
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("foo"));
693685
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
694686
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), nullValue());
695687
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), nullValue());
696-
assertTrue(getResponse.getAliases().get("bazbar").isEmpty());
697688
existsResponse = admin().indices().prepareAliasesExist("f*")
698689
.addIndices("*bar").get();
699690
assertThat(existsResponse.exists(), equalTo(true));
@@ -702,14 +693,13 @@ public void testIndicesGetAliases() throws Exception {
702693
logger.info("--> getting f* for index *bac");
703694
getResponse = admin().indices().prepareGetAliases("foo").addIndices("*bac").get();
704695
assertThat(getResponse, notNullValue());
705-
assertThat(getResponse.getAliases().size(), equalTo(2));
696+
assertThat(getResponse.getAliases().size(), equalTo(1));
706697
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(1));
707698
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
708699
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("foo"));
709700
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
710701
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), nullValue());
711702
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), nullValue());
712-
assertTrue(getResponse.getAliases().get("bazbar").isEmpty());
713703
existsResponse = admin().indices().prepareAliasesExist("foo")
714704
.addIndices("*bac").get();
715705
assertThat(existsResponse.exists(), equalTo(true));
@@ -727,6 +717,19 @@ public void testIndicesGetAliases() throws Exception {
727717
.addIndices("foobar").get();
728718
assertThat(existsResponse.exists(), equalTo(true));
729719

720+
for (String aliasName : new String[]{null, "_all", "*"}) {
721+
logger.info("--> getting {} alias for index foobar", aliasName);
722+
getResponse = aliasName != null ? admin().indices().prepareGetAliases(aliasName).addIndices("foobar").get() :
723+
admin().indices().prepareGetAliases().addIndices("foobar").get();
724+
assertThat(getResponse, notNullValue());
725+
assertThat(getResponse.getAliases().size(), equalTo(1));
726+
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(4));
727+
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("alias1"));
728+
assertThat(getResponse.getAliases().get("foobar").get(1).alias(), equalTo("alias2"));
729+
assertThat(getResponse.getAliases().get("foobar").get(2).alias(), equalTo("bac"));
730+
assertThat(getResponse.getAliases().get("foobar").get(3).alias(), equalTo("foo"));
731+
}
732+
730733
// alias at work again
731734
logger.info("--> getting * for index *bac");
732735
getResponse = admin().indices().prepareGetAliases("*").addIndices("*bac").get();

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
200200
if (aliasesRequest.expandAliasesWildcards()) {
201201
List<String> aliases = replaceWildcardsWithAuthorizedAliases(aliasesRequest.aliases(),
202202
loadAuthorizedAliases(authorizedIndices.get(), metaData));
203-
aliasesRequest.aliases(aliases.toArray(new String[aliases.size()]));
203+
aliasesRequest.replaceAliases(aliases.toArray(new String[aliases.size()]));
204204
}
205205
if (indicesReplacedWithNoIndices) {
206206
if (indicesRequest instanceof GetAliasesRequest == false) {

0 commit comments

Comments
 (0)