Add more validation for data stream aliases. (elastic#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 elastic#66163
martijnvg · May 27, 2021 · 292e4cd · 292e4cd
1 parent 78284c8
commit 292e4cd
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 0 deletions.
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
@@ -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/...main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java b/...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<String> 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<String> 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/...data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java b/...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"