From f95074589772f911d7d6762de6393ac4267b58e2 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 27 Jun 2018 10:47:11 +0200 Subject: [PATCH 1/5] Do not return all indices if a specific alias is requested via get aliases 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 #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 --- .../elasticsearch/action/AliasesRequest.java | 6 ++-- .../indices/alias/IndicesAliasesRequest.java | 6 +++- .../indices/alias/get/GetAliasesRequest.java | 36 ++++++++++++++----- .../alias/get/TransportGetAliasesAction.java | 15 +++++++- .../cluster/metadata/MetaData.java | 15 ++------ .../elasticsearch/aliases/IndexAliasesIT.java | 31 ++++++++-------- .../authz/IndicesAndAliasesResolver.java | 2 +- 7 files changed, 71 insertions(+), 40 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/AliasesRequest.java b/server/src/main/java/org/elasticsearch/action/AliasesRequest.java index a4ff57ebd2008..e85f38c777b77 100644 --- a/server/src/main/java/org/elasticsearch/action/AliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/AliasesRequest.java @@ -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 diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java index c7e7288e74f55..9249550871c12 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java @@ -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 + "]"); @@ -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 diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java index 04b0843e0ae8f..0c77a6254a63b 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java @@ -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; @@ -27,20 +28,18 @@ import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; +import java.util.Objects; public class GetAliasesRequest extends MasterNodeReadRequest 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() { @@ -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 @@ -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 @@ -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; } @@ -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; diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java index 96ecde4e4c6dd..e2a9678ba243c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java @@ -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; @@ -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 { @@ -63,6 +66,16 @@ protected GetAliasesResponse newResponse() { protected void masterOperation(GetAliasesRequest request, ClusterState state, ActionListener listener) { String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(state, request); ImmutableOpenMap> 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> mapBuilder = ImmutableOpenMap.builder(result); + Iterable 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())); } } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index 2bfe0d0a58f70..4ee7249c7bf35 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -263,7 +263,7 @@ public ImmutableOpenMap> findAliases(final String[] return ImmutableOpenMap.of(); } - boolean matchAllAliases = matchAllAliases(aliases); + boolean matchAllAliases = Strings.isAllOrWildcard(aliases); ImmutableOpenMap.Builder> mapBuilder = ImmutableOpenMap.builder(); Iterable intersection = HppcMaps.intersection(ObjectHashSet.from(concreteIndices), indices.keys()); for (String index : intersection) { @@ -276,24 +276,15 @@ public ImmutableOpenMap> 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. diff --git a/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java b/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java index 8bf074be551b1..d72b4c5f1ec16 100644 --- a/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java +++ b/server/src/test/java/org/elasticsearch/aliases/IndexAliasesIT.java @@ -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")); @@ -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)); @@ -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)); @@ -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)); @@ -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(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java index 941bf13daf584..77170f7a1cbfb 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java @@ -200,7 +200,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData if (aliasesRequest.expandAliasesWildcards()) { List 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) { From 288d63e0b3905fecf1e786b1ad6c72beabb8a535 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 27 Jun 2018 14:55:26 +0200 Subject: [PATCH 2/5] instead of tracking whether aliases were specified in original request, keep track the original aliases --- .../indices/alias/get/GetAliasesRequest.java | 22 +++++++++---------- .../alias/get/TransportGetAliasesAction.java | 4 +++- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java index 0c77a6254a63b..3961311f1e269 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java @@ -28,18 +28,17 @@ import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; -import java.util.Objects; public class GetAliasesRequest extends MasterNodeReadRequest implements AliasesRequest { private String[] indices = Strings.EMPTY_ARRAY; private String[] aliases = Strings.EMPTY_ARRAY; private IndicesOptions indicesOptions = IndicesOptions.strictExpand(); - private boolean aliasesProvided = false; + private String[] originalAliases; public GetAliasesRequest(String... aliases) { - this.aliases = Objects.requireNonNull(aliases); - this.aliasesProvided = aliases.length != 0; + this.aliases = aliases; + this.originalAliases = aliases; } public GetAliasesRequest() { @@ -51,7 +50,7 @@ public GetAliasesRequest(StreamInput in) throws IOException { aliases = in.readStringArray(); indicesOptions = IndicesOptions.readIndicesOptions(in); if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { - aliasesProvided = in.readBoolean(); + originalAliases = in.readStringArray(); } } @@ -62,7 +61,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringArray(aliases); indicesOptions.writeIndicesOptions(out); if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { - out.writeBoolean(aliasesProvided); + out.writeStringArray(originalAliases); } } @@ -73,8 +72,8 @@ public GetAliasesRequest indices(String... indices) { } public GetAliasesRequest aliases(String... aliases) { - this.aliases = Objects.requireNonNull(aliases); - this.aliasesProvided = aliases.length != 0; + this.aliases = aliases; + this.originalAliases = aliases; return this; } @@ -99,11 +98,10 @@ public void replaceAliases(String... 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 + * Returns aliases originally specified by the user */ - boolean isAliasesProvided() { - return aliasesProvided; + String[] getOriginalAliases() { + return originalAliases; } @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java index e2a9678ba243c..0f311c272a055 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java @@ -28,6 +28,7 @@ import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.HppcMaps; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.inject.Inject; @@ -68,10 +69,11 @@ protected void masterOperation(GetAliasesRequest request, ClusterState state, Ac ImmutableOpenMap> result = state.metaData().findAliases(request.aliases(), concreteIndices); // in case all aliases are requested then it is desired to return the concrete index with no aliases (#25114): + boolean aliasesProvided = Strings.isAllOrWildcard(request.getOriginalAliases()); ImmutableOpenMap.Builder> mapBuilder = ImmutableOpenMap.builder(result); Iterable intersection = HppcMaps.intersection(ObjectHashSet.from(concreteIndices), state.metaData().indices().keys()); for (String index : intersection) { - if (result.get(index) == null && request.isAliasesProvided() == false) { + if (result.get(index) == null && aliasesProvided) { mapBuilder.put(index, Collections.emptyList()); } } From 342610cacf117eaa8209fb0a0e6213e7fa922516 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Fri, 29 Jun 2018 11:56:51 +0200 Subject: [PATCH 3/5] Stop using HppcMaps.intersection(...) as concrete indices should always contains indices that exist --- .../admin/indices/alias/get/TransportGetAliasesAction.java | 5 +---- .../java/org/elasticsearch/cluster/metadata/MetaData.java | 3 +-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java index 0f311c272a055..078c7ec1e5257 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java @@ -18,7 +18,6 @@ */ 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; @@ -29,7 +28,6 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.collect.HppcMaps; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; @@ -71,8 +69,7 @@ protected void masterOperation(GetAliasesRequest request, ClusterState state, Ac // in case all aliases are requested then it is desired to return the concrete index with no aliases (#25114): boolean aliasesProvided = Strings.isAllOrWildcard(request.getOriginalAliases()); ImmutableOpenMap.Builder> mapBuilder = ImmutableOpenMap.builder(result); - Iterable intersection = HppcMaps.intersection(ObjectHashSet.from(concreteIndices), state.metaData().indices().keys()); - for (String index : intersection) { + for (String index : concreteIndices) { if (result.get(index) == null && aliasesProvided) { mapBuilder.put(index, Collections.emptyList()); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index 4ee7249c7bf35..632ea9e1f861d 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -265,8 +265,7 @@ public ImmutableOpenMap> findAliases(final String[] boolean matchAllAliases = Strings.isAllOrWildcard(aliases); ImmutableOpenMap.Builder> mapBuilder = ImmutableOpenMap.builder(); - Iterable intersection = HppcMaps.intersection(ObjectHashSet.from(concreteIndices), indices.keys()); - for (String index : intersection) { + for (String index : concreteIndices) { IndexMetaData indexMetaData = indices.get(index); List filteredValues = new ArrayList<>(); for (ObjectCursor cursor : indexMetaData.getAliases().values()) { From 4eb05f490b75f36a7bb7579cd2c79011a4dc90b9 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 2 Jul 2018 16:51:18 +0200 Subject: [PATCH 4/5] iter --- .../elasticsearch/action/AliasesRequest.java | 4 +- .../indices/alias/get/GetAliasesRequest.java | 4 +- .../alias/get/TransportGetAliasesAction.java | 24 ++++--- .../cluster/metadata/MetaData.java | 11 +++- .../admin/indices/RestGetAliasesAction.java | 4 ++ .../get/TransportGetAliasesActionTests.java | 64 +++++++++++++++++++ 6 files changed, 97 insertions(+), 14 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java diff --git a/server/src/main/java/org/elasticsearch/action/AliasesRequest.java b/server/src/main/java/org/elasticsearch/action/AliasesRequest.java index e85f38c777b77..bf7ceb28d5023 100644 --- a/server/src/main/java/org/elasticsearch/action/AliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/AliasesRequest.java @@ -33,9 +33,9 @@ public interface AliasesRequest extends IndicesRequest.Replaceable { String[] aliases(); /** - * Replaces the aliases that the action relates to + * Replaces current aliases with the provided aliases. * - * This is an internal method. + * Sometimes aliases expressions need to be resolved to concrete aliases prior to executing the transport action. */ void replaceAliases(String... aliases); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java index 3961311f1e269..728033c39375e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java @@ -98,9 +98,9 @@ public void replaceAliases(String... aliases) { } /** - * Returns aliases originally specified by the user + * Returns the aliases as was originally specified by the user */ - String[] getOriginalAliases() { + public String[] getOriginalAliases() { return originalAliases; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java index 078c7ec1e5257..1bacd652ee734 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java @@ -27,7 +27,6 @@ import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; @@ -64,17 +63,24 @@ protected GetAliasesResponse newResponse() { @Override protected void masterOperation(GetAliasesRequest request, ClusterState state, ActionListener listener) { String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(state, request); - ImmutableOpenMap> result = state.metaData().findAliases(request.aliases(), concreteIndices); + ImmutableOpenMap> aliases = state.metaData().findAliases(request.aliases(), concreteIndices); + listener.onResponse(new GetAliasesResponse(postProcess(request, concreteIndices, aliases))); + } - // in case all aliases are requested then it is desired to return the concrete index with no aliases (#25114): - boolean aliasesProvided = Strings.isAllOrWildcard(request.getOriginalAliases()); - ImmutableOpenMap.Builder> mapBuilder = ImmutableOpenMap.builder(result); + /** + * Fills alias result with empty entries for requested indices when no specific aliases were requested. + */ + static ImmutableOpenMap> postProcess(GetAliasesRequest request, String[] concreteIndices, + ImmutableOpenMap> aliases) { + boolean noAliasesSpecified = request.getOriginalAliases() == null || request.getOriginalAliases().length == 0; + ImmutableOpenMap.Builder> mapBuilder = ImmutableOpenMap.builder(aliases); for (String index : concreteIndices) { - if (result.get(index) == null && aliasesProvided) { - mapBuilder.put(index, Collections.emptyList()); + if (aliases.get(index) == null && noAliasesSpecified) { + List previous = mapBuilder.put(index, Collections.emptyList()); + assert previous == null; } } - - listener.onResponse(new GetAliasesResponse(mapBuilder.build())); + return mapBuilder.build(); } + } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index 632ea9e1f861d..4ed2adc9a1c9f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -263,7 +263,7 @@ public ImmutableOpenMap> findAliases(final String[] return ImmutableOpenMap.of(); } - boolean matchAllAliases = Strings.isAllOrWildcard(aliases); + boolean matchAllAliases = matchAllAliases(aliases); ImmutableOpenMap.Builder> mapBuilder = ImmutableOpenMap.builder(); for (String index : concreteIndices) { IndexMetaData indexMetaData = indices.get(index); @@ -284,6 +284,15 @@ public ImmutableOpenMap> findAliases(final String[] 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. diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetAliasesAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetAliasesAction.java index 8a1e4e74e819e..0d6d46e95b602 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetAliasesAction.java @@ -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); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java new file mode 100644 index 0000000000000..d445a63aea170 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java @@ -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> aliases = ImmutableOpenMap.>builder() + .fPut("b", Collections.singletonList(new AliasMetaData.Builder("y").build())) + .build(); + ImmutableOpenMap> 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.>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.>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)); + } + +} From fd1b0186e50a0b21250e2e8847a1c5c7d3661196 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 3 Jul 2018 11:39:33 +0200 Subject: [PATCH 5/5] fixed NPE error --- .../action/admin/indices/alias/get/GetAliasesRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java index 728033c39375e..e8dd93144b10e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java @@ -34,7 +34,7 @@ public class GetAliasesRequest extends MasterNodeReadRequest private String[] indices = Strings.EMPTY_ARRAY; private String[] aliases = Strings.EMPTY_ARRAY; private IndicesOptions indicesOptions = IndicesOptions.strictExpand(); - private String[] originalAliases; + private String[] originalAliases = Strings.EMPTY_ARRAY; public GetAliasesRequest(String... aliases) { this.aliases = aliases;