From 49fc11ae0b7422f881e28f9d72792d6f2e9cfb40 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Thu, 23 Jan 2025 11:32:35 -0800 Subject: [PATCH 1/4] Register UnmodifiableOnRestore settings Signed-off-by: AnnTian Shao --- .../opensearch/client/IndicesClientIT.java | 54 +++++++++++++++++++ .../admin/indices/create/CreateIndexIT.java | 22 ++++---- .../cluster/metadata/IndexMetadata.java | 26 +++++++++ .../common/settings/IndexScopedSettings.java | 3 ++ .../opensearch/snapshots/RestoreService.java | 11 +--- .../opensearch/index/IndexSettingsTests.java | 6 +-- .../test/InternalSettingsPlugin.java | 9 ---- 7 files changed, 96 insertions(+), 35 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java index 0399e4667d85d..d2ccccf1e1a1f 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java @@ -134,6 +134,7 @@ import java.util.Map; import java.util.stream.Collectors; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; import static org.opensearch.common.xcontent.support.XContentMapValues.extractRawValues; @@ -256,6 +257,26 @@ public void testCreateIndex() throws IOException { } } + public void testCreateIndexFailPrivateSetting() throws IOException { + { + // Create index with private setting + String indexName = "private_index"; + assertFalse(indexExists(indexName)); + + CreateIndexRequest createIndexRequest = new CreateIndexRequest(indexName); + + Settings.Builder settings = Settings.builder(); + settings.put(SETTING_CREATION_DATE, -1); + createIndexRequest.settings(settings); + + OpenSearchStatusException exception = expectThrows( + OpenSearchStatusException.class, + () -> execute(createIndexRequest, highLevelClient().indices()::create, highLevelClient().indices()::createAsync) + ); + assertTrue(exception.getMessage().contains("private index setting [index.creation_date] can not be set explicitly")); + } + } + public void testGetSettings() throws IOException { String indexName = "get_settings_index"; Settings basicSettings = Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).build(); @@ -281,6 +302,24 @@ public void testGetSettings() throws IOException { assertEquals("30s", updatedResponse.getSetting(indexName, "index.refresh_interval")); } + public void testGetPrivateSettings() throws IOException { + String indexName = "get_settings_index"; + Settings basicSettings = Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).build(); + + createIndex(indexName, basicSettings); + + GetSettingsRequest getSettingsRequest = new GetSettingsRequest().indices(indexName); + GetSettingsResponse getSettingsResponse = execute( + getSettingsRequest, + highLevelClient().indices()::getSettings, + highLevelClient().indices()::getSettingsAsync + ); + + assertNull(getSettingsResponse.getSetting(indexName, "index.refresh_interval")); + assertNotNull(getSettingsResponse.getSetting(indexName, "index.creation_date")); + assertNotNull(getSettingsResponse.getSetting(indexName, "index.uuid")); + } + public void testGetSettingsNonExistentIndex() throws IOException { String nonExistentIndex = "index_that_doesnt_exist"; assertFalse(indexExists(nonExistentIndex)); @@ -1366,6 +1405,9 @@ public void testIndexPutSettings() throws IOException { final String unmodifiableSettingKey = IndexMetadata.SETTING_NUMBER_OF_SHARDS; final int unmodifiableSettingValue = 3; + final String privateSettingKey = SETTING_CREATION_DATE; + final int privateSettingValue = -1; + String index = "index"; createIndex(index, Settings.EMPTY); @@ -1425,6 +1467,18 @@ public void testIndexPutSettings() throws IOException { + "reason=Can't update non dynamic settings [[index.number_of_shards]] for open indices [[index/" ) ); + + UpdateSettingsRequest privateSettingRequest = new UpdateSettingsRequest(index); + privateSettingRequest.settings(Settings.builder().put(privateSettingKey, privateSettingValue).build()); + exception = expectThrows( + OpenSearchException.class, + () -> execute(privateSettingRequest, highLevelClient().indices()::putSettings, highLevelClient().indices()::putSettingsAsync) + ); + assertThat( + exception.getMessage(), + containsString("can not update private setting [index.creation_date]; this setting is managed by OpenSearch") + ); + closeIndex(index); exception = expectThrows( OpenSearchException.class, diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java index d713c9cc86841..b41b3f07d3ac1 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java @@ -90,19 +90,6 @@ @ClusterScope(scope = Scope.TEST) public class CreateIndexIT extends OpenSearchIntegTestCase { - public void testCreationDateGivenFails() { - try { - prepareCreate("test").setSettings(Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 4L)).get(); - fail(); - } catch (SettingsException ex) { - assertEquals( - "unknown setting [index.creation_date] please check that any required plugins are installed, or check the " - + "breaking changes documentation for removed settings", - ex.getMessage() - ); - } - } - public void testCreationDateGenerated() { long timeBeforeRequest = System.currentTimeMillis(); prepareCreate("test").get(); @@ -224,6 +211,15 @@ public void testUnknownSettingFails() { } } + public void testPrivateSettingFails() { + try { + prepareCreate("test").setSettings(Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, -1).build()).get(); + fail("should have thrown an exception about private settings"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("private index setting [index.creation_date] can not be set explicitly")); + } + } + public void testInvalidShardCountSettingsWithoutPrefix() throws Exception { int value = randomIntBetween(-10, 0); try { 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 cee331788e4b7..88f13fc04eb47 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -576,6 +576,16 @@ public static APIBlock readFrom(StreamInput input) throws IOException { public static final String SETTING_VERSION_UPGRADED_STRING = "index.version.upgraded_string"; public static final String SETTING_CREATION_DATE = "index.creation_date"; + public static final Setting SETTING_INDEX_CREATION_DATE = Setting.longSetting( + SETTING_CREATION_DATE, + -1, + -1, + Property.IndexScope, + Property.NodeScope, + Property.PrivateIndex, + Property.UnmodifiableOnRestore + ); + /** * 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) @@ -602,6 +612,22 @@ public static APIBlock readFrom(StreamInput input) throws IOException { public static final String INDEX_UUID_NA_VALUE = Strings.UNKNOWN_UUID_VALUE; + public static final Setting INDEX_UUID_SETTING = Setting.simpleString( + SETTING_INDEX_UUID, + INDEX_UUID_NA_VALUE, + Property.IndexScope, + Property.PrivateIndex, + Property.UnmodifiableOnRestore + ); + + public static final Setting SETTING_INDEX_HISTORY_UUID = Setting.simpleString( + SETTING_HISTORY_UUID, + INDEX_UUID_NA_VALUE, + Property.IndexScope, + Property.PrivateIndex, + Property.UnmodifiableOnRestore + ); + public static final String INDEX_ROUTING_REQUIRE_GROUP_PREFIX = "index.routing.allocation.require"; public static final String INDEX_ROUTING_INCLUDE_GROUP_PREFIX = "index.routing.allocation.include"; public static final String INDEX_ROUTING_EXCLUDE_GROUP_PREFIX = "index.routing.allocation.exclude"; diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java index 8d56a942c5d6e..86a7112a777dc 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -91,6 +91,9 @@ public final class IndexScopedSettings extends AbstractScopedSettings { MergeSchedulerConfig.MAX_MERGE_COUNT_SETTING, MergeSchedulerConfig.MAX_THREAD_COUNT_SETTING, IndexMetadata.SETTING_INDEX_VERSION_CREATED, + IndexMetadata.SETTING_INDEX_CREATION_DATE, + IndexMetadata.INDEX_UUID_SETTING, + IndexMetadata.SETTING_INDEX_HISTORY_UUID, IndexMetadata.INDEX_ROUTING_EXCLUDE_GROUP_SETTING, IndexMetadata.INDEX_ROUTING_INCLUDE_GROUP_SETTING, IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_SETTING, diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 29ced9d5f0f0c..89403b15f6aca 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -118,9 +118,7 @@ import static java.util.Collections.unmodifiableSet; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS; -import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_HISTORY_UUID; -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_REMOTE_SEGMENT_STORE_REPOSITORY; @@ -162,14 +160,7 @@ public class RestoreService implements ClusterStateApplier { private static final Logger logger = LogManager.getLogger(RestoreService.class); private static final Set USER_UNMODIFIABLE_SETTINGS = unmodifiableSet( - newHashSet( - SETTING_INDEX_UUID, - SETTING_CREATION_DATE, - SETTING_HISTORY_UUID, - SETTING_REMOTE_STORE_ENABLED, - SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, - SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY - ) + newHashSet(SETTING_REMOTE_STORE_ENABLED, SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY) ); // It's OK to change some settings, but we shouldn't allow simply removing them diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java index 474ec73d5fe61..cbc9d8856a27e 100644 --- a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java @@ -608,7 +608,7 @@ public void testTranslogGenerationSizeThreshold() { } public void testPrivateSettingsValidation() { - final Settings settings = Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, System.currentTimeMillis()).build(); + final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_UPGRADED, Version.V_EMPTY).build(); final IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); { @@ -617,7 +617,7 @@ public void testPrivateSettingsValidation() { SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean()) ); - assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); + assertThat(e, hasToString(containsString("unknown setting [index.version.upgraded]"))); } { @@ -626,7 +626,7 @@ public void testPrivateSettingsValidation() { SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean(), false, randomBoolean()) ); - assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); + assertThat(e, hasToString(containsString("unknown setting [index.version.upgraded]"))); } // nothing should happen since we are ignoring private settings diff --git a/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java b/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java index 986cfd9c5b613..96919f65f88fc 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalSettingsPlugin.java @@ -31,7 +31,6 @@ package org.opensearch.test; -import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.unit.TimeValue; @@ -59,13 +58,6 @@ public final class InternalSettingsPlugin extends Plugin { Property.IndexScope, Property.NodeScope ); - public static final Setting INDEX_CREATION_DATE_SETTING = Setting.longSetting( - IndexMetadata.SETTING_CREATION_DATE, - -1, - -1, - Property.IndexScope, - Property.NodeScope - ); public static final Setting TRANSLOG_RETENTION_CHECK_INTERVAL_SETTING = Setting.timeSetting( "index.translog.retention.check_interval", new TimeValue(10, TimeUnit.MINUTES), @@ -78,7 +70,6 @@ public final class InternalSettingsPlugin extends Plugin { public List> getSettings() { return Arrays.asList( MERGE_ENABLED, - INDEX_CREATION_DATE_SETTING, PROVIDED_NAME_SETTING, TRANSLOG_RETENTION_CHECK_INTERVAL_SETTING, RemoteConnectionStrategy.REMOTE_MAX_PENDING_CONNECTION_LISTENERS, From d2b413982d95a737da80a482029e670c69e21f7c Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Wed, 29 Jan 2025 17:06:18 -0800 Subject: [PATCH 2/4] fixes to tests Signed-off-by: AnnTian Shao --- .../java/org/opensearch/client/IndicesClientIT.java | 11 ----------- .../opensearch/cluster/metadata/IndexMetadata.java | 1 - .../common/settings/IndexScopedSettings.java | 3 --- .../java/org/opensearch/index/IndexSettingsTests.java | 4 ++++ 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java index d2ccccf1e1a1f..82f5c3ea3ce18 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java @@ -1468,17 +1468,6 @@ public void testIndexPutSettings() throws IOException { ) ); - UpdateSettingsRequest privateSettingRequest = new UpdateSettingsRequest(index); - privateSettingRequest.settings(Settings.builder().put(privateSettingKey, privateSettingValue).build()); - exception = expectThrows( - OpenSearchException.class, - () -> execute(privateSettingRequest, highLevelClient().indices()::putSettings, highLevelClient().indices()::putSettingsAsync) - ); - assertThat( - exception.getMessage(), - containsString("can not update private setting [index.creation_date]; this setting is managed by OpenSearch") - ); - closeIndex(index); exception = expectThrows( OpenSearchException.class, 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 88f13fc04eb47..68392ffe6a21e 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -581,7 +581,6 @@ public static APIBlock readFrom(StreamInput input) throws IOException { -1, -1, Property.IndexScope, - Property.NodeScope, Property.PrivateIndex, Property.UnmodifiableOnRestore ); diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java index 86a7112a777dc..f6aadb8bdfccc 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -313,9 +313,6 @@ protected void validateSettingKey(Setting setting) { @Override public boolean isPrivateSetting(String key) { switch (key) { - case IndexMetadata.SETTING_CREATION_DATE: - case IndexMetadata.SETTING_INDEX_UUID: - case IndexMetadata.SETTING_HISTORY_UUID: case IndexMetadata.SETTING_VERSION_UPGRADED: case IndexMetadata.SETTING_INDEX_PROVIDED_NAME: case MergePolicyProvider.INDEX_MERGE_ENABLED: diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java index cbc9d8856a27e..8c56929c7ab2c 100644 --- a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java @@ -607,6 +607,10 @@ public void testTranslogGenerationSizeThreshold() { assertEquals(actual, settings.getGenerationThresholdSize()); } + /** + * Test private setting validation for private settings defined in isPrivateSetting() + * https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java + */ public void testPrivateSettingsValidation() { final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_UPGRADED, Version.V_EMPTY).build(); final IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); From 8576e431cc66ee111d2b7b7e8829d789ff037648 Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Thu, 30 Jan 2025 09:19:13 -0800 Subject: [PATCH 3/4] fix typo Signed-off-by: AnnTian Shao --- .../src/test/java/org/opensearch/client/IndicesClientIT.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java index 82f5c3ea3ce18..75022820c7fb8 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java @@ -1405,9 +1405,6 @@ public void testIndexPutSettings() throws IOException { final String unmodifiableSettingKey = IndexMetadata.SETTING_NUMBER_OF_SHARDS; final int unmodifiableSettingValue = 3; - final String privateSettingKey = SETTING_CREATION_DATE; - final int privateSettingValue = -1; - String index = "index"; createIndex(index, Settings.EMPTY); @@ -1467,7 +1464,6 @@ public void testIndexPutSettings() throws IOException { + "reason=Can't update non dynamic settings [[index.number_of_shards]] for open indices [[index/" ) ); - closeIndex(index); exception = expectThrows( OpenSearchException.class, From 970368ee1c3b128f1d1fc6cdb2b38dabbb1dccad Mon Sep 17 00:00:00 2001 From: AnnTian Shao Date: Thu, 30 Jan 2025 10:14:34 -0800 Subject: [PATCH 4/4] fix javadoc Signed-off-by: AnnTian Shao --- .../src/test/java/org/opensearch/index/IndexSettingsTests.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java index 8c56929c7ab2c..bc505daa607c1 100644 --- a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java @@ -608,8 +608,7 @@ public void testTranslogGenerationSizeThreshold() { } /** - * Test private setting validation for private settings defined in isPrivateSetting() - * https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java + * Test private setting validation for private settings defined in {@link IndexScopedSettings#isPrivateSetting(String)} */ public void testPrivateSettingsValidation() { final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_UPGRADED, Version.V_EMPTY).build();