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
This commit is contained in:
Martijn van Groningen 2018-07-06 08:54:32 +02:00 committed by GitHub
parent a5f5ea8422
commit 4761a1fa29
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 141 additions and 31 deletions

View File

@ -33,9 +33,11 @@ public interface AliasesRequest extends IndicesRequest.Replaceable {
String[] aliases();
/**
* Sets the array of aliases that the action relates to
* Replaces current aliases with the provided aliases.
*
* Sometimes aliases expressions need to be resolved to concrete aliases prior to executing the transport action.
*/
AliasesRequest aliases(String... aliases);
void replaceAliases(String... aliases);
/**
* Returns true if wildcards expressions among aliases should be resolved, false otherwise

View File

@ -302,7 +302,6 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
/**
* Aliases to use with this action.
*/
@Override
public AliasActions aliases(String... aliases) {
if (type == AliasActions.Type.REMOVE_INDEX) {
throw new IllegalArgumentException("[aliases] is unsupported for [" + type + "]");
@ -428,6 +427,11 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
return aliases;
}
@Override
public void replaceAliases(String... aliases) {
this.aliases = aliases;
}
@Override
public boolean expandAliasesWildcards() {
//remove operations support wildcards among aliases, add operations don't

View File

@ -18,6 +18,7 @@
*/
package org.elasticsearch.action.admin.indices.alias.get;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.AliasesRequest;
import org.elasticsearch.action.support.IndicesOptions;
@ -32,15 +33,12 @@ public class GetAliasesRequest extends MasterNodeReadRequest<GetAliasesRequest>
private String[] indices = Strings.EMPTY_ARRAY;
private String[] aliases = Strings.EMPTY_ARRAY;
private IndicesOptions indicesOptions = IndicesOptions.strictExpand();
private String[] originalAliases = Strings.EMPTY_ARRAY;
public GetAliasesRequest(String[] aliases) {
public GetAliasesRequest(String... aliases) {
this.aliases = aliases;
}
public GetAliasesRequest(String alias) {
this.aliases = new String[]{alias};
this.originalAliases = aliases;
}
public GetAliasesRequest() {
@ -51,6 +49,9 @@ public class GetAliasesRequest extends MasterNodeReadRequest<GetAliasesRequest>
indices = in.readStringArray();
aliases = in.readStringArray();
indicesOptions = IndicesOptions.readIndicesOptions(in);
if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
originalAliases = in.readStringArray();
}
}
@Override
@ -59,6 +60,9 @@ public class GetAliasesRequest extends MasterNodeReadRequest<GetAliasesRequest>
out.writeStringArray(indices);
out.writeStringArray(aliases);
indicesOptions.writeIndicesOptions(out);
if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
out.writeStringArray(originalAliases);
}
}
@Override
@ -67,9 +71,9 @@ public class GetAliasesRequest extends MasterNodeReadRequest<GetAliasesRequest>
return this;
}
@Override
public GetAliasesRequest aliases(String... aliases) {
this.aliases = aliases;
this.originalAliases = aliases;
return this;
}
@ -88,6 +92,18 @@ public class GetAliasesRequest extends MasterNodeReadRequest<GetAliasesRequest>
return aliases;
}
@Override
public void replaceAliases(String... aliases) {
this.aliases = aliases;
}
/**
* Returns the aliases as was originally specified by the user
*/
public String[] getOriginalAliases() {
return originalAliases;
}
@Override
public boolean expandAliasesWildcards() {
return true;

View File

@ -33,6 +33,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import java.util.Collections;
import java.util.List;
public class TransportGetAliasesAction extends TransportMasterNodeReadAction<GetAliasesRequest, GetAliasesResponse> {
@ -62,7 +63,24 @@ public class TransportGetAliasesAction extends TransportMasterNodeReadAction<Get
@Override
protected void masterOperation(GetAliasesRequest request, ClusterState state, ActionListener<GetAliasesResponse> listener) {
String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(state, request);
ImmutableOpenMap<String, List<AliasMetaData>> result = state.metaData().findAliases(request.aliases(), concreteIndices);
listener.onResponse(new GetAliasesResponse(result));
ImmutableOpenMap<String, List<AliasMetaData>> aliases = state.metaData().findAliases(request.aliases(), concreteIndices);
listener.onResponse(new GetAliasesResponse(postProcess(request, concreteIndices, aliases)));
}
/**
* Fills alias result with empty entries for requested indices when no specific aliases were requested.
*/
static ImmutableOpenMap<String, List<AliasMetaData>> postProcess(GetAliasesRequest request, String[] concreteIndices,
ImmutableOpenMap<String, List<AliasMetaData>> aliases) {
boolean noAliasesSpecified = request.getOriginalAliases() == null || request.getOriginalAliases().length == 0;
ImmutableOpenMap.Builder<String, List<AliasMetaData>> mapBuilder = ImmutableOpenMap.builder(aliases);
for (String index : concreteIndices) {
if (aliases.get(index) == null && noAliasesSpecified) {
List<AliasMetaData> previous = mapBuilder.put(index, Collections.emptyList());
assert previous == null;
}
}
return mapBuilder.build();
}
}

View File

@ -265,8 +265,7 @@ public class MetaData implements Iterable<IndexMetaData>, Diffable<MetaData>, To
boolean matchAllAliases = matchAllAliases(aliases);
ImmutableOpenMap.Builder<String, List<AliasMetaData>> mapBuilder = ImmutableOpenMap.builder();
Iterable<String> intersection = HppcMaps.intersection(ObjectHashSet.from(concreteIndices), indices.keys());
for (String index : intersection) {
for (String index : concreteIndices) {
IndexMetaData indexMetaData = indices.get(index);
List<AliasMetaData> filteredValues = new ArrayList<>();
for (ObjectCursor<AliasMetaData> cursor : indexMetaData.getAliases().values()) {
@ -276,11 +275,11 @@ public class MetaData implements Iterable<IndexMetaData>, Diffable<MetaData>, To
}
}
if (!filteredValues.isEmpty()) {
if (filteredValues.isEmpty() == false) {
// Make the list order deterministic
CollectionUtil.timSort(filteredValues, Comparator.comparing(AliasMetaData::alias));
mapBuilder.put(index, Collections.unmodifiableList(filteredValues));
}
mapBuilder.put(index, Collections.unmodifiableList(filteredValues));
}
return mapBuilder.build();
}

View File

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

View File

@ -0,0 +1,64 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.action.admin.indices.alias.get;
import org.elasticsearch.cluster.metadata.AliasMetaData;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.test.ESTestCase;
import java.util.Collections;
import java.util.List;
import static org.hamcrest.Matchers.equalTo;
public class TransportGetAliasesActionTests extends ESTestCase {
public void testPostProcess() {
GetAliasesRequest request = new GetAliasesRequest();
ImmutableOpenMap<String, List<AliasMetaData>> aliases = ImmutableOpenMap.<String, List<AliasMetaData>>builder()
.fPut("b", Collections.singletonList(new AliasMetaData.Builder("y").build()))
.build();
ImmutableOpenMap<String, List<AliasMetaData>> result =
TransportGetAliasesAction.postProcess(request, new String[]{"a", "b", "c"}, aliases);
assertThat(result.size(), equalTo(3));
assertThat(result.get("a").size(), equalTo(0));
assertThat(result.get("b").size(), equalTo(1));
assertThat(result.get("c").size(), equalTo(0));
request = new GetAliasesRequest();
request.replaceAliases("y", "z");
aliases = ImmutableOpenMap.<String, List<AliasMetaData>>builder()
.fPut("b", Collections.singletonList(new AliasMetaData.Builder("y").build()))
.build();
result = TransportGetAliasesAction.postProcess(request, new String[]{"a", "b", "c"}, aliases);
assertThat(result.size(), equalTo(3));
assertThat(result.get("a").size(), equalTo(0));
assertThat(result.get("b").size(), equalTo(1));
assertThat(result.get("c").size(), equalTo(0));
request = new GetAliasesRequest("y", "z");
aliases = ImmutableOpenMap.<String, List<AliasMetaData>>builder()
.fPut("b", Collections.singletonList(new AliasMetaData.Builder("y").build()))
.build();
result = TransportGetAliasesAction.postProcess(request, new String[]{"a", "b", "c"}, aliases);
assertThat(result.size(), equalTo(1));
assertThat(result.get("b").size(), equalTo(1));
}
}

View File

@ -570,24 +570,20 @@ public class IndexAliasesIT extends ESIntegTestCase {
logger.info("--> getting alias1");
GetAliasesResponse getResponse = admin().indices().prepareGetAliases("alias1").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(5));
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("alias1"));
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), nullValue());
assertTrue(getResponse.getAliases().get("test").isEmpty());
assertTrue(getResponse.getAliases().get("test123").isEmpty());
assertTrue(getResponse.getAliases().get("foobarbaz").isEmpty());
assertTrue(getResponse.getAliases().get("bazbar").isEmpty());
AliasesExistResponse existsResponse = admin().indices().prepareAliasesExist("alias1").get();
assertThat(existsResponse.exists(), equalTo(true));
logger.info("--> getting all aliases that start with alias*");
getResponse = admin().indices().prepareGetAliases("alias*").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(5));
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(2));
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("alias1"));
@ -599,10 +595,6 @@ public class IndexAliasesIT extends ESIntegTestCase {
assertThat(getResponse.getAliases().get("foobar").get(1).getFilter(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(1).getIndexRouting(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(1).getSearchRouting(), nullValue());
assertTrue(getResponse.getAliases().get("test").isEmpty());
assertTrue(getResponse.getAliases().get("test123").isEmpty());
assertTrue(getResponse.getAliases().get("foobarbaz").isEmpty());
assertTrue(getResponse.getAliases().get("bazbar").isEmpty());
existsResponse = admin().indices().prepareAliasesExist("alias*").get();
assertThat(existsResponse.exists(), equalTo(true));
@ -687,13 +679,12 @@ public class IndexAliasesIT extends ESIntegTestCase {
logger.info("--> getting f* for index *bar");
getResponse = admin().indices().prepareGetAliases("f*").addIndices("*bar").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(2));
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("foo"));
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), nullValue());
assertTrue(getResponse.getAliases().get("bazbar").isEmpty());
existsResponse = admin().indices().prepareAliasesExist("f*")
.addIndices("*bar").get();
assertThat(existsResponse.exists(), equalTo(true));
@ -702,14 +693,13 @@ public class IndexAliasesIT extends ESIntegTestCase {
logger.info("--> getting f* for index *bac");
getResponse = admin().indices().prepareGetAliases("foo").addIndices("*bac").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(2));
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("foo"));
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), nullValue());
assertTrue(getResponse.getAliases().get("bazbar").isEmpty());
existsResponse = admin().indices().prepareAliasesExist("foo")
.addIndices("*bac").get();
assertThat(existsResponse.exists(), equalTo(true));
@ -727,6 +717,19 @@ public class IndexAliasesIT extends ESIntegTestCase {
.addIndices("foobar").get();
assertThat(existsResponse.exists(), equalTo(true));
for (String aliasName : new String[]{null, "_all", "*"}) {
logger.info("--> getting {} alias for index foobar", aliasName);
getResponse = aliasName != null ? admin().indices().prepareGetAliases(aliasName).addIndices("foobar").get() :
admin().indices().prepareGetAliases().addIndices("foobar").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(4));
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("alias1"));
assertThat(getResponse.getAliases().get("foobar").get(1).alias(), equalTo("alias2"));
assertThat(getResponse.getAliases().get("foobar").get(2).alias(), equalTo("bac"));
assertThat(getResponse.getAliases().get("foobar").get(3).alias(), equalTo("foo"));
}
// alias at work again
logger.info("--> getting * for index *bac");
getResponse = admin().indices().prepareGetAliases("*").addIndices("*bac").get();

View File

@ -200,7 +200,7 @@ class IndicesAndAliasesResolver {
if (aliasesRequest.expandAliasesWildcards()) {
List<String> aliases = replaceWildcardsWithAuthorizedAliases(aliasesRequest.aliases(),
loadAuthorizedAliases(authorizedIndices.get(), metaData));
aliasesRequest.aliases(aliases.toArray(new String[aliases.size()]));
aliasesRequest.replaceAliases(aliases.toArray(new String[aliases.size()]));
}
if (indicesReplacedWithNoIndices) {
if (indicesRequest instanceof GetAliasesRequest == false) {