From 4f81d895f7f5214b8d335782ce7a16159ea4385d Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 12 Jun 2018 20:11:58 -0400 Subject: [PATCH 1/2] Add notion of internal index settings 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. --- .../MetaDataUpdateSettingsService.java | 6 +- .../settings/AbstractScopedSettings.java | 60 ++++- .../common/settings/Setting.java | 20 +- .../common/settings/ScopedSettingsTests.java | 24 ++ .../common/settings/SettingTests.java | 7 + .../indices/settings/UpdateSettingsIT.java | 205 +++++++++++++++++- 6 files changed, 311 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java index ce5ad12a53d6a..26e3b842a3743 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java @@ -82,8 +82,10 @@ public void updateSettings(final UpdateSettingsClusterStateUpdateRequest request Settings.Builder settingsForOpenIndices = Settings.builder(); final Set skippedSettings = new HashSet<>(); - indexScopedSettings.validate(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 + indexScopedSettings.validate( + 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 index internal settings for (String key : normalizedSettings.keySet()) { Setting setting = indexScopedSettings.get(key); boolean isWildcard = setting == null && Regex.isSimpleMatchPattern(key); diff --git a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index e8bb946c8a795..eb4e294642417 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -282,6 +282,18 @@ public final void validate(final Settings settings, final boolean validateDepend 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 final void validate( 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 exceptions = new ArrayList<>(); for (final String key : settings.keySet()) { // settings iterate in deterministic fashion if (isPrivateSetting(key) && ignorePrivateSettings) { @@ -305,7 +336,7 @@ public final void validate( continue; } try { - validate(key, settings, validateDependencies); + validate(key, settings, validateDependencies, validateInternalIndex); } catch (final RuntimeException ex) { exceptions.add(ex); } @@ -314,9 +345,27 @@ public final void validate( } /** - * 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 @@ void validate(String key, Settings settings, boolean validateDependencies) { } } } + // 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); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index f45f4bda9c9fe..7f3906ff5a251 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -120,7 +120,13 @@ public enum Property { * 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 @@ private Setting(Key key, @Nullable Setting fallbackSetting, Function 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. diff --git a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java index f00768651f917..2376d5663402e 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -876,4 +876,28 @@ public void testFinalSettingUpdateFail() { Settings.builder().put(currentSettings), Settings.builder(), "node")); assertThat(exc.getMessage(), containsString("final node setting [some.final.group.foo]")); } + + public void testInternalIndexSettingsFailsValidation() { + final Setting 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 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); + } + } diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index 1ab92526e3130..c32037f44525e 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -735,6 +735,13 @@ public void testRejectNonIndexScopedNotCopyableOnResizeSetting() { 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"); diff --git a/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java b/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java index 51c073c607e22..8093e7d38a14d 100644 --- a/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java +++ b/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.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 void testInvalidDynamicUpdate() { @Override protected Collection> nodePlugins() { - return Arrays.asList(DummySettingPlugin.class, FinalSettingPlugin.class); + return Arrays.asList(DummySettingPlugin.class, FinalSettingPlugin.class, InternalIndexSettingsPlugin.class); + } + + @Override + protected Collection> transportClientPlugins() { + return Collections.singletonList(InternalIndexSettingsPlugin.class); } public static class DummySettingPlugin extends Plugin { @@ -124,6 +151,151 @@ public List> getSettings() { } } + public static class InternalIndexSettingsPlugin extends Plugin implements ActionPlugin { + + public static final Setting INDEX_INTERNAL_SETTING = + Setting.simpleString("index.internal", Setting.Property.IndexScope, Setting.Property.InternalIndex); + + @Override + public List> getSettings() { + return Collections.singletonList(INDEX_INTERNAL_SETTING); + } + + public static class UpdateInternalIndexAction + extends Action { + + 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 { + + 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 { + + @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 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> 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 void testUpdateSettingsWithBlocks() { } } + 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")); + } + } From ab24609f4fd9334fb561f4a83ec71b8c219e3723 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 12 Jun 2018 23:03:55 -0400 Subject: [PATCH 2/2] comment --- .../cluster/metadata/MetaDataUpdateSettingsService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java index 26e3b842a3743..38766c08e08a3 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java @@ -85,7 +85,7 @@ public void updateSettings(final UpdateSettingsClusterStateUpdateRequest request indexScopedSettings.validate( 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 index internal settings + true); // validate internal index settings for (String key : normalizedSettings.keySet()) { Setting setting = indexScopedSettings.get(key); boolean isWildcard = setting == null && Regex.isSimpleMatchPattern(key);