Add notion of internal index settings (#31286)

We have some use cases for an index setting to only be manageable by
dedicated APIs rather than be updateable via the update settings
API. This commit adds the notion of an internal index setting. Such
settings can be set on create index requests, they can not be changed
via the update settings API, yet they can be changed by action on behalf
of or triggered by the user via dedicated APIs.

server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java

@@ -82,8 +82,10 @@ public class MetaDataUpdateSettingsService extends AbstractComponent {
         Settings.Builder settingsForOpenIndices = Settings.builder();
         final Set<String> skippedSettings = new HashSet<>();
 
-        indexScopedSettings.validate(normalizedSettings.filter(s -> Regex.isSimpleMatchPattern(s) == false  /* don't validate wildcards */),
+        indexScopedSettings.validate(
-            false); //don't validate dependencies here we check it below never allow to change the number of shards
+                normalizedSettings.filter(s -> Regex.isSimpleMatchPattern(s) == false), // don't validate wildcards
+                false, // don't validate dependencies here we check it below never allow to change the number of shards
+                true); // validate internal index settings
         for (String key : normalizedSettings.keySet()) {
             Setting setting = indexScopedSettings.get(key);
             boolean isWildcard = setting == null && Regex.isSimpleMatchPattern(key);

server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

@@ -282,6 +282,18 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
         validate(settings, validateDependencies, false, false);
     }
 
+    /**
+     * Validates that all settings are registered and valid.
+     *
+     * @param settings              the settings to validate
+     * @param validateDependencies  true if dependent settings should be validated
+     * @param validateInternalIndex true if internal index settings should be validated
+     * @see Setting#getSettingsDependencies(String)
+     */
+    public final void validate(final Settings settings, final boolean validateDependencies, final boolean validateInternalIndex) {
+        validate(settings, validateDependencies, false, false, validateInternalIndex);
+    }
+
     /**
      * Validates that all settings are registered and valid.
      *
@@ -296,6 +308,25 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
             final boolean validateDependencies,
             final boolean ignorePrivateSettings,
             final boolean ignoreArchivedSettings) {
+        validate(settings, validateDependencies, ignorePrivateSettings, ignoreArchivedSettings, false);
+    }
+
+    /**
+     * Validates that all settings are registered and valid.
+     *
+     * @param settings               the settings
+     * @param validateDependencies   true if dependent settings should be validated
+     * @param ignorePrivateSettings  true if private settings should be ignored during validation
+     * @param ignoreArchivedSettings true if archived settings should be ignored during validation
+     * @param validateInternalIndex  true if index internal settings should be validated
+     * @see Setting#getSettingsDependencies(String)
+     */
+    public final void validate(
+            final Settings settings,
+            final boolean validateDependencies,
+            final boolean ignorePrivateSettings,
+            final boolean ignoreArchivedSettings,
+            final boolean validateInternalIndex) {
         final List<RuntimeException> exceptions = new ArrayList<>();
         for (final String key : settings.keySet()) { // settings iterate in deterministic fashion
             if (isPrivateSetting(key) && ignorePrivateSettings) {
@@ -305,7 +336,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
                 continue;
             }
             try {
-                validate(key, settings, validateDependencies);
+                validate(key, settings, validateDependencies, validateInternalIndex);
             } catch (final RuntimeException ex) {
                 exceptions.add(ex);
             }
@@ -314,9 +345,27 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
     }
 
     /**
-     * Validates that the setting is valid
+     * Validates that the settings is valid.
+     *
+     * @param key the key of the setting to validate
+     * @param settings the settings
+     * @param validateDependencies true if dependent settings should be validated
+     * @throws IllegalArgumentException if the setting is invalid
      */
-    void validate(String key, Settings settings, boolean validateDependencies) {
+    void validate(final String key, final Settings settings, final boolean validateDependencies) {
+        validate(key, settings, validateDependencies, false);
+    }
+
+    /**
+     * Validates that the settings is valid.
+     *
+     * @param key the key of the setting to validate
+     * @param settings the settings
+     * @param validateDependencies true if dependent settings should be validated
+     * @param validateInternalIndex true if internal index settings should be validated
+     * @throws IllegalArgumentException if the setting is invalid
+     */
+    void validate(final String key, final Settings settings, final boolean validateDependencies, final boolean validateInternalIndex) {
         Setting setting = getRaw(key);
         if (setting == null) {
             LevensteinDistance ld = new LevensteinDistance();
@@ -356,6 +405,11 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
                     }
                 }
             }
+            // the only time that validateInternalIndex should be true is if this call is coming via the update settings API
+            if (validateInternalIndex && setting.getProperties().contains(Setting.Property.InternalIndex)) {
+                throw new IllegalArgumentException(
+                        "can not update internal setting [" + setting.getKey() + "]; this setting is managed via a dedicated API");
+            }
         }
         setting.get(settings);
     }

server/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -120,7 +120,13 @@ public class Setting<T> implements ToXContentObject {
          * Mark this setting as not copyable during an index resize (shrink or split). This property can only be applied to settings that
          * also have {@link Property#IndexScope}.
          */
-        NotCopyableOnResize
+        NotCopyableOnResize,
+
+        /**
+         * Indicates an index-level setting that is managed internally. Such a setting can only be added to an index on index creation but
+         * can not be updated via the update API.
+         */
+        InternalIndex
     }
 
     private final Key key;
@@ -152,14 +158,18 @@ public class Setting<T> implements ToXContentObject {
             if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.Final)) {
                 throw new IllegalArgumentException("final setting [" + key + "] cannot be dynamic");
             }
-            if (propertiesAsSet.contains(Property.NotCopyableOnResize) && propertiesAsSet.contains(Property.IndexScope) == false) {
+            checkPropertyRequiresIndexScope(propertiesAsSet, Property.NotCopyableOnResize);
-                throw new IllegalArgumentException(
+            checkPropertyRequiresIndexScope(propertiesAsSet, Property.InternalIndex);
-                        "non-index-scoped setting [" + key + "] can not have property [" + Property.NotCopyableOnResize + "]");
-            }
             this.properties = propertiesAsSet;
         }
     }
 
+    private void checkPropertyRequiresIndexScope(final EnumSet<Property> properties, final Property property) {
+        if (properties.contains(property) && properties.contains(Property.IndexScope) == false) {
+            throw new IllegalArgumentException("non-index-scoped setting [" + key + "] can not have property [" + property + "]");
+        }
+    }
+
     /**
      * Creates a new Setting instance
      * @param key the settings key for this setting.

server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

@@ -876,4 +876,28 @@ public class ScopedSettingsTests extends ESTestCase {
                 Settings.builder().put(currentSettings), Settings.builder(), "node"));
         assertThat(exc.getMessage(), containsString("final node setting [some.final.group.foo]"));
     }
+
+    public void testInternalIndexSettingsFailsValidation() {
+        final Setting<String> indexInternalSetting = Setting.simpleString("index.internal", Property.InternalIndex, Property.IndexScope);
+        final IndexScopedSettings indexScopedSettings =
+                new IndexScopedSettings(Settings.EMPTY, Collections.singleton(indexInternalSetting));
+        final IllegalArgumentException e = expectThrows(
+                IllegalArgumentException.class,
+                () -> {
+                    final Settings settings = Settings.builder().put("index.internal", "internal").build();
+                    indexScopedSettings.validate(settings, false, /* validateInternalIndex */ true);
+                });
+        final String message = "can not update internal setting [index.internal]; this setting is managed via a dedicated API";
+        assertThat(e, hasToString(containsString(message)));
+    }
+
+    public void testInternalIndexSettingsSkipValidation() {
+        final Setting<String> internalIndexSetting = Setting.simpleString("index.internal", Property.InternalIndex, Property.IndexScope);
+        final IndexScopedSettings indexScopedSettings = 
+                new IndexScopedSettings(Settings.EMPTY, Collections.singleton(internalIndexSetting));
+        // nothing should happen, validation should not throw an exception
+        final Settings settings = Settings.builder().put("index.internal", "internal").build();
+        indexScopedSettings.validate(settings, false, /* validateInternalIndex */ false);
+    }
+
 }

server/src/test/java/org/elasticsearch/common/settings/SettingTests.java

@@ -735,6 +735,13 @@ public class SettingTests extends ESTestCase {
         assertThat(e, hasToString(containsString("non-index-scoped setting [foo.bar] can not have property [NotCopyableOnResize]")));
     }
 
+    public void testRejectNonIndexScopedIndexInternalSetting() {
+        final IllegalArgumentException e = expectThrows(
+                IllegalArgumentException.class,
+                () -> Setting.simpleString("foo.bar", Property.InternalIndex));
+        assertThat(e, hasToString(containsString("non-index-scoped setting [foo.bar] can not have property [InternalIndex]")));
+    }
+
     public void testTimeValue() {
         final TimeValue random = TimeValue.parseTimeValue(randomTimeValue(), "test");
 

server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java

@@ -19,19 +19,40 @@
 
 package org.elasticsearch.indices.settings;
 
+import org.elasticsearch.action.Action;
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.ActionRequest;
+import org.elasticsearch.action.ActionRequestValidationException;
+import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
 import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse;
+import org.elasticsearch.action.support.ActionFilters;
+import org.elasticsearch.action.support.master.MasterNodeRequest;
+import org.elasticsearch.action.support.master.TransportMasterNodeAction;
+import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.cluster.ClusterStateUpdateTask;
+import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
+import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
+import org.elasticsearch.cluster.metadata.MetaData;
+import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Priority;
+import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.IndexModule;
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.engine.VersionConflictEngineException;
 import org.elasticsearch.indices.IndicesService;
+import org.elasticsearch.plugins.ActionPlugin;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.threadpool.ThreadPool;
+import org.elasticsearch.transport.TransportService;
 
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -46,6 +67,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBloc
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.nullValue;
 
 public class UpdateSettingsIT extends ESIntegTestCase {
@@ -79,7 +101,12 @@ public class UpdateSettingsIT extends ESIntegTestCase {
 
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
-        return Arrays.asList(DummySettingPlugin.class, FinalSettingPlugin.class);
+        return Arrays.asList(DummySettingPlugin.class, FinalSettingPlugin.class, InternalIndexSettingsPlugin.class);
+    }
+
+    @Override
+    protected Collection<Class<? extends Plugin>> transportClientPlugins() {
+        return Collections.singletonList(InternalIndexSettingsPlugin.class);
     }
 
     public static class DummySettingPlugin extends Plugin {
@@ -124,6 +151,151 @@ public class UpdateSettingsIT extends ESIntegTestCase {
         }
     }
 
+    public static class InternalIndexSettingsPlugin extends Plugin implements ActionPlugin {
+
+        public static final Setting<String> INDEX_INTERNAL_SETTING =
+                Setting.simpleString("index.internal", Setting.Property.IndexScope, Setting.Property.InternalIndex);
+
+        @Override
+        public List<Setting<?>> getSettings() {
+            return Collections.singletonList(INDEX_INTERNAL_SETTING);
+        }
+
+        public static class UpdateInternalIndexAction
+                extends Action<UpdateInternalIndexAction.Request, UpdateInternalIndexAction.Response> {
+
+            private static final UpdateInternalIndexAction INSTANCE = new UpdateInternalIndexAction();
+            private static final String NAME = "indices:admin/settings/update-internal-index";
+
+            public UpdateInternalIndexAction() {
+                super(NAME);
+            }
+
+            static class Request extends MasterNodeRequest<Request> {
+
+                private String index;
+                private String value;
+
+                Request() {
+
+                }
+
+                Request(final String index, final String value) {
+                    this.index = index;
+                    this.value = value;
+                }
+
+                @Override
+                public ActionRequestValidationException validate() {
+                    return null;
+                }
+
+                @Override
+                public void readFrom(final StreamInput in) throws IOException {
+                    super.readFrom(in);
+                    index = in.readString();
+                    value = in.readString();
+                }
+
+                @Override
+                public void writeTo(final StreamOutput out) throws IOException {
+                    super.writeTo(out);
+                    out.writeString(index);
+                    out.writeString(value);
+                }
+
+            }
+
+            static class Response extends ActionResponse {
+
+            }
+
+            @Override
+            public Response newResponse() {
+                return new Response();
+            }
+
+        }
+
+        public static class TransportUpdateInternalIndexAction
+                extends TransportMasterNodeAction<UpdateInternalIndexAction.Request, UpdateInternalIndexAction.Response> {
+
+            @Inject
+            public TransportUpdateInternalIndexAction(
+                    final Settings settings,
+                    final TransportService transportService,
+                    final ClusterService clusterService,
+                    final ThreadPool threadPool,
+                    final ActionFilters actionFilters,
+                    final IndexNameExpressionResolver indexNameExpressionResolver) {
+                super(
+                        settings,
+                        UpdateInternalIndexAction.NAME,
+                        transportService,
+                        clusterService,
+                        threadPool,
+                        actionFilters,
+                        indexNameExpressionResolver,
+                        UpdateInternalIndexAction.Request::new);
+            }
+
+            @Override
+            protected String executor() {
+                return ThreadPool.Names.SAME;
+            }
+
+            @Override
+            protected UpdateInternalIndexAction.Response newResponse() {
+                return new UpdateInternalIndexAction.Response();
+            }
+
+            @Override
+            protected void masterOperation(
+                    final UpdateInternalIndexAction.Request request,
+                    final ClusterState state,
+                    final ActionListener<UpdateInternalIndexAction.Response> listener) throws Exception {
+                clusterService.submitStateUpdateTask("update-index-internal", new ClusterStateUpdateTask() {
+                    @Override
+                    public ClusterState execute(final ClusterState currentState) throws Exception {
+                        final MetaData.Builder builder = MetaData.builder(currentState.metaData());
+                        final IndexMetaData.Builder imdBuilder = IndexMetaData.builder(currentState.metaData().index(request.index));
+                        final Settings.Builder settingsBuilder =
+                                Settings.builder()
+                                        .put(currentState.metaData().index(request.index).getSettings())
+                                        .put("index.internal", request.value);
+                        imdBuilder.settings(settingsBuilder);
+                        builder.put(imdBuilder.build(), true);
+                        return ClusterState.builder(currentState).metaData(builder).build();
+                    }
+
+                    @Override
+                    public void clusterStateProcessed(final String source, final ClusterState oldState, final ClusterState newState) {
+                        listener.onResponse(new UpdateInternalIndexAction.Response());
+                    }
+
+                    @Override
+                    public void onFailure(final String source, final Exception e) {
+                        listener.onFailure(e);
+                    }
+
+                });
+            }
+
+            @Override
+            protected ClusterBlockException checkBlock(UpdateInternalIndexAction.Request request, ClusterState state) {
+                return null;
+            }
+
+        }
+
+        @Override
+        public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
+            return Collections.singletonList(
+                    new ActionHandler<>(UpdateInternalIndexAction.INSTANCE, TransportUpdateInternalIndexAction.class));
+        }
+
+    }
+
     public void testUpdateDependentClusterSettings() {
         IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () ->
             client().admin().cluster().prepareUpdateSettings().setPersistentSettings(Settings.builder()
@@ -474,4 +646,35 @@ public class UpdateSettingsIT extends ESIntegTestCase {
         }
     }
 
+    public void testUpdateInternalIndexSettingViaSettingsAPI() {
+        final Settings settings = Settings.builder().put("index.internal", "internal").build();
+        createIndex("test", settings);
+        final GetSettingsResponse response = client().admin().indices().prepareGetSettings("test").get();
+        assertThat(response.getSetting("test", "index.internal"), equalTo("internal"));
+        // we can not update the setting via the update settings API
+        final IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> client().admin()
+                        .indices()
+                        .prepareUpdateSettings("test")
+                        .setSettings(Settings.builder().put("index.internal", "internal-update"))
+                        .get());
+        final String message = "can not update internal setting [index.internal]; this setting is managed via a dedicated API";
+        assertThat(e, hasToString(containsString(message)));
+        final GetSettingsResponse responseAfterAttemptedUpdate = client().admin().indices().prepareGetSettings("test").get();
+        assertThat(responseAfterAttemptedUpdate.getSetting("test", "index.internal"), equalTo("internal"));
+    }
+
+    public void testUpdateInternalIndexSettingViaDedicatedAPI() {
+        final Settings settings = Settings.builder().put("index.internal", "internal").build();
+        createIndex("test", settings);
+        final GetSettingsResponse response = client().admin().indices().prepareGetSettings("test").get();
+        assertThat(response.getSetting("test", "index.internal"), equalTo("internal"));
+        client().execute(
+                InternalIndexSettingsPlugin.UpdateInternalIndexAction.INSTANCE,
+                new InternalIndexSettingsPlugin.UpdateInternalIndexAction.Request("test", "internal-update"))
+                .actionGet();
+        final GetSettingsResponse responseAfterUpdate = client().admin().indices().prepareGetSettings("test").get();
+        assertThat(responseAfterUpdate.getSetting("test", "index.internal"), equalTo("internal-update"));
+    }
+
 }