Register UnmodifiableOnRestore settings (#17121)

* Register UnmodifiableOnRestore settings Signed-off-by: AnnTian Shao <[email protected]> * fixes to tests Signed-off-by: AnnTian Shao <[email protected]> * fix typo Signed-off-by: AnnTian Shao <[email protected]> * fix javadoc Signed-off-by: AnnTian Shao <[email protected]> --------- Signed-off-by: AnnTian Shao <[email protected]> Co-authored-by: AnnTian Shao <[email protected]>
opensearch-project · Jan 30, 2025 · d601c37 · d601c37
1 parent 2847695
commit d601c37
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 38 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
@@ -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));

diff --git a/...rc/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java b/...rc/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
@@ -588,6 +588,15 @@ 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<Long> SETTING_INDEX_CREATION_DATE = Setting.longSetting(
+        SETTING_CREATION_DATE,
+        -1,
+        -1,
+        Property.IndexScope,
+        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)
@@ -614,6 +623,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<String> INDEX_UUID_SETTING = Setting.simpleString(
+        SETTING_INDEX_UUID,
+        INDEX_UUID_NA_VALUE,
+        Property.IndexScope,
+        Property.PrivateIndex,
+        Property.UnmodifiableOnRestore
+    );
+
+    public static final Setting<String> 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
@@ -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,
@@ -316,9 +319,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/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<String> 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
@@ -607,8 +607,11 @@ public void testTranslogGenerationSizeThreshold() {
         assertEquals(actual, settings.getGenerationThresholdSize());
     }
 
+    /**
+     * Test private setting validation for private settings defined in {@link IndexScopedSettings#isPrivateSetting(String)}
+     */
     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 +620,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 +629,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
@@ -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<Long> INDEX_CREATION_DATE_SETTING = Setting.longSetting(
-        IndexMetadata.SETTING_CREATION_DATE,
-        -1,
-        -1,
-        Property.IndexScope,
-        Property.NodeScope
-    );
     public static final Setting<TimeValue> 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<Setting<?>> getSettings() {
         return Arrays.asList(
             MERGE_ENABLED,
-            INDEX_CREATION_DATE_SETTING,
             PROVIDED_NAME_SETTING,
             TRANSLOG_RETENTION_CHECK_INTERVAL_SETTING,
             RemoteConnectionStrategy.REMOTE_MAX_PENDING_CONNECTION_LISTENERS,