Browse Source

Fix corrupted Metadata from index and alias having the same name (#91456)

This fixes a bug introduced in https://github.com/elastic/elasticsearch/pull/87863
which added a Metadata copy constructor with separate name collision checks that
assumed index name and alias names were already validated in `IndexMetada`.
=> fixed corrupted states by actually adding the validation to `IndexMetadata`
to make it impossible to instantiate a broken `IndexMetadata` in the first place.
Armin Braun 2 years ago
parent
commit
a867ba128a

+ 5 - 0
docs/changelog/91456.yaml

@@ -0,0 +1,5 @@
+pr: 91456
+summary: Fix corrupted Metadata from index and alias having the same name
+area: Cluster Coordination
+type: bug
+issues: []

+ 9 - 0
server/src/internalClusterTest/java/org/elasticsearch/aliases/IndexAliasesIT.java

@@ -1376,6 +1376,15 @@ public class IndexAliasesIT extends ESIntegTestCase {
         assertThat(searchResponse.getHits().getHits(), emptyArray());
     }
 
+    public void testCreateIndexAndAliasWithSameNameFails() {
+        final String indexName = "index-name";
+        final IllegalArgumentException iae = expectThrows(
+            IllegalArgumentException.class,
+            () -> client().admin().indices().prepareCreate(indexName).addAlias(new Alias(indexName)).execute().actionGet()
+        );
+        assertEquals("alias name [" + indexName + "] self-conflicts with index name", iae.getMessage());
+    }
+
     public void testGetAliasAndAliasExistsForHiddenAliases() {
         final String writeIndex = randomAlphaOfLength(5).toLowerCase(Locale.ROOT);
         final String nonWriteIndex = randomAlphaOfLength(6).toLowerCase(Locale.ROOT);

+ 8 - 1
server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java

@@ -2076,6 +2076,13 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
                 );
             }
 
+            var aliasesMap = aliases.build();
+            for (AliasMetadata alias : aliasesMap.values()) {
+                if (alias.alias().equals(index)) {
+                    throw new IllegalArgumentException("alias name [" + index + "] self-conflicts with index name");
+                }
+            }
+
             final boolean isSearchableSnapshot = SearchableSnapshotsSettings.isSearchableSnapshotStore(settings);
             final String indexMode = settings.get(IndexSettings.MODE.getKey());
             final boolean isTsdb = indexMode != null && IndexMode.TIME_SERIES.getName().equals(indexMode.toLowerCase(Locale.ROOT));
@@ -2091,7 +2098,7 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
                 numberOfReplicas,
                 settings,
                 mapping,
-                aliases.build(),
+                aliasesMap,
                 newCustomMetadata,
                 Map.ofEntries(denseInSyncAllocationIds),
                 requireFilters,

+ 13 - 0
server/src/test/java/org/elasticsearch/cluster/metadata/IndexMetadataTests.java

@@ -487,6 +487,19 @@ public class IndexMetadataTests extends ESTestCase {
         assertThat(idxMeta2.getLifecyclePolicyName(), equalTo("some_policy"));
     }
 
+    public void testIndexAndAliasWithSameName() {
+        final IllegalArgumentException iae = expectThrows(
+            IllegalArgumentException.class,
+            () -> IndexMetadata.builder("index")
+                .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
+                .numberOfShards(1)
+                .numberOfReplicas(0)
+                .putAlias(AliasMetadata.builder("index").build())
+                .build()
+        );
+        assertEquals("alias name [index] self-conflicts with index name", iae.getMessage());
+    }
+
     private static Settings indexSettingsWithDataTier(String dataTier) {
         return Settings.builder()
             .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)

+ 6 - 21
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java

@@ -193,26 +193,6 @@ public class MetadataTests extends ESTestCase {
         assertThat(aliases.get(1).alias(), equalTo("bb"));
     }
 
-    public void testIndexAndAliasWithSameName() {
-        IndexMetadata.Builder builder = IndexMetadata.builder("index")
-            .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
-            .numberOfShards(1)
-            .numberOfReplicas(0)
-            .putAlias(AliasMetadata.builder("index").build());
-        try {
-            Metadata.builder().put(builder).build();
-            fail("exception should have been thrown");
-        } catch (IllegalStateException e) {
-            assertThat(
-                e.getMessage(),
-                equalTo(
-                    "index, alias, and data stream names need to be unique, but the following duplicates were found [index (alias "
-                        + "of [index]) conflicts with index]"
-                )
-            );
-        }
-    }
-
     public void testAliasCollidingWithAnExistingIndex() {
         int indexCount = randomIntBetween(10, 100);
         Set<String> indices = Sets.newHashSetWithExpectedSize(indexCount);
@@ -221,7 +201,12 @@ public class MetadataTests extends ESTestCase {
         }
         Map<String, Set<String>> aliasToIndices = new HashMap<>();
         for (String alias : randomSubsetOf(randomIntBetween(1, 10), indices)) {
-            aliasToIndices.put(alias, new HashSet<>(randomSubsetOf(randomIntBetween(1, 3), indices)));
+            Set<String> indicesInAlias;
+            do {
+                indicesInAlias = new HashSet<>(randomSubsetOf(randomIntBetween(1, 3), indices));
+                indicesInAlias.remove(alias);
+            } while (indicesInAlias.isEmpty());
+            aliasToIndices.put(alias, indicesInAlias);
         }
         int properAliases = randomIntBetween(0, 3);
         for (int i = 0; i < properAliases; i++) {