From 6952c0ef5b33891b9efae6f4f41ee768ac4feb1c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 7 Sep 2018 10:52:33 -0400 Subject: [PATCH 01/16] Add infrastructure to upgrade settings In some cases we want to deprecate a setting, and then automatically upgrade uses of that setting to a replacement setting. This commit adds infrastructure for this so that we can upgrade settings when recovering the cluster state, as well as when such settings are dynamically applied on cluster update settings requests. This commit only focuses on cluster settings, index settings can build on this infrastructure in a follow-up. --- .../TransportClusterUpdateSettingsAction.java | 8 +- .../settings/AbstractScopedSettings.java | 28 ++++ .../org/elasticsearch/gateway/Gateway.java | 11 +- .../common/settings/ScopedSettingsTests.java | 78 +++++++++++ .../common/settings/UpgradeSettingsIT.java | 132 ++++++++++++++++++ .../elasticsearch/gateway/GatewayTests.java | 73 ++++++++++ 6 files changed, 325 insertions(+), 5 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/common/settings/UpgradeSettingsIT.java create mode 100644 server/src/test/java/org/elasticsearch/gateway/GatewayTests.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java index 4cf74fbf865cc..8360797d66021 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java @@ -179,8 +179,12 @@ public void onFailure(String source, Exception e) { @Override public ClusterState execute(final ClusterState currentState) { - ClusterState clusterState = - updater.updateSettings(currentState, request.transientSettings(), request.persistentSettings(), logger); + final ClusterState clusterState = + updater.updateSettings( + currentState, + clusterSettings.upgradeSettings(request.transientSettings()), + clusterSettings.upgradeSettings(request.persistentSettings()), + logger); changed = clusterState != currentState; return clusterState; } 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 a77d739ffe0b4..e572ac666d1f5 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -22,6 +22,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.search.spell.LevenshteinDistance; import org.apache.lucene.util.CollectionUtil; +import org.elasticsearch.Assertions; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.component.AbstractComponent; @@ -34,10 +35,12 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.BiConsumer; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Predicate; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -757,6 +760,23 @@ private static Setting findOverlappingSetting(Setting newSetting, Map setting = getRaw(key); + final Function, Map.Entry> upgrader = upgraders.get(setting); + if (upgrader == null) { + builder.copy(key, settings); + } else { + changed = true; + final Map.Entry upgrade = upgrader.apply(new Entry(key, settings)); + builder.put(upgrade.getKey(), upgrade.getValue()); + } + } + return changed ? builder.build() : settings; + } + /** * Archives invalid or unknown settings. Any setting that is not recognized or fails validation * will be archived. This means the setting is prefixed with {@value ARCHIVED_SETTINGS_PREFIX} @@ -847,4 +867,12 @@ public String setValue(String value) { public boolean isPrivateSetting(String key) { return false; } + + private Map, Function, Map.Entry>> upgraders = new HashMap<>(); + + public synchronized void addSettingsUpgrader( + final Setting setting, Function, Map.Entry> upgrader) { + upgraders.put(Objects.requireNonNull(setting, "setting"), Objects.requireNonNull(upgrader, "upgrader")); + } + } diff --git a/server/src/main/java/org/elasticsearch/gateway/Gateway.java b/server/src/main/java/org/elasticsearch/gateway/Gateway.java index d2261e5d1b421..77d2c553c2c51 100644 --- a/server/src/main/java/org/elasticsearch/gateway/Gateway.java +++ b/server/src/main/java/org/elasticsearch/gateway/Gateway.java @@ -137,20 +137,25 @@ public void performStateRecovery(final GatewayStateRecoveredListener listener) t } } } + final ClusterState.Builder builder = upgradeAndArchiveUnknownOrInvalidSettings(metaDataBuilder); + listener.onSuccess(builder.build()); + } + + ClusterState.Builder upgradeAndArchiveUnknownOrInvalidSettings(MetaData.Builder metaDataBuilder) { final ClusterSettings clusterSettings = clusterService.getClusterSettings(); metaDataBuilder.persistentSettings( clusterSettings.archiveUnknownOrInvalidSettings( - metaDataBuilder.persistentSettings(), + clusterSettings.upgradeSettings(metaDataBuilder.persistentSettings()), e -> logUnknownSetting("persistent", e), (e, ex) -> logInvalidSetting("persistent", e, ex))); metaDataBuilder.transientSettings( clusterSettings.archiveUnknownOrInvalidSettings( - metaDataBuilder.transientSettings(), + clusterSettings.upgradeSettings(metaDataBuilder.transientSettings()), e -> logUnknownSetting("transient", e), (e, ex) -> logInvalidSetting("transient", e, ex))); ClusterState.Builder builder = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.get(settings)); builder.metaData(metaDataBuilder); - listener.onSuccess(builder.build()); + return builder; } private void logUnknownSetting(String settingType, Map.Entry e) { 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 f0f8b6c417f2f..14b84b712ed3d 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -32,6 +32,8 @@ import org.elasticsearch.transport.TransportService; import java.io.IOException; +import java.util.AbstractMap; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -53,6 +55,7 @@ import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.hasToString; +import static org.hamcrest.Matchers.sameInstance; public class ScopedSettingsTests extends ESTestCase { @@ -1045,4 +1048,79 @@ public void testPrivateIndexSettingsSkipValidation() { indexScopedSettings.validate(settings, false, /* validateInternalOrPrivateIndex */ false); } + public void testUpgradeSetting() { + final Setting oldSetting = Setting.simpleString("foo.old", Property.NodeScope); + final Setting newSetting = Setting.simpleString("foo.new", Property.NodeScope); + final Setting remainingSetting = Setting.simpleString("foo.remaining", Property.NodeScope); + + final AbstractScopedSettings service = + new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting))); + service.addSettingsUpgrader(oldSetting, entry -> new AbstractMap.SimpleEntry<>("foo.new", entry.getValue())); + + final Settings settings = + Settings.builder() + .put("foo.old", randomAlphaOfLength(8)) + .put("foo.remaining", randomAlphaOfLength(8)) + .build(); + final Settings upgradedSettings = service.upgradeSettings(settings); + assertFalse(oldSetting.exists(upgradedSettings)); + assertTrue(newSetting.exists(upgradedSettings)); + assertThat(newSetting.get(upgradedSettings), equalTo(oldSetting.get(settings))); + assertTrue(remainingSetting.exists(upgradedSettings)); + assertThat(remainingSetting.get(upgradedSettings), equalTo(remainingSetting.get(settings))); + } + + public void testUpgradeSettingsNoChangesPreservesInstance() { + final Setting oldSetting = Setting.simpleString("foo.old", Property.NodeScope); + final Setting newSetting = Setting.simpleString("foo.new", Property.NodeScope); + final Setting remainingSetting = Setting.simpleString("foo.remaining", Property.NodeScope); + + final AbstractScopedSettings service = + new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting))); + service.addSettingsUpgrader(oldSetting, entry -> new AbstractMap.SimpleEntry<>("foo.new", entry.getValue())); + + final Settings settings = + Settings.builder().put("foo.remaining", randomAlphaOfLength(8)).build(); + final Settings upgradedSettings = service.upgradeSettings(settings); + assertThat(upgradedSettings, sameInstance(settings)); + } + + public void testUpgradeComplexSetting() { + final Setting.AffixSetting oldSetting = + Setting.affixKeySetting("foo.old.", "suffix", key -> Setting.simpleString(key, Property.NodeScope)); + final Setting.AffixSetting newSetting = + Setting.affixKeySetting("foo.new.", "suffix", key -> Setting.simpleString(key, Property.NodeScope)); + final Setting.AffixSetting remainingSetting = + Setting.affixKeySetting("foo.remaining.", "suffix", key -> Setting.simpleString(key, Property.NodeScope)); + + final AbstractScopedSettings service = + new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting))); + service.addSettingsUpgrader( + oldSetting, + entry -> new AbstractMap.SimpleEntry<>(entry.getKey().replaceFirst("^foo.old", "foo.new"), entry.getValue())); + + final int count = randomIntBetween(1, 8); + final List concretes = new ArrayList<>(count); + final Settings.Builder builder = Settings.builder(); + for (int i = 0; i < count; i++) { + final String concrete = randomAlphaOfLength(8); + concretes.add(concrete); + builder.put("foo.old." + concrete + ".suffix", randomAlphaOfLength(8)); + builder.put("foo.remaining." + concrete + ".suffix", randomAlphaOfLength(8)); + } + final Settings settings = builder.build(); + final Settings upgradedSettings = service.upgradeSettings(settings); + for (final String concrete : concretes) { + assertFalse(oldSetting.getConcreteSettingForNamespace(concrete).exists(upgradedSettings)); + assertTrue(newSetting.getConcreteSettingForNamespace(concrete).exists(upgradedSettings)); + assertThat( + newSetting.getConcreteSettingForNamespace(concrete).get(upgradedSettings), + equalTo(oldSetting.getConcreteSettingForNamespace(concrete).get(settings))); + assertTrue(remainingSetting.getConcreteSettingForNamespace(concrete).exists(upgradedSettings)); + assertThat( + remainingSetting.getConcreteSettingForNamespace(concrete).get(upgradedSettings), + equalTo(remainingSetting.getConcreteSettingForNamespace(concrete).get(settings))); + } + } + } diff --git a/server/src/test/java/org/elasticsearch/common/settings/UpgradeSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/UpgradeSettingsIT.java new file mode 100644 index 0000000000000..8db5c5328d6e4 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/common/settings/UpgradeSettingsIT.java @@ -0,0 +1,132 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequestBuilder; +import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; +import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.env.Environment; +import org.elasticsearch.env.NodeEnvironment; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.script.ScriptService; +import org.elasticsearch.test.ESSingleNodeTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.watcher.ResourceWatcherService; +import org.junit.After; + +import java.util.AbstractMap; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.function.BiConsumer; +import java.util.function.Function; + +import static org.hamcrest.Matchers.equalTo; + +public class UpgradeSettingsIT extends ESSingleNodeTestCase { + + @After + public void cleanup() throws Exception { + client() + .admin() + .cluster() + .prepareUpdateSettings() + .setPersistentSettings(Settings.builder().putNull("*")) + .setTransientSettings(Settings.builder().putNull("*")) + .get(); + } + + @Override + protected Collection> getPlugins() { + return Collections.singletonList(UpgradeSettingsPlugin.class); + } + + public static class UpgradeSettingsPlugin extends Plugin { + + static final Setting oldSetting = Setting.simpleString("foo.old", Setting.Property.Dynamic, Setting.Property.NodeScope); + static final Setting newSetting = Setting.simpleString("foo.new", Setting.Property.Dynamic, Setting.Property.NodeScope); + + public UpgradeSettingsPlugin(){ + + } + + @Override + public Collection createComponents( + final Client client, + final ClusterService clusterService, + final ThreadPool threadPool, + final ResourceWatcherService resourceWatcherService, + final ScriptService scriptService, + final NamedXContentRegistry xContentRegistry, + final Environment environment, + final NodeEnvironment nodeEnvironment, + final NamedWriteableRegistry namedWriteableRegistry) { + clusterService.getClusterSettings().addSettingsUpgrader( + oldSetting, + entry -> new AbstractMap.SimpleEntry<>("foo.new", entry.getValue())); + return Collections.emptyList(); + } + + @Override + public List> getSettings() { + return Arrays.asList(oldSetting, newSetting); + } + + } + + public void testUpgradePersistentSettingsOnUpdate() { + runUpgradeSettingsOnUpdateTest((settings, builder) -> builder.setPersistentSettings(settings), MetaData::persistentSettings); + } + + public void testUpgradeTransientSettingsOnUpdate() { + runUpgradeSettingsOnUpdateTest((settings, builder) -> builder.setTransientSettings(settings), MetaData::transientSettings); + } + + private void runUpgradeSettingsOnUpdateTest( + final BiConsumer consumer, + final Function settingsFunction) { + final String value = randomAlphaOfLength(8); + final ClusterUpdateSettingsRequestBuilder builder = + client() + .admin() + .cluster() + .prepareUpdateSettings(); + consumer.accept(Settings.builder().put("foo.old", value).build(), builder); + builder.get(); + + final ClusterStateResponse response = client() + .admin() + .cluster() + .prepareState() + .clear() + .setMetaData(true) + .get(); + + assertFalse(UpgradeSettingsPlugin.oldSetting.exists(settingsFunction.apply(response.getState().metaData()))); + assertTrue(UpgradeSettingsPlugin.newSetting.exists(settingsFunction.apply(response.getState().metaData()))); + assertThat(UpgradeSettingsPlugin.newSetting.get(settingsFunction.apply(response.getState().metaData())), equalTo(value)); + } + +} diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java new file mode 100644 index 0000000000000..0c0c01a6f49f9 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java @@ -0,0 +1,73 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.gateway; + +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESTestCase; + +import java.util.AbstractMap; +import java.util.HashSet; +import java.util.Set; +import java.util.function.BiConsumer; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.hamcrest.Matchers.equalTo; + +public class GatewayTests extends ESTestCase { + + public void testUpgradePersistentSettings() { + runUpgradeSettings(MetaData.Builder::persistentSettings, MetaData::persistentSettings); + } + + public void testUpgradeTransientSettings() { + runUpgradeSettings(MetaData.Builder::transientSettings, MetaData::transientSettings); + } + + private void runUpgradeSettings( + final BiConsumer applySettingsToBuilder, final Function metaDataSettings) { + final Setting oldSetting = Setting.simpleString("foo.old", Setting.Property.Dynamic, Setting.Property.NodeScope); + final Setting newSetting = Setting.simpleString("foo.new", Setting.Property.Dynamic, Setting.Property.NodeScope); + final Set> settingsSet = + new HashSet<>( + Stream.concat( + ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream(), + Stream.of(oldSetting, newSetting)) + .collect(Collectors.toList())); + final ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, settingsSet); + clusterSettings.addSettingsUpgrader(oldSetting, entry -> new AbstractMap.SimpleEntry<>("foo.new", entry.getValue())); + final ClusterService clusterService = new ClusterService(Settings.EMPTY, clusterSettings, null); + final Gateway gateway = new Gateway(Settings.EMPTY, clusterService, null, null); + final MetaData.Builder builder = MetaData.builder(); + final Settings settings = Settings.builder().put("foo.old", randomAlphaOfLength(8)).build(); + applySettingsToBuilder.accept(builder, settings); + final ClusterState state = gateway.upgradeAndArchiveUnknownOrInvalidSettings(builder).build(); + assertFalse(oldSetting.exists(metaDataSettings.apply(state.metaData()))); + assertTrue(newSetting.exists(metaDataSettings.apply(state.metaData()))); + assertThat(newSetting.get(metaDataSettings.apply(state.metaData())), equalTo(oldSetting.get(settings))); + } + +} From c2a52762f58edc89b43596313d7c12bd785e3493 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 8 Sep 2018 15:03:28 -0400 Subject: [PATCH 02/16] Remove unused import --- .../elasticsearch/common/settings/AbstractScopedSettings.java | 1 - 1 file changed, 1 deletion(-) 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 e572ac666d1f5..03ba6cbd900a8 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -22,7 +22,6 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.search.spell.LevenshteinDistance; import org.apache.lucene.util.CollectionUtil; -import org.elasticsearch.Assertions; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.component.AbstractComponent; From 834e6f7d0f7585d80308a39b3d38a4ac1307507e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 8 Sep 2018 22:10:50 -0400 Subject: [PATCH 03/16] Plugin architecture --- .../client/transport/TransportClient.java | 5 +- .../settings/AbstractScopedSettings.java | 27 +++--- .../common/settings/ClusterSettings.java | 11 ++- .../common/settings/IndexScopedSettings.java | 2 +- .../common/settings/SettingUpgrader.java | 30 +++++++ .../common/settings/SettingsModule.java | 22 ++++- .../java/org/elasticsearch/node/Node.java | 12 ++- .../org/elasticsearch/plugins/Plugin.java | 10 +++ .../indices/get/GetIndexActionTests.java | 4 +- .../settings/get/GetSettingsActionTests.java | 3 +- .../common/settings/ScopedSettingsTests.java | 89 +++++++++++++++---- .../common/settings/SettingsModuleTests.java | 5 +- .../common/settings/UpgradeSettingsIT.java | 53 +++++------ .../elasticsearch/gateway/GatewayTests.java | 30 +++++-- .../test/AbstractBuilderTestCase.java | 3 +- .../core/security/authc/RealmSettings.java | 4 +- 16 files changed, 230 insertions(+), 80 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java diff --git a/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java index ba18105e3f1ca..b0b6fa73b7c48 100644 --- a/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -19,7 +19,6 @@ package org.elasticsearch.client.transport; -import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionModule; @@ -44,6 +43,7 @@ import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.PageCacheRecycler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.indices.IndicesModule; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.node.InternalSettingsPreparer; @@ -146,7 +146,8 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings for (final ExecutorBuilder builder : threadPool.builders()) { additionalSettings.addAll(builder.getRegisteredSettings()); } - SettingsModule settingsModule = new SettingsModule(settings, additionalSettings, additionalSettingsFilter); + SettingsModule settingsModule = + new SettingsModule(settings, additionalSettings, additionalSettingsFilter, Collections.emptyList()); SearchModule searchModule = new SearchModule(settings, true, pluginsService.filterPlugins(SearchPlugin.class)); IndicesModule indicesModule = new IndicesModule(Collections.emptyList()); 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 03ba6cbd900a8..607c4c10cb523 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.regex.Regex; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -34,7 +35,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.BiConsumer; @@ -54,14 +54,27 @@ public abstract class AbstractScopedSettings extends AbstractComponent { private final List> settingUpdaters = new CopyOnWriteArrayList<>(); private final Map> complexMatchers; private final Map> keySettings; + private final Map, Function, Map.Entry>> settingUpgraders; private final Setting.Property scope; private static final Pattern KEY_PATTERN = Pattern.compile("^(?:[-\\w]+[.])*[-\\w]+$"); private static final Pattern GROUP_KEY_PATTERN = Pattern.compile("^(?:[-\\w]+[.])+$"); private static final Pattern AFFIX_KEY_PATTERN = Pattern.compile("^(?:[-\\w]+[.])+[*](?:[.][-\\w]+)+$"); - protected AbstractScopedSettings(Settings settings, Set> settingsSet, Setting.Property scope) { + protected AbstractScopedSettings( + final Settings settings, + final Set> settingsSet, + final List> settingUpgraders, + final Setting.Property scope) { super(settings); this.lastSettingsApplied = Settings.EMPTY; + + this.settingUpgraders = + Collections.unmodifiableMap( + settingUpgraders.stream() + .collect(Collectors.toMap( + SettingUpgrader::getSetting, + u -> e -> new AbstractMap.SimpleEntry<>(u.getKey(e.getKey()), u.getValue(e.getValue()))))); + this.scope = scope; Map> complexMatchers = new HashMap<>(); Map> keySettings = new HashMap<>(); @@ -99,6 +112,7 @@ protected AbstractScopedSettings(Settings nodeSettings, Settings scopeSettings, this.scope = other.scope; complexMatchers = other.complexMatchers; keySettings = other.keySettings; + settingUpgraders = Collections.unmodifiableMap(new HashMap<>(other.settingUpgraders)); settingUpdaters.addAll(other.settingUpdaters); } @@ -764,7 +778,7 @@ public Settings upgradeSettings(final Settings settings) { boolean changed = false; for (final String key : settings.keySet()) { final Setting setting = getRaw(key); - final Function, Map.Entry> upgrader = upgraders.get(setting); + final Function, Map.Entry> upgrader = settingUpgraders.get(setting); if (upgrader == null) { builder.copy(key, settings); } else { @@ -867,11 +881,4 @@ public boolean isPrivateSetting(String key) { return false; } - private Map, Function, Map.Entry>> upgraders = new HashMap<>(); - - public synchronized void addSettingsUpgrader( - final Setting setting, Function, Map.Entry> upgrader) { - upgraders.put(Objects.requireNonNull(setting, "setting"), Objects.requireNonNull(upgrader, "upgrader")); - } - } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 10787140bdec8..ef1ed48c3346c 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -100,6 +100,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.function.Predicate; @@ -108,10 +109,15 @@ */ public final class ClusterSettings extends AbstractScopedSettings { public ClusterSettings(Settings nodeSettings, Set> settingsSet) { - super(nodeSettings, settingsSet, Property.NodeScope); + super(nodeSettings, settingsSet, Collections.emptyList(), Property.NodeScope); addSettingsUpdater(new LoggingSettingUpdater(nodeSettings)); } + public ClusterSettings( + final Settings nodeSettings, final Set> settingsSet, final List> settingUpgraders) { + super(nodeSettings, settingsSet, settingUpgraders, Property.NodeScope); + } + private static final class LoggingSettingUpdater implements SettingUpdater { final Predicate loggerPredicate = Loggers.LOG_LEVEL_SETTING::match; private final Settings settings; @@ -436,4 +442,7 @@ public void apply(Settings value, Settings current, Settings previous) { IndexGraveyard.SETTING_MAX_TOMBSTONES, EnableAssignmentDecider.CLUSTER_TASKS_ALLOCATION_ENABLE_SETTING ))); + + public static List> BUILT_IN_SETTING_UPGRADERS = Collections.emptyList(); + } diff --git a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java index 2116d1eff7510..591fe6b06d824 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java @@ -178,7 +178,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings { public static final IndexScopedSettings DEFAULT_SCOPED_SETTINGS = new IndexScopedSettings(Settings.EMPTY, BUILT_IN_INDEX_SETTINGS); public IndexScopedSettings(Settings settings, Set> settingsSet) { - super(settings, settingsSet, Property.IndexScope); + super(settings, settingsSet, Collections.emptyList(), Property.IndexScope); } private IndexScopedSettings(Settings settings, IndexScopedSettings other, IndexMetaData metaData) { diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java b/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java new file mode 100644 index 0000000000000..22bbce61b8167 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java @@ -0,0 +1,30 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +public interface SettingUpgrader { + + Setting getSetting(); + + String getKey(String key); + + String getValue(String value); + +} diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 67037b3708bee..e2bbe10782d81 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -54,10 +55,14 @@ public class SettingsModule implements Module { private final SettingsFilter settingsFilter; public SettingsModule(Settings settings, Setting... additionalSettings) { - this(settings, Arrays.asList(additionalSettings), Collections.emptyList()); + this(settings, Arrays.asList(additionalSettings), Collections.emptyList(), Collections.emptyList()); } - public SettingsModule(Settings settings, List> additionalSettings, List settingsFilter) { + public SettingsModule( + Settings settings, + List> additionalSettings, + List settingsFilter, + List> settingUpgraders) { logger = Loggers.getLogger(getClass(), settings); this.settings = settings; for (Setting setting : ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) { @@ -70,12 +75,20 @@ public SettingsModule(Settings settings, List> additionalSettings, Li for (Setting setting : additionalSettings) { registerSetting(setting); } - for (String filter : settingsFilter) { registerSettingsFilter(filter); } + final List> clusterSettingUpgraders = new ArrayList<>(); + for (final SettingUpgrader settingUpgrader : ClusterSettings.BUILT_IN_SETTING_UPGRADERS) { + assert settingUpgrader.getSetting().hasNodeScope() : settingUpgrader.getSetting().getKey(); + clusterSettingUpgraders.add(settingUpgrader); + } + for (final SettingUpgrader settingUpgrader : settingUpgraders) { + assert settingUpgrader.getSetting().hasNodeScope() : settingUpgrader.getSetting().getKey(); + clusterSettingUpgraders.add(settingUpgrader); + } this.indexScopedSettings = new IndexScopedSettings(settings, new HashSet<>(this.indexSettings.values())); - this.clusterSettings = new ClusterSettings(settings, new HashSet<>(this.nodeSettings.values())); + this.clusterSettings = new ClusterSettings(settings, new HashSet<>(this.nodeSettings.values()), clusterSettingUpgraders); Settings indexSettings = settings.filter((s) -> (s.startsWith("index.") && // special case - we want to get Did you mean indices.query.bool.max_clause_count // which means we need to by-pass this check for this setting @@ -205,4 +218,5 @@ public ClusterSettings getClusterSettings() { public SettingsFilter getSettingsFilter() { return settingsFilter; } + } diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index c2ef6d12331fe..45aaf6a2d162c 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -73,6 +73,7 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; +import org.elasticsearch.common.settings.SettingUpgrader; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsModule; import org.elasticsearch.common.transport.BoundTransportAddress; @@ -151,6 +152,7 @@ import org.elasticsearch.watcher.ResourceWatcherService; import javax.net.ssl.SNIHostName; + import java.io.BufferedWriter; import java.io.Closeable; import java.io.IOException; @@ -360,7 +362,15 @@ protected Node( AnalysisModule analysisModule = new AnalysisModule(this.environment, pluginsService.filterPlugins(AnalysisPlugin.class)); // this is as early as we can validate settings at this point. we already pass them to ScriptModule as well as ThreadPool // so we might be late here already - final SettingsModule settingsModule = new SettingsModule(this.settings, additionalSettings, additionalSettingsFilter); + + final List> settingsUpgraders = pluginsService.filterPlugins(Plugin.class) + .stream() + .map(Plugin::getSettingUpgraders) + .flatMap(List::stream) + .collect(Collectors.toList()); + + final SettingsModule settingsModule = + new SettingsModule(this.settings, additionalSettings, additionalSettingsFilter, settingsUpgraders); scriptModule.registerClusterSettingsListeners(settingsModule.getClusterSettings()); resourcesToClose.add(resourceWatcherService); final NetworkService networkService = new NetworkService( diff --git a/server/src/main/java/org/elasticsearch/plugins/Plugin.java b/server/src/main/java/org/elasticsearch/plugins/Plugin.java index 65d47682a95c0..faef27207e13a 100644 --- a/server/src/main/java/org/elasticsearch/plugins/Plugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/Plugin.java @@ -34,6 +34,7 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.SettingUpgrader; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsModule; import org.elasticsearch.common.xcontent.NamedXContentRegistry; @@ -172,6 +173,15 @@ public void onIndexModule(IndexModule indexModule) {} */ public List getSettingsFilter() { return Collections.emptyList(); } + /** + * Get the setting upgraders provided by this plugin. + * + * @return the settings upgraders + */ + public List> getSettingUpgraders() { + return Collections.emptyList(); + } + /** * Provides a function to modify global custom meta data on startup. *

diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java index b67c2e2954d04..61b8cd1ebb1e5 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java @@ -43,6 +43,8 @@ import java.util.Collections; import java.util.concurrent.TimeUnit; +import static java.util.Collections.emptyList; + public class GetIndexActionTests extends ESSingleNodeTestCase { private TransportService transportService; @@ -58,7 +60,7 @@ public class GetIndexActionTests extends ESSingleNodeTestCase { public void setUp() throws Exception { super.setUp(); - settingsFilter = new SettingsModule(Settings.EMPTY, Collections.emptyList(), Collections.emptyList()).getSettingsFilter(); + settingsFilter = new SettingsModule(Settings.EMPTY, emptyList(), emptyList(), emptyList()).getSettingsFilter(); threadPool = new TestThreadPool("GetIndexActionTests"); clusterService = getInstanceFromNode(ClusterService.class); indicesService = getInstanceFromNode(IndicesService.class); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java index 03ccebba10dbd..41f913a055e4f 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java @@ -42,6 +42,7 @@ import java.util.Collections; import java.util.concurrent.TimeUnit; +import static java.util.Collections.emptyList; import static org.elasticsearch.test.ClusterServiceUtils.createClusterService; public class GetSettingsActionTests extends ESTestCase { @@ -71,7 +72,7 @@ protected void masterOperation(GetSettingsRequest request, ClusterState state, A public void setUp() throws Exception { super.setUp(); - settingsFilter = new SettingsModule(Settings.EMPTY, Collections.emptyList(), Collections.emptyList()).getSettingsFilter(); + settingsFilter = new SettingsModule(Settings.EMPTY, emptyList(), emptyList(), emptyList()).getSettingsFilter(); threadPool = new TestThreadPool("GetSettingsActionTests"); clusterService = createClusterService(threadPool); CapturingTransport capturingTransport = new CapturingTransport(); 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 14b84b712ed3d..a09244d837c63 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -32,7 +32,6 @@ import org.elasticsearch.transport.TransportService; import java.io.IOException; -import java.util.AbstractMap; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -1049,13 +1048,30 @@ public void testPrivateIndexSettingsSkipValidation() { } public void testUpgradeSetting() { - final Setting oldSetting = Setting.simpleString("foo.old", Property.NodeScope); - final Setting newSetting = Setting.simpleString("foo.new", Property.NodeScope); - final Setting remainingSetting = Setting.simpleString("foo.remaining", Property.NodeScope); + final Setting oldSetting = Setting.simpleString("foo.old", Property.NodeScope); + final Setting newSetting = Setting.simpleString("foo.new", Property.NodeScope); + final Setting remainingSetting = Setting.simpleString("foo.remaining", Property.NodeScope); final AbstractScopedSettings service = - new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting))); - service.addSettingsUpgrader(oldSetting, entry -> new AbstractMap.SimpleEntry<>("foo.new", entry.getValue())); + new ClusterSettings( + Settings.EMPTY, + new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting)), + Collections.singletonList(new SettingUpgrader() { + @Override + public Setting getSetting() { + return oldSetting; + } + + @Override + public String getKey(final String key) { + return "foo.new"; + } + + @Override + public String getValue(final String value) { + return "new." + value; + } + })); final Settings settings = Settings.builder() @@ -1065,22 +1081,40 @@ public void testUpgradeSetting() { final Settings upgradedSettings = service.upgradeSettings(settings); assertFalse(oldSetting.exists(upgradedSettings)); assertTrue(newSetting.exists(upgradedSettings)); - assertThat(newSetting.get(upgradedSettings), equalTo(oldSetting.get(settings))); + assertThat(newSetting.get(upgradedSettings), equalTo("new." + oldSetting.get(settings))); assertTrue(remainingSetting.exists(upgradedSettings)); assertThat(remainingSetting.get(upgradedSettings), equalTo(remainingSetting.get(settings))); } public void testUpgradeSettingsNoChangesPreservesInstance() { - final Setting oldSetting = Setting.simpleString("foo.old", Property.NodeScope); - final Setting newSetting = Setting.simpleString("foo.new", Property.NodeScope); - final Setting remainingSetting = Setting.simpleString("foo.remaining", Property.NodeScope); + final Setting oldSetting = Setting.simpleString("foo.old", Property.NodeScope); + final Setting newSetting = Setting.simpleString("foo.new", Property.NodeScope); + final Setting remainingSetting = Setting.simpleString("foo.remaining", Property.NodeScope); final AbstractScopedSettings service = - new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting))); - service.addSettingsUpgrader(oldSetting, entry -> new AbstractMap.SimpleEntry<>("foo.new", entry.getValue())); + new ClusterSettings( + Settings.EMPTY, + new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting)), + Collections.singletonList(new SettingUpgrader() { - final Settings settings = - Settings.builder().put("foo.remaining", randomAlphaOfLength(8)).build(); + @Override + public Setting getSetting() { + return oldSetting; + } + + @Override + public String getKey(final String key) { + return "foo.new"; + } + + @Override + public String getValue(final String value) { + return value; + } + + })); + + final Settings settings = Settings.builder().put("foo.remaining", randomAlphaOfLength(8)).build(); final Settings upgradedSettings = service.upgradeSettings(settings); assertThat(upgradedSettings, sameInstance(settings)); } @@ -1094,10 +1128,27 @@ public void testUpgradeComplexSetting() { Setting.affixKeySetting("foo.remaining.", "suffix", key -> Setting.simpleString(key, Property.NodeScope)); final AbstractScopedSettings service = - new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting))); - service.addSettingsUpgrader( - oldSetting, - entry -> new AbstractMap.SimpleEntry<>(entry.getKey().replaceFirst("^foo.old", "foo.new"), entry.getValue())); + new ClusterSettings( + Settings.EMPTY, + new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting)), + + Collections.singletonList(new SettingUpgrader() { + @Override + public Setting getSetting() { + return oldSetting; + } + + @Override + public String getKey(final String key) { + return key.replaceFirst("^foo\\.old", "foo\\.new"); + } + + @Override + public String getValue(final String value) { + return "new." + value; + } + + })); final int count = randomIntBetween(1, 8); final List concretes = new ArrayList<>(count); @@ -1115,7 +1166,7 @@ public void testUpgradeComplexSetting() { assertTrue(newSetting.getConcreteSettingForNamespace(concrete).exists(upgradedSettings)); assertThat( newSetting.getConcreteSettingForNamespace(concrete).get(upgradedSettings), - equalTo(oldSetting.getConcreteSettingForNamespace(concrete).get(settings))); + equalTo("new." + oldSetting.getConcreteSettingForNamespace(concrete).get(settings))); assertTrue(remainingSetting.getConcreteSettingForNamespace(concrete).exists(upgradedSettings)); assertThat( remainingSetting.getConcreteSettingForNamespace(concrete).get(upgradedSettings), diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index 6a2be8217a661..5cd6ab7720b1a 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -24,6 +24,7 @@ import java.util.Arrays; +import static java.util.Collections.emptyList; import static org.hamcrest.Matchers.containsString; public class SettingsModuleTests extends ModuleTestCase { @@ -103,14 +104,14 @@ public void testRegisterSettingsFilter() { try { new SettingsModule(settings, Arrays.asList(Setting.boolSetting("foo.bar", true, Property.NodeScope), Setting.boolSetting("bar.foo", true, Property.NodeScope, Property.Filtered), - Setting.boolSetting("bar.baz", true, Property.NodeScope)), Arrays.asList("foo.*", "bar.foo")); + Setting.boolSetting("bar.baz", true, Property.NodeScope)), Arrays.asList("foo.*", "bar.foo"), emptyList()); fail(); } catch (IllegalArgumentException ex) { assertEquals("filter [bar.foo] has already been registered", ex.getMessage()); } SettingsModule module = new SettingsModule(settings, Arrays.asList(Setting.boolSetting("foo.bar", true, Property.NodeScope), Setting.boolSetting("bar.foo", true, Property.NodeScope, Property.Filtered), - Setting.boolSetting("bar.baz", true, Property.NodeScope)), Arrays.asList("foo.*")); + Setting.boolSetting("bar.baz", true, Property.NodeScope)), Arrays.asList("foo.*"), emptyList()); assertInstanceBinding(module, Settings.class, (s) -> s == settings); assertInstanceBinding(module, SettingsFilter.class, (s) -> s.filter(settings).size() == 1); assertInstanceBinding(module, SettingsFilter.class, (s) -> s.filter(settings).keySet().contains("bar.baz")); diff --git a/server/src/test/java/org/elasticsearch/common/settings/UpgradeSettingsIT.java b/server/src/test/java/org/elasticsearch/common/settings/UpgradeSettingsIT.java index 8db5c5328d6e4..839b96e641870 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/UpgradeSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/common/settings/UpgradeSettingsIT.java @@ -21,21 +21,11 @@ import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequestBuilder; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; -import org.elasticsearch.client.Client; import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.env.Environment; -import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESSingleNodeTestCase; -import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.watcher.ResourceWatcherService; import org.junit.After; -import java.util.AbstractMap; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -65,35 +55,38 @@ protected Collection> getPlugins() { public static class UpgradeSettingsPlugin extends Plugin { - static final Setting oldSetting = Setting.simpleString("foo.old", Setting.Property.Dynamic, Setting.Property.NodeScope); - static final Setting newSetting = Setting.simpleString("foo.new", Setting.Property.Dynamic, Setting.Property.NodeScope); + static final Setting oldSetting = Setting.simpleString("foo.old", Setting.Property.Dynamic, Setting.Property.NodeScope); + static final Setting newSetting = Setting.simpleString("foo.new", Setting.Property.Dynamic, Setting.Property.NodeScope); public UpgradeSettingsPlugin(){ } - @Override - public Collection createComponents( - final Client client, - final ClusterService clusterService, - final ThreadPool threadPool, - final ResourceWatcherService resourceWatcherService, - final ScriptService scriptService, - final NamedXContentRegistry xContentRegistry, - final Environment environment, - final NodeEnvironment nodeEnvironment, - final NamedWriteableRegistry namedWriteableRegistry) { - clusterService.getClusterSettings().addSettingsUpgrader( - oldSetting, - entry -> new AbstractMap.SimpleEntry<>("foo.new", entry.getValue())); - return Collections.emptyList(); - } - @Override public List> getSettings() { return Arrays.asList(oldSetting, newSetting); } + @Override + public List> getSettingUpgraders() { + return Collections.singletonList(new SettingUpgrader() { + + @Override + public Setting getSetting() { + return oldSetting; + } + + @Override + public String getKey(final String key) { + return "foo.new"; + } + + @Override + public String getValue(final String value) { + return "new." + value; + } + }); + } } public void testUpgradePersistentSettingsOnUpdate() { @@ -126,7 +119,7 @@ private void runUpgradeSettingsOnUpdateTest( assertFalse(UpgradeSettingsPlugin.oldSetting.exists(settingsFunction.apply(response.getState().metaData()))); assertTrue(UpgradeSettingsPlugin.newSetting.exists(settingsFunction.apply(response.getState().metaData()))); - assertThat(UpgradeSettingsPlugin.newSetting.get(settingsFunction.apply(response.getState().metaData())), equalTo(value)); + assertThat(UpgradeSettingsPlugin.newSetting.get(settingsFunction.apply(response.getState().metaData())), equalTo("new." + value)); } } diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java index 0c0c01a6f49f9..49cb377657ff7 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java @@ -24,10 +24,11 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.SettingUpgrader; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; -import java.util.AbstractMap; +import java.util.Collections; import java.util.HashSet; import java.util.Set; import java.util.function.BiConsumer; @@ -49,16 +50,33 @@ public void testUpgradeTransientSettings() { private void runUpgradeSettings( final BiConsumer applySettingsToBuilder, final Function metaDataSettings) { - final Setting oldSetting = Setting.simpleString("foo.old", Setting.Property.Dynamic, Setting.Property.NodeScope); - final Setting newSetting = Setting.simpleString("foo.new", Setting.Property.Dynamic, Setting.Property.NodeScope); + final Setting oldSetting = Setting.simpleString("foo.old", Setting.Property.Dynamic, Setting.Property.NodeScope); + final Setting newSetting = Setting.simpleString("foo.new", Setting.Property.Dynamic, Setting.Property.NodeScope); final Set> settingsSet = new HashSet<>( Stream.concat( ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream(), Stream.of(oldSetting, newSetting)) .collect(Collectors.toList())); - final ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, settingsSet); - clusterSettings.addSettingsUpgrader(oldSetting, entry -> new AbstractMap.SimpleEntry<>("foo.new", entry.getValue())); + final ClusterSettings clusterSettings = new ClusterSettings( + Settings.EMPTY, + settingsSet, + Collections.singletonList(new SettingUpgrader() { + @Override + public Setting getSetting() { + return oldSetting; + } + + @Override + public String getKey(final String key) { + return "foo.new"; + } + + @Override + public String getValue(final String value) { + return "new." + value; + } + })); final ClusterService clusterService = new ClusterService(Settings.EMPTY, clusterSettings, null); final Gateway gateway = new Gateway(Settings.EMPTY, clusterService, null, null); final MetaData.Builder builder = MetaData.builder(); @@ -67,7 +85,7 @@ private void runUpgradeSettings( final ClusterState state = gateway.upgradeAndArchiveUnknownOrInvalidSettings(builder).build(); assertFalse(oldSetting.exists(metaDataSettings.apply(state.metaData()))); assertTrue(newSetting.exists(metaDataSettings.apply(state.metaData()))); - assertThat(newSetting.get(metaDataSettings.apply(state.metaData())), equalTo(oldSetting.get(settings))); + assertThat(newSetting.get(metaDataSettings.apply(state.metaData())), equalTo("new." + oldSetting.get(settings))); } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java index cc3902893411a..e2469e0f71d5d 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java @@ -340,7 +340,8 @@ private static class ServiceHolder implements Closeable { clientInvocationHandler); ScriptModule scriptModule = createScriptModule(pluginsService.filterPlugins(ScriptPlugin.class)); List> additionalSettings = pluginsService.getPluginSettings(); - SettingsModule settingsModule = new SettingsModule(nodeSettings, additionalSettings, pluginsService.getPluginSettingsFilter()); + SettingsModule settingsModule = + new SettingsModule(nodeSettings, additionalSettings, pluginsService.getPluginSettingsFilter(), Collections.emptyList()); searchModule = new SearchModule(nodeSettings, false, pluginsService.filterPlugins(SearchPlugin.class)); IndicesModule indicesModule = new IndicesModule(pluginsService.filterPlugins(MapperPlugin.class)); List entries = new ArrayList<>(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java index f7fabab2799af..1633bbf5611df 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java @@ -12,6 +12,7 @@ import org.elasticsearch.xpack.core.security.SecurityExtension; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -181,7 +182,8 @@ private static void validateRealm(String name, String type, Settings settings, S settingSet.add(TYPE_SETTING); settingSet.add(ENABLED_SETTING); settingSet.add(ORDER_SETTING); - final AbstractScopedSettings validator = new AbstractScopedSettings(settings, settingSet, Setting.Property.NodeScope) { }; + final AbstractScopedSettings validator = + new AbstractScopedSettings(settings, settingSet, Collections.emptyList(), Setting.Property.NodeScope) { }; try { validator.validate(settings, false); } catch (RuntimeException e) { From 95c05bad2626d979b269bc76496d6a0eb12f153f Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 8 Sep 2018 22:14:08 -0400 Subject: [PATCH 04/16] Default implementation --- .../org/elasticsearch/common/settings/SettingUpgrader.java | 4 +++- .../elasticsearch/common/settings/ScopedSettingsTests.java | 5 ----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java b/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java index 22bbce61b8167..3f57075436b2d 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java @@ -25,6 +25,8 @@ public interface SettingUpgrader { String getKey(String key); - String getValue(String value); + default String getValue(final String value) { + return value; + } } 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 a09244d837c63..1d640f08e427b 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -1107,11 +1107,6 @@ public String getKey(final String key) { return "foo.new"; } - @Override - public String getValue(final String value) { - return value; - } - })); final Settings settings = Settings.builder().put("foo.remaining", randomAlphaOfLength(8)).build(); From 279bad4e1f82badc96ecae0934e1b27a123a1c99 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 8 Sep 2018 22:35:08 -0400 Subject: [PATCH 05/16] Fix compilation --- .../test/java/org/elasticsearch/test/SettingsFilterTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java index 1886dd4249b14..2b8f3d3b6e81d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java @@ -21,6 +21,7 @@ import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.TrustManagerFactory; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -121,7 +122,7 @@ public void testFiltering() throws Exception { List settingsFilterList = new ArrayList<>(); settingsFilterList.addAll(securityPlugin.getSettingsFilter()); // custom settings, potentially added by a plugin - SettingsModule settingsModule = new SettingsModule(settings, settingList, settingsFilterList); + SettingsModule settingsModule = new SettingsModule(settings, settingList, settingsFilterList, Collections.emptyList()); Injector injector = Guice.createInjector(settingsModule); SettingsFilter settingsFilter = injector.getInstance(SettingsFilter.class); From 1981e56251e9b01ab0b38637018a0b63959a04ea Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 08:13:50 -0400 Subject: [PATCH 06/16] Fix logging updaters --- .../java/org/elasticsearch/common/settings/ClusterSettings.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index ef1ed48c3346c..0bab33fa4848b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -110,12 +110,12 @@ public final class ClusterSettings extends AbstractScopedSettings { public ClusterSettings(Settings nodeSettings, Set> settingsSet) { super(nodeSettings, settingsSet, Collections.emptyList(), Property.NodeScope); - addSettingsUpdater(new LoggingSettingUpdater(nodeSettings)); } public ClusterSettings( final Settings nodeSettings, final Set> settingsSet, final List> settingUpgraders) { super(nodeSettings, settingsSet, settingUpgraders, Property.NodeScope); + addSettingsUpdater(new LoggingSettingUpdater(nodeSettings)); } private static final class LoggingSettingUpdater implements SettingUpdater { From acb812d72ac45fab294d2389a6e768c1da076111 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 08:42:30 -0400 Subject: [PATCH 07/16] Fix constructor chaining --- .../org/elasticsearch/common/settings/ClusterSettings.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 0bab33fa4848b..dd1f7025b1c98 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -108,8 +108,8 @@ * Encapsulates all valid cluster level settings. */ public final class ClusterSettings extends AbstractScopedSettings { - public ClusterSettings(Settings nodeSettings, Set> settingsSet) { - super(nodeSettings, settingsSet, Collections.emptyList(), Property.NodeScope); + public ClusterSettings(final Settings nodeSettings, final Set> settingsSet) { + this(nodeSettings, settingsSet, Collections.emptyList()); } public ClusterSettings( From 259c2199a0747467605ac030edc544f0274081eb Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 09:43:48 -0400 Subject: [PATCH 08/16] Fix indentaiton --- .../common/settings/AbstractScopedSettings.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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 607c4c10cb523..5c245dca3d490 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -70,10 +70,12 @@ protected AbstractScopedSettings( this.settingUpgraders = Collections.unmodifiableMap( - settingUpgraders.stream() - .collect(Collectors.toMap( - SettingUpgrader::getSetting, - u -> e -> new AbstractMap.SimpleEntry<>(u.getKey(e.getKey()), u.getValue(e.getValue()))))); + settingUpgraders + .stream() + .collect( + Collectors.toMap( + SettingUpgrader::getSetting, + u -> e -> new AbstractMap.SimpleEntry<>(u.getKey(e.getKey()), u.getValue(e.getValue()))))); this.scope = scope; Map> complexMatchers = new HashMap<>(); From 69f9abd854a08aa965d5e8e7669827884ce7cac9 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 10:31:24 -0400 Subject: [PATCH 09/16] Add javadocs --- .../common/settings/SettingUpgrader.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java b/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java index 3f57075436b2d..a4625aa10d32b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java @@ -19,12 +19,34 @@ package org.elasticsearch.common.settings; +/** + * Represents the logic to upgrade a setting. + * + * @param + */ public interface SettingUpgrader { + /** + * The setting upgraded by this upgrader. + * + * @return + */ Setting getSetting(); + /** + * The logic to upgrade the setting key, for example by mapping the old setting key to the new setting key. + * + * @param key the setting key to upgrade + * @return the upgraded setting key + */ String getKey(String key); + /** + * The logic to upgrade the setting value. + * + * @param value the setting value to upgrade + * @return the upgraded setting value + */ default String getValue(final String value) { return value; } From 49445908311245abaf90991c4a5ad56b898fe2c6 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 10:34:36 -0400 Subject: [PATCH 10/16] Javadocs and comments --- .../common/settings/AbstractScopedSettings.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 5c245dca3d490..9717dd7026300 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -775,20 +775,29 @@ private static Setting findOverlappingSetting(Setting newSetting, Map setting = getRaw(key); final Function, Map.Entry> upgrader = settingUpgraders.get(setting); if (upgrader == null) { + // the setting does not have an upgrader, copy the setting builder.copy(key, settings); } else { + // the setting has an upgrader, so mark that we have changed a setting and apply the upgrade logic changed = true; final Map.Entry upgrade = upgrader.apply(new Entry(key, settings)); builder.put(upgrade.getKey(), upgrade.getValue()); } } + // we only return a new instance of there was an upgrade return changed ? builder.build() : settings; } From f69d76efefcf18f2871bed2d1f9d9e4aa8eb0429 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 10:35:02 -0400 Subject: [PATCH 11/16] Fix typo --- .../elasticsearch/common/settings/AbstractScopedSettings.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9717dd7026300..9ccd04e7742ac 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -797,7 +797,7 @@ public Settings upgradeSettings(final Settings settings) { builder.put(upgrade.getKey(), upgrade.getValue()); } } - // we only return a new instance of there was an upgrade + // we only return a new instance if there was an upgrade return changed ? builder.build() : settings; } From f7a019e10ba8c73335cb58d258cdea9b6796212a Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 11:53:20 -0400 Subject: [PATCH 12/16] Fix strict Javadocs checks --- .../org/elasticsearch/common/settings/SettingUpgrader.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java b/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java index a4625aa10d32b..91f2bead300d3 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingUpgrader.java @@ -22,14 +22,14 @@ /** * Represents the logic to upgrade a setting. * - * @param + * @param the type of the underlying setting */ public interface SettingUpgrader { /** * The setting upgraded by this upgrader. * - * @return + * @return the setting */ Setting getSetting(); From 1dc145725a57d406f707b0b9e3467e576a1e4110 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 12:15:05 -0400 Subject: [PATCH 13/16] Use stream collection --- .../test/java/org/elasticsearch/gateway/GatewayTests.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java index 49cb377657ff7..6f8604546af8a 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java @@ -53,11 +53,9 @@ private void runUpgradeSettings( final Setting oldSetting = Setting.simpleString("foo.old", Setting.Property.Dynamic, Setting.Property.NodeScope); final Setting newSetting = Setting.simpleString("foo.new", Setting.Property.Dynamic, Setting.Property.NodeScope); final Set> settingsSet = - new HashSet<>( - Stream.concat( - ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream(), - Stream.of(oldSetting, newSetting)) - .collect(Collectors.toList())); + Stream.concat( + ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream(), + Stream.of(oldSetting, newSetting)).collect(Collectors.toSet())) final ClusterSettings clusterSettings = new ClusterSettings( Settings.EMPTY, settingsSet, From 9510a76683b3894a370a1141117b65411fdff652 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 12:15:12 -0400 Subject: [PATCH 14/16] Remove unused import --- server/src/test/java/org/elasticsearch/gateway/GatewayTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java index 6f8604546af8a..39d16d0d0c478 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java @@ -29,7 +29,6 @@ import org.elasticsearch.test.ESTestCase; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Function; From 81eae98277f9ca6ff6d86ae86cddcbfeb0eb3932 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 12:15:28 -0400 Subject: [PATCH 15/16] Fix compilation --- .../src/test/java/org/elasticsearch/gateway/GatewayTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java index 39d16d0d0c478..d6ee2456eaa97 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java @@ -54,7 +54,7 @@ private void runUpgradeSettings( final Set> settingsSet = Stream.concat( ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream(), - Stream.of(oldSetting, newSetting)).collect(Collectors.toSet())) + Stream.of(oldSetting, newSetting)).collect(Collectors.toSet()); final ClusterSettings clusterSettings = new ClusterSettings( Settings.EMPTY, settingsSet, From be2e5de89707ba8770eafd268b1aaa55b82bba50 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 9 Sep 2018 16:33:06 -0400 Subject: [PATCH 16/16] Use a set --- .../client/transport/TransportClient.java | 2 +- .../common/settings/AbstractScopedSettings.java | 2 +- .../common/settings/ClusterSettings.java | 4 ++-- .../common/settings/IndexScopedSettings.java | 2 +- .../common/settings/SettingsModule.java | 13 +++++++------ .../src/main/java/org/elasticsearch/node/Node.java | 4 ++-- .../admin/indices/get/GetIndexActionTests.java | 8 ++++---- .../settings/get/GetSettingsActionTests.java | 3 ++- .../common/settings/ScopedSettingsTests.java | 8 +++++--- .../common/settings/SettingsModuleTests.java | 6 +++--- .../org/elasticsearch/gateway/GatewayTests.java | 4 +++- .../elasticsearch/test/AbstractBuilderTestCase.java | 2 +- .../xpack/core/security/authc/RealmSettings.java | 2 +- .../org/elasticsearch/test/SettingsFilterTests.java | 3 ++- 14 files changed, 35 insertions(+), 28 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java index b0b6fa73b7c48..39829615fb3fe 100644 --- a/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/server/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -147,7 +147,7 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings additionalSettings.addAll(builder.getRegisteredSettings()); } SettingsModule settingsModule = - new SettingsModule(settings, additionalSettings, additionalSettingsFilter, Collections.emptyList()); + new SettingsModule(settings, additionalSettings, additionalSettingsFilter, Collections.emptySet()); SearchModule searchModule = new SearchModule(settings, true, pluginsService.filterPlugins(SearchPlugin.class)); IndicesModule indicesModule = new IndicesModule(Collections.emptyList()); 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 9ccd04e7742ac..e25d954aa4f1c 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -63,7 +63,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent { protected AbstractScopedSettings( final Settings settings, final Set> settingsSet, - final List> settingUpgraders, + final Set> settingUpgraders, final Setting.Property scope) { super(settings); this.lastSettingsApplied = Settings.EMPTY; diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index dd1f7025b1c98..cb369d6cfda02 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -109,11 +109,11 @@ */ public final class ClusterSettings extends AbstractScopedSettings { public ClusterSettings(final Settings nodeSettings, final Set> settingsSet) { - this(nodeSettings, settingsSet, Collections.emptyList()); + this(nodeSettings, settingsSet, Collections.emptySet()); } public ClusterSettings( - final Settings nodeSettings, final Set> settingsSet, final List> settingUpgraders) { + final Settings nodeSettings, final Set> settingsSet, final Set> settingUpgraders) { super(nodeSettings, settingsSet, settingUpgraders, Property.NodeScope); addSettingsUpdater(new LoggingSettingUpdater(nodeSettings)); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java index 591fe6b06d824..ae8529af5b53e 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java @@ -178,7 +178,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings { public static final IndexScopedSettings DEFAULT_SCOPED_SETTINGS = new IndexScopedSettings(Settings.EMPTY, BUILT_IN_INDEX_SETTINGS); public IndexScopedSettings(Settings settings, Set> settingsSet) { - super(settings, settingsSet, Collections.emptyList(), Property.IndexScope); + super(settings, settingsSet, Collections.emptySet(), Property.IndexScope); } private IndexScopedSettings(Settings settings, IndexScopedSettings other, IndexMetaData metaData) { diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index e2bbe10782d81..1eca3eb415f12 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -29,7 +29,6 @@ import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -55,14 +54,14 @@ public class SettingsModule implements Module { private final SettingsFilter settingsFilter; public SettingsModule(Settings settings, Setting... additionalSettings) { - this(settings, Arrays.asList(additionalSettings), Collections.emptyList(), Collections.emptyList()); + this(settings, Arrays.asList(additionalSettings), Collections.emptyList(), Collections.emptySet()); } public SettingsModule( Settings settings, List> additionalSettings, List settingsFilter, - List> settingUpgraders) { + Set> settingUpgraders) { logger = Loggers.getLogger(getClass(), settings); this.settings = settings; for (Setting setting : ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) { @@ -78,14 +77,16 @@ public SettingsModule( for (String filter : settingsFilter) { registerSettingsFilter(filter); } - final List> clusterSettingUpgraders = new ArrayList<>(); + final Set> clusterSettingUpgraders = new HashSet<>(); for (final SettingUpgrader settingUpgrader : ClusterSettings.BUILT_IN_SETTING_UPGRADERS) { assert settingUpgrader.getSetting().hasNodeScope() : settingUpgrader.getSetting().getKey(); - clusterSettingUpgraders.add(settingUpgrader); + final boolean added = clusterSettingUpgraders.add(settingUpgrader); + assert added : settingUpgrader.getSetting().getKey(); } for (final SettingUpgrader settingUpgrader : settingUpgraders) { assert settingUpgrader.getSetting().hasNodeScope() : settingUpgrader.getSetting().getKey(); - clusterSettingUpgraders.add(settingUpgrader); + final boolean added = clusterSettingUpgraders.add(settingUpgrader); + assert added : settingUpgrader.getSetting().getKey(); } this.indexScopedSettings = new IndexScopedSettings(settings, new HashSet<>(this.indexSettings.values())); this.clusterSettings = new ClusterSettings(settings, new HashSet<>(this.nodeSettings.values()), clusterSettingUpgraders); diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 45aaf6a2d162c..67c3894ddf40a 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -363,11 +363,11 @@ protected Node( // this is as early as we can validate settings at this point. we already pass them to ScriptModule as well as ThreadPool // so we might be late here already - final List> settingsUpgraders = pluginsService.filterPlugins(Plugin.class) + final Set> settingsUpgraders = pluginsService.filterPlugins(Plugin.class) .stream() .map(Plugin::getSettingUpgraders) .flatMap(List::stream) - .collect(Collectors.toList()); + .collect(Collectors.toSet()); final SettingsModule settingsModule = new SettingsModule(this.settings, additionalSettings, additionalSettingsFilter, settingsUpgraders); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java index 61b8cd1ebb1e5..9bf4d9d32f622 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java @@ -40,10 +40,10 @@ import org.junit.After; import org.junit.Before; -import java.util.Collections; import java.util.concurrent.TimeUnit; import static java.util.Collections.emptyList; +import static java.util.Collections.emptySet; public class GetIndexActionTests extends ESSingleNodeTestCase { @@ -60,14 +60,14 @@ public class GetIndexActionTests extends ESSingleNodeTestCase { public void setUp() throws Exception { super.setUp(); - settingsFilter = new SettingsModule(Settings.EMPTY, emptyList(), emptyList(), emptyList()).getSettingsFilter(); + settingsFilter = new SettingsModule(Settings.EMPTY, emptyList(), emptyList(), emptySet()).getSettingsFilter(); threadPool = new TestThreadPool("GetIndexActionTests"); clusterService = getInstanceFromNode(ClusterService.class); indicesService = getInstanceFromNode(IndicesService.class); CapturingTransport capturingTransport = new CapturingTransport(); transportService = capturingTransport.createCapturingTransportService(clusterService.getSettings(), threadPool, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - boundAddress -> clusterService.localNode(), null, Collections.emptySet()); + boundAddress -> clusterService.localNode(), null, emptySet()); transportService.start(); transportService.acceptIncomingRequests(); getIndexAction = new GetIndexActionTests.TestTransportGetIndexAction(); @@ -108,7 +108,7 @@ class TestTransportGetIndexAction extends TransportGetIndexAction { TestTransportGetIndexAction() { super(Settings.EMPTY, GetIndexActionTests.this.transportService, GetIndexActionTests.this.clusterService, - GetIndexActionTests.this.threadPool, settingsFilter, new ActionFilters(Collections.emptySet()), + GetIndexActionTests.this.threadPool, settingsFilter, new ActionFilters(emptySet()), new GetIndexActionTests.Resolver(Settings.EMPTY), indicesService, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java index 41f913a055e4f..85b85cf9e1469 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java @@ -43,6 +43,7 @@ import java.util.concurrent.TimeUnit; import static java.util.Collections.emptyList; +import static java.util.Collections.emptySet; import static org.elasticsearch.test.ClusterServiceUtils.createClusterService; public class GetSettingsActionTests extends ESTestCase { @@ -72,7 +73,7 @@ protected void masterOperation(GetSettingsRequest request, ClusterState state, A public void setUp() throws Exception { super.setUp(); - settingsFilter = new SettingsModule(Settings.EMPTY, emptyList(), emptyList(), emptyList()).getSettingsFilter(); + settingsFilter = new SettingsModule(Settings.EMPTY, emptyList(), emptyList(), emptySet()).getSettingsFilter(); threadPool = new TestThreadPool("GetSettingsActionTests"); clusterService = createClusterService(threadPool); CapturingTransport capturingTransport = new CapturingTransport(); 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 1d640f08e427b..0ee1d2e9c4a80 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -1056,7 +1056,8 @@ public void testUpgradeSetting() { new ClusterSettings( Settings.EMPTY, new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting)), - Collections.singletonList(new SettingUpgrader() { + Collections.singleton(new SettingUpgrader() { + @Override public Setting getSetting() { return oldSetting; @@ -1071,6 +1072,7 @@ public String getKey(final String key) { public String getValue(final String value) { return "new." + value; } + })); final Settings settings = @@ -1095,7 +1097,7 @@ public void testUpgradeSettingsNoChangesPreservesInstance() { new ClusterSettings( Settings.EMPTY, new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting)), - Collections.singletonList(new SettingUpgrader() { + Collections.singleton(new SettingUpgrader() { @Override public Setting getSetting() { @@ -1126,8 +1128,8 @@ public void testUpgradeComplexSetting() { new ClusterSettings( Settings.EMPTY, new HashSet<>(Arrays.asList(oldSetting, newSetting, remainingSetting)), + Collections.singleton(new SettingUpgrader() { - Collections.singletonList(new SettingUpgrader() { @Override public Setting getSetting() { return oldSetting; diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index 5cd6ab7720b1a..c6182eac8f680 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -24,7 +24,7 @@ import java.util.Arrays; -import static java.util.Collections.emptyList; +import static java.util.Collections.emptySet; import static org.hamcrest.Matchers.containsString; public class SettingsModuleTests extends ModuleTestCase { @@ -104,14 +104,14 @@ public void testRegisterSettingsFilter() { try { new SettingsModule(settings, Arrays.asList(Setting.boolSetting("foo.bar", true, Property.NodeScope), Setting.boolSetting("bar.foo", true, Property.NodeScope, Property.Filtered), - Setting.boolSetting("bar.baz", true, Property.NodeScope)), Arrays.asList("foo.*", "bar.foo"), emptyList()); + Setting.boolSetting("bar.baz", true, Property.NodeScope)), Arrays.asList("foo.*", "bar.foo"), emptySet()); fail(); } catch (IllegalArgumentException ex) { assertEquals("filter [bar.foo] has already been registered", ex.getMessage()); } SettingsModule module = new SettingsModule(settings, Arrays.asList(Setting.boolSetting("foo.bar", true, Property.NodeScope), Setting.boolSetting("bar.foo", true, Property.NodeScope, Property.Filtered), - Setting.boolSetting("bar.baz", true, Property.NodeScope)), Arrays.asList("foo.*"), emptyList()); + Setting.boolSetting("bar.baz", true, Property.NodeScope)), Arrays.asList("foo.*"), emptySet()); assertInstanceBinding(module, Settings.class, (s) -> s == settings); assertInstanceBinding(module, SettingsFilter.class, (s) -> s.filter(settings).size() == 1); assertInstanceBinding(module, SettingsFilter.class, (s) -> s.filter(settings).keySet().contains("bar.baz")); diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java index d6ee2456eaa97..457b3a14ebf4a 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayTests.java @@ -58,7 +58,8 @@ private void runUpgradeSettings( final ClusterSettings clusterSettings = new ClusterSettings( Settings.EMPTY, settingsSet, - Collections.singletonList(new SettingUpgrader() { + Collections.singleton(new SettingUpgrader() { + @Override public Setting getSetting() { return oldSetting; @@ -73,6 +74,7 @@ public String getKey(final String key) { public String getValue(final String value) { return "new." + value; } + })); final ClusterService clusterService = new ClusterService(Settings.EMPTY, clusterSettings, null); final Gateway gateway = new Gateway(Settings.EMPTY, clusterService, null, null); diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java index e2469e0f71d5d..60f93f8ea30fe 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java @@ -341,7 +341,7 @@ private static class ServiceHolder implements Closeable { ScriptModule scriptModule = createScriptModule(pluginsService.filterPlugins(ScriptPlugin.class)); List> additionalSettings = pluginsService.getPluginSettings(); SettingsModule settingsModule = - new SettingsModule(nodeSettings, additionalSettings, pluginsService.getPluginSettingsFilter(), Collections.emptyList()); + new SettingsModule(nodeSettings, additionalSettings, pluginsService.getPluginSettingsFilter(), Collections.emptySet()); searchModule = new SearchModule(nodeSettings, false, pluginsService.filterPlugins(SearchPlugin.class)); IndicesModule indicesModule = new IndicesModule(pluginsService.filterPlugins(MapperPlugin.class)); List entries = new ArrayList<>(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java index 1633bbf5611df..daf1775a80a52 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java @@ -183,7 +183,7 @@ private static void validateRealm(String name, String type, Settings settings, S settingSet.add(ENABLED_SETTING); settingSet.add(ORDER_SETTING); final AbstractScopedSettings validator = - new AbstractScopedSettings(settings, settingSet, Collections.emptyList(), Setting.Property.NodeScope) { }; + new AbstractScopedSettings(settings, settingSet, Collections.emptySet(), Setting.Property.NodeScope) { }; try { validator.validate(settings, false); } catch (RuntimeException e) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java index 2b8f3d3b6e81d..3bf3bb4dc8641 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java @@ -20,6 +20,7 @@ import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.TrustManagerFactory; + import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -122,7 +123,7 @@ public void testFiltering() throws Exception { List settingsFilterList = new ArrayList<>(); settingsFilterList.addAll(securityPlugin.getSettingsFilter()); // custom settings, potentially added by a plugin - SettingsModule settingsModule = new SettingsModule(settings, settingList, settingsFilterList, Collections.emptyList()); + SettingsModule settingsModule = new SettingsModule(settings, settingList, settingsFilterList, Collections.emptySet()); Injector injector = Guice.createInjector(settingsModule); SettingsFilter settingsFilter = injector.getInstance(SettingsFilter.class);