From ee8bd11d0665330b137c2411875e302f08213128 Mon Sep 17 00:00:00 2001 From: Tommy Shao <69884021+anntians@users.noreply.github.com> Date: Fri, 24 Jan 2025 11:03:57 -0800 Subject: [PATCH] Added new Setting property `UnmodifiableOnRestore` to prevent updating settings on restore snapshot (#16957) * Add index.knn setting to list of unmodifiable settings when restore snapshot Signed-off-by: AnnTian Shao * Add index.knn setting to list of unmodifiable settings when restore snapshot Signed-off-by: AnnTian Shao * Add new Setting property UnmodifiableOnRestore to mark setting as immutable on restore snapshot Signed-off-by: AnnTian Shao * Add tests for new Setting property UnmodifiableOnRestore Signed-off-by: AnnTian Shao * fixes and added more tests for new setting property UnmodifiableOnRestore Signed-off-by: AnnTian Shao * fix test failures Signed-off-by: AnnTian Shao * Revert "fix test failures" This reverts commit 252100cb1174316198044e93647c544aac1e4394. Signed-off-by: AnnTian Shao * fixes by adding back USER_UNMODIFIABLE_SETTINGS for settings without Setting implementation Signed-off-by: AnnTian Shao * testing CI config for registering plugin settings Signed-off-by: AnnTian Shao * Revert "testing CI config for registering plugin settings" This reverts commit 9ebab5a7ac9fa3a15b436168f227556bf18ace87. Signed-off-by: AnnTian Shao * Add UnmodifiableOnRestore only to unmodifiable settings that are already registered, will raise separate PR for settings not yet registered. Add validation check in Setting.java. Add UnmodifiableOnRestore settings cannot be removed on restore Signed-off-by: AnnTian Shao * fixes and move tests to RestoreSnapshotIT Signed-off-by: AnnTian Shao * Add back testInvalidRestoreRequestScenarios test method Signed-off-by: AnnTian Shao --------- Signed-off-by: AnnTian Shao Signed-off-by: Tommy Shao <69884021+anntians@users.noreply.github.com> Co-authored-by: AnnTian Shao Signed-off-by: Eugene Tolbakov --- CHANGELOG.md | 1 + .../remotestore/RemoteRestoreSnapshotIT.java | 149 ++++++- .../RestoreShallowSnapshotV2IT.java | 149 ++++++- .../snapshots/RestoreSnapshotIT.java | 388 ++++++++++++++++++ .../cluster/metadata/IndexMetadata.java | 15 +- .../metadata/MetadataCreateIndexService.java | 4 + .../settings/AbstractScopedSettings.java | 8 + .../opensearch/common/settings/Setting.java | 15 +- .../opensearch/snapshots/RestoreService.java | 22 +- .../common/settings/ScopedSettingsTests.java | 24 ++ .../common/settings/SettingTests.java | 16 + 11 files changed, 766 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43e822f01ffb2..104c30b453a80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Added a new `time` field to replace the deprecated `getTime` field in `GetStats`. ([#17009](https://github.com/opensearch-project/OpenSearch/pull/17009)) - Improve flat_object field parsing performance by reducing two passes to a single pass ([#16297](https://github.com/opensearch-project/OpenSearch/pull/16297)) - Improve performance of the bitmap filtering([#16936](https://github.com/opensearch-project/OpenSearch/pull/16936/)) +- Added new Setting property UnmodifiableOnRestore to prevent updating settings on restore snapshot ([#16957](https://github.com/opensearch-project/OpenSearch/pull/16957)) - Introduce Template query ([#16818](https://github.com/opensearch-project/OpenSearch/pull/16818)) - Propagate the sourceIncludes and excludes fields from fetchSourceContext to FieldsVisitor. ([#17080](https://github.com/opensearch-project/OpenSearch/pull/17080)) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index 70e283791fc3e..3b96636cfe771 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -72,7 +72,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS; import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA; @@ -494,6 +496,51 @@ public void testRemoteRestoreIndexRestoredFromSnapshot() throws IOException, Exe assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1); } + public void testSuccessfulIndexRestoredFromSnapshotWithUpdatedSetting() throws IOException, ExecutionException, InterruptedException { + internalCluster().startClusterManagerOnlyNode(); + internalCluster().startDataOnlyNodes(2); + + String indexName1 = "testindex1"; + String snapshotRepoName = "test-restore-snapshot-repo"; + String snapshotName1 = "test-restore-snapshot1"; + Path absolutePath1 = randomRepoPath().toAbsolutePath(); + logger.info("Snapshot Path [{}]", absolutePath1); + + createRepository(snapshotRepoName, "fs", getRepositorySettings(absolutePath1, true)); + + Settings indexSettings = getIndexSettings(1, 0).build(); + createIndex(indexName1, indexSettings); + + final int numDocsInIndex1 = randomIntBetween(20, 30); + indexDocuments(client(), indexName1, numDocsInIndex1); + flushAndRefresh(indexName1); + ensureGreen(indexName1); + + logger.info("--> snapshot"); + SnapshotInfo snapshotInfo1 = createSnapshot(snapshotRepoName, snapshotName1, new ArrayList<>(Arrays.asList(indexName1))); + assertThat(snapshotInfo1.successfulShards(), greaterThan(0)); + assertThat(snapshotInfo1.successfulShards(), equalTo(snapshotInfo1.totalShards())); + assertThat(snapshotInfo1.state(), equalTo(SnapshotState.SUCCESS)); + + assertAcked(client().admin().indices().delete(new DeleteIndexRequest(indexName1)).get()); + assertFalse(indexExists(indexName1)); + + // try index restore with index.number_of_replicas setting modified. index.number_of_replicas can be modified on restore + Settings numberOfReplicasSettings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1).build(); + + RestoreSnapshotResponse restoreSnapshotResponse1 = client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepoName, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(numberOfReplicasSettings) + .setIndices(indexName1) + .get(); + + assertEquals(restoreSnapshotResponse1.status(), RestStatus.ACCEPTED); + ensureGreen(indexName1); + assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1); + } + protected IndexShard getIndexShard(String node, String indexName) { final Index index = resolveIndex(indexName); IndicesService indicesService = internalCluster().getInstance(IndicesService.class, node); @@ -706,7 +753,7 @@ public void testInvalidRestoreRequestScenarios() throws Exception { indexDocuments(client, index, numDocsInIndex, numDocsInIndex + randomIntBetween(2, 5)); ensureGreen(index); - // try index restore with remote store disabled + // try index restore with index.remote_store.enabled ignored SnapshotRestoreException exception = expectThrows( SnapshotRestoreException.class, () -> client().admin() @@ -721,26 +768,37 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.enabled] on restore")); - // try index restore with remote store repository modified - Settings remoteStoreIndexSettings = Settings.builder() - .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo) - .build(); + // try index restore with index.remote_store.segment.repository ignored + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIgnoreIndexSettings(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.segment.repository] on restore")); + // try index restore with index.remote_store.translog.repository ignored exception = expectThrows( SnapshotRestoreException.class, () -> client().admin() .cluster() .prepareRestoreSnapshot(snapshotRepo, snapshotName1) .setWaitForCompletion(false) - .setIndexSettings(remoteStoreIndexSettings) + .setIgnoreIndexSettings(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY) .setIndices(index) .setRenamePattern(index) .setRenameReplacement(restoredIndex) .get() ); - assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); + assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.translog.repository] on restore")); - // try index restore with remote store repository and translog store repository disabled + // try index restore with index.remote_store.segment.repository and index.remote_store.translog.repository ignored exception = expectThrows( SnapshotRestoreException.class, () -> client().admin() @@ -757,6 +815,81 @@ public void testInvalidRestoreRequestScenarios() throws Exception { .get() ); assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.segment.repository]" + " on restore")); + + // try index restore with index.remote_store.enabled modified + Settings remoteStoreIndexSettings = Settings.builder().put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false).build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(remoteStoreIndexSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.enabled]" + " on restore")); + + // try index restore with index.remote_store.segment.repository modified + Settings remoteStoreSegmentIndexSettings = Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(remoteStoreSegmentIndexSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); + + // try index restore with index.remote_store.translog.repository modified + Settings remoteStoreTranslogIndexSettings = Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, newRemoteStoreRepo) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(remoteStoreTranslogIndexSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.translog.repository]" + " on restore")); + + // try index restore with index.remote_store.translog.repository and index.remote_store.segment.repository modified + Settings multipleRemoteStoreIndexSettings = Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo) + .put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, newRemoteStoreRepo) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(multipleRemoteStoreIndexSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); } public void testCreateSnapshotV2_Orphan_Timestamp_Cleanup() throws Exception { diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java index 1493a1b259e13..19953b147bdf9 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java @@ -67,7 +67,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; import static org.opensearch.index.query.QueryBuilders.matchAllQuery; import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS; import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG; @@ -561,6 +563,51 @@ public void testRemoteRestoreIndexRestoredFromSnapshot() throws IOException, Exe assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1); } + public void testSuccessfulIndexRestoredFromSnapshotWithUpdatedSetting() throws IOException, ExecutionException, InterruptedException { + internalCluster().startClusterManagerOnlyNode(); + internalCluster().startDataOnlyNodes(2); + + String indexName1 = "testindex1"; + String snapshotRepoName = "test-restore-snapshot-repo"; + String snapshotName1 = "test-restore-snapshot1"; + Path absolutePath1 = randomRepoPath().toAbsolutePath(); + logger.info("Snapshot Path [{}]", absolutePath1); + + createRepository(snapshotRepoName, "fs", getRepositorySettings(absolutePath1, true)); + + Settings indexSettings = getIndexSettings(1, 0).build(); + createIndex(indexName1, indexSettings); + + final int numDocsInIndex1 = randomIntBetween(20, 30); + indexDocuments(client(), indexName1, numDocsInIndex1); + flushAndRefresh(indexName1); + ensureGreen(indexName1); + + logger.info("--> snapshot"); + SnapshotInfo snapshotInfo1 = createSnapshot(snapshotRepoName, snapshotName1, new ArrayList<>(Arrays.asList(indexName1))); + assertThat(snapshotInfo1.successfulShards(), greaterThan(0)); + assertThat(snapshotInfo1.successfulShards(), equalTo(snapshotInfo1.totalShards())); + assertThat(snapshotInfo1.state(), equalTo(SnapshotState.SUCCESS)); + + assertAcked(client().admin().indices().delete(new DeleteIndexRequest(indexName1)).get()); + assertFalse(indexExists(indexName1)); + + // try index restore with index.number_of_replicas setting modified. index.number_of_replicas can be modified on restore + Settings numberOfReplicasSettings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1).build(); + + RestoreSnapshotResponse restoreSnapshotResponse1 = client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepoName, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(numberOfReplicasSettings) + .setIndices(indexName1) + .get(); + + assertEquals(restoreSnapshotResponse1.status(), RestStatus.ACCEPTED); + ensureGreen(indexName1); + assertDocsPresentInIndex(client(), indexName1, numDocsInIndex1); + } + private IndexShard getIndexShard(String node, String indexName) { final Index index = resolveIndex(indexName); IndicesService indicesService = internalCluster().getInstance(IndicesService.class, node); @@ -773,7 +820,7 @@ public void testInvalidRestoreRequestScenarios() throws Exception { indexDocuments(client, index, numDocsInIndex, numDocsInIndex + randomIntBetween(2, 5)); ensureGreen(index); - // try index restore with remote store disabled + // try index restore with index.remote_store.enabled ignored SnapshotRestoreException exception = expectThrows( SnapshotRestoreException.class, () -> client().admin() @@ -788,26 +835,37 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.enabled] on restore")); - // try index restore with remote store repository modified - Settings remoteStoreIndexSettings = Settings.builder() - .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo) - .build(); + // try index restore with index.remote_store.segment.repository ignored + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIgnoreIndexSettings(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.segment.repository] on restore")); + // try index restore with index.remote_store.translog.repository ignored exception = expectThrows( SnapshotRestoreException.class, () -> client().admin() .cluster() .prepareRestoreSnapshot(snapshotRepo, snapshotName1) .setWaitForCompletion(false) - .setIndexSettings(remoteStoreIndexSettings) + .setIgnoreIndexSettings(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY) .setIndices(index) .setRenamePattern(index) .setRenameReplacement(restoredIndex) .get() ); - assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); + assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.translog.repository] on restore")); - // try index restore with remote store repository and translog store repository disabled + // try index restore with index.remote_store.segment.repository and index.remote_store.translog.repository ignored exception = expectThrows( SnapshotRestoreException.class, () -> client().admin() @@ -824,6 +882,81 @@ public void testInvalidRestoreRequestScenarios() throws Exception { .get() ); assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.segment.repository]" + " on restore")); + + // try index restore with index.remote_store.enabled modified + Settings remoteStoreIndexSettings = Settings.builder().put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false).build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(remoteStoreIndexSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.enabled]" + " on restore")); + + // try index restore with index.remote_store.segment.repository modified + Settings remoteStoreSegmentIndexSettings = Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(remoteStoreSegmentIndexSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); + + // try index restore with index.remote_store.translog.repository modified + Settings remoteStoreTranslogIndexSettings = Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, newRemoteStoreRepo) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(remoteStoreTranslogIndexSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.translog.repository]" + " on restore")); + + // try index restore with index.remote_store.translog.repository and index.remote_store.segment.repository modified + Settings multipleRemoteStoreIndexSettings = Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, newRemoteStoreRepo) + .put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, newRemoteStoreRepo) + .build(); + + exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(multipleRemoteStoreIndexSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.segment.repository]" + " on restore")); } public void testRestoreOperationsUsingDifferentRepos() throws Exception { diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java index e76587653e99a..36ab97d0b730f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/RestoreSnapshotIT.java @@ -32,6 +32,7 @@ package org.opensearch.snapshots; +import org.opensearch.Version; import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; @@ -49,10 +50,12 @@ import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.rest.RestStatus; +import org.opensearch.index.IndexSettings; import org.opensearch.indices.InvalidIndexNameException; import org.opensearch.repositories.RepositoriesService; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -1112,4 +1115,389 @@ public void testRestoreBalancedReplica() { } } + private String index; + private String snapshotRepo; + private String snapshotName1; + private String snapshotName2; + private Path absolutePath1; + private String restoredIndex; + private Settings indexSettings; + private SnapshotInfo snapshotInfo; + private SnapshotInfo snapshotInfo2; + + public void setupSnapshotRestore() { + index = "test-index"; + snapshotRepo = "test-restore-snapshot-repo"; + snapshotName1 = "test-restore-snapshot1"; + snapshotName2 = "test-restore-snapshot2"; + absolutePath1 = randomRepoPath().toAbsolutePath(); + + logger.info("Snapshot Path [{}]", absolutePath1); + restoredIndex = index + "-restored"; + + createRepository(snapshotRepo, "fs", getRepositorySettings(absolutePath1, true)); + + indexSettings = getIndexSettings(1, 0).build(); + createIndex(index, indexSettings); + ensureGreen(index); + + logger.info("--> snapshot"); + + snapshotInfo = createSnapshot(snapshotRepo, snapshotName1, new ArrayList<>(List.of(index))); + assertThat(snapshotInfo.state(), equalTo(SnapshotState.SUCCESS)); + assertThat(snapshotInfo.successfulShards(), greaterThan(0)); + assertThat(snapshotInfo.successfulShards(), equalTo(snapshotInfo.totalShards())); + + updateRepository(snapshotRepo, "fs", getRepositorySettings(absolutePath1, false)); + snapshotInfo2 = createSnapshot(snapshotRepo, snapshotName2, new ArrayList<>(List.of(index))); + assertThat(snapshotInfo2.state(), equalTo(SnapshotState.SUCCESS)); + assertThat(snapshotInfo2.successfulShards(), greaterThan(0)); + assertThat(snapshotInfo2.successfulShards(), equalTo(snapshotInfo2.totalShards())); + ensureGreen(index); + } + + public void testInvalidRestoreRequest_UserUnRemovableSettingsIgnored() throws Exception { + setupSnapshotRestore(); + + // try index restore with USER_UNREMOVABLE_SETTINGS setting ignored + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIgnoreIndexSettings(IndexMetadata.SETTING_REMOTE_STORE_ENABLED) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.enabled] on restore")); + + } + + public void testInvalidRestoreRequest_UnmodifiableOnRestoreIgnored() throws Exception { + setupSnapshotRestore(); + + // try index restore with UnmodifiableOnRestore setting ignored + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIgnoreIndexSettings(IndexMetadata.SETTING_NUMBER_OF_SHARDS) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot remove UnmodifiableOnRestore setting [index.number_of_shards] on restore")); + + } + + public void testInvalidRestoreRequest_MixRemovableAndUnmodifiableOnRestoreIgnored() throws Exception { + setupSnapshotRestore(); + + // try index restore with mix of removable and UnmodifiableOnRestore settings ignored + // index.version.created is UnmodifiableOnRestore, index.number_of_search_only_replicas is removable + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIgnoreIndexSettings(IndexMetadata.SETTING_VERSION_CREATED, IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot remove UnmodifiableOnRestore setting [index.version.created] on restore")); + } + + public void testInvalidRestoreRequest_MixRemovableAndUserUnRemovableSettingsIgnored() throws Exception { + setupSnapshotRestore(); + + // try index restore with mix of removable and USER_UNREMOVABLE_SETTINGS settings ignored + // index.number_of_replicas is USER_UNREMOVABLE_SETTINGS, index.number_of_search_only_replicas is removable + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIgnoreIndexSettings(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot remove setting [index.number_of_replicas] on restore")); + + } + + public void testInvalidRestoreRequest_MixUnmodifiableOnRestoreAndUserUnRemovableSettingsIgnored() throws Exception { + setupSnapshotRestore(); + + // try index restore with mix of UnmodifiableOnRestore and USER_UNREMOVABLE_SETTINGS settings ignored + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIgnoreIndexSettings(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, IndexMetadata.SETTING_NUMBER_OF_SHARDS) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot remove setting [index.number_of_replicas]" + " on restore")); + + } + + public void testInvalidRestoreRequest_MultipleUnmodifiableOnRestoreIgnored() throws Exception { + setupSnapshotRestore(); + + // try index restore with multiple UnmodifiableOnRestore settings ignored + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIgnoreIndexSettings(IndexMetadata.SETTING_NUMBER_OF_SHARDS, IndexMetadata.SETTING_VERSION_CREATED) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot remove UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); + + } + + public void testInvalidRestoreRequest_MultipleUserUnRemovableSettingsIgnored() throws Exception { + setupSnapshotRestore(); + + // try index restore with multiple USER_UNREMOVABLE_SETTINGS settings ignored + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIgnoreIndexSettings(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot remove setting [index.number_of_replicas]" + " on restore")); + + } + + public void testInvalidRestoreRequest_UnmodifiableOnRestoreModified() throws Exception { + setupSnapshotRestore(); + + // try index restore with UnmodifiableOnRestore setting modified + Settings numberOfShardsSettingsDiff = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3).build(); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(numberOfShardsSettingsDiff) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); + + } + + public void testInvalidRestoreRequest_UnmodifiableOnRestoreUnchanged() throws Exception { + setupSnapshotRestore(); + + // try index restore with UnmodifiableOnRestore setting unchanged + Settings numberOfShardsSettingsSame = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).build(); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(numberOfShardsSettingsSame) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); + + } + + public void testInvalidRestoreRequest_UserUnmodifiableSettingsModified() throws Exception { + setupSnapshotRestore(); + + // try index restore with USER_UNMODIFIABLE_SETTINGS setting modified + Settings remoteStoreEnabledSetting = Settings.builder().put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false).build(); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(remoteStoreEnabledSetting) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.enabled]" + " on restore")); + + } + + public void testInvalidRestoreRequest_MixModifiableAndUnmodifiableOnRestoreModified() throws Exception { + setupSnapshotRestore(); + + // try index restore with mix of modifiable and UnmodifiableOnRestore settings modified + // index.version.created is UnmodifiableOnRestore, index.number_of_search_only_replicas is modifiable + Settings mixedSettingsUnmodifiableOnRestore = Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_EMPTY) + .put(IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS, 1) + .build(); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(mixedSettingsUnmodifiableOnRestore) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.version.created]" + " on restore")); + + } + + public void testInvalidRestoreRequest_MixModifiableAndUserUnmodifiableSettingsModified() throws Exception { + setupSnapshotRestore(); + + // try index restore with mix of modifiable and USER_UNMODIFIABLE_SETTINGS settings modified + // index.remote_store.enabled is USER_UNMODIFIABLE_SETTINGS, index.number_of_search_only_replicas is modifiable + Settings mixedSettingsUserUnmodifiableSettings = Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false) + .put(IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS, 1) + .build(); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(mixedSettingsUserUnmodifiableSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.enabled]" + " on restore")); + + } + + public void testInvalidRestoreRequest_MixUnmodifiableOnRestoreAndUserUnmodifiableSettingsModified() throws Exception { + setupSnapshotRestore(); + + // try index restore with mix of UnmodifiableOnRestore and USER_UNMODIFIABLE_SETTINGS settings modified + // index.remote_store.enabled is USER_UNMODIFIABLE_SETTINGS, index.version.created is UnmodifiableOnRestore + Settings mixedSettings = Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false) + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_EMPTY) + .build(); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(mixedSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.enabled]" + " on restore")); + + } + + public void testInvalidRestoreRequest_MultipleUnmodifiableOnRestoreModified() throws Exception { + setupSnapshotRestore(); + + // try index restore with multiple UnmodifiableOnRestore settings modified + Settings unmodifiableOnRestoreSettings = Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_EMPTY) + .build(); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(unmodifiableOnRestoreSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify UnmodifiableOnRestore setting [index.number_of_shards]" + " on restore")); + + } + + public void testInvalidRestoreRequest_MultipleUserUnmodifiableSettingsModified() throws Exception { + setupSnapshotRestore(); + + // try index restore with multiple USER_UNMODIFIABLE_SETTINGS settings modified + Settings userUnmodifiableSettings = Settings.builder() + .put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false) + .put(IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS, 1) + .build(); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> client().admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndexSettings(userUnmodifiableSettings) + .setIndices(index) + .setRenamePattern(index) + .setRenameReplacement(restoredIndex) + .get() + ); + assertTrue(exception.getMessage().contains("cannot modify setting [index.remote_store.enabled]" + " on restore")); + + } + + protected Settings.Builder getIndexSettings(int numOfShards, int numOfReplicas) { + Settings.Builder settingsBuilder = Settings.builder() + .put(super.indexSettings()) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numOfShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numOfReplicas) + .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "300s"); + return settingsBuilder; + } + } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index f70282986ad4e..cee331788e4b7 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -230,7 +230,15 @@ static Setting buildNumberOfShardsSetting() { + "]" ); } - return Setting.intSetting(SETTING_NUMBER_OF_SHARDS, defaultNumShards, 1, maxNumShards, Property.IndexScope, Property.Final); + return Setting.intSetting( + SETTING_NUMBER_OF_SHARDS, + defaultNumShards, + 1, + maxNumShards, + Property.IndexScope, + Property.Final, + Property.UnmodifiableOnRestore + ); } public static final String INDEX_SETTING_PREFIX = "index."; @@ -559,13 +567,15 @@ public static APIBlock readFrom(StreamInput input) throws IOException { SETTING_VERSION_CREATED, Version.V_EMPTY, Property.IndexScope, - Property.PrivateIndex + Property.PrivateIndex, + Property.UnmodifiableOnRestore ); public static final String SETTING_VERSION_CREATED_STRING = "index.version.created_string"; public static final String SETTING_VERSION_UPGRADED = "index.version.upgraded"; public static final String SETTING_VERSION_UPGRADED_STRING = "index.version.upgraded_string"; public static final String SETTING_CREATION_DATE = "index.creation_date"; + /** * The user provided name for an index. This is the plain string provided by the user when the index was created. * It might still contain date math expressions etc. (added in 5.0) @@ -589,6 +599,7 @@ public static APIBlock readFrom(StreamInput input) throws IOException { Function.identity(), Property.IndexScope ); + public static final String INDEX_UUID_NA_VALUE = Strings.UNKNOWN_UUID_VALUE; public static final String INDEX_ROUTING_REQUIRE_GROUP_PREFIX = "index.routing.allocation.require"; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index b5b2b71f977fa..f052e9940bb9a 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -231,6 +231,10 @@ public MetadataCreateIndexService( : null; } + public IndexScopedSettings getIndexScopedSettings() { + return indexScopedSettings; + } + /** * Add a provider to be invoked to get additional index settings prior to an index being created */ diff --git a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java index 7655135b06d6c..8c10623e48fe4 100644 --- a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java @@ -759,6 +759,14 @@ public boolean isFinalSetting(String key) { return setting != null && setting.isFinal(); } + /** + * Returns true if the setting for the given key is unmodifiableOnRestore. Otherwise false. + */ + public boolean isUnmodifiableOnRestoreSetting(String key) { + final Setting setting = get(key); + return setting != null && setting.isUnmodifiableOnRestore(); + } + /** * Returns a settings object that contains all settings that are not * already set in the given source. The diff contains either the default value for each diff --git a/server/src/main/java/org/opensearch/common/settings/Setting.java b/server/src/main/java/org/opensearch/common/settings/Setting.java index 081029c1c106c..eb63522270e87 100644 --- a/server/src/main/java/org/opensearch/common/settings/Setting.java +++ b/server/src/main/java/org/opensearch/common/settings/Setting.java @@ -171,7 +171,13 @@ public enum Property { /** * Extension scope */ - ExtensionScope + ExtensionScope, + + /** + * Mark this setting as immutable on snapshot restore + * i.e. the setting will not be allowed to be removed or modified during restore + */ + UnmodifiableOnRestore } private final Key key; @@ -208,10 +214,13 @@ private Setting( final EnumSet propertiesAsSet = EnumSet.copyOf(Arrays.asList(properties)); if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.Final)) { throw new IllegalArgumentException("final setting [" + key + "] cannot be dynamic"); + } else if (propertiesAsSet.contains(Property.UnmodifiableOnRestore) && propertiesAsSet.contains(Property.Dynamic)) { + throw new IllegalArgumentException("UnmodifiableOnRestore setting [" + key + "] cannot be dynamic"); } checkPropertyRequiresIndexScope(propertiesAsSet, Property.NotCopyableOnResize); checkPropertyRequiresIndexScope(propertiesAsSet, Property.InternalIndex); checkPropertyRequiresIndexScope(propertiesAsSet, Property.PrivateIndex); + checkPropertyRequiresIndexScope(propertiesAsSet, Property.UnmodifiableOnRestore); checkPropertyRequiresNodeScope(propertiesAsSet, Property.Consistent); this.properties = propertiesAsSet; } @@ -348,6 +357,10 @@ public final boolean isFinal() { return properties.contains(Property.Final); } + public final boolean isUnmodifiableOnRestore() { + return properties.contains(Property.UnmodifiableOnRestore); + } + public final boolean isInternalIndex() { return properties.contains(Property.InternalIndex); } diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 4b5bd951f80a0..29ced9d5f0f0c 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -77,6 +77,7 @@ import org.opensearch.common.lucene.Lucene; import org.opensearch.common.regex.Regex; import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.ArrayUtils; @@ -122,12 +123,10 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS; -import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; 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.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_UPGRADED; import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY; import static org.opensearch.common.util.IndexUtils.filterIndices; @@ -164,8 +163,6 @@ public class RestoreService implements ClusterStateApplier { private static final Set USER_UNMODIFIABLE_SETTINGS = unmodifiableSet( newHashSet( - SETTING_NUMBER_OF_SHARDS, - SETTING_VERSION_CREATED, SETTING_INDEX_UUID, SETTING_CREATION_DATE, SETTING_HISTORY_UUID, @@ -180,7 +177,7 @@ public class RestoreService implements ClusterStateApplier { private static final String REMOTE_STORE_INDEX_SETTINGS_REGEX = "index.remote_store.*"; static { - Set unremovable = new HashSet<>(USER_UNMODIFIABLE_SETTINGS.size() + 4); + Set unremovable = new HashSet<>(USER_UNMODIFIABLE_SETTINGS.size() + 3); unremovable.addAll(USER_UNMODIFIABLE_SETTINGS); unremovable.add(SETTING_NUMBER_OF_REPLICAS); unremovable.add(SETTING_AUTO_EXPAND_REPLICAS); @@ -202,6 +199,8 @@ public class RestoreService implements ClusterStateApplier { private final ClusterSettings clusterSettings; + private final IndexScopedSettings indexScopedSettings; + private final IndicesService indicesService; private final Supplier clusterInfoSupplier; @@ -234,6 +233,7 @@ public RestoreService( this.clusterSettings = clusterService.getClusterSettings(); this.shardLimitValidator = shardLimitValidator; this.indicesService = indicesService; + this.indexScopedSettings = createIndexService.getIndexScopedSettings(); this.clusterInfoSupplier = clusterInfoSupplier; this.dataToFileCacheSizeRatioSupplier = dataToFileCacheSizeRatioSupplier; @@ -835,6 +835,11 @@ private IndexMetadata updateIndexSettings( snapshot, "cannot remove setting [" + ignoredSetting + "] on restore" ); + } else if (indexScopedSettings.isUnmodifiableOnRestoreSetting(ignoredSetting)) { + throw new SnapshotRestoreException( + snapshot, + "cannot remove UnmodifiableOnRestore setting [" + ignoredSetting + "] on restore" + ); } else { keyFilters.add(ignoredSetting); } @@ -853,7 +858,7 @@ private IndexMetadata updateIndexSettings( } Predicate settingsFilter = k -> { - if (USER_UNREMOVABLE_SETTINGS.contains(k) == false) { + if (USER_UNREMOVABLE_SETTINGS.contains(k) == false && !indexScopedSettings.isUnmodifiableOnRestoreSetting(k)) { for (String filterKey : keyFilters) { if (k.equals(filterKey)) { return false; @@ -872,6 +877,11 @@ private IndexMetadata updateIndexSettings( .put(normalizedChangeSettings.filter(k -> { if (USER_UNMODIFIABLE_SETTINGS.contains(k)) { throw new SnapshotRestoreException(snapshot, "cannot modify setting [" + k + "] on restore"); + } else if (indexScopedSettings.isUnmodifiableOnRestoreSetting(k)) { + throw new SnapshotRestoreException( + snapshot, + "cannot modify UnmodifiableOnRestore setting [" + k + "] on restore" + ); } else { return true; } diff --git a/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java b/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java index 7780481c9deff..55e3cfa34040b 100644 --- a/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java @@ -789,6 +789,30 @@ public void testIsFinal() { assertTrue(settings.isFinalSetting("foo.group.key")); } + public void testIsUnmodifiableOnRestore() { + ClusterSettings settings = new ClusterSettings( + Settings.EMPTY, + new HashSet<>( + Arrays.asList( + Setting.intSetting("foo.int", 1, Property.UnmodifiableOnRestore, Property.IndexScope, Property.NodeScope), + Setting.groupSetting("foo.group.", Property.UnmodifiableOnRestore, Property.IndexScope, Property.NodeScope), + Setting.groupSetting("foo.list.", Property.UnmodifiableOnRestore, Property.IndexScope, Property.NodeScope), + Setting.intSetting("foo.int.baz", 1, Property.IndexScope, Property.NodeScope) + ) + ) + ); + + assertFalse(settings.isUnmodifiableOnRestoreSetting("foo.int.baz")); + assertTrue(settings.isUnmodifiableOnRestoreSetting("foo.int")); + + assertFalse(settings.isUnmodifiableOnRestoreSetting("foo.list")); + assertTrue(settings.isUnmodifiableOnRestoreSetting("foo.list.0.key")); + assertTrue(settings.isUnmodifiableOnRestoreSetting("foo.list.key")); + + assertFalse(settings.isUnmodifiableOnRestoreSetting("foo.group")); + assertTrue(settings.isUnmodifiableOnRestoreSetting("foo.group.key")); + } + public void testDiff() throws IOException { Setting fooBarBaz = Setting.intSetting("foo.bar.baz", 1, Property.NodeScope); Setting fooBar = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope); diff --git a/server/src/test/java/org/opensearch/common/settings/SettingTests.java b/server/src/test/java/org/opensearch/common/settings/SettingTests.java index c3c399a9d88b2..a0788b0c83e11 100644 --- a/server/src/test/java/org/opensearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/opensearch/common/settings/SettingTests.java @@ -1439,6 +1439,22 @@ public void testRejectConflictingDynamicAndFinalProperties() { assertThat(ex.getMessage(), containsString("final setting [foo.bar] cannot be dynamic")); } + public void testRejectConflictingDynamicAndUnmodifiableOnRestoreProperties() { + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> Setting.simpleString("foo.bar", Property.UnmodifiableOnRestore, Property.Dynamic) + ); + assertThat(ex.getMessage(), containsString("UnmodifiableOnRestore setting [foo.bar] cannot be dynamic")); + } + + public void testRejectNonIndexScopedUnmodifiableOnRestoreSetting() { + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> Setting.simpleString("foo.bar", Property.UnmodifiableOnRestore) + ); + assertThat(e, hasToString(containsString("non-index-scoped setting [foo.bar] can not have property [UnmodifiableOnRestore]"))); + } + public void testRejectNonIndexScopedNotCopyableOnResizeSetting() { final IllegalArgumentException e = expectThrows( IllegalArgumentException.class,