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

Author: DaveCTurner

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java

@@ -25,6 +25,7 @@
 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<RestoreSnapshotReq
     private Settings indexSettings = EMPTY_SETTINGS;
     private String[] ignoreIndexSettings = Strings.EMPTY_ARRAY;
 
+    @Nullable // if any snapshot UUID will do
+    private String snapshotUuid;
+
     public RestoreSnapshotRequest() {
     }
 
@@ -99,6 +103,9 @@ public RestoreSnapshotRequest(StreamInput in) throws IOException {
         }
         indexSettings = readSettingsFromStream(in);
         ignoreIndexSettings = in.readStringArray();
+        if (in.getVersion().onOrAfter(Version.V_7_10_0)) {
+            snapshotUuid = in.readOptionalString();
+        }
     }
 
     @Override
@@ -119,6 +126,12 @@ public void writeTo(StreamOutput out) throws IOException {
         }
         writeSettingsToStream(indexSettings, out);
         out.writeStringArray(ignoreIndexSettings);
+        if (out.getVersion().onOrAfter(Version.V_7_10_0)) {
+            out.writeOptionalString(snapshotUuid);
+        } else if (snapshotUuid != null) {
+            throw new IllegalStateException(
+                    "restricting the snapshot UUID is forbidden in a cluster with version [" + out.getVersion() + "] nodes");
+        }
     }
 
     @Override
@@ -439,6 +452,28 @@ public Settings indexSettings() {
         return this.indexSettings;
     }
 
+    /**
+     * Sometimes a client has identified precisely which snapshot is to be restored via a separate mechanism and wishes to guarantee that
+     * this is the snapshot that this request restores. If the client can only identify a snapshot by its name then there is a risk that the
+     * desired snapshot may be deleted and replaced by a new snapshot with the same name which is inconsistent with the original one. This
+     * method lets us fail the restore if the precise snapshot we want is not available.
+     *
+     * This is for internal use only and is not exposed in the REST layer.
+     */
+    public RestoreSnapshotRequest snapshotUuid(String snapshotUuid) {
+        this.snapshotUuid = snapshotUuid;
+        return this;
+    }
+
+    /**
+     * @return the UUID that identifies the specific snapshot in the repository to be restored, or {@code null} if the snapshot name is
+     * a sufficient identifier.
+     */
+    @Nullable
+    public String snapshotUuid() {
+        return snapshotUuid;
+    }
+
     /**
      * Parses restore definition
      *
@@ -561,13 +596,14 @@ public boolean equals(Object o) {
             Objects.equals(renamePattern, that.renamePattern) &&
             Objects.equals(renameReplacement, that.renameReplacement) &&
             Objects.equals(indexSettings, that.indexSettings) &&
-            Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings);
+            Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings) &&
+            Objects.equals(snapshotUuid, that.snapshotUuid);
     }
 
     @Override
     public int hashCode() {
         int result = Objects.hash(snapshot, repository, indicesOptions, renamePattern, renameReplacement, waitForCompletion,
-            includeGlobalState, partial, includeAliases, indexSettings);
+            includeGlobalState, partial, includeAliases, indexSettings, snapshotUuid);
         result = 31 * result + Arrays.hashCode(indices);
         result = 31 * result + Arrays.hashCode(ignoreIndexSettings);
         return result;

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

@@ -199,6 +199,11 @@ public void restoreSnapshot(final RestoreSnapshotRequest request, final ActionLi
                 }
 
                 final SnapshotId snapshotId = matchingSnapshotId.get();
+                if (request.snapshotUuid() != null && request.snapshotUuid().equals(snapshotId.getUUID()) == false) {
+                    throw new SnapshotRestoreException(repositoryName, snapshotName,
+                        "snapshot UUID mismatch: expected [" + request.snapshotUuid() + "] but got [" + snapshotId.getUUID() + "]");
+                }
+
                 final SnapshotInfo snapshotInfo = repository.getSnapshotInfo(snapshotId);
                 final Snapshot snapshot = new Snapshot(repositoryName, snapshotId);
 

server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java

@@ -89,6 +89,11 @@ private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) {
         if (randomBoolean()) {
             instance.masterNodeTimeout(randomTimeValue());
         }
+
+        if (randomBoolean()) {
+            instance.snapshotUuid(randomBoolean() ? null : randomAlphaOfLength(10));
+        }
+
         return instance;
     }
 

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java

@@ -153,11 +153,6 @@ protected void masterOperation(
             }
             final SnapshotId snapshotId = matchingSnapshotId.get();
 
-            // TODO validate IDs in the restore:
-            // We must fail the restore if it obtains different IDs from the ones we just obtained (e.g. the target snapshot was replaced
-            // by one with the same name while we are restoring it) or else the index metadata might bear no relation to the snapshot we're
-            // searching.
-
             final String[] ignoreIndexSettings = Arrays.copyOf(request.ignoreIndexSettings(), request.ignoreIndexSettings().length + 1);
             ignoreIndexSettings[ignoreIndexSettings.length - 1] = IndexMetadata.SETTING_DATA_PATH;
 
@@ -188,7 +183,9 @@ protected void masterOperation(
                         // Pass through the wait-for-completion flag
                         .waitForCompletion(request.waitForCompletion())
                         // Pass through the master-node timeout
-                        .masterNodeTimeout(request.masterNodeTimeout()),
+                        .masterNodeTimeout(request.masterNodeTimeout())
+                        // Fail the restore if the snapshot found above is swapped out from under us before the restore happens
+                        .snapshotUuid(snapshotId.getUUID()),
                     listener
                 );
         }, listener::onFailure);

x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsUuidValidationIntegTests.java

@@ -0,0 +1,150 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+package org.elasticsearch.xpack.searchablesnapshots;
+
+import org.elasticsearch.action.ActionFuture;
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.ActionRequest;
+import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
+import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotAction;
+import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
+import org.elasticsearch.action.support.ActionFilter;
+import org.elasticsearch.action.support.ActionFilters;
+import org.elasticsearch.action.support.PlainActionFuture;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.plugins.ActionPlugin;
+import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.snapshots.SnapshotInfo;
+import org.elasticsearch.snapshots.SnapshotRestoreException;
+import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotAction;
+import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Locale;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.util.Collections.singletonList;
+import static org.elasticsearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+
+public class SearchableSnapshotsUuidValidationIntegTests extends BaseSearchableSnapshotsIntegTestCase {
+
+    public static class TestPlugin extends Plugin implements ActionPlugin {
+
+        private final RestoreBlockingActionFilter restoreBlockingActionFilter;
+
+        public TestPlugin() {
+            restoreBlockingActionFilter = new RestoreBlockingActionFilter();
+        }
+
+        @Override
+        public List<ActionFilter> getActionFilters() {
+            return singletonList(restoreBlockingActionFilter);
+        }
+    }
+
+    public static class RestoreBlockingActionFilter extends org.elasticsearch.action.support.ActionFilter.Simple {
+        private final PlainActionFuture<Void> executed = new PlainActionFuture<>();
+        private final PlainActionFuture<Void> 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<Class<? extends Plugin>> 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<RestoreSnapshotResponse> 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");
+    }
+
+}