From 292e4cd9ea13d7eeae2aba5feaa83e0a7343d6b0 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 27 May 2021 13:22:01 +0200 Subject: [PATCH] Add more validation for data stream aliases. (#73416) Currently when attempting to an alias to points to both data streams and regular indices the alias does get created, but points only to data streams. With this change attempting to add such aliases results in a client error. Currently when adding data stream aliases with unsupported parameters (e.g. filter or routing) the alias does get created, but without the unsupported parameters. With this change attempting to create aliases to point to data streams with unsupported parameters will result in a client error. Relates to #66163 --- .../indices/alias/IndicesAliasesRequest.java | 4 + .../alias/TransportIndicesAliasesAction.java | 34 +++++++ .../datastreams/DataStreamIT.java | 90 +++++++++++++++++++ 3 files changed, 128 insertions(+) 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 305d085c001a6..59dd63c1c7891 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 @@ -384,6 +384,10 @@ public AliasActions searchRouting(String searchRouting) { return this; } + public String routing() { + return routing; + } + public String indexRouting() { return indexRouting == null ? routing : indexRouting; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java index 4e63aa4496260..e9a40f6ce842d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java @@ -35,12 +35,14 @@ import org.elasticsearch.transport.TransportService; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import static java.util.Collections.unmodifiableList; @@ -91,6 +93,38 @@ protected void masterOperation(final IndicesAliasesRequest request, final Cluste List concreteDataStreams = indexNameExpressionResolver.dataStreamNames(state, request.indicesOptions(), action.indices()); if (concreteDataStreams.size() != 0) { + // Fail if parameters are used that data streams don't support: + if (action.filter() != null) { + throw new IllegalArgumentException("aliases that point to data streams don't support filters"); + } + if (action.routing() != null) { + throw new IllegalArgumentException("aliases that point to data streams don't support routing"); + } + if (action.indexRouting() != null) { + throw new IllegalArgumentException("aliases that point to data streams don't support index_routing"); + } + if (action.searchRouting() != null) { + throw new IllegalArgumentException("aliases that point to data streams don't support search_routing"); + } + if (action.writeIndex() != null) { + throw new IllegalArgumentException("aliases that point to data streams don't support is_write_index"); + } + if (action.isHidden() != null) { + throw new IllegalArgumentException("aliases that point to data streams don't support is_hidden"); + } + // Fail if expressions match both data streams and regular indices: + String[] concreteIndices = + indexNameExpressionResolver.concreteIndexNames(state, request.indicesOptions(), true, action.indices()); + List nonBackingIndices = Arrays.stream(concreteIndices) + .map(resolvedIndex -> state.metadata().getIndicesLookup().get(resolvedIndex)) + .filter(ia -> ia.getParentDataStream() == null) + .map(IndexAbstraction::getName) + .collect(Collectors.toList()); + if (nonBackingIndices.isEmpty() == false) { + throw new IllegalArgumentException("expressions " + Arrays.toString(action.indices()) + + " that match with both data streams and regular indices are disallowed"); + } + switch (action.actionType()) { case ADD: for (String dataStreamName : concreteDataStreams) { diff --git a/x-pack/plugin/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java b/x-pack/plugin/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java index 8b0ff8cdddad2..257407e8a9521 100644 --- a/x-pack/plugin/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java +++ b/x-pack/plugin/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java @@ -737,6 +737,96 @@ public void testAliasActionsFailOnDataStreamBackingIndices() throws Exception { ); } + public void testDataStreamAliasesMixedExpressionValidation() throws Exception { + createIndex("metrics-myindex"); + putComposableIndexTemplate("id1", List.of("metrics-*")); + String dataStreamName = "metrics-foo"; + CreateDataStreamAction.Request createDataStreamRequest = new CreateDataStreamAction.Request(dataStreamName); + client().execute(CreateDataStreamAction.INSTANCE, createDataStreamRequest).get(); + + AliasActions addAction = new AliasActions(AliasActions.Type.ADD).index("metrics-*").aliases("my-alias"); + IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest(); + aliasesAddRequest.addAliasAction(addAction); + Exception e = expectThrows(IllegalArgumentException.class, () -> client().admin().indices().aliases(aliasesAddRequest).actionGet()); + assertThat(e.getMessage(), equalTo("expressions [metrics-*] that match with both data streams and regular indices are disallowed")); + } + + public void testDataStreamAliasesUnsupportedParametersValidation() throws Exception { + putComposableIndexTemplate("id1", List.of("metrics-*")); + String dataStreamName = "metrics-foo"; + CreateDataStreamAction.Request createDataStreamRequest = new CreateDataStreamAction.Request(dataStreamName); + client().execute(CreateDataStreamAction.INSTANCE, createDataStreamRequest).get(); + + { + AliasActions addAction = new AliasActions(AliasActions.Type.ADD).index("metrics-*").aliases("my-alias").filter("[filter]"); + IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest(); + aliasesAddRequest.addAliasAction(addAction); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> client().admin().indices().aliases(aliasesAddRequest).actionGet() + ); + assertThat(e.getMessage(), equalTo("aliases that point to data streams don't support filters")); + } + { + AliasActions addAction = new AliasActions(AliasActions.Type.ADD).index("metrics-*").aliases("my-alias").routing("[routing]"); + IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest(); + aliasesAddRequest.addAliasAction(addAction); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> client().admin().indices().aliases(aliasesAddRequest).actionGet() + ); + assertThat(e.getMessage(), equalTo("aliases that point to data streams don't support routing")); + } + { + AliasActions addAction = new AliasActions(AliasActions.Type.ADD).index("metrics-*") + .aliases("my-alias") + .indexRouting("[index_routing]"); + IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest(); + aliasesAddRequest.addAliasAction(addAction); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> client().admin().indices().aliases(aliasesAddRequest).actionGet() + ); + assertThat(e.getMessage(), equalTo("aliases that point to data streams don't support index_routing")); + } + { + AliasActions addAction = new AliasActions(AliasActions.Type.ADD).index("metrics-*") + .aliases("my-alias") + .searchRouting("[search_routing]"); + IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest(); + aliasesAddRequest.addAliasAction(addAction); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> client().admin().indices().aliases(aliasesAddRequest).actionGet() + ); + assertThat(e.getMessage(), equalTo("aliases that point to data streams don't support search_routing")); + } + { + AliasActions addAction = new AliasActions(AliasActions.Type.ADD).index("metrics-*") + .aliases("my-alias") + .writeIndex(randomBoolean()); + IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest(); + aliasesAddRequest.addAliasAction(addAction); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> client().admin().indices().aliases(aliasesAddRequest).actionGet() + ); + assertThat(e.getMessage(), equalTo("aliases that point to data streams don't support is_write_index")); + } + { + AliasActions addAction = new AliasActions(AliasActions.Type.ADD).index("metrics-*") + .aliases("my-alias") + .isHidden(randomBoolean()); + IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest(); + aliasesAddRequest.addAliasAction(addAction); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> client().admin().indices().aliases(aliasesAddRequest).actionGet() + ); + assertThat(e.getMessage(), equalTo("aliases that point to data streams don't support is_hidden")); + } + } + public void testTimestampFieldCustomAttributes() throws Exception { String mapping = "{\n" + " \"properties\": {\n"