Prevent deletion of snapshot backing index

CHANGELOG.md

@@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Apply reproducible builds configuration for OpenSearch plugins through gradle plugin ([#4746](https://github.com/opensearch-project/OpenSearch/pull/4746))
 - Add project health badges to the README.md ([#4843](https://github.com/opensearch-project/OpenSearch/pull/4843))
 - [Test] Add IAE test for deprecated edgeNGram analyzer name ([#5040](https://github.com/opensearch-project/OpenSearch/pull/5040))
+- Prevent deletion of snapshots that are backing searchable snapshot indexes ([#5069](https://github.com/opensearch-project/OpenSearch/pull/5069))
 - Allow mmap to use new JDK-19 preview APIs in Apache Lucene 9.4+ ([#5151](https://github.com/opensearch-project/OpenSearch/pull/5151))
 - Add feature flag for extensions ([#5211](https://github.com/opensearch-project/OpenSearch/pull/5211))
 
@@ -89,4 +90,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Security
 
 [Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.4...HEAD
-[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.4...2.x
+[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.4...2.x

server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java

@@ -7,11 +7,14 @@
 import com.carrotsearch.randomizedtesting.generators.RandomPicks;
 import org.hamcrest.MatcherAssert;
 import org.junit.BeforeClass;
+import org.opensearch.action.StepListener;
 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;
+import org.opensearch.action.support.master.AcknowledgedResponse;
 import org.opensearch.client.Client;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.block.ClusterBlockException;
@@ -27,6 +30,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 +78,29 @@ 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");
+        takeSnapshot(client, snapshotName, repoName, false, 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 +111,18 @@ 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);
+        takeSnapshot(client, repositorySettings, snapshotName, repoName, false, indexName);
 
         deleteIndicesAndEnsureGreen(client, indexName);
-        restoreSnapshotAndEnsureGreen(client);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
 
         assertDocCount(restoredIndexName, 1000L);
     }
@@ -124,13 +136,15 @@ 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);
+        takeSnapshot(client, snapshotName, repoName, false, indexName);
 
-        restoreSnapshotAndEnsureGreen(client);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
 
         assertDocCount(restoredIndexName, 100L);
         assertDocCount(indexName, 100L);
@@ -145,16 +159,18 @@ 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);
+        takeSnapshot(client, snapshotName, repoName, false, 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 +199,16 @@ 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);
+        takeSnapshot(client, snapshotName, repoName, false, indexName);
         deleteIndicesAndEnsureGreen(client, indexName);
 
         internalCluster().ensureAtLeastNumSearchNodes(1);
-        restoreSnapshotAndEnsureGreen(client);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
 
         assertIndexingBlocked(restoredIndexName);
         assertIndexSettingChangeBlocked(restoredIndexName);
@@ -202,6 +220,47 @@ public void testSearchableSnapshotIndexIsReadOnly() throws Exception {
         );
     }
 
+    public void testDeleteSearchableSnapshotBackingIndexThrowsException() throws Exception {
+        disableRepoConsistencyCheck("reason");
+        final String indexName = "test-index";
+        final Client client = client();
+        final String repoName = "test-repo";
+        final String snapshotName = "test-snap";
+        final StepListener<AcknowledgedResponse> deleteSnapshotStepListener = new StepListener<>();
+        createRepository(repoName, FsRepository.TYPE);
+        createIndexWithDocsAndEnsureGreen(0, 100, indexName);
+        takeSnapshot(client, snapshotName, repoName, false, indexName);
+        internalCluster().ensureAtLeastNumSearchNodes(1);
+        restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertThrows(
+            SnapshotDeletionException.class,
+            () -> client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest(repoName, snapshotName)).actionGet()
+        );
+    }
+
+    public void testDeleteSearchableSnapshotBackingIndex() throws Exception {
+        disableRepoConsistencyCheck("reason");
+        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";
+        final StepListener<AcknowledgedResponse> deleteSnapshotStepListener = new StepListener<>();
+        createRepository(repoName, FsRepository.TYPE);
+        createIndexWithDocsAndEnsureGreen(0, 100, indexName1);
+        createIndexWithDocsAndEnsureGreen(0, 100, indexName2);
+        takeSnapshot(client, snapshotName1, repoName, false, indexName1);
+        takeSnapshot(client, snapshotName2, repoName, true, indexName2);
+        internalCluster().ensureAtLeastNumSearchNodes(1);
+        restoreSnapshotAndEnsureGreen(client, snapshotName2, repoName);
+        try {
+            client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest(repoName, snapshotName1)).actionGet();
+        } catch (Exception e) {
+            fail("Should not have thrown any exception");
+        }
+    }
+
     private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDocs, String indexName) throws InterruptedException {
         createIndex(
             indexName,
@@ -216,21 +275,30 @@ private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDo
         ensureGreen();
     }
 
-    private void takeSnapshot(Client client, String... indices) {
-        takeSnapshot(client, null, indices);
+    private void takeSnapshot(Client client, String snapshotName, String repoName, Boolean doesRepoExist, String... indices) {
+        takeSnapshot(client, null, snapshotName, repoName, doesRepoExist, indices);
     }
 
-    private void takeSnapshot(Client client, Settings.Builder repositorySettings, String... indices) {
+    private void takeSnapshot(
+        Client client,
+        Settings.Builder repositorySettings,
+        String snapshotName,
+        String repoName,
+        Boolean doesRepoExist,
+        String... indices
+    ) {
         logger.info("--> Create a repository");
-        if (repositorySettings == null) {
-            createRepository("test-repo", "fs");
-        } else {
-            createRepository("test-repo", "fs", repositorySettings);
+        if (!doesRepoExist) {
+            if (repositorySettings == null) {
+                createRepository(repoName, FsRepository.TYPE);
+            } else {
+                createRepository(repoName, FsRepository.TYPE, repositorySettings);
+            }
         }
         logger.info("--> Take a snapshot");
         final CreateSnapshotResponse createSnapshotResponse = client.admin()
             .cluster()
-            .prepareCreateSnapshot("test-repo", "test-snap")
+            .prepareCreateSnapshot(repoName, snapshotName)
             .setWaitForCompletion(true)
             .setIndices(indices)
             .get();
@@ -247,11 +315,11 @@ private void deleteIndicesAndEnsureGreen(Client client, String... indices) {
         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)

server/src/main/java/org/opensearch/OpenSearchException.java

@@ -1611,6 +1611,12 @@ private enum OpenSearchExceptionHandle {
             ClusterManagerThrottlingException::new,
             165,
             Version.V_2_4_0
+        ),
+        SNAPSHOT_DELETION_EXCEPTION(
+            org.opensearch.snapshots.SnapshotDeletionException.class,
+            org.opensearch.snapshots.SnapshotDeletionException::new,
+            166,
+            UNKNOWN_VERSION_ADDED
         );
 
         final Class<? extends OpenSearchException> exceptionClass;

server/src/main/java/org/opensearch/snapshots/SnapshotDeletionException.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 SnapshotDeletionException extends SnapshotException {
+
+    public SnapshotDeletionException(final String repositoryName, final String snapshotName, final String msg) {
+        super(repositoryName, snapshotName, msg);
+    }
+
+    public SnapshotDeletionException(StreamInput in) throws IOException {
+        super(in);
+    }
+
+    @Override
+    public RestStatus status() {
+        return RestStatus.FORBIDDEN;
+    }
+}

server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java

@@ -32,16 +32,24 @@
 package org.opensearch.snapshots;
 
 import org.opensearch.action.support.IndicesOptions;
+import org.opensearch.cluster.ClusterState;
+import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
+import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.common.regex.Regex;
+import org.opensearch.index.IndexModule;
 import org.opensearch.index.IndexNotFoundException;
+import org.opensearch.index.IndexSettings;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Iterator;
 
 /**
  * Snapshot utilities
@@ -135,4 +143,36 @@ public static List<String> filterIndices(List<String> availableIndices, String[]
         }
         return Collections.unmodifiableList(new ArrayList<>(result));
     }
+
+    /**
+     * Validates if there are any remote snapshots backing an index
+     * @param currentState current cluster state
+     * @param snapshotIds list of snapshot Ids to be verified
+     * @param repoName repo name for which the verification is being done
+     */
+    public static void validateSnapshotsBackingAnyIndex(ClusterState currentState, List<SnapshotId> snapshotIds, String repoName) {
+        final Map<String, SnapshotId> uuidToSnapshotId = new HashMap<>();
+        final Set<String> snapshotsToBeNotDeleted = new HashSet<>();
+        ImmutableOpenMap<String, IndexMetadata> indicesMap = currentState.getMetadata().getIndices();
+        snapshotIds.forEach(snapshotId -> uuidToSnapshotId.put(snapshotId.getUUID(), snapshotId));
+
+        for (Iterator<IndexMetadata> it = indicesMap.valuesIt(); it.hasNext();) {
+            IndexMetadata indexMetadata = it.next();
+            String storeType = indexMetadata.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey());
+            if (IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey().equals(storeType)) {
+                String snapshotId = indexMetadata.getSettings().get(IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID.getKey());
+                if (uuidToSnapshotId.get(snapshotId) != null) {
+                    snapshotsToBeNotDeleted.add(uuidToSnapshotId.get(snapshotId).getName());
+                }
+            }
+        }
+
+        if (!snapshotsToBeNotDeleted.isEmpty()) {
+            throw new SnapshotDeletionException(
+                repoName,
+                snapshotsToBeNotDeleted.toString(),
+                "These remote snapshots are backing some indices and hence can't be deleted! No snapshots were deleted."
+            );
+        }
+    }
 }

server/src/main/java/org/opensearch/snapshots/SnapshotsService.java

@@ -132,6 +132,7 @@
 import static java.util.Collections.emptySet;
 import static java.util.Collections.unmodifiableList;
 import static org.opensearch.cluster.SnapshotsInProgress.completed;
+import static org.opensearch.snapshots.SnapshotUtils.validateSnapshotsBackingAnyIndex;
 
 /**
  * Service responsible for creating snapshots. This service runs all the steps executed on the cluster-manager node during snapshot creation and
@@ -1769,6 +1770,7 @@ public ClusterState execute(ClusterState currentState) throws Exception {
                     snapshotNames,
                     repoName
                 );
+                validateSnapshotsBackingAnyIndex(currentState, snapshotIds, repoName);
                 deleteFromRepoTask = createDeleteStateUpdate(snapshotIds, repoName, repositoryData, Priority.NORMAL, listener);
                 return deleteFromRepoTask.execute(currentState);
             }

server/src/test/java/org/opensearch/ExceptionSerializationTests.java

@@ -106,6 +106,7 @@
 import org.opensearch.snapshots.SnapshotException;
 import org.opensearch.snapshots.SnapshotId;
 import org.opensearch.snapshots.SnapshotInProgressException;
+import org.opensearch.snapshots.SnapshotDeletionException;
 import org.opensearch.test.OpenSearchTestCase;
 import org.opensearch.test.VersionUtils;
 import org.opensearch.transport.ActionNotFoundTransportException;
@@ -862,6 +863,7 @@ public void testIds() {
         ids.put(163, DecommissioningFailedException.class);
         ids.put(164, NodeDecommissionedException.class);
         ids.put(165, ClusterManagerThrottlingException.class);
+        ids.put(166, SnapshotDeletionException.class);
 
         Map<Class<? extends OpenSearchException>, Integer> reverse = new HashMap<>();
         for (Map.Entry<Integer, Class<? extends OpenSearchException>> entry : ids.entrySet()) {