diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdater.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdater.java index 5d1990a48d0..dc13913652a 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdater.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdater.java @@ -54,15 +54,23 @@ final class SettingsUpdater { transientSettings.put(currentState.metaData().transientSettings()); changed |= clusterSettings.updateDynamicSettings(transientToApply, transientSettings, transientUpdates, "transient"); + Settings.Builder persistentSettings = Settings.builder(); persistentSettings.put(currentState.metaData().persistentSettings()); changed |= clusterSettings.updateDynamicSettings(persistentToApply, persistentSettings, persistentUpdates, "persistent"); final ClusterState clusterState; if (changed) { + Settings transientFinalSettings = transientSettings.build(); + Settings persistentFinalSettings = persistentSettings.build(); + // both transient and persistent settings must be consistent by itself we can't allow dependencies to be + // in either of them otherwise a full cluster restart will break the settings validation + clusterSettings.validate(transientFinalSettings, true); + clusterSettings.validate(persistentFinalSettings, true); + MetaData.Builder metaData = MetaData.builder(currentState.metaData()) - .persistentSettings(persistentSettings.build()) - .transientSettings(transientSettings.build()); + .persistentSettings(persistentFinalSettings) + .transientSettings(transientFinalSettings); ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks()); boolean updatedReadOnly = MetaData.SETTING_READ_ONLY_SETTING.get(metaData.persistentSettings()) diff --git a/core/src/main/java/org/elasticsearch/action/admin/indices/template/put/TransportPutIndexTemplateAction.java b/core/src/main/java/org/elasticsearch/action/admin/indices/template/put/TransportPutIndexTemplateAction.java index 7d9897b112e..1624c7950e7 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/indices/template/put/TransportPutIndexTemplateAction.java +++ b/core/src/main/java/org/elasticsearch/action/admin/indices/template/put/TransportPutIndexTemplateAction.java @@ -77,7 +77,7 @@ public class TransportPutIndexTemplateAction extends TransportMasterNodeAction

listener) { Settings.Builder updatedSettingsBuilder = Settings.builder(); - updatedSettingsBuilder.put(request.settings()).normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX); - indexScopedSettings.validate(updatedSettingsBuilder); - request.settings(updatedSettingsBuilder.build()); - + Settings build = updatedSettingsBuilder.put(request.settings()).normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX).build(); + indexScopedSettings.validate(build, true); // we do validate here - index setting must be consistent + request.settings(build); clusterService.submitStateUpdateTask("create-index [" + request.index() + "], cause [" + request.cause() + "]", new IndexCreationTask(logger, allocationService, request, listener, indicesService, aliasValidator, xContentRegistry, settings, this::validate)); @@ -420,7 +419,6 @@ public class MetaDataCreateIndexService extends AbstractComponent { tmpImdBuilder.primaryTerm(shardId, primaryTerm); } } - // Set up everything, now locally create the index to see that things are ok, and apply final IndexMetaData tmpImd = tmpImdBuilder.build(); ActiveShardCount waitForActiveShards = request.waitForActiveShards(); diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java index c96895b94e7..883d7f2fc47 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java @@ -276,7 +276,7 @@ public class MetaDataIndexTemplateService extends AbstractComponent { } try { - indexScopedSettings.validate(request.settings); + indexScopedSettings.validate(request.settings, true); // templates must be consistent with regards to dependencies } catch (IllegalArgumentException iae) { validationErrors.add(iae.getMessage()); for (Throwable t : iae.getSuppressed()) { diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java index abc0a4e8ea2..2c0bc929294 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java @@ -54,6 +54,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.function.Predicate; import static org.elasticsearch.action.support.ContextPreservingActionListener.wrapPreservingContext; @@ -163,7 +164,7 @@ public class MetaDataUpdateSettingsService extends AbstractComponent implements Settings.Builder settingsForOpenIndices = Settings.builder(); final Set skippedSettings = new HashSet<>(); - indexScopedSettings.validate(normalizedSettings); + indexScopedSettings.validate(normalizedSettings, false); // don't validate dependencies here we check it below // never allow to change the number of shards for (String key : normalizedSettings.keySet()) { Setting setting = indexScopedSettings.get(key); @@ -240,7 +241,9 @@ public class MetaDataUpdateSettingsService extends AbstractComponent implements if (preserveExisting) { indexSettings.put(indexMetaData.getSettings()); } - metaDataBuilder.put(IndexMetaData.builder(indexMetaData).settings(indexSettings)); + Settings finalSettings = indexSettings.build(); + indexScopedSettings.validate(finalSettings.filter(k -> indexScopedSettings.isPrivateSetting(k) == false), true); + metaDataBuilder.put(IndexMetaData.builder(indexMetaData).settings(finalSettings)); } } } @@ -254,7 +257,9 @@ public class MetaDataUpdateSettingsService extends AbstractComponent implements if (preserveExisting) { indexSettings.put(indexMetaData.getSettings()); } - metaDataBuilder.put(IndexMetaData.builder(indexMetaData).settings(indexSettings)); + Settings finalSettings = indexSettings.build(); + indexScopedSettings.validate(finalSettings.filter(k -> indexScopedSettings.isPrivateSetting(k) == false), true); + metaDataBuilder.put(IndexMetaData.builder(indexMetaData).settings(finalSettings)); } } } diff --git a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index 61f32c67c20..38eaef1d14d 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -264,20 +264,16 @@ public abstract class AbstractScopedSettings extends AbstractComponent { } /** - * Validates that all settings in the builder are registered and valid + * Validates that all given settings are registered and valid + * @param settings the settings to validate + * @param validateDependencies if true settings dependencies are validated as well. + * @see Setting#getSettingsDependencies(String) */ - public final void validate(Settings.Builder settingsBuilder) { - validate(settingsBuilder.build()); - } - - /** - * * Validates that all given settings are registered and valid - */ - public final void validate(Settings settings) { + public final void validate(Settings settings, boolean validateDependencies) { List exceptions = new ArrayList<>(); for (String key : settings.keySet()) { // settings iterate in deterministic fashion try { - validate(key, settings); + validate(key, settings, validateDependencies); } catch (RuntimeException ex) { exceptions.add(ex); } @@ -285,12 +281,11 @@ public abstract class AbstractScopedSettings extends AbstractComponent { ExceptionsHelper.rethrowAndSuppress(exceptions); } - /** * Validates that the setting is valid */ - public final void validate(String key, Settings settings) { - Setting setting = get(key); + void validate(String key, Settings settings, boolean validateDependencies) { + Setting setting = getRaw(key); if (setting == null) { LevensteinDistance ld = new LevensteinDistance(); List> scoredKeys = new ArrayList<>(); @@ -315,6 +310,20 @@ public abstract class AbstractScopedSettings extends AbstractComponent { "settings"; } throw new IllegalArgumentException(msg); + } else { + Set settingsDependencies = setting.getSettingsDependencies(key); + if (setting.hasComplexMatcher()) { + setting = setting.getConcreteSetting(key); + } + if (validateDependencies && settingsDependencies.isEmpty() == false) { + Set settingKeys = settings.keySet(); + for (String requiredSetting : settingsDependencies) { + if (settingKeys.contains(requiredSetting) == false) { + throw new IllegalArgumentException("Missing required setting [" + + requiredSetting + "] for setting [" + setting.getKey() + "]"); + } + } + } } setting.get(settings); } @@ -375,7 +384,18 @@ public abstract class AbstractScopedSettings extends AbstractComponent { /** * Returns the {@link Setting} for the given key or null if the setting can not be found. */ - public Setting get(String key) { + public final Setting get(String key) { + Setting raw = getRaw(key); + if (raw == null) { + return null; + } if (raw.hasComplexMatcher()) { + return raw.getConcreteSetting(key); + } else { + return raw; + } + } + + private Setting getRaw(String key) { Setting setting = keySettings.get(key); if (setting != null) { return setting; @@ -383,7 +403,8 @@ public abstract class AbstractScopedSettings extends AbstractComponent { for (Map.Entry> entry : complexMatchers.entrySet()) { if (entry.getValue().match(key)) { assert assertMatcher(key, 1); - return entry.getValue().getConcreteSetting(key); + assert entry.getValue().hasComplexMatcher(); + return entry.getValue(); } } return null; @@ -513,7 +534,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent { } else if (get(key) == null) { throw new IllegalArgumentException(type + " setting [" + key + "], not recognized"); } else if (isNull == false && canUpdate.test(key)) { - validate(key, toApply); + validate(key, toApply, false); // we might not have a full picture here do to a dependency validation settingsBuilder.copy(key, toApply); updates.copy(key, toApply); changed = true; @@ -654,7 +675,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent { * representation. Otherwise false */ // TODO this should be replaced by Setting.Property.HIDDEN or something like this. - protected boolean isPrivateSetting(String key) { + public boolean isPrivateSetting(String key) { return false; } } diff --git a/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java index 962e61b5c3c..d40488eaa34 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java @@ -191,7 +191,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings { } @Override - protected boolean isPrivateSetting(String key) { + public boolean isPrivateSetting(String key) { switch (key) { case IndexMetaData.SETTING_CREATION_DATE: case IndexMetaData.SETTING_INDEX_UUID: diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java index 9b99e67c8c4..abc589aedaf 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -42,6 +42,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.IdentityHashMap; import java.util.Iterator; import java.util.List; @@ -126,7 +127,7 @@ public class Setting implements ToXContentObject { private static final EnumSet EMPTY_PROPERTIES = EnumSet.noneOf(Property.class); private Setting(Key key, @Nullable Setting fallbackSetting, Function defaultValue, Function parser, - Validator validator, Property... properties) { + Validator validator, Property... properties) { assert this instanceof SecureSetting || this.isGroupSetting() || parser.apply(defaultValue.apply(Settings.EMPTY)) != null : "parser returned null"; this.key = key; @@ -457,6 +458,14 @@ public class Setting implements ToXContentObject { return this; } + /** + * Returns a set of settings that are required at validation time. Unless all of the dependencies are present in the settings + * object validation of setting must fail. + */ + public Set getSettingsDependencies(String key) { + return Collections.emptySet(); + } + /** * Build a new updater with a noop validator. */ @@ -519,11 +528,13 @@ public class Setting implements ToXContentObject { public static class AffixSetting extends Setting { private final AffixKey key; private final Function> delegateFactory; + private final Set dependencies; - public AffixSetting(AffixKey key, Setting delegate, Function> delegateFactory) { + public AffixSetting(AffixKey key, Setting delegate, Function> delegateFactory, AffixSetting... dependencies) { super(key, delegate.defaultValue, delegate.parser, delegate.properties.toArray(new Property[0])); this.key = key; this.delegateFactory = delegateFactory; + this.dependencies = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(dependencies))); } boolean isGroupSetting() { @@ -534,6 +545,15 @@ public class Setting implements ToXContentObject { return settings.keySet().stream().filter((key) -> match(key)).map(settingKey -> key.getConcreteString(settingKey)); } + public Set getSettingsDependencies(String settingsKey) { + if (dependencies.isEmpty()) { + return Collections.emptySet(); + } else { + String namespace = key.getNamespace(settingsKey); + return dependencies.stream().map(s -> s.key.toConcreteKey(namespace).key).collect(Collectors.toSet()); + } + } + AbstractScopedSettings.SettingUpdater, T>> newAffixUpdater( BiConsumer consumer, Logger logger, BiConsumer validator) { return new AbstractScopedSettings.SettingUpdater, T>>() { @@ -659,6 +679,13 @@ public class Setting implements ToXContentObject { return matchStream(settings).distinct().map(this::getConcreteSetting); } + /** + * Returns distinct namespaces for the given settings + */ + public Set getNamespaces(Settings settings) { + return settings.keySet().stream().filter(this::match).map(key::getNamespace).collect(Collectors.toSet()); + } + /** * Returns a map of all namespaces to it's values give the provided settings */ @@ -1184,13 +1211,15 @@ public class Setting implements ToXContentObject { * storage.${backend}.enable=[true|false] can easily be added with this setting. Yet, affix key settings don't support updaters * out of the box unless {@link #getConcreteSetting(String)} is used to pull the updater. */ - public static AffixSetting affixKeySetting(String prefix, String suffix, Function> delegateFactory) { - return affixKeySetting(new AffixKey(prefix, suffix), delegateFactory); + public static AffixSetting affixKeySetting(String prefix, String suffix, Function> delegateFactory, + AffixSetting... dependencies) { + return affixKeySetting(new AffixKey(prefix, suffix), delegateFactory, dependencies); } - private static AffixSetting affixKeySetting(AffixKey key, Function> delegateFactory) { + private static AffixSetting affixKeySetting(AffixKey key, Function> delegateFactory, + AffixSetting... dependencies) { Setting delegate = delegateFactory.apply("_na_"); - return new AffixSetting<>(key, delegate, delegateFactory); + return new AffixSetting<>(key, delegate, delegateFactory, dependencies); }; diff --git a/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 45b511e1cc1..0304b20e992 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -132,7 +132,7 @@ public class SettingsModule implements Module { } } // by now we are fully configured, lets check node level settings for unregistered index settings - clusterSettings.validate(settings); + clusterSettings.validate(settings, true); this.settingsFilter = new SettingsFilter(settings, settingsFilterPattern); } diff --git a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java index bd4ac25a874..2015a6b42d1 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -122,7 +122,7 @@ public class ScopedSettingsTests extends ESTestCase { Settings.Builder builder = Settings.builder(); Settings updates = Settings.builder().putNull("index.routing.allocation.require._ip") .put("index.some.dyn.setting", 1).build(); - settings.validate(updates); + settings.validate(updates, false); settings.updateDynamicSettings(updates, Settings.builder().put(currentSettings), builder, "node"); currentSettings = builder.build(); @@ -160,6 +160,26 @@ public class ScopedSettingsTests extends ESTestCase { assertEquals(0, consumer2.get()); } + public void testDependentSettings() { + Setting.AffixSetting stringSetting = Setting.affixKeySetting("foo.", "name", + (k) -> Setting.simpleString(k, Property.Dynamic, Property.NodeScope)); + Setting.AffixSetting intSetting = Setting.affixKeySetting("foo.", "bar", + (k) -> Setting.intSetting(k, 1, Property.Dynamic, Property.NodeScope), stringSetting); + + AbstractScopedSettings service = new ClusterSettings(Settings.EMPTY,new HashSet<>(Arrays.asList(intSetting, stringSetting))); + + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, + () -> service.validate(Settings.builder().put("foo.test.bar", 7).build(), true)); + assertEquals("Missing required setting [foo.test.name] for setting [foo.test.bar]", iae.getMessage()); + + service.validate(Settings.builder() + .put("foo.test.name", "test") + .put("foo.test.bar", 7) + .build(), true); + + service.validate(Settings.builder().put("foo.test.bar", 7).build(), false); + } + public void testAddConsumerAffix() { Setting.AffixSetting intSetting = Setting.affixKeySetting("foo.", "bar", (k) -> Setting.intSetting(k, 1, Property.Dynamic, Property.NodeScope)); @@ -585,7 +605,7 @@ public class ScopedSettingsTests extends ESTestCase { Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, - () -> settings.validate(Settings.builder().put("index.numbe_of_replica", "1").build())); + () -> settings.validate(Settings.builder().put("index.numbe_of_replica", "1").build(), false)); assertEquals(iae.getMessage(), "unknown setting [index.numbe_of_replica] did you mean [index.number_of_replicas]?"); } @@ -595,26 +615,23 @@ public class ScopedSettingsTests extends ESTestCase { IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); String unknownMsgSuffix = " please check that any required plugins are installed, or check the breaking changes documentation for" + " removed settings"; - settings.validate(Settings.builder().put("index.store.type", "boom")); - settings.validate(Settings.builder().put("index.store.type", "boom").build()); + settings.validate(Settings.builder().put("index.store.type", "boom").build(), false); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> - settings.validate(Settings.builder().put("index.store.type", "boom").put("i.am.not.a.setting", true))); + settings.validate(Settings.builder().put("index.store.type", "boom").put("i.am.not.a.setting", true).build(), false)); assertEquals("unknown setting [i.am.not.a.setting]" + unknownMsgSuffix, e.getMessage()); e = expectThrows(IllegalArgumentException.class, () -> - settings.validate(Settings.builder().put("index.store.type", "boom").put("i.am.not.a.setting", true).build())); - assertEquals("unknown setting [i.am.not.a.setting]" + unknownMsgSuffix, e.getMessage()); - - e = expectThrows(IllegalArgumentException.class, () -> - settings.validate(Settings.builder().put("index.store.type", "boom").put("index.number_of_replicas", true).build())); + settings.validate(Settings.builder().put("index.store.type", "boom").put("index.number_of_replicas", true).build(), false)); assertEquals("Failed to parse value [true] for setting [index.number_of_replicas]", e.getMessage()); e = expectThrows(IllegalArgumentException.class, () -> - settings.validate("index.number_of_replicas", Settings.builder().put("index.number_of_replicas", "true").build())); + settings.validate("index.number_of_replicas", Settings.builder().put("index.number_of_replicas", "true").build(), false)); assertEquals("Failed to parse value [true] for setting [index.number_of_replicas]", e.getMessage()); e = expectThrows(IllegalArgumentException.class, () -> - settings.validate("index.similarity.classic.type", Settings.builder().put("index.similarity.classic.type", "mine").build())); + settings.validate("index.similarity.classic.type", Settings.builder().put("index.similarity.classic.type", "mine").build(), + false)); assertEquals("illegal value for [index.similarity.classic] cannot redefine built-in similarity", e.getMessage()); } @@ -624,12 +641,12 @@ public class ScopedSettingsTests extends ESTestCase { Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); final ClusterSettings clusterSettings = new ClusterSettings(settings, Collections.emptySet()); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> clusterSettings.validate(settings)); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> clusterSettings.validate(settings, false)); assertThat(e.getMessage(), startsWith("unknown secure setting [some.secure.setting]")); ClusterSettings clusterSettings2 = new ClusterSettings(settings, Collections.singleton(SecureSetting.secureString("some.secure.setting", null))); - clusterSettings2.validate(settings); + clusterSettings2.validate(settings, false); } public void testDiffSecureSettings() { @@ -722,7 +739,7 @@ public class ScopedSettingsTests extends ESTestCase { IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, - () -> settings.validate(Settings.builder().put("logger._root", "boom").build())); + () -> settings.validate(Settings.builder().put("logger._root", "boom").build(), false)); assertEquals("Unknown level constant [BOOM].", ex.getMessage()); assertEquals(level, ESLoggerFactory.getRootLogger().getLevel()); settings.applySettings(Settings.builder().put("logger._root", "TRACE").build()); diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java index 65d51e126c9..4a4beb2e0e3 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -30,6 +30,7 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.stream.Collectors; @@ -42,6 +43,7 @@ import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; public class SettingTests extends ESTestCase { + public void testGet() { Setting booleanSetting = Setting.boolSetting("foo.bar", false, Property.Dynamic, Property.NodeScope); assertFalse(booleanSetting.get(Settings.EMPTY)); @@ -577,6 +579,22 @@ public class SettingTests extends ESTestCase { assertFalse(listAffixSetting.match("foo")); } + public void testAffixSettingNamespaces() { + Setting.AffixSetting setting = + Setting.affixKeySetting("foo.", "enable", (key) -> Setting.boolSetting(key, false, Property.NodeScope)); + Settings build = Settings.builder() + .put("foo.bar.enable", "true") + .put("foo.baz.enable", "true") + .put("foo.boom.enable", "true") + .put("something.else", "true") + .build(); + Set namespaces = setting.getNamespaces(build); + assertEquals(3, namespaces.size()); + assertTrue(namespaces.contains("bar")); + assertTrue(namespaces.contains("baz")); + assertTrue(namespaces.contains("boom")); + } + public void testAffixAsMap() { Setting.AffixSetting setting = Setting.prefixKeySetting("foo.bar.", key -> Setting.simpleString(key, Property.NodeScope)); diff --git a/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java b/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java index 6be786aff88..79c306f43f1 100644 --- a/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java @@ -498,7 +498,7 @@ public class IndexSettingsTests extends ESTestCase { assertTrue(index.isSingleType()); expectThrows(IllegalArgumentException.class, () -> { index.getScopedSettings() - .validate(Settings.builder().put(IndexSettings.INDEX_MAPPING_SINGLE_TYPE_SETTING_KEY, randomBoolean()).build()); + .validate(Settings.builder().put(IndexSettings.INDEX_MAPPING_SINGLE_TYPE_SETTING_KEY, randomBoolean()).build(), false); }); } { diff --git a/core/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java b/core/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java index e8c1604c799..7ff0725449e 100644 --- a/core/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java +++ b/core/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java @@ -85,6 +85,17 @@ public class UpdateSettingsIT extends ESIntegTestCase { public static class DummySettingPlugin extends Plugin { public static final Setting DUMMY_SETTING = Setting.simpleString("index.dummy", Setting.Property.IndexScope, Setting.Property.Dynamic); + + public static final Setting.AffixSetting DUMMY_ACCOUNT_USER = Setting.affixKeySetting("index.acc.", "user", + k -> Setting.simpleString(k, Setting.Property.IndexScope, Setting.Property.Dynamic)); + public static final Setting DUMMY_ACCOUNT_PW = Setting.affixKeySetting("index.acc.", "pw", + k -> Setting.simpleString(k, Setting.Property.IndexScope, Setting.Property.Dynamic), DUMMY_ACCOUNT_USER); + + public static final Setting.AffixSetting DUMMY_ACCOUNT_USER_CLUSTER = Setting.affixKeySetting("cluster.acc.", "user", + k -> Setting.simpleString(k, Setting.Property.NodeScope, Setting.Property.Dynamic)); + public static final Setting DUMMY_ACCOUNT_PW_CLUSTER = Setting.affixKeySetting("cluster.acc.", "pw", + k -> Setting.simpleString(k, Setting.Property.NodeScope, Setting.Property.Dynamic), DUMMY_ACCOUNT_USER_CLUSTER); + @Override public void onIndexModule(IndexModule indexModule) { indexModule.addSettingsUpdateConsumer(DUMMY_SETTING, (s) -> {}, (s) -> { @@ -95,7 +106,8 @@ public class UpdateSettingsIT extends ESIntegTestCase { @Override public List> getSettings() { - return Collections.singletonList(DUMMY_SETTING); + return Arrays.asList(DUMMY_SETTING, DUMMY_ACCOUNT_PW, DUMMY_ACCOUNT_USER, + DUMMY_ACCOUNT_PW_CLUSTER, DUMMY_ACCOUNT_USER_CLUSTER); } } @@ -112,6 +124,124 @@ public class UpdateSettingsIT extends ESIntegTestCase { } } + public void testUpdateDependentClusterSettings() { + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> + client().admin().cluster().prepareUpdateSettings().setPersistentSettings(Settings.builder() + .put("cluster.acc.test.pw", "asdf")).get()); + assertEquals("Missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + + iae = expectThrows(IllegalArgumentException.class, () -> + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder() + .put("cluster.acc.test.pw", "asdf")).get()); + assertEquals("Missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + + iae = expectThrows(IllegalArgumentException.class, () -> + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder() + .put("cluster.acc.test.pw", "asdf")).setPersistentSettings(Settings.builder() + .put("cluster.acc.test.user", "asdf")).get()); + assertEquals("Missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + + if (randomBoolean()) { + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder() + .put("cluster.acc.test.pw", "asdf") + .put("cluster.acc.test.user", "asdf")).get(); + iae = expectThrows(IllegalArgumentException.class, () -> + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder() + .putNull("cluster.acc.test.user")).get()); + assertEquals("Missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder() + .putNull("cluster.acc.test.pw") + .putNull("cluster.acc.test.user")).get(); + } else { + client().admin().cluster().prepareUpdateSettings().setPersistentSettings(Settings.builder() + .put("cluster.acc.test.pw", "asdf") + .put("cluster.acc.test.user", "asdf")).get(); + + iae = expectThrows(IllegalArgumentException.class, () -> + client().admin().cluster().prepareUpdateSettings().setPersistentSettings(Settings.builder() + .putNull("cluster.acc.test.user")).get()); + assertEquals("Missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + + client().admin().cluster().prepareUpdateSettings().setPersistentSettings(Settings.builder() + .putNull("cluster.acc.test.pw") + .putNull("cluster.acc.test.user")).get(); + } + + } + + public void testUpdateDependentIndexSettings() { + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> + prepareCreate("test", Settings.builder().put("index.acc.test.pw", "asdf")).get()); + assertEquals("Missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); + + createIndex("test"); + for (int i = 0; i < 2; i++) { + if (i == 1) { + // now do it on a closed index + client().admin().indices().prepareClose("test").get(); + } + + iae = expectThrows(IllegalArgumentException.class, () -> + client() + .admin() + .indices() + .prepareUpdateSettings("test") + .setSettings( + Settings.builder() + .put("index.acc.test.pw", "asdf")) + .execute() + .actionGet()); + assertEquals("Missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); + + // user has no dependency + client() + .admin() + .indices() + .prepareUpdateSettings("test") + .setSettings( + Settings.builder() + .put("index.acc.test.user", "asdf")) + .execute() + .actionGet(); + + // now we are consistent + client() + .admin() + .indices() + .prepareUpdateSettings("test") + .setSettings( + Settings.builder() + .put("index.acc.test.pw", "test")) + .execute() + .actionGet(); + + // now try to remove it and make sure it fails + iae = expectThrows(IllegalArgumentException.class, () -> + client() + .admin() + .indices() + .prepareUpdateSettings("test") + .setSettings( + Settings.builder() + .putNull("index.acc.test.user")) + .execute() + .actionGet()); + assertEquals("Missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); + + // now we are consistent + client() + .admin() + .indices() + .prepareUpdateSettings("test") + .setSettings( + Settings.builder() + .putNull("index.acc.test.pw") + .putNull("index.acc.test.user")) + .execute() + .actionGet(); + } + } + public void testResetDefault() { createIndex("test"); diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java index 472ab121e83..e360558933c 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java @@ -38,44 +38,47 @@ import java.util.Collections; import java.util.HashMap; import java.util.Locale; import java.util.Map; -import java.util.Set; public final class AzureStorageSettings { + // prefix for azure client settings - private static final String PREFIX = "azure.client."; + private static final String AZURE_CLIENT_PREFIX_KEY = "azure.client."; /** Azure account name */ public static final AffixSetting ACCOUNT_SETTING = - Setting.affixKeySetting(PREFIX, "account", key -> SecureSetting.secureString(key, null)); + Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "account", key -> SecureSetting.secureString(key, null)); + + /** Azure key */ + public static final AffixSetting KEY_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "key", + key -> SecureSetting.secureString(key, null)); /** max_retries: Number of retries in case of Azure errors. Defaults to 3 (RetryPolicy.DEFAULT_CLIENT_RETRY_COUNT). */ private static final Setting MAX_RETRIES_SETTING = - Setting.affixKeySetting(PREFIX, "max_retries", - (key) -> Setting.intSetting(key, RetryPolicy.DEFAULT_CLIENT_RETRY_COUNT, Setting.Property.NodeScope)); + Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "max_retries", + (key) -> Setting.intSetting(key, RetryPolicy.DEFAULT_CLIENT_RETRY_COUNT, Setting.Property.NodeScope), + ACCOUNT_SETTING, KEY_SETTING); /** * Azure endpoint suffix. Default to core.windows.net (CloudStorageAccount.DEFAULT_DNS). */ - public static final Setting ENDPOINT_SUFFIX_SETTING = Setting.affixKeySetting(PREFIX, "endpoint_suffix", - key -> Setting.simpleString(key, Property.NodeScope)); + public static final Setting ENDPOINT_SUFFIX_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "endpoint_suffix", + key -> Setting.simpleString(key, Property.NodeScope), ACCOUNT_SETTING, KEY_SETTING); - /** Azure key */ - public static final AffixSetting KEY_SETTING = Setting.affixKeySetting(PREFIX, "key", - key -> SecureSetting.secureString(key, null)); - - public static final AffixSetting TIMEOUT_SETTING = Setting.affixKeySetting(PREFIX, "timeout", - (key) -> Setting.timeSetting(key, TimeValue.timeValueMinutes(-1), Property.NodeScope)); + public static final AffixSetting TIMEOUT_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "timeout", + (key) -> Setting.timeSetting(key, TimeValue.timeValueMinutes(-1), Property.NodeScope), ACCOUNT_SETTING, KEY_SETTING); /** The type of the proxy to connect to azure through. Can be direct (no proxy, default), http or socks */ - public static final AffixSetting PROXY_TYPE_SETTING = Setting.affixKeySetting(PREFIX, "proxy.type", - (key) -> new Setting<>(key, "direct", s -> Proxy.Type.valueOf(s.toUpperCase(Locale.ROOT)), Property.NodeScope)); + public static final AffixSetting PROXY_TYPE_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "proxy.type", + (key) -> new Setting<>(key, "direct", s -> Proxy.Type.valueOf(s.toUpperCase(Locale.ROOT)), Property.NodeScope) + , ACCOUNT_SETTING, KEY_SETTING); /** The host name of a proxy to connect to azure through. */ - public static final Setting PROXY_HOST_SETTING = Setting.affixKeySetting(PREFIX, "proxy.host", - (key) -> Setting.simpleString(key, Property.NodeScope)); + public static final AffixSetting PROXY_HOST_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "proxy.host", + (key) -> Setting.simpleString(key, Property.NodeScope), KEY_SETTING, ACCOUNT_SETTING, PROXY_TYPE_SETTING); /** The port of a proxy to connect to azure through. */ - public static final Setting PROXY_PORT_SETTING = Setting.affixKeySetting(PREFIX, "proxy.port", - (key) -> Setting.intSetting(key, 0, 0, 65535, Setting.Property.NodeScope)); + public static final Setting PROXY_PORT_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "proxy.port", + (key) -> Setting.intSetting(key, 0, 0, 65535, Setting.Property.NodeScope), ACCOUNT_SETTING, KEY_SETTING, PROXY_TYPE_SETTING, + PROXY_HOST_SETTING); private final String account; private final String key; @@ -157,9 +160,8 @@ public final class AzureStorageSettings { */ public static Map load(Settings settings) { // Get the list of existing named configurations - Set clientNames = settings.getGroups(PREFIX).keySet(); Map storageSettings = new HashMap<>(); - for (String clientName : clientNames) { + for (String clientName : ACCOUNT_SETTING.getNamespaces(settings)) { storageSettings.put(clientName, getClientSettings(settings, clientName)); } diff --git a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java index 50fd071accf..dcd1d650628 100644 --- a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java +++ b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java @@ -40,6 +40,7 @@ import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.util.iterable.Iterables; import org.elasticsearch.env.Environment; import java.io.IOException; @@ -54,10 +55,8 @@ import java.util.Set; interface GoogleCloudStorageService { - String SETTINGS_PREFIX = "gcs.client."; - /** A json credentials file loaded from secure settings. */ - Setting.AffixSetting CREDENTIALS_FILE_SETTING = Setting.affixKeySetting(SETTINGS_PREFIX, "credentials_file", + Setting.AffixSetting CREDENTIALS_FILE_SETTING = Setting.affixKeySetting("gcs.client.", "credentials_file", key -> SecureSetting.secureFile(key, null)); /** @@ -176,16 +175,15 @@ interface GoogleCloudStorageService { /** Load all secure credentials from the settings. */ static Map loadClientCredentials(Settings settings) { - Set clientNames = settings.getGroups(SETTINGS_PREFIX).keySet(); Map credentials = new HashMap<>(); - for (String clientName : clientNames) { - Setting concreteSetting = CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName); + Iterable> iterable = CREDENTIALS_FILE_SETTING.getAllConcreteSettings(settings)::iterator; + for (Setting concreteSetting : iterable) { try (InputStream credStream = concreteSetting.get(settings)) { GoogleCredential credential = GoogleCredential.fromStream(credStream); if (credential.createScopedRequired()) { credential = credential.createScoped(Collections.singleton(StorageScopes.DEVSTORAGE_FULL_CONTROL)); } - credentials.put(clientName, credential); + credentials.put(CREDENTIALS_FILE_SETTING.getNamespace(concreteSetting), credential); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java index 9230a4eb248..2cd4ef94ae0 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java @@ -2568,7 +2568,7 @@ public abstract class AbstractSimpleTransportTestCase extends ESTestCase { Settings randomSettings = randomFrom(random(), globalSettings, transportSettings, profileSettings); ClusterSettings clusterSettings = new ClusterSettings(randomSettings, ClusterSettings .BUILT_IN_CLUSTER_SETTINGS); - clusterSettings.validate(randomSettings); + clusterSettings.validate(randomSettings, false); TcpTransport.ProfileSettings settings = new TcpTransport.ProfileSettings( Settings.builder().put(randomSettings).put("transport.profiles.some_profile.port", "9700-9800").build(), // port is required "some_profile");