From 691759fb1fc3b3cde08b3976e105dabcbb0f7b22 Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 15 Jul 2020 16:23:20 +0100 Subject: [PATCH] Validate snapshot UUID during restore (#59601) Today when mounting a searchable snapshot we obtain the snapshot/index UUIDs and then assume that these are the UUIDs used during the subsequent restore. If you concurrently delete the snapshot and replace it with one with the same name then this assumption is violated, with chaotic consequences. This commit introduces a check that ensures that the snapshot UUID does not change during the mount process. If the snapshot remains in place then the index UUID necessarily does not change either. Relates #50999 --- .../restore/RestoreSnapshotRequest.java | 40 ++++- .../snapshots/RestoreService.java | 5 + .../restore/RestoreSnapshotRequestTests.java | 5 + ...ransportMountSearchableSnapshotAction.java | 9 +- ...ableSnapshotsUuidValidationIntegTests.java | 150 ++++++++++++++++++ 5 files changed, 201 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsUuidValidationIntegTests.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 1c6d72c948b..0e4eab3f6c3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -25,6 +25,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.master.MasterNodeRequest; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -68,6 +69,9 @@ public class RestoreSnapshotRequest extends MasterNodeRequest getActionFilters() { + return singletonList(restoreBlockingActionFilter); + } + } + + public static class RestoreBlockingActionFilter extends org.elasticsearch.action.support.ActionFilter.Simple { + private final PlainActionFuture executed = new PlainActionFuture<>(); + private final PlainActionFuture unblocked = new PlainActionFuture<>(); + + @Override + protected boolean apply(String action, ActionRequest request, ActionListener listener) { + if (RestoreSnapshotAction.NAME.equals(action)) { + executed.onResponse(null); + unblocked.actionGet(); + } + return true; + } + + @Override + public int order() { + return 0; + } + + public void unblock() { + unblocked.onResponse(null); + } + + public void awaitExecution() { + executed.actionGet(); + } + } + + @Override + protected Collection> nodePlugins() { + return Stream.concat(super.nodePlugins().stream(), Stream.of(TestPlugin.class)).collect(Collectors.toList()); + } + + public void testMountFailsIfSnapshotChanged() throws Exception { + final String fsRepoName = randomAlphaOfLength(10); + final String indexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + final String restoredIndexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + final String snapshotName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + + createRepo(fsRepoName); + + final Settings.Builder originalIndexSettings = Settings.builder().put(INDEX_SOFT_DELETES_SETTING.getKey(), true); + createAndPopulateIndex(indexName, originalIndexSettings); + + createSnapshot(fsRepoName, snapshotName); + + final MountSearchableSnapshotRequest req = new MountSearchableSnapshotRequest( + restoredIndexName, + fsRepoName, + snapshotName, + indexName, + Settings.EMPTY, + Strings.EMPTY_ARRAY, + true + ); + + final ActionFuture responseFuture = client().execute(MountSearchableSnapshotAction.INSTANCE, req); + + final RestoreBlockingActionFilter restoreBlockingActionFilter = getBlockingActionFilter(); + restoreBlockingActionFilter.awaitExecution(); + + assertAcked(client().admin().cluster().prepareDeleteSnapshot(fsRepoName, snapshotName).get()); + createSnapshot(fsRepoName, snapshotName); + + assertFalse(responseFuture.isDone()); + restoreBlockingActionFilter.unblock(); + + assertThat( + expectThrows(SnapshotRestoreException.class, responseFuture::actionGet).getMessage(), + containsString("snapshot UUID mismatch") + ); + + assertAcked(client().admin().indices().prepareDelete(indexName)); + } + + private static void createSnapshot(String fsRepoName, String snapshotName) { + final CreateSnapshotResponse createSnapshotResponse = client().admin() + .cluster() + .prepareCreateSnapshot(fsRepoName, snapshotName) + .setWaitForCompletion(true) + .get(); + final SnapshotInfo snapshotInfo = createSnapshotResponse.getSnapshotInfo(); + assertThat(snapshotInfo.successfulShards(), greaterThan(0)); + assertThat(snapshotInfo.successfulShards(), equalTo(snapshotInfo.totalShards())); + } + + private static RestoreBlockingActionFilter getBlockingActionFilter() { + for (final ActionFilter filter : internalCluster().getCurrentMasterNodeInstance(ActionFilters.class).filters()) { + if (filter instanceof RestoreBlockingActionFilter) { + return (RestoreBlockingActionFilter) filter; + } + } + throw new AssertionError("did not find BlockingActionFilter"); + } + +}