Add update settings allowlist for searchable snapshot

Allow search related settings to be updated for searchable snapshots. Signed-off-by: Rabi Panda <adnapibar@gmail.com>
opensearch-project · Jan 17, 2023 · 5ec928c · 5ec928c
1 parent 7bd1291
commit 5ec928c
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 2 deletions.
diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java
@@ -13,6 +13,7 @@
 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;
@@ -38,6 +39,7 @@
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest.Metric.FS;
 import static org.opensearch.common.util.CollectionUtils.iterableAsArrayList;
 
@@ -215,6 +217,7 @@ public void testSearchableSnapshotIndexIsReadOnly() throws Exception {
 
         assertIndexingBlocked(restoredIndexName);
         assertIndexSettingChangeBlocked(restoredIndexName);
+        assertAllowedIndexSettingUpdate(restoredIndexName);
         assertTrue(client.admin().indices().prepareDelete(restoredIndexName).get().isAcknowledged());
         assertThrows(
             "Expect index to not exist",
@@ -336,6 +339,13 @@ private void assertIndexSettingChangeBlocked(String index) {
         }
     }
 
+    private void assertAllowedIndexSettingUpdate(String index) {
+        final UpdateSettingsRequestBuilder builder = client().admin().indices().prepareUpdateSettings(index);
+        builder.setSettings(Map.of("index.max_result_window", 100));
+        AcknowledgedResponse settingsResponse = builder.execute().actionGet();
+        assertThat(settingsResponse, notNullValue());
+    }
+
     /**
      * Picks a shard out of the cluster state for each given index and asserts
      * that the 'index' directory does not exist in the node's file system.

diff --git a/.../java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java b/.../java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java
@@ -50,10 +50,15 @@
 import org.opensearch.common.inject.Inject;
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.index.Index;
+import org.opensearch.index.IndexModule;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportService;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.stream.Stream;
+
+import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
 
 /**
  * Transport action for updating index settings
@@ -64,6 +69,18 @@ public class TransportUpdateSettingsAction extends TransportClusterManagerNodeAc
 
     private static final Logger logger = LogManager.getLogger(TransportUpdateSettingsAction.class);
 
+    private final static String[] ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS = {
+        "index.max_result_window",
+        "index.max_inner_result_window",
+        "index.max_rescore_window",
+        "index.max_docvalue_fields_search",
+        "index.max_script_fields",
+        "index.max_terms_count",
+        "index.max_regex_length",
+        "index.highlight.max_analyzed_offset" };
+
+    private final static String[] ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS_PREFIXES = { "index.search.slowlog" };
+
     private final MetadataUpdateSettingsService updateSettingsService;
 
     @Inject
@@ -106,8 +123,31 @@ protected ClusterBlockException checkBlock(UpdateSettingsRequest request, Cluste
             || IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.exists(request.settings())) {
             return null;
         }
-        return state.blocks()
-            .indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indexNameExpressionResolver.concreteIndexNames(state, request));
+
+        final Index[] requestIndices = indexNameExpressionResolver.concreteIndices(state, request);
+        boolean allowSearchableSnapshotSettingsUpdate = true;
+        // check if all indices in the request are remote snapshot
+        for (Index index : requestIndices) {
+            if (state.blocks().indexBlocked(ClusterBlockLevel.METADATA_WRITE, index.getName())) {
+                allowSearchableSnapshotSettingsUpdate = allowSearchableSnapshotSettingsUpdate
+                    && IndexModule.Type.REMOTE_SNAPSHOT.match(
+                        state.getMetadata().getIndexSafe(index).getSettings().get(INDEX_STORE_TYPE_SETTING.getKey())
+                    );
+            }
+        }
+        // check if all settings in the request are in the allow list
+        if (allowSearchableSnapshotSettingsUpdate) {
+            for (String setting : request.settings().keySet()) {
+                allowSearchableSnapshotSettingsUpdate = allowSearchableSnapshotSettingsUpdate
+                    && (Stream.of(ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS_PREFIXES).anyMatch(setting::startsWith)
+                        || (Arrays.asList(ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS).contains(setting)));
+            }
+        }
+
+        return allowSearchableSnapshotSettingsUpdate
+            ? null
+            : state.blocks()
+                .indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indexNameExpressionResolver.concreteIndexNames(state, request));
     }
 
     @Override