Sfoglia il codice sorgente

validate snapshot has global state before restoring it (#82037)

It is possible to restore from a snapshot with a global
state even if it does not have one. This pr adds validation
to prevent this from happening.
Ievgen Degtiarenko 3 anni fa
parent
commit
e7d89910c2

+ 3 - 0
docs/reference/snapshot-restore/apis/restore-snapshot-api.asciidoc

@@ -170,6 +170,9 @@ ingest pipelines and {ilm-init} lifecycle policies that exist in your cluster
 and replaces them with the corresponding items from the snapshot.
 
 Use the `feature_states` parameter to configure how feature states are restored.
+
+If `include_global_state` is `true` and a snapshot was created without a global
+state then the restore request will fail.
 --
 
 [[restore-snapshot-api-feature-states]]

+ 3 - 3
server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotCustomPluginStateIT.java

@@ -155,10 +155,10 @@ public class SnapshotCustomPluginStateIT extends AbstractSnapshotIntegTestCase {
             assertAcked(clusterAdmin().prepareDeleteStoredScript("foobar").get());
         }
 
-        logger.info("--> try restoring cluster state from snapshot without global state");
+        logger.info("--> try restoring from snapshot without global state");
         RestoreSnapshotResponse restoreSnapshotResponse = clusterAdmin().prepareRestoreSnapshot("test-repo", "test-snap-no-global-state")
             .setWaitForCompletion(true)
-            .setRestoreGlobalState(true)
+            .setRestoreGlobalState(false)
             .execute()
             .actionGet();
         assertThat(restoreSnapshotResponse.getRestoreInfo().totalShards(), equalTo(0));
@@ -227,7 +227,7 @@ public class SnapshotCustomPluginStateIT extends AbstractSnapshotIntegTestCase {
         logger.info("--> try restoring index and cluster state from snapshot without global state");
         restoreSnapshotResponse = clusterAdmin().prepareRestoreSnapshot("test-repo", "test-snap-no-global-state-with-index")
             .setWaitForCompletion(true)
-            .setRestoreGlobalState(true)
+            .setRestoreGlobalState(false)
             .execute()
             .actionGet();
         assertThat(restoreSnapshotResponse.getRestoreInfo().totalShards(), greaterThan(0));

+ 1 - 1
server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndicesSnapshotIT.java

@@ -132,7 +132,7 @@ public class SystemIndicesSnapshotIT extends AbstractSnapshotIntegTestCase {
 
         // snapshot by feature
         CreateSnapshotResponse createSnapshotResponse = clusterAdmin().prepareCreateSnapshot(REPO_NAME, "test-snap")
-            .setIncludeGlobalState(false)
+            .setIncludeGlobalState(true)
             .setWaitForCompletion(true)
             .setFeatureStates(SystemIndexTestPlugin.class.getSimpleName(), AnotherSystemIndexTestPlugin.class.getSimpleName())
             .get();

+ 8 - 4
server/src/main/java/org/elasticsearch/snapshots/RestoreService.java

@@ -52,7 +52,6 @@ import org.elasticsearch.common.Priority;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.ClusterSettings;
@@ -131,7 +130,6 @@ import static org.elasticsearch.snapshots.SnapshotsService.NO_FEATURE_STATES_VAL
 public class RestoreService implements ClusterStateApplier {
 
     private static final Logger logger = LogManager.getLogger(RestoreService.class);
-    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestoreService.class);
 
     public static final Setting<Boolean> REFRESH_REPO_UUID_ON_RESTORE_SETTING = Setting.boolSetting(
         "snapshot.refresh_repo_uuid_on_restore",
@@ -300,7 +298,7 @@ public class RestoreService implements ClusterStateApplier {
         final Snapshot snapshot = new Snapshot(repositoryName, snapshotId);
 
         // Make sure that we can restore from this snapshot
-        validateSnapshotRestorable(repository.getMetadata(), snapshotInfo);
+        validateSnapshotRestorable(request, repository.getMetadata(), snapshotInfo);
 
         // Get the global state if necessary
         Metadata globalMetadata = null;
@@ -961,7 +959,7 @@ public class RestoreService implements ClusterStateApplier {
      *  @param repository      repository name
      * @param snapshotInfo    snapshot metadata
      */
-    private static void validateSnapshotRestorable(final RepositoryMetadata repository, final SnapshotInfo snapshotInfo) {
+    static void validateSnapshotRestorable(RestoreSnapshotRequest request, RepositoryMetadata repository, SnapshotInfo snapshotInfo) {
         if (snapshotInfo.state().restorable() == false) {
             throw new SnapshotRestoreException(
                 new Snapshot(repository.name(), snapshotInfo.snapshotId()),
@@ -988,6 +986,12 @@ public class RestoreService implements ClusterStateApplier {
                     + "]"
             );
         }
+        if (request.includeGlobalState() && snapshotInfo.includeGlobalState() == Boolean.FALSE) {
+            throw new SnapshotRestoreException(
+                new Snapshot(repository.name(), snapshotInfo.snapshotId()),
+                "cannot restore global state since the snapshot was created without global state"
+            );
+        }
     }
 
     public static final Setting<Boolean> ALLOW_BWC_INDICES_SETTING = Setting.boolSetting(

+ 42 - 2
server/src/test/java/org/elasticsearch/snapshots/RestoreServiceTests.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.snapshots;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest;
 import org.elasticsearch.action.support.PlainActionFuture;
@@ -24,7 +25,6 @@ import org.elasticsearch.repositories.Repository;
 import org.elasticsearch.repositories.RepositoryData;
 import org.elasticsearch.repositories.blobstore.BlobStoreRepository;
 import org.elasticsearch.test.ESTestCase;
-import org.hamcrest.Matchers;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -36,6 +36,8 @@ import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.createTimestampField;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
@@ -178,8 +180,46 @@ public class RestoreServiceTests extends ESTestCase {
         when(repositoriesService.getRepositories()).thenReturn(repositories);
         RestoreService.refreshRepositoryUuids(true, repositoriesService, listener);
         assertNull(listener.get(0L, TimeUnit.SECONDS));
-        assertThat(pendingRefreshes, Matchers.empty());
+        assertThat(pendingRefreshes, empty());
         finalAssertions.forEach(Runnable::run);
     }
 
+    public void testNotAllowToRestoreGlobalStateFromSnapshotWithoutOne() {
+
+        var request = new RestoreSnapshotRequest().includeGlobalState(true);
+        var repository = new RepositoryMetadata("name", "type", Settings.EMPTY);
+        var snapshot = new Snapshot("repository", new SnapshotId("name", "uuid"));
+
+        var snapshotInfo = createSnapshotInfo(snapshot, Boolean.FALSE);
+
+        var exception = expectThrows(
+            SnapshotRestoreException.class,
+            () -> RestoreService.validateSnapshotRestorable(request, repository, snapshotInfo)
+        );
+        assertThat(
+            exception.getMessage(),
+            equalTo("[name:name/uuid] cannot restore global state since the snapshot was created without global state")
+        );
+    }
+
+    private static SnapshotInfo createSnapshotInfo(Snapshot snapshot, Boolean includeGlobalState) {
+        var shards = randomIntBetween(0, 100);
+        return new SnapshotInfo(
+            snapshot,
+            List.of(),
+            List.of(),
+            List.of(),
+            randomAlphaOfLengthBetween(10, 100),
+            Version.CURRENT,
+            randomNonNegativeLong(),
+            randomNonNegativeLong(),
+            shards,
+            shards,
+            List.of(),
+            includeGlobalState,
+            Map.of(),
+            SnapshotState.SUCCESS,
+            Map.of()
+        );
+    }
 }

+ 9 - 20
x-pack/plugin/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamsSnapshotsIT.java

@@ -349,7 +349,7 @@ public class DataStreamsSnapshotsIT extends AbstractSnapshotIntegTestCase {
 
         RestoreSnapshotRequest restoreSnapshotRequest = new RestoreSnapshotRequest(REPO, SNAPSHOT);
         restoreSnapshotRequest.waitForCompletion(true);
-        restoreSnapshotRequest.includeGlobalState(true);
+        restoreSnapshotRequest.includeGlobalState(false);
         if (filterDuringSnapshotting == false) {
             restoreSnapshotRequest.indices(dataStreamToSnapshot);
         }
@@ -391,9 +391,7 @@ public class DataStreamsSnapshotsIT extends AbstractSnapshotIntegTestCase {
     }
 
     public void testSnapshotAndRestoreReplaceAll() throws Exception {
-        CreateSnapshotRequest createSnapshotRequest = new CreateSnapshotRequest(REPO, SNAPSHOT);
-        createSnapshotRequest.waitForCompletion(true);
-        createSnapshotRequest.includeGlobalState(false);
+        var createSnapshotRequest = new CreateSnapshotRequest(REPO, SNAPSHOT).waitForCompletion(true).includeGlobalState(false);
         CreateSnapshotResponse createSnapshotResponse = client.admin().cluster().createSnapshot(createSnapshotRequest).actionGet();
 
         RestStatus status = createSnapshotResponse.getSnapshotInfo().status();
@@ -403,9 +401,7 @@ public class DataStreamsSnapshotsIT extends AbstractSnapshotIntegTestCase {
         assertAcked(client.execute(DeleteDataStreamAction.INSTANCE, new DeleteDataStreamAction.Request(new String[] { "*" })).get());
         assertAcked(client.admin().indices().prepareDelete("*").setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN));
 
-        RestoreSnapshotRequest restoreSnapshotRequest = new RestoreSnapshotRequest(REPO, SNAPSHOT);
-        restoreSnapshotRequest.waitForCompletion(true);
-        restoreSnapshotRequest.includeGlobalState(true);
+        var restoreSnapshotRequest = new RestoreSnapshotRequest(REPO, SNAPSHOT).waitForCompletion(true).includeGlobalState(false);
         RestoreSnapshotResponse restoreSnapshotResponse = client.admin().cluster().restoreSnapshot(restoreSnapshotRequest).actionGet();
 
         assertEquals(2, restoreSnapshotResponse.getRestoreInfo().successfulShards());
@@ -449,9 +445,7 @@ public class DataStreamsSnapshotsIT extends AbstractSnapshotIntegTestCase {
     }
 
     public void testSnapshotAndRestoreAll() throws Exception {
-        CreateSnapshotRequest createSnapshotRequest = new CreateSnapshotRequest(REPO, SNAPSHOT);
-        createSnapshotRequest.waitForCompletion(true);
-        createSnapshotRequest.includeGlobalState(false);
+        var createSnapshotRequest = new CreateSnapshotRequest(REPO, SNAPSHOT).waitForCompletion(true).includeGlobalState(false);
         CreateSnapshotResponse createSnapshotResponse = client.admin().cluster().createSnapshot(createSnapshotRequest).actionGet();
 
         RestStatus status = createSnapshotResponse.getSnapshotInfo().status();
@@ -461,9 +455,7 @@ public class DataStreamsSnapshotsIT extends AbstractSnapshotIntegTestCase {
         assertAcked(client.execute(DeleteDataStreamAction.INSTANCE, new DeleteDataStreamAction.Request(new String[] { "*" })).get());
         assertAcked(client.admin().indices().prepareDelete("*").setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN));
 
-        RestoreSnapshotRequest restoreSnapshotRequest = new RestoreSnapshotRequest(REPO, SNAPSHOT);
-        restoreSnapshotRequest.waitForCompletion(true);
-        restoreSnapshotRequest.includeGlobalState(true);
+        var restoreSnapshotRequest = new RestoreSnapshotRequest(REPO, SNAPSHOT).waitForCompletion(true).includeGlobalState(false);
         RestoreSnapshotResponse restoreSnapshotResponse = client.admin().cluster().restoreSnapshot(restoreSnapshotRequest).actionGet();
         assertEquals(2, restoreSnapshotResponse.getRestoreInfo().successfulShards());
 
@@ -507,9 +499,7 @@ public class DataStreamsSnapshotsIT extends AbstractSnapshotIntegTestCase {
     }
 
     public void testSnapshotAndRestoreIncludeAliasesFalse() throws Exception {
-        CreateSnapshotRequest createSnapshotRequest = new CreateSnapshotRequest(REPO, SNAPSHOT);
-        createSnapshotRequest.waitForCompletion(true);
-        createSnapshotRequest.includeGlobalState(false);
+        var createSnapshotRequest = new CreateSnapshotRequest(REPO, SNAPSHOT).waitForCompletion(true).includeGlobalState(false);
         CreateSnapshotResponse createSnapshotResponse = client.admin().cluster().createSnapshot(createSnapshotRequest).actionGet();
 
         RestStatus status = createSnapshotResponse.getSnapshotInfo().status();
@@ -519,10 +509,9 @@ public class DataStreamsSnapshotsIT extends AbstractSnapshotIntegTestCase {
         assertAcked(client.execute(DeleteDataStreamAction.INSTANCE, new DeleteDataStreamAction.Request(new String[] { "*" })).get());
         assertAcked(client.admin().indices().prepareDelete("*").setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN));
 
-        RestoreSnapshotRequest restoreSnapshotRequest = new RestoreSnapshotRequest(REPO, SNAPSHOT);
-        restoreSnapshotRequest.waitForCompletion(true);
-        restoreSnapshotRequest.includeGlobalState(true);
-        restoreSnapshotRequest.includeAliases(false);
+        var restoreSnapshotRequest = new RestoreSnapshotRequest(REPO, SNAPSHOT).waitForCompletion(true)
+            .includeGlobalState(false)
+            .includeAliases(false);
         RestoreSnapshotResponse restoreSnapshotResponse = client.admin().cluster().restoreSnapshot(restoreSnapshotRequest).actionGet();
         assertEquals(2, restoreSnapshotResponse.getRestoreInfo().successfulShards());
 

+ 1 - 1
x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java

@@ -427,7 +427,7 @@ public class SearchableSnapshotActionIT extends ESRestTestCase {
         // restore the datastream
         Request restoreSnapshot = new Request("POST", "/_snapshot/" + snapshotRepo + "/" + dsSnapshotName + "/_restore");
         restoreSnapshot.addParameter("wait_for_completion", "true");
-        restoreSnapshot.setJsonEntity("{\"indices\": \"" + dataStream + "\", \"include_global_state\": true}");
+        restoreSnapshot.setJsonEntity("{\"indices\": \"" + dataStream + "\", \"include_global_state\": false}");
         assertOK(client().performRequest(restoreSnapshot));
 
         assertThat(indexExists(searchableSnapMountedIndexName), is(true));