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

) (#91469) This fixes a bug introduced in #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.
elastic · Nov 9, 2022 · c1310c4 · c1310c4
1 parent a1ed654
commit c1310c4
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 22 deletions.
diff --git a/docs/changelog/91456.yaml b/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: []
diff --git a/server/src/internalClusterTest/java/org/elasticsearch/aliases/IndexAliasesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/aliases/IndexAliasesIT.java
@@ -1376,6 +1376,15 @@ public void testIndexingAndQueryingHiddenAliases() throws Exception {
         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);

diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java
@@ -2017,6 +2017,13 @@ public IndexMetadata build() {
                 lifecycleExecutionState = LifecycleExecutionState.EMPTY_STATE;
             }
 
+            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));
@@ -2032,7 +2039,7 @@ public IndexMetadata build() {
                 numberOfReplicas,
                 settings,
                 mapping,
-                aliases.build(),
+                aliasesMap,
                 newCustomMetadata,
                 Map.ofEntries(denseInSyncAllocationIds),
                 requireFilters,

diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexMetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexMetadataTests.java
@@ -482,6 +482,19 @@ public void testLifeCyclePolicyName() {
         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)

diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java
@@ -193,26 +193,6 @@ public void testFindAliasWithExclusionAndOverride() {
         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 void testAliasCollidingWithAnExistingIndex() {
         }
         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++) {