Skip to content

Commit

Permalink
Do not return all indices if a specific alias is requested via get al…
Browse files Browse the repository at this point in the history
…iases api.

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 elastic#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 elastic#27763
  • Loading branch information
martijnvg committed Jul 2, 2018
1 parent 5d94003 commit f950745
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ public interface AliasesRequest extends IndicesRequest.Replaceable {
String[] aliases();

/**
* Sets the array of aliases that the action relates to
* Replaces the aliases that the action relates to
*
* This is an internal method.
*/
AliasesRequest aliases(String... aliases);
void replaceAliases(String... aliases);

/**
* Returns true if wildcards expressions among aliases should be resolved, false otherwise
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ public AliasActions index(String index) {
/**
* 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 + "]");
Expand Down Expand Up @@ -428,6 +427,11 @@ public String[] aliases() {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,20 +28,18 @@
import org.elasticsearch.common.io.stream.StreamOutput;

import java.io.IOException;
import java.util.Objects;

public class GetAliasesRequest extends MasterNodeReadRequest<GetAliasesRequest> implements AliasesRequest {

private String[] indices = Strings.EMPTY_ARRAY;
private String[] aliases = Strings.EMPTY_ARRAY;

private IndicesOptions indicesOptions = IndicesOptions.strictExpand();
private boolean aliasesProvided = false;

public GetAliasesRequest(String[] aliases) {
this.aliases = aliases;
}

public GetAliasesRequest(String alias) {
this.aliases = new String[]{alias};
public GetAliasesRequest(String... aliases) {
this.aliases = Objects.requireNonNull(aliases);
this.aliasesProvided = aliases.length != 0;
}

public GetAliasesRequest() {
Expand All @@ -51,6 +50,9 @@ public GetAliasesRequest(StreamInput in) throws IOException {
indices = in.readStringArray();
aliases = in.readStringArray();
indicesOptions = IndicesOptions.readIndicesOptions(in);
if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
aliasesProvided = in.readBoolean();
}
}

@Override
Expand All @@ -59,6 +61,9 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeStringArray(indices);
out.writeStringArray(aliases);
indicesOptions.writeIndicesOptions(out);
if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
out.writeBoolean(aliasesProvided);
}
}

@Override
Expand All @@ -67,9 +72,9 @@ public GetAliasesRequest indices(String... indices) {
return this;
}

@Override
public GetAliasesRequest aliases(String... aliases) {
this.aliases = aliases;
this.aliases = Objects.requireNonNull(aliases);
this.aliasesProvided = aliases.length != 0;
return this;
}

Expand All @@ -88,6 +93,19 @@ public String[] aliases() {
return aliases;
}

@Override
public void replaceAliases(String... aliases) {
this.aliases = aliases;
}

/**
* @return Whether aliases that where originally provided by the user via {@link #aliases(String...)} or
* {@link #GetAliasesRequest(String...)}. If this is not the case and there are aliases then
*/
boolean isAliasesProvided() {
return aliasesProvided;
}

@Override
public boolean expandAliasesWildcards() {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.elasticsearch.action.admin.indices.alias.get;

import com.carrotsearch.hppc.ObjectHashSet;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.master.TransportMasterNodeReadAction;
Expand All @@ -27,12 +28,14 @@
import org.elasticsearch.cluster.metadata.AliasMetaData;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.collect.HppcMaps;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.inject.Inject;
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> {
Expand Down Expand Up @@ -63,6 +66,16 @@ protected GetAliasesResponse newResponse() {
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));

// in case all aliases are requested then it is desired to return the concrete index with no aliases (#25114):
ImmutableOpenMap.Builder<String, List<AliasMetaData>> mapBuilder = ImmutableOpenMap.builder(result);
Iterable<String> intersection = HppcMaps.intersection(ObjectHashSet.from(concreteIndices), state.metaData().indices().keys());
for (String index : intersection) {
if (result.get(index) == null && request.isAliasesProvided() == false) {
mapBuilder.put(index, Collections.emptyList());
}
}

listener.onResponse(new GetAliasesResponse(mapBuilder.build()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ public ImmutableOpenMap<String, List<AliasMetaData>> findAliases(final String[]
return ImmutableOpenMap.of();
}

boolean matchAllAliases = matchAllAliases(aliases);
boolean matchAllAliases = Strings.isAllOrWildcard(aliases);
ImmutableOpenMap.Builder<String, List<AliasMetaData>> mapBuilder = ImmutableOpenMap.builder();
Iterable<String> intersection = HppcMaps.intersection(ObjectHashSet.from(concreteIndices), indices.keys());
for (String index : intersection) {
Expand All @@ -276,24 +276,15 @@ public ImmutableOpenMap<String, List<AliasMetaData>> findAliases(final String[]
}
}

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();
}

private static boolean matchAllAliases(final String[] aliases) {
for (String alias : aliases) {
if (alias.equals(ALL)) {
return true;
}
}
return aliases.length == 0;
}

/**
* Checks if at least one of the specified aliases exists in the specified concrete indices. Wildcards are supported in the
* alias names for partial matches.
Expand Down
31 changes: 17 additions & 14 deletions server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -570,24 +570,20 @@ public void testIndicesGetAliases() throws Exception {
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"));
Expand All @@ -599,10 +595,6 @@ public void testIndicesGetAliases() throws Exception {
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));

Expand Down Expand Up @@ -687,13 +679,12 @@ public void testIndicesGetAliases() throws Exception {
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));
Expand All @@ -702,14 +693,13 @@ public void testIndicesGetAliases() throws Exception {
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));
Expand All @@ -727,6 +717,19 @@ public void testIndicesGetAliases() throws Exception {
.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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
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) {
Expand Down

0 comments on commit f950745

Please sign in to comment.