Validate alias operations don't target data streams (#58327)

andreidan · web-flow · commit 816448990e46 · 2020-06-18T16:57:36.000+01:00
This adds validation to make sure alias operations (add, remove, remove index)
don't target data streams or the backing indices.
diff --git a/server/src/internalClusterTest/java/org/elasticsearch/indices/DataStreamIT.java b/server/src/internalClusterTest/java/org/elasticsearch/indices/DataStreamIT.java
@@ -21,6 +21,7 @@
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.action.ActionRequestBuilder;
 import org.elasticsearch.action.DocWriteRequest;
+import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
 import org.elasticsearch.action.admin.indices.datastream.CreateDataStreamAction;
 import org.elasticsearch.action.admin.indices.datastream.DeleteDataStreamAction;
 import org.elasticsearch.action.admin.indices.datastream.GetDataStreamAction;
@@ -433,6 +434,36 @@ public void testCannotDeleteComposableTemplateUsedByDataStream() throws Exceptio
         assertTrue(maybeE.isPresent());
     }
 
+    public void testAliasActionsFailOnDataStreams() throws Exception {
+        createIndexTemplate("id1", "metrics-foo*", "@timestamp1");
+        String dataStreamName = "metrics-foo";
+        CreateDataStreamAction.Request createDataStreamRequest = new CreateDataStreamAction.Request(dataStreamName);
+        client().admin().indices().createDataStream(createDataStreamRequest).get();
+
+        IndicesAliasesRequest.AliasActions addAction = new IndicesAliasesRequest.AliasActions(IndicesAliasesRequest.AliasActions.Type.ADD)
+            .index(dataStreamName).aliases("foo");
+        IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest();
+        aliasesAddRequest.addAliasAction(addAction);
+        expectFailure(dataStreamName, () -> client().admin().indices().aliases(aliasesAddRequest).actionGet());
+    }
+
+    public void testAliasActionsFailOnDataStreamBackingIndices() throws Exception {
+        createIndexTemplate("id1", "metrics-foo*", "@timestamp1");
+        String dataStreamName = "metrics-foo";
+        CreateDataStreamAction.Request createDataStreamRequest = new CreateDataStreamAction.Request(dataStreamName);
+        client().admin().indices().createDataStream(createDataStreamRequest).get();
+
+        String backingIndex = DataStream.getDefaultBackingIndexName(dataStreamName, 1);
+        IndicesAliasesRequest.AliasActions addAction = new IndicesAliasesRequest.AliasActions(IndicesAliasesRequest.AliasActions.Type.ADD)
+            .index(backingIndex).aliases("first_gen");
+        IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest();
+        aliasesAddRequest.addAliasAction(addAction);
+        Exception e = expectThrows(IllegalArgumentException.class, () -> client().admin().indices().aliases(aliasesAddRequest).actionGet());
+        assertThat(e.getMessage(), equalTo("The provided expressions [" + backingIndex
+            + "] match a backing index belonging to data stream [" + dataStreamName + "]. Data streams and their backing indices don't " +
+            "support aliases."));
+    }
+
     private static void verifyResolvability(String dataStream, ActionRequestBuilder requestBuilder, boolean fail) {
         verifyResolvability(dataStream, requestBuilder, fail, 0);
     }
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
@@ -34,6 +34,7 @@
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.metadata.AliasAction;
 import org.elasticsearch.cluster.metadata.AliasMetadata;
+import org.elasticsearch.cluster.metadata.IndexAbstraction;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.metadata.MetadataIndexAliasesService;
@@ -110,16 +111,26 @@ protected void masterOperation(Task task, final IndicesAliasesRequest request, f
         //Expand the indices names
         List<AliasActions> actions = request.aliasActions();
         List<AliasAction> finalActions = new ArrayList<>();
-
         // Resolve all the AliasActions into AliasAction instances and gather all the aliases
         Set<String> aliases = new HashSet<>();
         for (AliasActions action : actions) {
-            final Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(state, request.indicesOptions(), action.indices());
+            final Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(state, request.indicesOptions(), false,
+                action.indices());
+            for (Index concreteIndex : concreteIndices) {
+                IndexAbstraction indexAbstraction = state.metadata().getIndicesLookup().get(concreteIndex.getName());
+                assert indexAbstraction != null : "invalid cluster metadata. index [" + concreteIndex.getName() + "] was not found";
+                if (indexAbstraction.getParentDataStream() != null) {
+                    throw new IllegalArgumentException("The provided expressions [" + String.join(",", action.indices())
+                        + "] match a backing index belonging to data stream [" + indexAbstraction.getParentDataStream().getName()
+                        + "]. Data streams and their backing indices don't support aliases.");
+                }
+            }
             final Optional<Exception> maybeException = requestValidators.validateRequest(request, state, concreteIndices);
             if (maybeException.isPresent()) {
                 listener.onFailure(maybeException.get());
                 return;
             }
+
             Collections.addAll(aliases, action.getOriginalAliases());
             for (final Index index : concreteIndices) {
                 switch (action.actionType()) {
diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java
@@ -90,7 +90,7 @@ public ClusterState execute(ClusterState currentState) {
             });
     }
 
-     /**
+    /**
      * Handles the cluster state transition to a version that reflects the provided {@link AliasAction}s.
      */
      public ClusterState applyAliasActions(ClusterState currentState, Iterable<AliasAction> actions) {
@@ -108,6 +108,7 @@ public ClusterState applyAliasActions(ClusterState currentState, Iterable<AliasA
                     if (index == null) {
                         throw new IndexNotFoundException(action.getIndex());
                     }
+                    validateAliasTargetIsNotDSBackingIndex(currentState, action);
                     indicesToDelete.add(index.getIndex());
                     changed = true;
                 }
@@ -128,6 +129,7 @@ public ClusterState applyAliasActions(ClusterState currentState, Iterable<AliasA
                 if (index == null) {
                     throw new IndexNotFoundException(action.getIndex());
                 }
+                validateAliasTargetIsNotDSBackingIndex(currentState, action);
                 NewAliasValidator newAliasValidator = (alias, indexRouting, filter, writeIndex) -> {
                     /* It is important that we look up the index using the metadata builder we are modifying so we can remove an
                      * index and replace it with an alias. */
@@ -187,4 +189,13 @@ public ClusterState applyAliasActions(ClusterState currentState, Iterable<AliasA
         }
     }
 
+    private void validateAliasTargetIsNotDSBackingIndex(ClusterState currentState, AliasAction action) {
+        IndexAbstraction indexAbstraction = currentState.metadata().getIndicesLookup().get(action.getIndex());
+        assert indexAbstraction != null : "invalid cluster metadata. index [" + action.getIndex() + "] was not found";
+        if (indexAbstraction.getParentDataStream() != null) {
+            throw new IllegalArgumentException("The provided index [ " + action.getIndex()
+                + "] is a backing index belonging to data stream [" + indexAbstraction.getParentDataStream().getName()
+                + "]. Data streams and their backing indices don't support alias operations.");
+        }
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java
@@ -1688,6 +1688,40 @@ public void testIndicesAliasesRequestIgnoresAliases() {
         }
     }
 
+    public void testIndicesAliasesRequestTargetDataStreams() {
+        final String dataStreamName = "my-data-stream";
+        IndexMetadata backingIndex = createBackingIndex(dataStreamName, 1).build();
+
+        Metadata.Builder mdBuilder = Metadata.builder()
+            .put(backingIndex, false)
+            .put(new DataStream(dataStreamName, "ts", List.of(backingIndex.getIndex()), 1));
+        ClusterState state = ClusterState.builder(new ClusterName("_name")).metadata(mdBuilder).build();
+
+        {
+            IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.add().index(dataStreamName);
+            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
+                () -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions, false));
+            assertEquals("The provided expression [" + dataStreamName + "] matches a data stream, specify the corresponding " +
+                "concrete indices instead.", iae.getMessage());
+        }
+
+        {
+            IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.add().index("my-data-*").alias("my-data");
+            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
+                () -> indexNameExpressionResolver.concreteIndexNames(state, aliasActions, false));
+            assertEquals("The provided expression [my-data-*] matches a data stream, specify the corresponding concrete indices instead.",
+                iae.getMessage());
+        }
+
+        {
+            IndicesAliasesRequest.AliasActions aliasActions = IndicesAliasesRequest.AliasActions.add().index(dataStreamName)
+                .alias("my-data");
+            String[] indices = indexNameExpressionResolver.concreteIndexNames(state, aliasActions, true);
+            assertEquals(1, indices.length);
+            assertEquals(backingIndex.getIndex().getName(), indices[0]);
+        }
+    }
+
     public void testInvalidIndex() {
         Metadata.Builder mdBuilder = Metadata.builder().put(indexBuilder("test"));
         ClusterState state = ClusterState.builder(new ClusterName("_name")).metadata(mdBuilder).build();
diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java
@@ -40,6 +40,7 @@
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.startsWith;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anySetOf;
@@ -487,6 +488,24 @@ public void testSimultaneousHiddenPropertyValidation() {
         }
     }
 
+    public void testAliasesForDataStreamBackingIndicesNotSupported() {
+        String dataStreamName = "foo-stream";
+        String backingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, 1);
+        IndexMetadata indexMetadata = IndexMetadata.builder(backingIndexName)
+            .settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(1).build();
+        ClusterState state = ClusterState.builder(ClusterName.DEFAULT)
+            .metadata(
+                Metadata.builder()
+                    .put(indexMetadata, true)
+                    .put(new DataStream(dataStreamName, "@timestamp", singletonList(indexMetadata.getIndex()))))
+            .build();
+
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> service.applyAliasActions(state,
+            singletonList(new AliasAction.Add(backingIndexName, "test", null, null, null, null, null))));
+        assertThat(exception.getMessage(), is("The provided index [ .ds-foo-stream-000001] is a backing index belonging to data stream " +
+            "[foo-stream]. Data streams and their backing indices don't support alias operations."));
+    }
+
     private ClusterState applyHiddenAliasMix(ClusterState before, Boolean isHidden1, Boolean isHidden2) {
         return service.applyAliasActions(before, Arrays.asList(
             new AliasAction.Add("test", "alias", null, null, null, null, isHidden1),

Original file line number	Diff line number	Diff line change
`@@ -90,7 +90,7 @@ public ClusterState execute(ClusterState currentState) {`
`90`	`90`	`});`
`91`	`91`	`}`
`92`	`92`
`93`		`- /**`
	`93`	`+ /**`
`94`	`94`	`* Handles the cluster state transition to a version that reflects the provided {@link AliasAction}s.`
`95`	`95`	`*/`
`96`	`96`	`public ClusterState applyAliasActions(ClusterState currentState, Iterable<AliasAction> actions) {`
`@@ -108,6 +108,7 @@ public ClusterState applyAliasActions(ClusterState currentState, Iterable<AliasA`
`108`	`108`	`if (index == null) {`
`109`	`109`	`throw new IndexNotFoundException(action.getIndex());`
`110`	`110`	`}`
	`111`	`+ validateAliasTargetIsNotDSBackingIndex(currentState, action);`
`111`	`112`	`indicesToDelete.add(index.getIndex());`
`112`	`113`	`changed = true;`
`113`	`114`	`}`
`@@ -128,6 +129,7 @@ public ClusterState applyAliasActions(ClusterState currentState, Iterable<AliasA`
`128`	`129`	`if (index == null) {`
`129`	`130`	`throw new IndexNotFoundException(action.getIndex());`
`130`	`131`	`}`
	`132`	`+ validateAliasTargetIsNotDSBackingIndex(currentState, action);`
`131`	`133`	`NewAliasValidator newAliasValidator = (alias, indexRouting, filter, writeIndex) -> {`
`132`	`134`	`/* It is important that we look up the index using the metadata builder we are modifying so we can remove an`
`133`	`135`	`* index and replace it with an alias. */`
`@@ -187,4 +189,13 @@ public ClusterState applyAliasActions(ClusterState currentState, Iterable<AliasA`
`187`	`189`	`}`
`188`	`190`	`}`
`189`	`191`
	`192`	`+ private void validateAliasTargetIsNotDSBackingIndex(ClusterState currentState, AliasAction action) {`
	`193`	`+ IndexAbstraction indexAbstraction = currentState.metadata().getIndicesLookup().get(action.getIndex());`
	`194`	`+ assert indexAbstraction != null : "invalid cluster metadata. index [" + action.getIndex() + "] was not found";`
	`195`	`+ if (indexAbstraction.getParentDataStream() != null) {`
	`196`	`+ throw new IllegalArgumentException("The provided index [ " + action.getIndex()`
	`197`	`+ + "] is a backing index belonging to data stream [" + indexAbstraction.getParentDataStream().getName()`
	`198`	`+ + "]. Data streams and their backing indices don't support alias operations.");`
	`199`	`+ }`
	`200`	`+ }`
`190`	`201`	`}`