From 2cbe1b9d593f50509611ee69eb823c1f156bd5e1 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Mon, 29 Sep 2014 23:23:59 +0400 Subject: [PATCH] Snapshot/Restore: Make sure indices cannot be renamed into restored aliases Fixes #7915 --- .../snapshots/RestoreService.java | 27 +++++++++--- .../SharedClusterSnapshotRestoreTests.java | 43 ++++++++++++++++++- 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/src/main/java/org/elasticsearch/snapshots/RestoreService.java index aaba177827f6c..98696c5fe3a6b 100644 --- a/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/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 shards; + Set aliases = newHashSet(); if (!renamedIndices.isEmpty()) { // We have some indices to restore ImmutableMap.Builder 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 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 alias : currentIndexMetaData.aliases().values()) { indexMdBuilder.putAlias(alias.value); } + } else { + for (ObjectCursor 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.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 renamedIndices, Set aliases) { + for(Map.Entry 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())) { diff --git a/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java b/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java index b4b03e4a03653..3fd25637902e0 100644 --- a/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java +++ b/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