From 30e777856f71467fc99f4d2c8254a52d56d49eff Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 18 Jun 2020 20:23:07 +0100 Subject: [PATCH] [7.x] Validate alias operations don't target data streams (#58327) (#58337) This adds validation to make sure alias operations (add, remove, remove index) don't target data streams or the backing indices. (cherry picked from commit 816448990e464a02f3960f12f6f6644a8cce36a4) Signed-off-by: Andrei Dan --- .../elasticsearch/indices/DataStreamIT.java | 31 +++++++++++++++++ .../alias/TransportIndicesAliasesAction.java | 15 ++++++-- .../metadata/MetadataIndexAliasesService.java | 13 ++++++- .../IndexNameExpressionResolverTests.java | 34 +++++++++++++++++++ .../MetadataIndexAliasesServiceTests.java | 19 +++++++++++ 5 files changed, 109 insertions(+), 3 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/indices/DataStreamIT.java b/server/src/internalClusterTest/java/org/elasticsearch/indices/DataStreamIT.java index 3cf4d847c63..76a3b750c93 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/indices/DataStreamIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/indices/DataStreamIT.java @@ -21,6 +21,7 @@ package org.elasticsearch.indices; 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; @@ -434,6 +435,36 @@ public class DataStreamIT extends ESIntegTestCase { 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 index 90f9970a6b8..f3acb28e7a8 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 @@ -34,6 +34,7 @@ import org.elasticsearch.cluster.block.ClusterBlockException; 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; @@ -109,16 +110,26 @@ public class TransportIndicesAliasesAction extends TransportMasterNodeAction actions = request.aliasActions(); List finalActions = new ArrayList<>(); - // Resolve all the AliasActions into AliasAction instances and gather all the aliases Set 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 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 index e67478eb17f..2ea44d04401 100644 --- 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 class MetadataIndexAliasesService { }); } - /** + /** * Handles the cluster state transition to a version that reflects the provided {@link AliasAction}s. */ public ClusterState applyAliasActions(ClusterState currentState, Iterable actions) { @@ -108,6 +108,7 @@ public class MetadataIndexAliasesService { if (index == null) { throw new IndexNotFoundException(action.getIndex()); } + validateAliasTargetIsNotDSBackingIndex(currentState, action); indicesToDelete.add(index.getIndex()); changed = true; } @@ -128,6 +129,7 @@ public class MetadataIndexAliasesService { 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 class MetadataIndexAliasesService { } } + 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 index ddeb1fa6d87..0314b6413e4 100644 --- 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 class IndexNameExpressionResolverTests extends ESTestCase { } } + 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", org.elasticsearch.common.collect.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 index 7284e96edb3..82d8b522679 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java @@ -41,6 +41,7 @@ import static java.util.Collections.singletonList; 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; @@ -488,6 +489,24 @@ public class MetadataIndexAliasesServiceTests extends ESTestCase { } } + 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),