Snapshot/Restore: Make sure indices cannot be renamed into restored aliases

Fixes #7915

Author: imotov

src/main/java/org/elasticsearch/snapshots/RestoreService.java

@@ -46,14 +46,12 @@
 import org.elasticsearch.transport.*;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
+import static com.google.common.collect.Sets.newHashSet;
 import static org.elasticsearch.cluster.metadata.MetaDataIndexStateService.INDEX_CLOSED_BLOCK;
 
 /**
@@ -146,6 +144,7 @@ public ClusterState execute(ClusterState currentState) {
                     ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
                     RoutingTable.Builder rtBuilder = RoutingTable.builder(currentState.routingTable());
                     final ImmutableMap<ShardId, RestoreMetaData.ShardRestoreStatus> shards;
+                    Set<String> aliases = newHashSet();
                     if (!renamedIndices.isEmpty()) {
                         // We have some indices to restore
                         ImmutableMap.Builder<ShardId, RestoreMetaData.ShardRestoreStatus> shardsBuilder = ImmutableMap.builder();
@@ -166,6 +165,10 @@ public ClusterState execute(ClusterState currentState) {
                                 if (!request.includeAliases() && !snapshotIndexMetaData.aliases().isEmpty()) {
                                     // Remove all aliases - they shouldn't be restored
                                     indexMdBuilder.removeAllAliases();
+                                } else {
+                                    for (ObjectCursor<String> alias : snapshotIndexMetaData.aliases().keys()) {
+                                        aliases.add(alias.value);
+                                    }
                                 }
                                 IndexMetaData updatedIndexMetaData = indexMdBuilder.build();
                                 if (partial) {
@@ -187,6 +190,10 @@ public ClusterState execute(ClusterState currentState) {
                                     for (ObjectCursor<AliasMetaData> alias : currentIndexMetaData.aliases().values()) {
                                         indexMdBuilder.putAlias(alias.value);
                                     }
+                                } else {
+                                    for (ObjectCursor<String> alias : snapshotIndexMetaData.aliases().keys()) {
+                                        aliases.add(alias.value);
+                                    }
                                 }
                                 IndexMetaData updatedIndexMetaData = indexMdBuilder.index(renamedIndex).build();
                                 rtBuilder.addAsRestore(updatedIndexMetaData, restoreSource);
@@ -209,12 +216,14 @@ public ClusterState execute(ClusterState currentState) {
                         shards = ImmutableMap.of();
                     }
 
+                    checkAliasNameConflicts(renamedIndices, aliases);
+
                     // Restore global state if needed
                     restoreGlobalStateIfRequested(mdBuilder);
 
                     if (completed(shards)) {
                         // We don't have any indices to restore - we are done
-                        restoreInfo = new RestoreInfo(request.name(), ImmutableList.<String>copyOf(renamedIndices.keySet()),
+                        restoreInfo = new RestoreInfo(request.name(), ImmutableList.copyOf(renamedIndices.keySet()),
                                 shards.size(), shards.size() - failedShards(shards));
                     }
 
@@ -223,6 +232,14 @@ public ClusterState execute(ClusterState currentState) {
                     return ClusterState.builder(updatedState).routingResult(routingResult).build();
                 }
 
+                private void checkAliasNameConflicts(Map<String, String> renamedIndices, Set<String> aliases) {
+                    for(Map.Entry<String, String> renamedIndex: renamedIndices.entrySet()) {
+                        if (aliases.contains(renamedIndex.getKey())) {
+                            throw new SnapshotRestoreException(snapshotId, "cannot rename index [" + renamedIndex.getValue() + "] into [" + renamedIndex.getKey() + "] because of conflict with an alias with the same name");
+                        }
+                    }
+                }
+
                 private void populateIgnoredShards(String index, IntSet ignoreShards) {
                     for (SnapshotShardFailure failure : snapshot.shardFailures()) {
                         if (index.equals(failure.index())) {

src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java

@@ -776,9 +776,15 @@ public void renameOnRestoreTest() throws Exception {
                 .setType("fs").setSettings(ImmutableSettings.settingsBuilder()
                         .put("location", newTempDir(LifecycleScope.SUITE))));
 
-        createIndex("test-idx-1", "test-idx-2");
+        createIndex("test-idx-1", "test-idx-2", "test-idx-3");
         ensureGreen();
 
+        assertAcked(client.admin().indices().prepareAliases()
+                        .addAlias("test-idx-1", "alias-1")
+                        .addAlias("test-idx-2", "alias-2")
+                        .addAlias("test-idx-3", "alias-3")
+        );
+
         logger.info("--> indexing some data");
         for (int i = 0; i < 100; i++) {
             index("test-idx-1", "doc", Integer.toString(i), "foo", "bar" + i);
@@ -823,6 +829,9 @@ public void renameOnRestoreTest() throws Exception {
                 .setRenamePattern("(.+-2)").setRenameReplacement("$1-copy").setWaitForCompletion(true).execute().actionGet();
         assertThat(restoreSnapshotResponse.getRestoreInfo().totalShards(), greaterThan(0));
 
+        logger.info("--> delete indices");
+        cluster().wipeIndices("test-idx-1", "test-idx-1-copy", "test-idx-2", "test-idx-2-copy");
+
         logger.info("--> try renaming indices using the same name");
         try {
             client.admin().cluster().prepareRestoreSnapshot("test-repo", "test-snap").setRenamePattern("(.+)").setRenameReplacement("same-name").setWaitForCompletion(true).execute().actionGet();
@@ -846,6 +855,38 @@ public void renameOnRestoreTest() throws Exception {
         } catch (InvalidIndexNameException ex) {
             // Expected
         }
+
+        logger.info("--> try renaming indices into existing alias name");
+        try {
+            client.admin().cluster().prepareRestoreSnapshot("test-repo", "test-snap").setIndices("test-idx-1").setRenamePattern(".+").setRenameReplacement("alias-3").setWaitForCompletion(true).execute().actionGet();
+            fail("Shouldn't be here");
+        } catch (InvalidIndexNameException ex) {
+            // Expected
+        }
+
+        logger.info("--> try renaming indices into existing alias of itself");
+        try {
+            client.admin().cluster().prepareRestoreSnapshot("test-repo", "test-snap").setIndices("test-idx-1").setRenamePattern("test-idx").setRenameReplacement("alias").setWaitForCompletion(true).execute().actionGet();
+            fail("Shouldn't be here");
+        } catch (SnapshotRestoreException ex) {
+            // Expected
+        }
+
+        logger.info("--> try renaming indices into existing alias of another restored index");
+        try {
+            client.admin().cluster().prepareRestoreSnapshot("test-repo", "test-snap").setIndices("test-idx-1", "test-idx-2").setRenamePattern("test-idx-1").setRenameReplacement("alias-2").setWaitForCompletion(true).execute().actionGet();
+            fail("Shouldn't be here");
+        } catch (SnapshotRestoreException ex) {
+            // Expected
+        }
+
+        logger.info("--> try renaming indices into existing alias of itself, but don't restore aliases ");
+        restoreSnapshotResponse = client.admin().cluster().prepareRestoreSnapshot("test-repo", "test-snap")
+                .setIndices("test-idx-1").setRenamePattern("test-idx").setRenameReplacement("alias")
+                .setWaitForCompletion(true).setIncludeAliases(false).execute().actionGet();
+        assertThat(restoreSnapshotResponse.getRestoreInfo().totalShards(), greaterThan(0));
+
+
     }
 
     @Test