From 4b2c3ab0b7b157423dbd08ba7914f8fb2767ea76 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 19 May 2021 10:07:11 +0200 Subject: [PATCH] The get aliases api should not return entries for data streams with no aliases (#72953) The get alias api should take into account the aliases parameter when returning aliases that refer to data streams and don't return entries for data streams that don't have any aliases pointing to it. Relates to #66163 --- docs/reference/indices/aliases.asciidoc | 3 -- .../alias/get/TransportGetAliasesAction.java | 13 ++++--- .../cluster/metadata/Metadata.java | 12 +++--- .../get/TransportGetAliasesActionTests.java | 37 +++++++++++++++++++ .../xpack/datastreams/DataStreamsRestIT.java | 13 ++----- 5 files changed, 55 insertions(+), 23 deletions(-) diff --git a/docs/reference/indices/aliases.asciidoc b/docs/reference/indices/aliases.asciidoc index 01dd4d5452cf1..31b56c5e8cba1 100644 --- a/docs/reference/indices/aliases.asciidoc +++ b/docs/reference/indices/aliases.asciidoc @@ -599,9 +599,6 @@ The get index alias API returns: [source,console-result] ---- { - "logs-my_app-default": { - "aliases": {} - }, "logs-nginx.access-prod": { "aliases": { "logs": {} 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 bb6033d07ccec..4a1e6c85f464c 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 @@ -73,7 +73,8 @@ protected void masterOperation(Task task, GetAliasesRequest request, ClusterStat final SystemIndexAccessLevel systemIndexAccessLevel = indexNameExpressionResolver.getSystemIndexAccessLevel(); ImmutableOpenMap> aliases = state.metadata().findAliases(request, concreteIndices); listener.onResponse(new GetAliasesResponse(postProcess(request, concreteIndices, aliases, state, - systemIndexAccessLevel, threadPool.getThreadContext(), systemIndices), postProcess(request, state))); + systemIndexAccessLevel, threadPool.getThreadContext(), systemIndices), + postProcess(indexNameExpressionResolver, request, state))); } /** @@ -107,17 +108,19 @@ static ImmutableOpenMap> postProcess(GetAliasesReque return finalResponse; } - Map> postProcess(GetAliasesRequest request, ClusterState state) { + static Map> postProcess(IndexNameExpressionResolver resolver, GetAliasesRequest request, + ClusterState state) { Map> result = new HashMap<>(); boolean noAliasesSpecified = request.getOriginalAliases() == null || request.getOriginalAliases().length == 0; - List requestedDataStreams = - indexNameExpressionResolver.dataStreamNames(state, request.indicesOptions(), request.indices()); + List requestedDataStreams = resolver.dataStreamNames(state, request.indicesOptions(), request.indices()); for (String requestedDataStream : requestedDataStreams) { List aliases = state.metadata().dataStreamAliases().values().stream() .filter(alias -> alias.getDataStreams().contains(requestedDataStream)) .filter(alias -> noAliasesSpecified || Regex.simpleMatch(request.aliases(), alias.getName())) .collect(Collectors.toList()); - result.put(requestedDataStream, aliases); + if (aliases.isEmpty() == false) { + result.put(requestedDataStream, aliases); + } } return result; } 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 5801a69ceacf1..ca2fdc2391d9e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java @@ -1177,7 +1177,7 @@ public Builder put(DataStream dataStream) { return this; } - public boolean put(String name, String dataStream) { + public boolean put(String aliasName, String dataStream) { Map existingDataStream = Optional.ofNullable((DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE)) .map(dsmd -> new HashMap<>(dsmd.dataStreams())) @@ -1188,21 +1188,21 @@ public boolean put(String name, String dataStream) { .orElse(new HashMap<>()); if (existingDataStream.containsKey(dataStream) == false) { - throw new IllegalArgumentException("alias [" + name + "] refers to a non existing data stream [" + dataStream + "]"); + throw new IllegalArgumentException("alias [" + aliasName + "] refers to a non existing data stream [" + dataStream + "]"); } - DataStreamAlias alias = dataStreamAliases.get(name); + DataStreamAlias alias = dataStreamAliases.get(aliasName); if (alias == null) { - alias = new DataStreamAlias(name, List.of(dataStream)); + alias = new DataStreamAlias(aliasName, List.of(dataStream)); } else { Set dataStreams = new HashSet<>(alias.getDataStreams()); boolean added = dataStreams.add(dataStream); if (added == false) { return false; } - alias = new DataStreamAlias(name, List.copyOf(dataStreams)); + alias = new DataStreamAlias(aliasName, List.copyOf(dataStreams)); } - dataStreamAliases.put(name, alias); + dataStreamAliases.put(aliasName, alias); this.customs.put(DataStreamMetadata.TYPE, new DataStreamMetadata(existingDataStream, dataStreamAliases)); return true; 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 index 229de26d68713..5c4a5ba4babf6 100644 --- 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 @@ -10,20 +10,26 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.AliasMetadata; +import org.elasticsearch.cluster.metadata.DataStreamAlias; +import org.elasticsearch.cluster.metadata.DataStreamTestHelper; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.indices.EmptySystemIndices; import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.indices.SystemIndices.SystemIndexAccessLevel; +import org.elasticsearch.indices.TestIndexNameExpressionResolver; import org.elasticsearch.test.ESTestCase; import java.util.Collections; import java.util.List; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; public class TransportGetAliasesActionTests extends ESTestCase { @@ -184,6 +190,37 @@ public void testDeprecationWarningEmittedWhenRequestingNonExistingAliasInSystemP "access to system indices and their aliases will not be allowed"); } + public void testPostProcessDataStreamAliases() { + var resolver = TestIndexNameExpressionResolver.newInstance(); + var tuples = List.of(new Tuple<>("logs-foo", 1), new Tuple<>("logs-bar", 1), new Tuple<>("logs-baz", 1)); + var clusterState = DataStreamTestHelper.getClusterStateWithDataStreams(tuples, List.of()); + var builder = Metadata.builder(clusterState.metadata()); + builder.put("logs", "logs-foo"); + builder.put("logs", "logs-bar"); + builder.put("secret", "logs-bar"); + clusterState = ClusterState.builder(clusterState).metadata(builder).build(); + + // return all all data streams with aliases + var getAliasesRequest = new GetAliasesRequest(); + var result = TransportGetAliasesAction.postProcess(resolver, getAliasesRequest, clusterState); + assertThat(result.keySet(), containsInAnyOrder("logs-foo", "logs-bar")); + assertThat(result.get("logs-foo"), contains(new DataStreamAlias("logs", List.of("logs-bar", "logs-foo")))); + assertThat(result.get("logs-bar"), containsInAnyOrder(new DataStreamAlias("logs", List.of("logs-bar", "logs-foo")), + new DataStreamAlias("secret", List.of("logs-bar")))); + + // filter by alias name + getAliasesRequest = new GetAliasesRequest("secret"); + result = TransportGetAliasesAction.postProcess(resolver, getAliasesRequest, clusterState); + assertThat(result.keySet(), containsInAnyOrder("logs-bar")); + assertThat(result.get("logs-bar"), contains(new DataStreamAlias("secret", List.of("logs-bar")))); + + // filter by data stream: + getAliasesRequest = new GetAliasesRequest().indices("logs-foo"); + result = TransportGetAliasesAction.postProcess(resolver, getAliasesRequest, clusterState); + assertThat(result.keySet(), containsInAnyOrder("logs-foo")); + assertThat(result.get("logs-foo"), contains(new DataStreamAlias("logs", List.of("logs-bar", "logs-foo")))); + } + public ClusterState systemIndexTestClusterState() { return ClusterState.builder(ClusterState.EMPTY_STATE) .metadata(Metadata.builder() diff --git a/x-pack/plugin/data-streams/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/datastreams/DataStreamsRestIT.java b/x-pack/plugin/data-streams/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/datastreams/DataStreamsRestIT.java index c29a795915cb1..cafcd90459818 100644 --- a/x-pack/plugin/data-streams/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/datastreams/DataStreamsRestIT.java +++ b/x-pack/plugin/data-streams/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/datastreams/DataStreamsRestIT.java @@ -130,8 +130,7 @@ public void testDataStreamAliases() throws Exception { getAliasesRequest = new Request("GET", "/_aliases"); getAliasesResponse = entityAsMap(client().performRequest(getAliasesRequest)); - assertEquals(Map.of(), XContentMapValues.extractValue("logs-myapp1.aliases", getAliasesResponse)); - assertEquals(Map.of(), XContentMapValues.extractValue("logs-myapp2.aliases", getAliasesResponse)); + assertEquals(Map.of(), getAliasesResponse); expectThrows(ResponseException.class, () -> client().performRequest(new Request("GET", "/logs/_search"))); // Add logs-* -> logs @@ -155,8 +154,7 @@ public void testDataStreamAliases() throws Exception { getAliasesRequest = new Request("GET", "/_aliases"); getAliasesResponse = entityAsMap(client().performRequest(getAliasesRequest)); - assertEquals(Map.of(), XContentMapValues.extractValue("logs-myapp1.aliases", getAliasesResponse)); - assertEquals(Map.of(), XContentMapValues.extractValue("logs-myapp2.aliases", getAliasesResponse)); + assertEquals(Map.of(), getAliasesResponse); expectThrows(ResponseException.class, () -> client().performRequest(new Request("GET", "/logs/_search"))); } @@ -225,20 +223,17 @@ public void testGetAliasApiFilterByDataStreamAlias() throws Exception { response = client().performRequest(new Request("GET", "/_alias/emea")); assertOK(response); getAliasesResponse = entityAsMap(response); - assertThat(getAliasesResponse.size(), equalTo(2)); // Adjust to equalTo(1) when #72953 is merged + assertThat(getAliasesResponse.size(), equalTo(1)); assertEquals(Map.of("emea", Map.of()), XContentMapValues.extractValue("logs-emea.aliases", getAliasesResponse)); - assertEquals(Map.of(), XContentMapValues.extractValue("logs-nasa.aliases", getAliasesResponse)); // Remove when #72953 is merged ResponseException exception = expectThrows(ResponseException.class, () -> client().performRequest(new Request("GET", "/_alias/wrong_name"))); response = exception.getResponse(); assertThat(response.getStatusLine().getStatusCode(), equalTo(404)); getAliasesResponse = entityAsMap(response); - assertThat(getAliasesResponse.size(), equalTo(4)); // Adjust to equalTo(2) when #72953 is merged + assertThat(getAliasesResponse.size(), equalTo(2)); assertEquals("alias [wrong_name] missing", getAliasesResponse.get("error")); assertEquals(404, getAliasesResponse.get("status")); - assertEquals(Map.of(), XContentMapValues.extractValue("logs-emea.aliases", getAliasesResponse)); // Remove when #72953 is merged - assertEquals(Map.of(), XContentMapValues.extractValue("logs-nasa.aliases", getAliasesResponse)); // Remove when #72953 is merged } }