[7.16] Fix data stream alias validation. (#81040) (#81136)

* Fix data stream alias validation. (#81040)

In case of restoring a snapshot, it is possible to overwrite an existing
data stream with a data stream alias from a snapshot. This change fixes
this by improving the generic duplicate name validation.

On top of this the lack of data stream alias validation in Metadata.Builder#build()
method resulted in cases where data stream aliases could be added for existing
index aliases, data streams or indices with the same name.

Closes #80972

* adjust to 7.16 reality

* Unmute DataStreamsSnapshotsIT#testRestoreDataStreamAliasWithConflictingIndicesAlias() test
and fix the test problem, which is that testRestoreDataStreamAliasWithConflictingDataStream()
test needs to remove the composable index template that it adds. The base test class
doesn't remove any composable index templates and this template interferes with the
testRestoreDataStreamAliasWithConflictingIndicesAlias() test.

Relates to #81040

Author: martijnvg

server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java

@@ -1740,17 +1740,23 @@ public Metadata build(boolean builtIndicesLookupEagerly) {
                 indexMetadata.getAliases().keysIt().forEachRemaining(allAliases::add);
             }
 
+            final ArrayList<String> duplicates = new ArrayList<>();
             final Set<String> allDataStreams = new HashSet<>();
             DataStreamMetadata dataStreamMetadata = (DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE);
             if (dataStreamMetadata != null) {
                 for (DataStream dataStream : dataStreamMetadata.dataStreams().values()) {
                     allDataStreams.add(dataStream.getName());
                 }
+                // Adding data stream aliases:
+                for (String dataStreamAlias : dataStreamMetadata.getDataStreamAliases().keySet()) {
+                    if (allAliases.add(dataStreamAlias) == false) {
+                        duplicates.add("data stream alias and indices alias have the same name (" + dataStreamAlias + ")");
+                    }
+                }
             }
 
             final Set<String> aliasDuplicatesWithIndices = new HashSet<>(allAliases);
             aliasDuplicatesWithIndices.retainAll(allIndices);
-            ArrayList<String> duplicates = new ArrayList<>();
             if (aliasDuplicatesWithIndices.isEmpty() == false) {
                 // iterate again and constructs a helpful message
                 for (ObjectCursor<IndexMetadata> cursor : indices.values()) {
@@ -1766,12 +1772,19 @@ public Metadata build(boolean builtIndicesLookupEagerly) {
             aliasDuplicatesWithDataStreams.retainAll(allDataStreams);
             if (aliasDuplicatesWithDataStreams.isEmpty() == false) {
                 // iterate again and constructs a helpful message
-                for (ObjectCursor<IndexMetadata> cursor : indices.values()) {
-                    for (String alias : aliasDuplicatesWithDataStreams) {
+                for (String alias : aliasDuplicatesWithDataStreams) {
+                    // reported var avoids adding a message twice if an index alias has the same name as a data stream.
+                    boolean reported = false;
+                    for (ObjectCursor<IndexMetadata> cursor : indices.values()) {
                         if (cursor.value.getAliases().containsKey(alias)) {
                             duplicates.add(alias + " (alias of " + cursor.value.getIndex() + ") conflicts with data stream");
+                            reported = true;
                         }
                     }
+                    // This is for adding an error message for when a data steam alias has the same name as a data stream.
+                    if (reported == false && dataStreamMetadata != null && dataStreamMetadata.dataStreams().containsKey(alias)) {
+                        duplicates.add("data stream alias and data stream have the same name (" + alias + ")");
+                    }
                 }
             }
 

server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java

@@ -48,6 +48,7 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
+import static java.util.Collections.singletonList;
 import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.createBackingIndex;
 import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.createFirstBackingIndex;
 import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.createTimestampField;
@@ -1231,9 +1232,7 @@ public void testOverlappingDataStreamNamesWithBackingIndexDatePattern() {
             .numberOfReplicas(1)
             .build();
         b.put(ds2Index1, false);
-        b.put(
-            new DataStream(dataStreamName2, createTimestampField("@timestamp"), Collections.singletonList(ds2Index1.getIndex()), 1, null)
-        );
+        b.put(new DataStream(dataStreamName2, createTimestampField("@timestamp"), singletonList(ds2Index1.getIndex()), 1, null));
 
         Metadata metadata = b.build();
         assertThat(metadata.dataStreams().size(), equalTo(2));
@@ -1314,6 +1313,74 @@ public void testBuildIndicesLookupForDataStreamAliases() {
         assertThat(value.getAliases(), nullValue());
     }
 
+    public void testDataStreamAliasValidation() {
+        Metadata.Builder b = Metadata.builder();
+        addDataStream("my-alias", b);
+        b.put("my-alias", "my-alias", null, null);
+        Exception e = expectThrows(IllegalStateException.class, b::build);
+        assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));
+
+        b = Metadata.builder();
+        addDataStream("d1", b);
+        addDataStream("my-alias", b);
+        b.put("my-alias", "d1", null, null);
+        e = expectThrows(IllegalStateException.class, b::build);
+        assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));
+
+        b = Metadata.builder();
+        b.put(
+            IndexMetadata.builder("index1")
+                .settings(
+                    Settings.builder()
+                        .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
+                        .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
+                        .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
+                )
+                .putAlias(new AliasMetadata.Builder("my-alias"))
+        );
+
+        addDataStream("d1", b);
+        b.put("my-alias", "d1", null, null);
+        e = expectThrows(IllegalStateException.class, b::build);
+        assertThat(e.getMessage(), containsString("data stream alias and indices alias have the same name (my-alias)"));
+    }
+
+    public void testDataStreamAliasValidationRestoreScenario() {
+        Metadata.Builder b = Metadata.builder();
+        b.dataStreams(
+            org.elasticsearch.core.Map.of("my-alias", createDataStream("my-alias")),
+            org.elasticsearch.core.Map.of("my-alias", new DataStreamAlias("my-alias", singletonList("my-alias"), null, null))
+        );
+        Exception e = expectThrows(IllegalStateException.class, b::build);
+        assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));
+
+        b = Metadata.builder();
+        b.dataStreams(
+            org.elasticsearch.core.Map.of("d1", createDataStream("d1"), "my-alias", createDataStream("my-alias")),
+            org.elasticsearch.core.Map.of("my-alias", new DataStreamAlias("my-alias", singletonList("d1"), null, null))
+        );
+        e = expectThrows(IllegalStateException.class, b::build);
+        assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));
+
+        b = Metadata.builder();
+        b.put(
+            IndexMetadata.builder("index1")
+                .settings(
+                    Settings.builder()
+                        .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
+                        .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
+                        .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
+                )
+                .putAlias(new AliasMetadata.Builder("my-alias"))
+        );
+        b.dataStreams(
+            org.elasticsearch.core.Map.of("d1", createDataStream("d1")),
+            org.elasticsearch.core.Map.of("my-alias", new DataStreamAlias("my-alias", singletonList("d1"), null, null))
+        );
+        e = expectThrows(IllegalStateException.class, b::build);
+        assertThat(e.getMessage(), containsString("data stream alias and indices alias have the same name (my-alias)"));
+    }
+
     private void addDataStream(String name, Metadata.Builder b) {
         int numBackingIndices = randomIntBetween(1, 4);
         List<Index> indices = new ArrayList<>(numBackingIndices);
@@ -1325,6 +1392,16 @@ private void addDataStream(String name, Metadata.Builder b) {
         b.put(new DataStream(name, createTimestampField("@timestamp"), indices));
     }
 
+    private DataStream createDataStream(String name) {
+        int numBackingIndices = randomIntBetween(1, 4);
+        List<Index> indices = new ArrayList<>(numBackingIndices);
+        for (int j = 1; j <= numBackingIndices; j++) {
+            IndexMetadata idx = createBackingIndex(name, j).build();
+            indices.add(idx.getIndex());
+        }
+        return new DataStream(name, createTimestampField("@timestamp"), indices);
+    }
+
     public void testIndicesLookupRecordsDataStreamForBackingIndices() {
         final int numIndices = randomIntBetween(2, 5);
         final int numBackingIndices = randomIntBetween(2, 5);
@@ -1772,7 +1849,7 @@ public void testReuseIndicesLookup() {
             DataStream dataStream = new DataStream(
                 dataStreamName,
                 new DataStream.TimestampField("@timestamp"),
-                Collections.singletonList(idx.getIndex())
+                singletonList(idx.getIndex())
             );
             builder.put(dataStream);
             Metadata metadata = builder.build();