use PrivateIndex setting property to avoid user overrides on remote i…

…ndex settings

Signed-off-by: Varun Bansal <bansvaru@amazon.com>

server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java

@@ -29,7 +29,6 @@
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
 import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING;
-import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_ENABLED_SETTING;
 import static org.opensearch.remotestore.RemoteStoreBaseIntegTestCase.remoteStoreClusterSettings;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
@@ -47,7 +46,7 @@ public void teardown() {
     protected Settings nodeSettings(int nodeOriginal) {
         Settings settings = super.nodeSettings(nodeOriginal);
         Settings.Builder builder = Settings.builder()
-            .put(remoteStoreClusterSettings("my-segment-repo-1", "my-translog-repo-1", true))
+            .put(remoteStoreClusterSettings("my-segment-repo-1", "my-translog-repo-1"))
             .put(settings);
         return builder.build();
     }
@@ -115,9 +114,8 @@ public void testRemoteStoreDisabledByUser() throws Exception {
             containsString(
                 String.format(
                     Locale.ROOT,
-                    "Cannot override [%s] settings when [%s] is set to [true].",
-                    SETTING_REMOTE_STORE_ENABLED,
-                    CLUSTER_REMOTE_STORE_ENABLED_SETTING.getKey()
+                    "Validation Failed: 1: private index setting [%s] can not be set explicitly;",
+                    SETTING_REMOTE_STORE_ENABLED
                 )
             )
         );
@@ -157,9 +155,8 @@ public void testRemoteStoreEnabledByUserWithoutRemoteRepoIllegalArgumentExceptio
             containsString(
                 String.format(
                     Locale.ROOT,
-                    "Cannot override [%s] settings when [%s] is set to [true].",
-                    SETTING_REMOTE_STORE_ENABLED,
-                    CLUSTER_REMOTE_STORE_ENABLED_SETTING.getKey()
+                    "Validation Failed: 1: private index setting [%s] can not be set explicitly;",
+                    SETTING_REMOTE_STORE_ENABLED
                 )
             )
         );
@@ -230,10 +227,9 @@ public void testRemoteStoreEnabledByUserWithRemoteRepoIllegalArgumentException()
             containsString(
                 String.format(
                     Locale.ROOT,
-                    "Cannot override [%s][%s] settings when [%s] is set to [true].",
+                    "Validation Failed: 1: private index setting [%s] can not be set explicitly;2: private index setting [%s] can not be set explicitly;",
                     SETTING_REMOTE_STORE_ENABLED,
-                    SETTING_REMOTE_SEGMENT_STORE_REPOSITORY,
-                    CLUSTER_REMOTE_STORE_ENABLED_SETTING.getKey()
+                    SETTING_REMOTE_SEGMENT_STORE_REPOSITORY
                 )
             )
         );
@@ -280,35 +276,10 @@ public void testRemoteStoreOverrideTranslogRepoCorrectly() throws Exception {
             containsString(
                 String.format(
                     Locale.ROOT,
-                    "Cannot override [%s][%s][%s] settings when [%s] is set to [true].",
+                    "Validation Failed: 1: private index setting [%s] can not be set explicitly;2: private index setting [%s] can not be set explicitly;3: private index setting [%s] can not be set explicitly;",
                     SETTING_REMOTE_STORE_ENABLED,
                     SETTING_REMOTE_SEGMENT_STORE_REPOSITORY,
-                    SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY,
-                    CLUSTER_REMOTE_STORE_ENABLED_SETTING.getKey()
-                )
-            )
-        );
-    }
-
-    public void testRemoteStoreOverrideReplicationTypeIndexSettings() throws Exception {
-        Settings settings = Settings.builder()
-            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
-            .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
-            .put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT)
-            .build();
-        IllegalArgumentException exc = expectThrows(
-            IllegalArgumentException.class,
-            () -> client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get()
-        );
-        assertThat(
-            exc.getMessage(),
-            containsString(
-                String.format(
-                    Locale.ROOT,
-                    "To enable %s, %s should be set to %s",
-                    SETTING_REMOTE_STORE_ENABLED,
-                    SETTING_REPLICATION_TYPE,
-                    ReplicationType.SEGMENT
+                    SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY
                 )
             )
         );

server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java

@@ -82,6 +82,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Function;
 
@@ -285,6 +286,32 @@ public Iterator<Setting<?>> settings() {
         SETTING_REPLICATION_TYPE,
         ReplicationType.DOCUMENT.toString(),
         ReplicationType::parseString,
+        new Setting.Validator<>() {
+
+            @Override
+            public void validate(final ReplicationType value) {}
+
+            @Override
+            public void validate(final ReplicationType value, final Map<Setting<?>, Object> settings) {
+                final Object remoteStoreEnabled = settings.get(INDEX_REMOTE_STORE_ENABLED_SETTING);
+                if (ReplicationType.SEGMENT.equals(value) == false && Objects.equals(remoteStoreEnabled, true)) {
+                    throw new IllegalArgumentException(
+                        "To enable "
+                            + INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()
+                            + ", "
+                            + INDEX_REPLICATION_TYPE_SETTING.getKey()
+                            + " should be set to "
+                            + ReplicationType.SEGMENT
+                    );
+                }
+            }
+
+            @Override
+            public Iterator<Setting<?>> settings() {
+                final List<Setting<?>> settings = List.of(INDEX_REMOTE_STORE_ENABLED_SETTING);
+                return settings.iterator();
+            }
+        },
         Property.IndexScope,
         Property.Final
     );
@@ -328,7 +355,8 @@ public Iterator<Setting<?>> settings() {
             }
         },
         Property.IndexScope,
-        Property.Final
+        Property.PrivateIndex,
+        Property.Dynamic
     );
 
     /**
@@ -361,7 +389,8 @@ public Iterator<Setting<?>> settings() {
             }
         },
         Property.IndexScope,
-        Property.Final
+        Property.PrivateIndex,
+        Property.Dynamic
     );
 
     private static void validateRemoteStoreSettingEnabled(final Map<Setting<?>, Object> settings, Setting<?> setting) {
@@ -411,7 +440,8 @@ public Iterator<Setting<?>> settings() {
             }
         },
         Property.IndexScope,
-        Property.Final
+        Property.PrivateIndex,
+        Property.Dynamic
     );
 
     public static final String SETTING_AUTO_EXPAND_REPLICAS = "index.auto_expand_replicas";

server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java

@@ -114,14 +114,10 @@
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
-import java.util.stream.Stream;
 
 import static java.util.stream.Collectors.toList;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
-import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING;
-import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REMOTE_SEGMENT_STORE_REPOSITORY_SETTING;
-import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REPLICATION_TYPE_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE;
@@ -891,7 +887,7 @@ static Settings aggregateIndexSettings(
         indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID());
 
         updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings);
-        updateRemoteStoreSettings(indexSettingsBuilder, request.settings(), settings);
+        updateRemoteStoreSettings(indexSettingsBuilder, settings);
 
         if (sourceMetadata != null) {
             assert request.resizeType() != null;
@@ -946,39 +942,16 @@ private static void updateReplicationStrategy(Settings.Builder settingsBuilder,
     /**
      * Updates index settings to enable remote store by default based on cluster level settings
      * @param settingsBuilder index settings builder to be updated with relevant settings
-     * @param requestSettings settings passed in during index create request
      * @param clusterSettings cluster level settings
      */
-    private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder, Settings requestSettings, Settings clusterSettings) {
-        boolean isRemoteStoreClusterEnabled = CLUSTER_REMOTE_STORE_ENABLED_SETTING.get(clusterSettings);
-        List<String> overriddenSettings = getRemoteStoreOverriddenSetting(requestSettings);
-        if (overriddenSettings.isEmpty() == false) {
-            throw new IllegalArgumentException(
-                String.format(
-                    Locale.ROOT,
-                    "Cannot override [%s] settings when [%s] is set to [%s].",
-                    String.join("][", overriddenSettings),
-                    CLUSTER_REMOTE_STORE_ENABLED_SETTING.getKey(),
-                    isRemoteStoreClusterEnabled
-                )
-            );
-        }
-
-        if (isRemoteStoreClusterEnabled == true) {
+    private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder, Settings clusterSettings) {
+        if (CLUSTER_REMOTE_STORE_ENABLED_SETTING.get(clusterSettings) == true) {
             settingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, true)
                 .put(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, CLUSTER_REMOTE_SEGMENT_STORE_REPOSITORY_SETTING.get(clusterSettings))
                 .put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING.get(clusterSettings));
         }
     }
 
-    private static List<String> getRemoteStoreOverriddenSetting(Settings requestSettings) {
-        return Stream.of(
-            INDEX_REMOTE_STORE_ENABLED_SETTING,
-            INDEX_REMOTE_SEGMENT_STORE_REPOSITORY_SETTING,
-            INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING
-        ).filter(setting -> setting.exists(requestSettings)).map(Setting::getKey).collect(toList());
-    }
-
     public static void validateStoreTypeSettings(Settings settings) {
         // deprecate simplefs store type:
         if (IndexModule.Type.SIMPLEFS.match(IndexModule.INDEX_STORE_TYPE_SETTING.get(settings))) {