Prevent deletion of snapshot backing index

CHANGELOG.md

@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add feature flag for extensions ([#5211](https://github.com/opensearch-project/OpenSearch/pull/5211))
 ### Added
 - Apply reproducible builds configuration for OpenSearch plugins through gradle plugin ([#4746](https://github.com/opensearch-project/OpenSearch/pull/4746))
+- Prevent deletion of snapshots that are backing searchable snapshot indexes ([#5069](https://github.com/opensearch-project/OpenSearch/pull/5069))
 
 ### Dependencies
 - Bump bcpg-fips from 1.0.5.1 to 1.0.7.1 ([#5148](https://github.com/opensearch-project/OpenSearch/pull/5148))

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)

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;

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;
+    }
+}

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

@@ -31,17 +31,24 @@
 
 package org.opensearch.snapshots;
 
+import com.carrotsearch.hppc.cursors.ObjectCursor;
 import org.opensearch.action.support.IndicesOptions;
+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;
 
 /**
  * Snapshot utilities
@@ -135,4 +142,39 @@ 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 metadata index metadata from 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(
+        ImmutableOpenMap<String, IndexMetadata> metadata,
+        List<SnapshotId> snapshotIds,
+        String repoName
+    ) {
+        final Map<String, SnapshotId> uuidToSnapshotId = new HashMap<>();
+        final Set<String> snapshotsToBeNotDeleted = new HashSet<>();
+        snapshotIds.forEach(snapshotId -> uuidToSnapshotId.put(snapshotId.getUUID(), snapshotId));
+
+        for (ObjectCursor<IndexMetadata> cursor : metadata.values()) {
+            IndexMetadata indexMetadata = cursor.value;
+            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 SnapshotInUseDeletionException(
+                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

@@ -134,6 +134,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
@@ -2217,6 +2218,7 @@ public ClusterState execute(ClusterState currentState) throws Exception {
                     snapshotNames,
                     repoName
                 );
+                validateSnapshotsBackingAnyIndex(currentState.getMetadata().getIndices(), snapshotIds, repoName);
                 if (snapshotEntries.isEmpty() || minNodeVersion.onOrAfter(SnapshotsService.FULL_CONCURRENCY_VERSION)) {
                     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.SnapshotInUseDeletionException;
 import org.opensearch.test.OpenSearchTestCase;
 import org.opensearch.test.VersionUtils;
 import org.opensearch.transport.ActionNotFoundTransportException;
@@ -866,6 +867,7 @@ public void testIds() {
         ids.put(163, DecommissioningFailedException.class);
         ids.put(164, NodeDecommissionedException.class);
         ids.put(165, ClusterManagerThrottlingException.class);
+        ids.put(166, SnapshotInUseDeletionException.class);
 
         Map<Class<? extends OpenSearchException>, Integer> reverse = new HashMap<>();
         for (Map.Entry<Integer, Class<? extends OpenSearchException>> entry : ids.entrySet()) {