From 1690e786463c7844049dfa769d4ca1307f045bc5 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Fri, 27 Mar 2020 10:07:46 -0500 Subject: [PATCH] Validation for data stream creation --- .../datastream/CreateDataStreamAction.java | 5 +- .../cluster/metadata/MetaData.java | 25 ++++++++-- .../CreateDataStreamRequestTests.java | 9 ++++ .../cluster/metadata/MetaDataTests.java | 46 +++++++++++++++++++ 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/datastream/CreateDataStreamAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/datastream/CreateDataStreamAction.java index 5281c977908..0db3f0eb168 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/datastream/CreateDataStreamAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/datastream/CreateDataStreamAction.java @@ -35,6 +35,7 @@ import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.DataStream; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.metadata.MetaDataCreateIndexService; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; import org.elasticsearch.common.Strings; @@ -164,9 +165,11 @@ public class CreateDataStreamAction extends ActionType { throw new IllegalArgumentException("data_stream [" + request.name + "] already exists"); } + MetaDataCreateIndexService.validateIndexOrAliasName(request.name, + (s1, s2) -> new IllegalArgumentException("data_stream [" + s1 + "] " + s2)); + MetaData.Builder builder = MetaData.builder(currentState.metaData()).put( new DataStream(request.name, request.timestampFieldName, Collections.emptyList())); - logger.info("adding data stream [{}]", request.name); return ClusterState.builder(currentState).metaData(builder).build(); } 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 9143ffb3217..5bd48647cbe 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -1353,7 +1353,7 @@ public class MetaData implements Iterable, Diffable, To public MetaData build() { // TODO: We should move these datastructures to IndexNameExpressionResolver, this will give the following benefits: - // 1) The datastructures will only be rebuilded when needed. Now during serializing we rebuild these datastructures + // 1) The datastructures will be rebuilt only when needed. Now during serializing we rebuild these datastructures // while these datastructures aren't even used. // 2) The aliasAndIndexLookup can be updated instead of rebuilding it all the time. @@ -1391,7 +1391,7 @@ public class MetaData implements Iterable, Diffable, To // iterate again and constructs a helpful message ArrayList duplicates = new ArrayList<>(); for (ObjectCursor cursor : indices.values()) { - for (String alias: duplicateAliasesIndices) { + for (String alias : duplicateAliasesIndices) { if (cursor.value.getAliases().containsKey(alias)) { duplicates.add(alias + " (alias of " + cursor.value.getIndex() + ")"); } @@ -1399,12 +1399,13 @@ public class MetaData implements Iterable, Diffable, To } assert duplicates.size() > 0; throw new IllegalStateException("index and alias names need to be unique, but the following duplicates were found [" - + Strings.collectionToCommaDelimitedString(duplicates)+ "]"); + + Strings.collectionToCommaDelimitedString(duplicates) + "]"); } SortedMap aliasAndIndexLookup = Collections.unmodifiableSortedMap(buildAliasAndIndexLookup()); + validateDataStreams(aliasAndIndexLookup); // build all concrete indices arrays: // TODO: I think we can remove these arrays. it isn't worth the effort, for operations on all indices. @@ -1447,6 +1448,24 @@ public class MetaData implements Iterable, Diffable, To return aliasAndIndexLookup; } + private void validateDataStreams(SortedMap aliasAndIndexLookup) { + DataStreamMetadata dsMetadata = (DataStreamMetadata) customs.get(DataStreamMetadata.TYPE); + if (dsMetadata != null) { + for (DataStream ds : dsMetadata.dataStreams().values()) { + if (aliasAndIndexLookup.containsKey(ds.getName())) { + throw new IllegalStateException("data stream [" + ds.getName() + "] conflicts with existing index or alias"); + } + + SortedMap map = aliasAndIndexLookup.subMap(ds.getName() + "-", ds.getName() + "."); // '.' is the char after '-' + if (map.size() != 0) { + throw new IllegalStateException("data stream [" + ds.getName() + + "] could create backing indices that conflict with " + map.size() + " existing index(s) or alias(s)" + + " including '" + map.firstKey() + "'"); + } + } + } + } + public static void toXContent(MetaData metaData, XContentBuilder builder, ToXContent.Params params) throws IOException { XContentContext context = XContentContext.valueOf(params.param(CONTEXT_MODE_PARAM, CONTEXT_MODE_API)); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/datastream/CreateDataStreamRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/datastream/CreateDataStreamRequestTests.java index db8ef692cd0..a1926f95ff8 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/datastream/CreateDataStreamRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/datastream/CreateDataStreamRequestTests.java @@ -81,4 +81,13 @@ public class CreateDataStreamRequestTests extends AbstractWireSerializingTestCas () -> CreateDataStreamAction.TransportAction.createDataStream(cs, req)); assertThat(e.getMessage(), containsString("data_stream [" + dataStreamName + "] already exists")); } + + public void testCreateDataStreamWithInvalidName() { + final String dataStreamName = "_My-da#ta- ,stream-"; + ClusterState cs = ClusterState.builder(new ClusterName("_name")).build(); + CreateDataStreamAction.Request req = new CreateDataStreamAction.Request(dataStreamName); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> CreateDataStreamAction.TransportAction.createDataStream(cs, req)); + assertThat(e.getMessage(), containsString("must not contain the following characters")); + } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java index b48124026d7..264f896c88d 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java @@ -44,6 +44,7 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -941,6 +942,51 @@ public class MetaDataTests extends ESTestCase { assertThat(expectThrows(NullPointerException.class, () -> builder.customs(map)).getMessage(), containsString(key)); } + public void testBuilderRejectsDataStreamThatConflictsWithIndex() { + final String dataStreamName = "my-data-stream"; + MetaData.Builder b = MetaData.builder() + .put(IndexMetaData.builder(dataStreamName) + .settings(settings(Version.CURRENT)) + .numberOfShards(1) + .numberOfReplicas(1) + .build(), false) + .put(new DataStream(dataStreamName, "ts", Collections.emptyList())); + + IllegalStateException e = expectThrows(IllegalStateException.class, b::build); + assertThat(e.getMessage(), containsString("data stream [" + dataStreamName + "] conflicts with existing index or alias")); + } + + public void testBuilderRejectsDataStreamThatConflictsWithAlias() { + final String dataStreamName = "my-data-stream"; + MetaData.Builder b = MetaData.builder() + .put(IndexMetaData.builder(dataStreamName + "z") + .settings(settings(Version.CURRENT)) + .numberOfShards(1) + .numberOfReplicas(1) + .putAlias(AliasMetaData.builder(dataStreamName).build()) + .build(), false) + .put(new DataStream(dataStreamName, "ts", Collections.emptyList())); + + IllegalStateException e = expectThrows(IllegalStateException.class, b::build); + assertThat(e.getMessage(), containsString("data stream [" + dataStreamName + "] conflicts with existing index or alias")); + } + + public void testBuilderRejectsDataStreamWithConflictingBackingIndices() { + final String dataStreamName = "my-data-stream"; + final String conflictingIndex = dataStreamName + "-00001"; + MetaData.Builder b = MetaData.builder() + .put(IndexMetaData.builder(conflictingIndex) + .settings(settings(Version.CURRENT)) + .numberOfShards(1) + .numberOfReplicas(1) + .build(), false) + .put(new DataStream(dataStreamName, "ts", Collections.emptyList())); + + IllegalStateException e = expectThrows(IllegalStateException.class, b::build); + assertThat(e.getMessage(), containsString("data stream [" + dataStreamName + + "] could create backing indices that conflict with 1 existing index(s) or alias(s) including '" + conflictingIndex + "'")); + } + public void testSerialization() throws IOException { final MetaData orig = randomMetaData(); final BytesStreamOutput out = new BytesStreamOutput();