Prevent deletion of snapshot backing index (opensearch-project#5069)

Prevent deletion of snapshots that are backing searchable snapshot indexes Signed-off-by: Vishal Sarda <vsarda@amazon.com> (cherry picked from commit 08ac17f) Signed-off-by: Vishal Sarda <vsarda@amazon.com>
Vishalks · Nov 23, 2022 · ca90434 · ca90434
1 parent 6bdad79
commit ca90434
Show file tree

Hide file tree

Showing 8 changed files with 262 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,51 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Bumps `json-schema-validator` from 1.0.69 to 1.0.73 ([#5316](https://github.com/opensearch-project/OpenSearch/pull/5316))
 - Bumps `proto-google-common-protos` from 2.8.0 to 2.10.0 ([#5318](https://github.com/opensearch-project/OpenSearch/pull/5318))
 - Bumps `protobuf-java` from 3.21.7 to 3.21.9 ([#5319](https://github.com/opensearch-project/OpenSearch/pull/5319))
+- Update Apache Lucene to 9.5.0-snapshot-a4ef70f ([#4979](https://github.com/opensearch-project/OpenSearch/pull/4979))
+- Update to Gradle 7.6 and JDK-19 ([#4973](https://github.com/opensearch-project/OpenSearch/pull/4973))
+
+### Changed
+- [CCR] Add getHistoryOperationsFromTranslog method to fetch the history snapshot from translogs ([#3948](https://github.com/opensearch-project/OpenSearch/pull/3948))
+- Relax visibility of the HTTP_CHANNEL_KEY and HTTP_SERVER_CHANNEL_KEY to make it possible for the plugins to access associated Netty4HttpChannel / Netty4HttpServerChannel instance ([#4638](https://github.com/opensearch-project/OpenSearch/pull/4638))
+- Use ReplicationFailedException instead of OpensearchException in ReplicationTarget ([#4725](https://github.com/opensearch-project/OpenSearch/pull/4725))
+- Migrate client transports to Apache HttpClient / Core 5.x ([#4459](https://github.com/opensearch-project/OpenSearch/pull/4459))
+- Support remote translog transfer for request level durability([#4480](https://github.com/opensearch-project/OpenSearch/pull/4480))
+
+### Deprecated
+
+### Removed
+- Remove deprecated code to add node name into log pattern of log4j property file ([#4568](https://github.com/opensearch-project/OpenSearch/pull/4568))
+- Unused object and import within TransportClusterAllocationExplainAction ([#4639](https://github.com/opensearch-project/OpenSearch/pull/4639))
+- Remove LegacyESVersion.V_7_0_* and V_7_1_* Constants ([#2768](https://https://github.com/opensearch-project/OpenSearch/pull/2768))
+- Remove LegacyESVersion.V_7_2_ and V_7_3_ Constants ([#4702](https://github.com/opensearch-project/OpenSearch/pull/4702))
+- Always auto release the flood stage block ([#4703](https://github.com/opensearch-project/OpenSearch/pull/4703))
+- Remove LegacyESVersion.V_7_4_ and V_7_5_ Constants ([#4704](https://github.com/opensearch-project/OpenSearch/pull/4704))
+- Remove Legacy Version support from Snapshot/Restore Service ([#4728](https://github.com/opensearch-project/OpenSearch/pull/4728))
+- Remove deprecated serialization logic from pipeline aggs ([#4847](https://github.com/opensearch-project/OpenSearch/pull/4847))
+- Remove unused private methods ([#4926](https://github.com/opensearch-project/OpenSearch/pull/4926))
+- Remove LegacyESVersion.V_7_8_ and V_7_9_ Constants ([#4855](https://github.com/opensearch-project/OpenSearch/pull/4855))
+- Remove LegacyESVersion.V_7_6_ and V_7_7_ Constants ([#4837](https://github.com/opensearch-project/OpenSearch/pull/4837))
+- Remove LegacyESVersion.V_7_10_ Constants ([#5018](https://github.com/opensearch-project/OpenSearch/pull/5018))
+- Remove Version.V_1_ Constants ([#5021](https://github.com/opensearch-project/OpenSearch/pull/5021))
+
+### Fixed
+- Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
+- Fixed compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944))
+- Reject bulk requests with invalid actions ([#5299](https://github.com/opensearch-project/OpenSearch/issues/5299))
+
+### Security
+
+## [Unreleased 2.x]
+### Added
+- Prevent deletion of snapshots that are backing searchable snapshot indexes ([#5069](https://github.com/opensearch-project/OpenSearch/pull/5069))
+
+### Dependencies
+- Bumps `bcpg-fips` from 1.0.5.1 to 1.0.7.1
+- Bumps `azure-storage-blob` from 12.16.1 to 12.20.0 ([#4995](https://github.com/opensearch-project/OpenSearch/pull/4995))
+- Bumps `commons-compress` from 1.21 to 1.22 ([#5104](https://github.com/opensearch-project/OpenSearch/pull/5104))
+- Bump `opencensus-contrib-http-util` from 0.18.0 to 0.31.1 ([#3633](https://github.com/opensearch-project/OpenSearch/pull/3633))
+- Bump `geoip2` from 3.0.1 to 3.0.2 ([#5103](https://github.com/opensearch-project/OpenSearch/pull/5103))
+- Bump gradle-extra-configurations-plugin from 7.0.0 to 8.0.0 ([#4808](https://github.com/opensearch-project/OpenSearch/pull/4808))
 ### Changed
 ### Deprecated
 ### Removed

diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java
@@ -9,6 +9,7 @@
 import org.junit.BeforeClass;
 import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse;
 import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
+import org.opensearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest;
 import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest;
 import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequestBuilder;
 import org.opensearch.action.index.IndexRequestBuilder;
@@ -27,6 +28,7 @@
 import org.opensearch.index.Index;
 import org.opensearch.index.IndexNotFoundException;
 import org.opensearch.monitor.fs.FsInfo;
+import org.opensearch.repositories.fs.FsRepository;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -74,23 +76,30 @@ private Settings.Builder chunkedRepositorySettings() {
      * Ensures availability of sufficient data nodes and search capable nodes.
      */
     public void testCreateSearchableSnapshot() throws Exception {
+        final String snapshotName = "test-snap";
+        final String repoName = "test-repo";
+        final String indexName1 = "test-idx-1";
+        final String restoredIndexName1 = indexName1 + "-copy";
+        final String indexName2 = "test-idx-2";
+        final String restoredIndexName2 = indexName2 + "-copy";
         final int numReplicasIndex1 = randomIntBetween(1, 4);
         final int numReplicasIndex2 = randomIntBetween(0, 2);
         final Client client = client();
 
         internalCluster().ensureAtLeastNumDataNodes(Math.max(numReplicasIndex1, numReplicasIndex2) + 1);
-        createIndexWithDocsAndEnsureGreen(numReplicasIndex1, 100, "test-idx-1");
-        createIndexWithDocsAndEnsureGreen(numReplicasIndex2, 100, "test-idx-2");
+        createIndexWithDocsAndEnsureGreen(numReplicasIndex1, 100, indexName1);
+        createIndexWithDocsAndEnsureGreen(numReplicasIndex2, 100, indexName2);
 
-        takeSnapshot(client, "test-idx-1", "test-idx-2");
-        deleteIndicesAndEnsureGreen(client, "test-idx-1", "test-idx-2");
+        createRepositoryWithSettings(null, repoName);
+        takeSnapshot(client, snapshotName, repoName, indexName1, indexName2);
+        deleteIndicesAndEnsureGreen(client, indexName1, indexName2);
 
         internalCluster().ensureAtLeastNumSearchNodes(Math.max(numReplicasIndex1, numReplicasIndex2) + 1);
-        restoreSnapshotAndEnsureGreen(client);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
 
-        assertDocCount("test-idx-1-copy", 100L);
-        assertDocCount("test-idx-2-copy", 100L);
-        assertIndexDirectoryDoesNotExist("test-idx-1-copy", "test-idx-2-copy");
+        assertDocCount(restoredIndexName1, 100L);
+        assertDocCount(restoredIndexName2, 100L);
+        assertIndexDirectoryDoesNotExist(restoredIndexName1, restoredIndexName2);
     }
 
     /**
@@ -101,16 +110,19 @@ public void testCreateSearchableSnapshotWithChunks() throws Exception {
         final int numReplicasIndex = randomIntBetween(1, 4);
         final String indexName = "test-idx";
         final String restoredIndexName = indexName + "-copy";
+        final String repoName = "test-repo";
+        final String snapshotName = "test-snap";
         final Client client = client();
 
         Settings.Builder repositorySettings = chunkedRepositorySettings();
 
         internalCluster().ensureAtLeastNumSearchAndDataNodes(numReplicasIndex + 1);
         createIndexWithDocsAndEnsureGreen(numReplicasIndex, 1000, indexName);
-        takeSnapshot(client, repositorySettings, indexName);
+        createRepositoryWithSettings(repositorySettings, repoName);
+        takeSnapshot(client, snapshotName, repoName, indexName);
 
         deleteIndicesAndEnsureGreen(client, indexName);
-        restoreSnapshotAndEnsureGreen(client);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
 
         assertDocCount(restoredIndexName, 1000L);
     }
@@ -124,13 +136,16 @@ public void testSearchableSnapshotAllocationForLocalAndRemoteShardsOnSameNode()
         final int numReplicasIndex = randomIntBetween(1, 4);
         final String indexName = "test-idx";
         final String restoredIndexName = indexName + "-copy";
+        final String repoName = "test-repo";
+        final String snapshotName = "test-snap";
         final Client client = client();
 
         internalCluster().ensureAtLeastNumSearchAndDataNodes(numReplicasIndex + 1);
         createIndexWithDocsAndEnsureGreen(numReplicasIndex, 100, indexName);
-        takeSnapshot(client, indexName);
+        createRepositoryWithSettings(null, repoName);
+        takeSnapshot(client, snapshotName, repoName, indexName);
 
-        restoreSnapshotAndEnsureGreen(client);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
 
         assertDocCount(restoredIndexName, 100L);
         assertDocCount(indexName, 100L);
@@ -145,16 +160,19 @@ public void testSearchableSnapshotAllocationForFailoverAndRecovery() throws Exce
         final int numReplicasIndex = 1;
         final String indexName = "test-idx";
         final String restoredIndexName = indexName + "-copy";
+        final String repoName = "test-repo";
+        final String snapshotName = "test-snap";
         final Client client = client();
 
         internalCluster().ensureAtLeastNumDataNodes(numReplicasIndex + 1);
         createIndexWithDocsAndEnsureGreen(numReplicasIndex, 100, indexName);
 
-        takeSnapshot(client, indexName);
+        createRepositoryWithSettings(null, repoName);
+        takeSnapshot(client, snapshotName, repoName, indexName);
         deleteIndicesAndEnsureGreen(client, indexName);
 
         internalCluster().ensureAtLeastNumSearchNodes(numReplicasIndex + 1);
-        restoreSnapshotAndEnsureGreen(client);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
         assertDocCount(restoredIndexName, 100L);
 
         logger.info("--> stop a random search node");
@@ -183,14 +201,17 @@ public void testSearchableSnapshotAllocationForFailoverAndRecovery() throws Exce
     public void testSearchableSnapshotIndexIsReadOnly() throws Exception {
         final String indexName = "test-index";
         final String restoredIndexName = indexName + "-copy";
+        final String repoName = "test-repo";
+        final String snapshotName = "test-snap";
         final Client client = client();
 
         createIndexWithDocsAndEnsureGreen(0, 100, indexName);
-        takeSnapshot(client, indexName);
+        createRepositoryWithSettings(null, repoName);
+        takeSnapshot(client, snapshotName, repoName, indexName);
         deleteIndicesAndEnsureGreen(client, indexName);
 
         internalCluster().ensureAtLeastNumSearchNodes(1);
-        restoreSnapshotAndEnsureGreen(client);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
 
         assertIndexingBlocked(restoredIndexName);
         assertIndexSettingChangeBlocked(restoredIndexName);
@@ -202,6 +223,39 @@ public void testSearchableSnapshotIndexIsReadOnly() throws Exception {
         );
     }
 
+    public void testDeleteSearchableSnapshotBackingIndexThrowsException() throws Exception {
+        final String indexName = "test-index";
+        final Client client = client();
+        final String repoName = "test-repo";
+        final String snapshotName = "test-snap";
+        createRepositoryWithSettings(null, repoName);
+        createIndexWithDocsAndEnsureGreen(0, 100, indexName);
+        takeSnapshot(client, snapshotName, repoName, indexName);
+        internalCluster().ensureAtLeastNumSearchNodes(1);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertThrows(
+            SnapshotInUseDeletionException.class,
+            () -> client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest(repoName, snapshotName)).actionGet()
+        );
+    }
+
+    public void testDeleteSearchableSnapshotBackingIndex() throws Exception {
+        final String indexName1 = "test-index1";
+        final String indexName2 = "test-index2";
+        final Client client = client();
+        final String repoName = "test-repo";
+        final String snapshotName1 = "test-snapshot1";
+        final String snapshotName2 = "test-snap";
+        createRepositoryWithSettings(null, repoName);
+        createIndexWithDocsAndEnsureGreen(0, 100, indexName1);
+        createIndexWithDocsAndEnsureGreen(0, 100, indexName2);
+        takeSnapshot(client, snapshotName1, repoName, indexName1);
+        takeSnapshot(client, snapshotName2, repoName, indexName2);
+        internalCluster().ensureAtLeastNumSearchNodes(1);
+        restoreSnapshotAndEnsureGreen(client, snapshotName2, repoName);
+        client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest(repoName, snapshotName1)).actionGet();
+    }
+
     private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDocs, String indexName) throws InterruptedException {
         createIndex(
             indexName,
@@ -216,21 +270,11 @@ private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDo
         ensureGreen();
     }
 
-    private void takeSnapshot(Client client, String... indices) {
-        takeSnapshot(client, null, indices);
-    }
-
-    private void takeSnapshot(Client client, Settings.Builder repositorySettings, String... indices) {
-        logger.info("--> Create a repository");
-        if (repositorySettings == null) {
-            createRepository("test-repo", "fs");
-        } else {
-            createRepository("test-repo", "fs", repositorySettings);
-        }
+    private void takeSnapshot(Client client, String snapshotName, String repoName, String... indices) {
         logger.info("--> Take a snapshot");
         final CreateSnapshotResponse createSnapshotResponse = client.admin()
             .cluster()
-            .prepareCreateSnapshot("test-repo", "test-snap")
+            .prepareCreateSnapshot(repoName, snapshotName)
             .setWaitForCompletion(true)
             .setIndices(indices)
             .get();
@@ -242,16 +286,25 @@ private void takeSnapshot(Client client, Settings.Builder repositorySettings, St
         );
     }
 
+    private void createRepositoryWithSettings(Settings.Builder repositorySettings, String repoName) {
+        logger.info("--> Create a repository");
+        if (repositorySettings == null) {
+            createRepository(repoName, FsRepository.TYPE);
+        } else {
+            createRepository(repoName, FsRepository.TYPE, repositorySettings);
+        }
+    }
+
     private void deleteIndicesAndEnsureGreen(Client client, String... indices) {
         assertTrue(client.admin().indices().prepareDelete(indices).get().isAcknowledged());
         ensureGreen();
     }
 
-    private void restoreSnapshotAndEnsureGreen(Client client) {
+    private void restoreSnapshotAndEnsureGreen(Client client, String snapshotName, String repoName) {
         logger.info("--> restore indices as 'remote_snapshot'");
         client.admin()
             .cluster()
-            .prepareRestoreSnapshot("test-repo", "test-snap")
+            .prepareRestoreSnapshot(repoName, snapshotName)
             .setRenamePattern("(.+)")
             .setRenameReplacement("$1-copy")
             .setStorageType(RestoreSnapshotRequest.StorageType.REMOTE_SNAPSHOT)

diff --git a/server/src/main/java/org/opensearch/OpenSearchException.java b/server/src/main/java/org/opensearch/OpenSearchException.java
@@ -52,6 +52,7 @@
 import org.opensearch.rest.RestStatus;
 import org.opensearch.search.SearchException;
 import org.opensearch.search.aggregations.MultiBucketConsumerService;
+import org.opensearch.snapshots.SnapshotInUseDeletionException;
 import org.opensearch.transport.TcpTransport;
 
 import java.io.IOException;
@@ -1626,6 +1627,12 @@ private enum OpenSearchExceptionHandle {
             ClusterManagerThrottlingException::new,
             165,
             Version.V_2_4_0
+        ),
+        SNAPSHOT_IN_USE_DELETION_EXCEPTION(
+            SnapshotInUseDeletionException.class,
+            SnapshotInUseDeletionException::new,
+            166,
+            UNKNOWN_VERSION_ADDED
         );
 
         final Class<? extends OpenSearchException> exceptionClass;

diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotInUseDeletionException.java b/server/src/main/java/org/opensearch/snapshots/SnapshotInUseDeletionException.java
@@ -0,0 +1,35 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.snapshots;
+
+import org.opensearch.common.io.stream.StreamInput;
+import org.opensearch.rest.RestStatus;
+
+import java.io.IOException;
+
+/**
+ * Thrown if requested snapshot/s can't be deleted
+ *
+ * @opensearch.internal
+ */
+public class SnapshotInUseDeletionException extends SnapshotException {
+
+    public SnapshotInUseDeletionException(final String repositoryName, final String snapshotName, final String msg) {
+        super(repositoryName, snapshotName, msg);
+    }
+
+    public SnapshotInUseDeletionException(StreamInput in) throws IOException {
+        super(in);
+    }
+
+    @Override
+    public RestStatus status() {
+        return RestStatus.CONFLICT;
+    }
+}