Restrict user overrides for remote store related index settings (open…

…search-project#8812) Signed-off-by: Varun Bansal <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Gaurav614 · Apr 25, 2024 · e6d5086 · e6d5086
1 parent b8333e8
commit e6d5086
Show file tree

Hide file tree

Showing 34 changed files with 1,731 additions and 1,462 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -48,6 +48,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Remote Segment Store Repository setting moved from `index.remote_store.repository` to `index.remote_store.segment.repository` and `cluster.remote_store.repository` to `cluster.remote_store.segment.repository` respectively for Index and Cluster level settings ([#8719](https:/opensearch-project/OpenSearch/pull/8719))
 - [Remote Store] Add support to restore only unassigned shards of an index ([#8792](https:/opensearch-project/OpenSearch/pull/8792))
 - Replace the deprecated IndexReader APIs with new storedFields() & termVectors() ([#7792](https:/opensearch-project/OpenSearch/pull/7792))
+- [Remote Store] Restrict user override for remote store index level settings ([#8812](https:/opensearch-project/OpenSearch/pull/8812))
 
 ### Deprecated
 

diff --git a/server/src/internalClusterTest/java/org/opensearch/index/SegmentReplicationPressureIT.java b/server/src/internalClusterTest/java/org/opensearch/index/SegmentReplicationPressureIT.java
@@ -15,6 +15,7 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.lease.Releasable;
+import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException;
 import org.opensearch.index.shard.IndexShard;
 import org.opensearch.index.shard.IndexShardState;
@@ -30,6 +31,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -260,7 +262,7 @@ public void testFailStaleReplica() throws Exception {
  public void testWithDocumentReplicationEnabledIndex() throws Exception {
  assumeTrue(
  "Can't create DocRep index with remote store enabled. Skipping.",
- indexSettings().getAsBoolean(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false) == false
+ Objects.equals(featureFlagSettings().get(FeatureFlags.REMOTE_STORE, "false"), "false")
  );
  Settings settings = Settings.builder().put(MAX_REPLICATION_TIME_SETTING.getKey(), TimeValue.timeValueMillis(500)).build();
  // Starts a primary and replica node.

diff --git a/...rTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java b/...rTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java
@@ -29,6 +29,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import static org.opensearch.remotestore.RemoteStoreBaseIntegTestCase.remoteStoreClusterSettings;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
 public abstract class AbstractRemoteStoreMockRepositoryIntegTestCase extends AbstractSnapshotIntegTestCase {
@@ -46,7 +47,7 @@ protected Settings featureFlagSettings() {
  public void setup() {
  FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
  FeatureFlagSetter.set(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL);
- internalCluster().startClusterManagerOnlyNode();
+ internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REPOSITORY_NAME, TRANSLOG_REPOSITORY_NAME));
  }
 
  @Override
@@ -62,9 +63,6 @@ protected Settings remoteStoreIndexSettings(int numberOfReplicas) {
  .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numberOfReplicas)
  .put(IndexModule.INDEX_QUERY_CACHE_ENABLED_SETTING.getKey(), false)
  .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
- .put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, true)
- .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, REPOSITORY_NAME)
- .put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, TRANSLOG_REPOSITORY_NAME)
  .build();
  }
 

diff --git a/...reateRemoteIndexClusterDefaultDocRep.java → ...ateRemoteIndexClusterDefaultDocRepIT.java b/...reateRemoteIndexClusterDefaultDocRep.java → ...ateRemoteIndexClusterDefaultDocRepIT.java
@@ -16,12 +16,16 @@
 import org.opensearch.indices.replication.common.ReplicationType;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
+import java.util.Locale;
+
 import static org.hamcrest.Matchers.containsString;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
 import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
-public class CreateRemoteIndexClusterDefaultDocRep extends CreateRemoteIndexIT {
+public class CreateRemoteIndexClusterDefaultDocRepIT extends CreateRemoteIndexIT {
 
  @Override
  protected Settings nodeSettings(int nodeOriginal) {
@@ -44,7 +48,15 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception {
  );
  assertThat(
  exc.getMessage(),
- containsString("Cannot enable [index.remote_store.enabled] when [index.replication.type] is DOCUMENT")
+ containsString(
+ String.format(
+ Locale.ROOT,
+ "To enable %s, %s should be set to %s",
+ SETTING_REMOTE_STORE_ENABLED,
+ SETTING_REPLICATION_TYPE,
+ ReplicationType.SEGMENT
+ )
+ )
  );
  }
 

diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java
@@ -26,14 +26,10 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY;
-import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING;
 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.indices.IndicesService.CLUSTER_REMOTE_STORE_REPOSITORY_SETTING;
-import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING;
-import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_ENABLED_SETTING;
-import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;
-import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE;
+import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING;
+import static org.opensearch.remotestore.RemoteStoreBaseIntegTestCase.remoteStoreClusterSettings;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
@@ -50,10 +46,7 @@ public void teardown() {
  protected Settings nodeSettings(int nodeOriginal) {
  Settings settings = super.nodeSettings(nodeOriginal);
  Settings.Builder builder = Settings.builder()
- .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT)
- .put(CLUSTER_REMOTE_STORE_ENABLED_SETTING.getKey(), true)
- .put(CLUSTER_REMOTE_STORE_REPOSITORY_SETTING.getKey(), "my-segment-repo-1")
- .put(CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING.getKey(), "my-translog-repo-1")
+ .put(remoteStoreClusterSettings("my-segment-repo-1", "my-translog-repo-1"))
  .put(settings);
  return builder.build();
  }
@@ -111,19 +104,20 @@ public void testRemoteStoreDisabledByUser() throws Exception {
  .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
  .put(SETTING_REMOTE_STORE_ENABLED, false)
  .build();
- assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
- GetIndexResponse getIndexResponse = client().admin()
- .indices()
- .getIndex(new GetIndexRequest().indices("test-idx-1").includeDefaults(true))
- .get();
- Settings indexSettings = getIndexResponse.settings().get("test-idx-1");
- verifyRemoteStoreIndexSettings(
- indexSettings,
- "false",
- null,
- null,
- client().settings().get(CLUSTER_SETTING_REPLICATION_TYPE),
- IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
+
+ IllegalArgumentException exc = expectThrows(
+ IllegalArgumentException.class,
+ () -> client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get()
+ );
+ assertThat(
+ exc.getMessage(),
+ containsString(
+ String.format(
+ Locale.ROOT,
+ "Validation Failed: 1: private index setting [%s] can not be set explicitly;",
+ SETTING_REMOTE_STORE_ENABLED
+ )
+ )
  );
  }
 
@@ -161,8 +155,8 @@ public void testRemoteStoreEnabledByUserWithoutRemoteRepoIllegalArgumentExceptio
  containsString(
  String.format(
  Locale.ROOT,
- "Setting %s should be provided with non-empty repository ID",
- SETTING_REMOTE_SEGMENT_STORE_REPOSITORY
+ "Validation Failed: 1: private index setting [%s] can not be set explicitly;",
+ SETTING_REMOTE_STORE_ENABLED
  )
  )
  );
@@ -174,19 +168,21 @@ public void testReplicationTypeDocumentByUser() throws Exception {
  .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
  .put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT)
  .build();
- assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
- GetIndexResponse getIndexResponse = client().admin()
- .indices()
- .getIndex(new GetIndexRequest().indices("test-idx-1").includeDefaults(true))
- .get();
- Settings indexSettings = getIndexResponse.settings().get("test-idx-1");
- verifyRemoteStoreIndexSettings(
- indexSettings,
- null,
- null,
- null,
- ReplicationType.DOCUMENT.toString(),
- IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
+ 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
+ )
+ )
  );
  }
 
@@ -213,7 +209,7 @@ public void testRemoteStoreSegmentRepoWithoutRemoteEnabledAndSegmentReplicationI
  );
  }
 
- public void testRemoteStoreEnabledByUserWithRemoteRepo() throws Exception {
+ public void testRemoteStoreEnabledByUserWithRemoteRepoIllegalArgumentException() throws Exception {
  Settings settings = Settings.builder()
  .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
  .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
@@ -222,19 +218,20 @@ public void testRemoteStoreEnabledByUserWithRemoteRepo() throws Exception {
  .put(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, "my-custom-repo")
  .build();
 
- assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
- GetIndexResponse getIndexResponse = client().admin()
- .indices()
- .getIndex(new GetIndexRequest().indices("test-idx-1").includeDefaults(true))
- .get();
- Settings indexSettings = getIndexResponse.settings().get("test-idx-1");
- verifyRemoteStoreIndexSettings(
- indexSettings,
- "true",
- "my-custom-repo",
- "my-translog-repo-1",
- ReplicationType.SEGMENT.toString(),
- IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
+ IllegalArgumentException exc = expectThrows(
+ IllegalArgumentException.class,
+ () -> client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get()
+ );
+ assertThat(
+ exc.getMessage(),
+ containsString(
+ String.format(
+ Locale.ROOT,
+ "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
+ )
+ )
  );
  }
 
@@ -270,41 +267,21 @@ public void testRemoteStoreOverrideTranslogRepoCorrectly() throws Exception {
  .put(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, "my-custom-repo")
  .put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, "my-custom-repo")
  .build();
- assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
- GetIndexResponse getIndexResponse = client().admin()
- .indices()
- .getIndex(new GetIndexRequest().indices("test-idx-1").includeDefaults(true))
- .get();
- Settings indexSettings = getIndexResponse.settings().get("test-idx-1");
- verifyRemoteStoreIndexSettings(
- indexSettings,
- "true",
- "my-custom-repo",
- "my-custom-repo",
- ReplicationType.SEGMENT.toString(),
- IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
+ IllegalArgumentException exc = expectThrows(
+ IllegalArgumentException.class,
+ () -> client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get()
  );
- }
-
- 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();
- assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
- GetIndexResponse getIndexResponse = client().admin()
- .indices()
- .getIndex(new GetIndexRequest().indices("test-idx-1").includeDefaults(true))
- .get();
- Settings indexSettings = getIndexResponse.settings().get("test-idx-1");
- verifyRemoteStoreIndexSettings(
- indexSettings,
- null,
- null,
- null,
- ReplicationType.DOCUMENT.toString(),
- IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
+ assertThat(
+ exc.getMessage(),
+ containsString(
+ String.format(
+ Locale.ROOT,
+ "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
+ )
+ )
  );
  }
 

diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/PrimaryTermValidationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/PrimaryTermValidationIT.java
@@ -61,6 +61,7 @@ public void testPrimaryTermValidation() throws Exception {
  .put(FollowersChecker.FOLLOWER_CHECK_TIMEOUT_SETTING.getKey(), "1s")
  .put(FollowersChecker.FOLLOWER_CHECK_INTERVAL_SETTING.getKey(), "1s")
  .put(FollowersChecker.FOLLOWER_CHECK_RETRY_COUNT_SETTING.getKey(), 1)
+ .put(remoteStoreClusterSettings(REPOSITORY_NAME, REPOSITORY_2_NAME, true))
  .build();
  internalCluster().startClusterManagerOnlyNode(clusterSettings);