Skip to content
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

Do not return all indices if a specific alias is requested via get aliases api. #29538

Merged
merged 5 commits into from
Jul 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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
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 @@ -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() {
Expand All @@ -51,6 +49,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)) {
originalAliases = in.readStringArray();
}
}

@Override
Expand All @@ -59,6 +60,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.writeStringArray(originalAliases);
}
}

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

@Override
public GetAliasesRequest aliases(String... aliases) {
this.aliases = aliases;
this.originalAliases = aliases;
return this;
}

Expand All @@ -88,6 +92,18 @@ public String[] aliases() {
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
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 @@ -62,7 +63,24 @@ protected GetAliasesResponse newResponse() {
@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();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,7 @@ public ImmutableOpenMap<String, List<AliasMetaData>> findAliases(final String[]

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()) {
Expand All @@ -276,11 +275,11 @@ 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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ public String getName() {

@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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}

}
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()]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

}
if (indicesReplacedWithNoIndices) {
if (indicesRequest instanceof GetAliasesRequest == false) {
Expand Down