exclude searchable snapshot when check block in _aliases and _rollove…

…r api

Signed-off-by: kkewwei <[email protected]>

CHANGELOG.md

@@ -101,6 +101,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [Workload Management] Fixing Create/Update QueryGroup TransportActions to execute from non-cluster manager nodes ([16422](https://github.com/opensearch-project/OpenSearch/pull/16422))
 - Fix flaky test in `testApproximateRangeWithSizeOverDefault` by adjusting totalHits assertion logic ([#16434](https://github.com/opensearch-project/OpenSearch/pull/16434#pullrequestreview-2386999409))
 - Revert changes to upload remote state manifest using minimum codec version([#16403](https://github.com/opensearch-project/OpenSearch/pull/16403))
+- Fix Rollover alias supports remote snapshot index
 
 ### Security
 

server/src/main/java/org/opensearch/action/admin/indices/alias/TransportIndicesAliasesAction.java

@@ -41,7 +41,7 @@
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.ack.ClusterStateUpdateResponse;
 import org.opensearch.cluster.block.ClusterBlockException;
-import org.opensearch.cluster.block.ClusterBlockLevel;
+import org.opensearch.cluster.block.ClusterBlocks;
 import org.opensearch.cluster.metadata.AliasAction;
 import org.opensearch.cluster.metadata.AliasMetadata;
 import org.opensearch.cluster.metadata.IndexAbstraction;
@@ -123,7 +123,7 @@ protected ClusterBlockException checkBlock(IndicesAliasesRequest request, Cluste
         for (IndicesAliasesRequest.AliasActions aliasAction : request.aliasActions()) {
             Collections.addAll(indices, aliasAction.indices());
         }
-        return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indices.toArray(new String[0]));
+        return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, state);
     }
 
     @Override

server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java

@@ -44,7 +44,7 @@
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.ClusterStateUpdateTask;
 import org.opensearch.cluster.block.ClusterBlockException;
-import org.opensearch.cluster.block.ClusterBlockLevel;
+import org.opensearch.cluster.block.ClusterBlocks;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.metadata.Metadata;
@@ -62,8 +62,10 @@
 import org.opensearch.transport.TransportService;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -127,11 +129,10 @@ protected ClusterBlockException checkBlock(RolloverRequest request, ClusterState
             request.indicesOptions().expandWildcardsClosed()
         );
 
-        return state.blocks()
-            .indicesBlockedException(
-                ClusterBlockLevel.METADATA_WRITE,
-                indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request)
-            );
+        return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(
+            new HashSet<>(Arrays.asList(indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request))),
+            state
+        );
     }
 
     @Override

server/src/main/java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java

@@ -42,7 +42,7 @@
 import org.opensearch.cluster.ack.ClusterStateUpdateResponse;
 import org.opensearch.cluster.block.ClusterBlockException;
 import org.opensearch.cluster.block.ClusterBlockLevel;
-import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.cluster.block.ClusterBlocks;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.metadata.MetadataUpdateSettingsService;
 import org.opensearch.cluster.service.ClusterService;
@@ -118,9 +118,8 @@ protected ClusterBlockException checkBlock(UpdateSettingsRequest request, Cluste
             return globalBlock;
         }
         if (request.settings().size() == 1 &&  // we have to allow resetting these settings otherwise users can't unblock an index
-            IndexMetadata.INDEX_BLOCKS_METADATA_SETTING.exists(request.settings())
-            || IndexMetadata.INDEX_READ_ONLY_SETTING.exists(request.settings())
-            || IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.exists(request.settings())) {
+            ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS.stream()
+                .anyMatch(booleanSetting -> booleanSetting.exists(request.settings()))) {
             return null;
         }
 

server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java

@@ -33,19 +33,23 @@
 package org.opensearch.cluster.block;
 
 import org.opensearch.cluster.AbstractDiffable;
+import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.Diff;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.metadata.MetadataIndexStateService;
 import org.opensearch.common.Nullable;
 import org.opensearch.common.annotation.PublicApi;
+import org.opensearch.common.settings.Setting;
 import org.opensearch.common.util.set.Sets;
 import org.opensearch.core.common.io.stream.BufferedChecksumStreamOutput;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.common.io.stream.VerifiableWriteable;
 import org.opensearch.core.rest.RestStatus;
+import org.opensearch.index.IndexModule;
 
 import java.io.IOException;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumMap;
 import java.util.HashMap;
@@ -57,6 +61,7 @@
 import static java.util.Collections.emptySet;
 import static java.util.Collections.unmodifiableSet;
 import static java.util.stream.Collectors.toSet;
+import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
 
 /**
  * Represents current cluster level blocks to block dirty operations done against the cluster.
@@ -66,7 +71,11 @@
 @PublicApi(since = "1.0.0")
 public class ClusterBlocks extends AbstractDiffable<ClusterBlocks> implements VerifiableWriteable {
     public static final ClusterBlocks EMPTY_CLUSTER_BLOCK = new ClusterBlocks(emptySet(), Map.of());
-
+    public static final Set<Setting<Boolean>> INDEX_DATA_READ_ONLY_BLOCK_SETTINGS = Set.of(
+        IndexMetadata.INDEX_READ_ONLY_SETTING,
+        IndexMetadata.INDEX_BLOCKS_METADATA_SETTING,
+        IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING
+    );
     private final Set<ClusterBlock> global;
 
     private final Map<String, Set<ClusterBlock>> indicesBlocks;
@@ -276,6 +285,21 @@ public ClusterBlockException indicesAllowReleaseResources(String[] indices) {
         return new ClusterBlockException(indexLevelBlocks);
     }
 
+    public static ClusterBlockException indicesWithRemoteSnapshotBlockedException(Collection<String> concreteIndices, ClusterState state) {
+        for (String index : concreteIndices) {
+            if (state.blocks().indexBlocked(ClusterBlockLevel.METADATA_WRITE, index)) {
+                IndexMetadata indexMeta = state.metadata().index(index);
+                if (indexMeta != null
+                    && (IndexModule.Type.REMOTE_SNAPSHOT.match(indexMeta.getSettings().get(INDEX_STORE_TYPE_SETTING.getKey())) == false
+                        || ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS.stream()
+                            .anyMatch(booleanSetting -> booleanSetting.exists(indexMeta.getSettings())))) {
+                    return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, concreteIndices.toArray(new String[0]));
+                }
+            }
+        }
+        return null;
+    }
+
     @Override
     public String toString() {
         if (global.isEmpty() && indices().isEmpty()) {

server/src/test/java/org/opensearch/cluster/block/ClusterBlocksTests.java

@@ -8,12 +8,40 @@
 
 package org.opensearch.cluster.block;
 
+import com.carrotsearch.randomizedtesting.RandomizedTest;
+
+import org.opensearch.Version;
+import org.opensearch.cluster.ClusterName;
+import org.opensearch.cluster.ClusterState;
+import org.opensearch.cluster.metadata.AliasMetadata;
+import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.common.io.stream.BytesStreamOutput;
+import org.opensearch.common.settings.Setting;
+import org.opensearch.common.settings.Settings;
 import org.opensearch.core.common.io.stream.BufferedChecksumStreamOutput;
 import org.opensearch.core.common.io.stream.StreamInput;
+import org.opensearch.index.IndexModule;
+import org.opensearch.index.IndexSettings;
 import org.opensearch.test.OpenSearchTestCase;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
 import static org.opensearch.cluster.block.ClusterBlockTests.randomClusterBlock;
+import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_METADATA_BLOCK;
+import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_BLOCK;
+import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK;
+import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK;
+import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_WRITE_BLOCK;
+import static org.opensearch.cluster.metadata.IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
 
 public class ClusterBlocksTests extends OpenSearchTestCase {
 
@@ -52,4 +80,153 @@ public void testWriteVerifiableTo() throws Exception {
         clusterBlocks2.writeVerifiableTo(checksumOut2);
         assertEquals(checksumOut.getChecksum(), checksumOut2.getChecksum());
     }
+
+    public void testGlobalBlock() {
+        String index = "test-000001";
+        ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
+        Set<String> indices = new HashSet<>();
+        indices.add(index);
+
+        // no global blocks
+        {
+            stateBuilder.blocks(ClusterBlocks.EMPTY_CLUSTER_BLOCK);
+            ClusterState clusterState = stateBuilder.build();
+            clusterState.blocks();
+            assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
+        }
+
+        // has global block
+        {
+            for (ClusterBlock block : Arrays.asList(
+                INDEX_READ_ONLY_BLOCK,
+                INDEX_READ_BLOCK,
+                INDEX_WRITE_BLOCK,
+                INDEX_METADATA_BLOCK,
+                INDEX_READ_ONLY_ALLOW_DELETE_BLOCK,
+                REMOTE_READ_ONLY_ALLOW_DELETE
+            )) {
+                stateBuilder.blocks(ClusterBlocks.builder().addGlobalBlock(block).build());
+                ClusterState clusterState = stateBuilder.build();
+                clusterState.blocks();
+                assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
+            }
+        }
+    }
+
+    public void testIndexWithBlock() {
+        String index = "test-000001";
+        Set<String> indices = new HashSet<>();
+        indices.add(index);
+        ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
+        stateBuilder.blocks(ClusterBlocks.builder().addIndexBlock(index, IndexMetadata.INDEX_METADATA_BLOCK));
+        stateBuilder.metadata(Metadata.builder().put(createIndexMetadata(index, false, null, null), false));
+        ClusterState clusterState = stateBuilder.build();
+        clusterState.blocks();
+        assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, stateBuilder.build()));
+    }
+
+    public void testRemoteIndexBlock() {
+        String remoteIndex = "remote_index";
+        Set<String> indices = new HashSet<>();
+        indices.add(remoteIndex);
+        ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
+
+        {
+            IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, null, null);
+            stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false));
+            stateBuilder.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
+
+            ClusterState clusterState = stateBuilder.build();
+            assertTrue(clusterState.blocks().hasIndexBlock(remoteIndex, IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE));
+            clusterState.blocks();
+            assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
+        }
+
+        // searchable snapshot index with block
+        {
+            Setting<Boolean> setting = RandomizedTest.randomFrom(new ArrayList<>(ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS));
+            IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, null, setting);
+            stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false));
+            stateBuilder.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
+            ClusterState clusterState = stateBuilder.build();
+            clusterState.blocks();
+            assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
+        }
+    }
+
+    public void testRemoteIndexWithoutBlock() {
+        String remoteIndex = "remote_index";
+        ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
+
+        String alias = "alias1";
+        IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, null);
+        String index = "test-000001";
+        IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null);
+        stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false));
+
+        Set<String> indices = new HashSet<>();
+        indices.add(remoteIndex);
+        ClusterState clusterState = stateBuilder.build();
+        clusterState.blocks();
+        assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
+    }
+
+    public void testRemoteIndexWithIndexBlock() {
+        String index = "test-000001";
+        String remoteIndex = "remote_index";
+        String alias = "alias1";
+        {
+            ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
+            IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, null);
+            IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null);
+            stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false))
+                .blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
+            ClusterState clusterState = stateBuilder.build();
+            clusterState.blocks();
+            assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(index), clusterState));
+            clusterState.blocks();
+            assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(remoteIndex), clusterState));
+        }
+
+        {
+            ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
+            Setting<Boolean> setting = RandomizedTest.randomFrom(new ArrayList<>(ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS));
+            IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, setting);
+            IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null);
+            stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false))
+                .blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
+            ClusterState clusterState = stateBuilder.build();
+            assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(index), clusterState));
+            assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(remoteIndex), clusterState));
+        }
+    }
+
+    private IndexMetadata createIndexMetadata(String index, boolean isRemoteIndex, String alias, Setting<Boolean> blockSetting) {
+        IndexMetadata.Builder builder = IndexMetadata.builder(index).settings(createIndexSettingBuilder(isRemoteIndex, blockSetting));
+        if (alias != null) {
+            AliasMetadata.Builder aliasBuilder = AliasMetadata.builder(alias);
+            return builder.putAlias(aliasBuilder.build()).build();
+        }
+        return builder.build();
+    }
+
+    private Settings.Builder createIndexSettingBuilder(boolean isRemoteIndex, Setting<Boolean> blockSetting) {
+        Settings.Builder builder = Settings.builder()
+            .put(IndexMetadata.SETTING_INDEX_UUID, "abc")
+            .put(SETTING_VERSION_CREATED, Version.CURRENT)
+            .put(SETTING_CREATION_DATE, System.currentTimeMillis())
+            .put(SETTING_NUMBER_OF_SHARDS, 1)
+            .put(SETTING_NUMBER_OF_REPLICAS, 1);
+
+        if (isRemoteIndex) {
+            builder.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey())
+                .put(IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY.getKey(), "repo")
+                .put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME.getKey(), "snapshot");
+        }
+        if (blockSetting != null) {
+            builder.put(blockSetting.getKey(), true);
+        }
+
+        return builder;
+    }
 }