Skip to content

Commit

Permalink
Retrict remote only indices in a remote enabled cluster
Browse files Browse the repository at this point in the history 
Signed-off-by: bansvaru <[email protected]>
  • Loading branch information
@linuxpi
linuxpi committed Jul 21, 2023
1 parent c47ec1b commit 65c2168
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 173 deletions.
6 changes: 5 additions & 1 deletion ...nalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexClusterDefaultDocRep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@
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;

Expand Down Expand Up @@ -44,7 +48,7 @@ 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))
);
}

Expand Down
114 changes: 38 additions & 76 deletions server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,8 @@
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_REMOTE_SEGMENT_STORE_REPOSITORY;
import static org.opensearch.cluster.metadata.IndexMetadata.*;
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;
Expand Down Expand Up @@ -111,19 +108,14 @@ 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("Cannot override settings related to remote store.")
);
}

Expand Down Expand Up @@ -158,13 +150,7 @@ public void testRemoteStoreEnabledByUserWithoutRemoteRepoIllegalArgumentExceptio
);
assertThat(
exc.getMessage(),
containsString(
String.format(
Locale.ROOT,
"Setting %s should be provided with non-empty repository ID",
SETTING_REMOTE_SEGMENT_STORE_REPOSITORY
)
)
containsString("Cannot override settings related to remote store.")
);
}

Expand All @@ -174,19 +160,13 @@ 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))
);
}

Expand Down Expand Up @@ -222,19 +202,13 @@ 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("Cannot override settings related to remote store.")
);
}

Expand Down Expand Up @@ -270,19 +244,13 @@ 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()
);
assertThat(
exc.getMessage(),
containsString("Cannot override settings related to remote store.")
);
}

Expand All @@ -292,19 +260,13 @@ public void testRemoteStoreOverrideReplicationTypeIndexSettings() throws Excepti
.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))
);
}

Expand Down
39 changes: 7 additions & 32 deletions server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -954,44 +954,19 @@ private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder,
if (CLUSTER_REMOTE_STORE_ENABLED_SETTING.get(clusterSettings)) {
// Verify if we can create a remote store based index based on user provided settings
if (canCreateRemoteStoreIndex(requestSettings) == false) {
return;
throw new IllegalArgumentException("Cannot override settings related to remote store.");
}

// Verify index has replication type as SEGMENT
if (ReplicationType.DOCUMENT.equals(ReplicationType.parseString(settingsBuilder.get(SETTING_REPLICATION_TYPE)))) {
throw new IllegalArgumentException(
"Cannot enable ["
+ SETTING_REMOTE_STORE_ENABLED
+ "] when ["
+ SETTING_REPLICATION_TYPE
+ "] is "
+ ReplicationType.DOCUMENT
);
}

settingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, true);
String remoteStoreRepo;
if (Objects.equals(requestSettings.get(INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()), "true")) {
remoteStoreRepo = requestSettings.get(INDEX_REMOTE_STORE_REPOSITORY_SETTING.getKey());
} else {
remoteStoreRepo = CLUSTER_REMOTE_STORE_REPOSITORY_SETTING.get(clusterSettings);
}
settingsBuilder.put(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, remoteStoreRepo)
.put(
SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY,
requestSettings.get(
INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING.getKey(),
CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING.get(clusterSettings)
)
);
settingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, CLUSTER_REMOTE_STORE_REPOSITORY_SETTING.get(clusterSettings))
.put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING.get(clusterSettings));
}
}

private static boolean canCreateRemoteStoreIndex(Settings requestSettings) {
return (INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings) == false
|| INDEX_REPLICATION_TYPE_SETTING.get(requestSettings).equals(ReplicationType.SEGMENT))
&& (INDEX_REMOTE_STORE_ENABLED_SETTING.exists(requestSettings) == false
|| INDEX_REMOTE_STORE_ENABLED_SETTING.get(requestSettings));
return INDEX_REMOTE_STORE_ENABLED_SETTING.exists(requestSettings) == false
&& INDEX_REMOTE_STORE_REPOSITORY_SETTING.exists(requestSettings) == false
&& INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING.exists(requestSettings) == false;
}

public static void validateStoreTypeSettings(Settings settings) {
Expand Down
124 changes: 60 additions & 64 deletions server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1296,23 +1296,22 @@ public void testRemoteStoreDisabledByUserIndexSettings() {
final Settings.Builder requestSettings = Settings.builder();
requestSettings.put(SETTING_REMOTE_STORE_ENABLED, false);
request.settings(requestSettings.build());
Settings indexSettings = aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet()
IllegalArgumentException exc = expectThrows(
IllegalArgumentException.class,
() -> aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet()
)
);
verifyRemoteStoreIndexSettings(
indexSettings,
"false",
null,
null,
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
assertThat(
exc.getMessage(),
containsString("Cannot override settings related to remote store.")
);
}

Expand All @@ -1331,23 +1330,22 @@ public void testRemoteStoreOverrideSegmentRepoIndexSettings() {
.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, "my-custom-repo");
request.settings(requestSettings.build());
Settings indexSettings = aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet()
IllegalArgumentException exc = expectThrows(
IllegalArgumentException.class,
() -> aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet()
)
);
verifyRemoteStoreIndexSettings(
indexSettings,
"true",
"my-custom-repo",
"my-translog-repo-1",
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
assertThat(
exc.getMessage(),
containsString("Cannot override settings related to remote store.")
);
}

Expand All @@ -1364,23 +1362,22 @@ public void testRemoteStoreOverrideTranslogRepoIndexSettings() {
final Settings.Builder requestSettings = Settings.builder();
requestSettings.put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, "my-custom-repo");
request.settings(requestSettings.build());
Settings indexSettings = aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet()
IllegalArgumentException exc = expectThrows(
IllegalArgumentException.class,
() -> aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet()
)
);
verifyRemoteStoreIndexSettings(
indexSettings,
"true",
"my-segment-repo-1",
"my-custom-repo",
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
assertThat(
exc.getMessage(),
containsString("Cannot override settings related to remote store.")
);
}

Expand All @@ -1397,23 +1394,22 @@ public void testRemoteStoreOverrideReplicationTypeIndexSettings() {
final Settings.Builder requestSettings = Settings.builder();
requestSettings.put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT);
request.settings(requestSettings.build());
Settings indexSettings = aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet()
IllegalArgumentException exc = expectThrows(
IllegalArgumentException.class,
() -> aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet()
)
);
verifyRemoteStoreIndexSettings(
indexSettings,
null,
null,
null,
ReplicationType.DOCUMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
assertThat(
exc.getMessage(),
containsString("Cannot override settings related to remote store.")
);
}

Expand Down

0 comments on commit 65c2168

Please sign in to comment.