diff --git a/qa/ccs-unavailable-clusters/src/test/java/org/elasticsearch/search/CrossClusterSearchUnavailableClusterIT.java b/qa/ccs-unavailable-clusters/src/test/java/org/elasticsearch/search/CrossClusterSearchUnavailableClusterIT.java index fbcf55c91b739..0c42e4be89ac1 100644 --- a/qa/ccs-unavailable-clusters/src/test/java/org/elasticsearch/search/CrossClusterSearchUnavailableClusterIT.java +++ b/qa/ccs-unavailable-clusters/src/test/java/org/elasticsearch/search/CrossClusterSearchUnavailableClusterIT.java @@ -235,7 +235,7 @@ public void testSkipUnavailableDependsOnSeeds() throws IOException { () -> client().performRequest(request)); assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode()); assertThat(responseException.getMessage(), - containsString("Missing required setting [cluster.remote.remote1.seeds] " + + containsString("missing required setting [cluster.remote.remote1.seeds] " + "for setting [cluster.remote.remote1.skip_unavailable]")); } @@ -251,7 +251,7 @@ public void testSkipUnavailableDependsOnSeeds() throws IOException { ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(request)); assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode()); - assertThat(responseException.getMessage(), containsString("Missing required setting [cluster.remote.remote1.seeds] " + + assertThat(responseException.getMessage(), containsString("missing required setting [cluster.remote.remote1.seeds] " + "for setting [cluster.remote.remote1.skip_unavailable]")); } 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 52439f7c89d14..a77d739ffe0b4 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -32,6 +32,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; @@ -461,16 +462,19 @@ void validate( } throw new IllegalArgumentException(msg); } else { - Set settingsDependencies = setting.getSettingsDependencies(key); + 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() + "]"); + for (final Setting settingDependency : settingsDependencies) { + if (settingDependency.existsOrFallbackExists(settings) == false) { + final String message = String.format( + Locale.ROOT, + "missing required setting [%s] for setting [%s]", + settingDependency.getKey(), + setting.getKey()); + throw new IllegalArgumentException(message); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index eabf2ef498406..89bbe752a1ffc 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -366,12 +366,25 @@ public T getDefault(Settings settings) { } /** - * Returns true iff this setting is present in the given settings object. Otherwise false + * Returns true if and only if this setting is present in the given settings instance. Note that fallback settings are excluded. + * + * @param settings the settings + * @return true if the setting is present in the given settings instance, otherwise false */ - public boolean exists(Settings settings) { + public boolean exists(final Settings settings) { return settings.keySet().contains(getKey()); } + /** + * Returns true if and only if this setting including fallback settings is present in the given settings instance. + * + * @param settings the settings + * @return true if the setting including fallback settings is present in the given settings instance, otherwise false + */ + public boolean existsOrFallbackExists(final Settings settings) { + return settings.keySet().contains(getKey()) || (fallbackSetting != null && fallbackSetting.existsOrFallbackExists(settings)); + } + /** * Returns the settings value. If the setting is not present in the given settings object the default value is returned * instead. @@ -511,7 +524,7 @@ public Setting getConcreteSetting(String key) { * 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) { + public Set> getSettingsDependencies(String key) { return Collections.emptySet(); } @@ -634,12 +647,12 @@ private Stream matchStream(Settings settings) { return settings.keySet().stream().filter(this::match).map(key::getConcreteString); } - public Set getSettingsDependencies(String settingsKey) { + 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()); + return dependencies.stream().map(s -> (Setting)s.getConcreteSettingForNamespace(namespace)).collect(Collectors.toSet()); } } @@ -914,40 +927,6 @@ public String toString() { } } - private static class ListSetting extends Setting> { - private final Function> defaultStringValue; - - private ListSetting(String key, Function> defaultStringValue, Function> parser, - Property... properties) { - super(new ListKey(key), (s) -> Setting.arrayToParsableString(defaultStringValue.apply(s)), parser, - properties); - this.defaultStringValue = defaultStringValue; - } - - @Override - String innerGetRaw(final Settings settings) { - List array = settings.getAsList(getKey(), null); - return array == null ? defaultValue.apply(settings) : arrayToParsableString(array); - } - - @Override - boolean hasComplexMatcher() { - return true; - } - - @Override - public void diff(Settings.Builder builder, Settings source, Settings defaultSettings) { - if (exists(source) == false) { - List asList = defaultSettings.getAsList(getKey(), null); - if (asList == null) { - builder.putList(getKey(), defaultStringValue.apply(defaultSettings)); - } else { - builder.putList(getKey(), asList); - } - } - } - } - private final class Updater implements AbstractScopedSettings.SettingUpdater { private final Consumer consumer; private final Logger logger; @@ -1209,26 +1188,44 @@ public static Setting memorySizeSetting(String key, String defaul return new Setting<>(key, (s) -> defaultPercentage, (s) -> MemorySizeValue.parseBytesSizeValueOrHeapRatio(s, key), properties); } - public static Setting> listSetting(String key, List defaultStringValue, Function singleValueParser, - Property... properties) { - return listSetting(key, (s) -> defaultStringValue, singleValueParser, properties); + public static Setting> listSetting( + final String key, + final List defaultStringValue, + final Function singleValueParser, + final Property... properties) { + return listSetting(key, null, singleValueParser, (s) -> defaultStringValue, properties); } // TODO this one's two argument get is still broken - public static Setting> listSetting(String key, Setting> fallbackSetting, Function singleValueParser, - Property... properties) { - return listSetting(key, (s) -> parseableStringToList(fallbackSetting.getRaw(s)), singleValueParser, properties); + public static Setting> listSetting( + final String key, + final Setting> fallbackSetting, + final Function singleValueParser, + final Property... properties) { + return listSetting(key, fallbackSetting, singleValueParser, (s) -> parseableStringToList(fallbackSetting.getRaw(s)), properties); + } + + public static Setting> listSetting( + final String key, + final Function singleValueParser, + final Function> defaultStringValue, + final Property... properties) { + return listSetting(key, null, singleValueParser, defaultStringValue, properties); } - public static Setting> listSetting(String key, Function> defaultStringValue, - Function singleValueParser, Property... properties) { + public static Setting> listSetting( + final String key, + final @Nullable Setting> fallbackSetting, + final Function singleValueParser, + final Function> defaultStringValue, + final Property... properties) { if (defaultStringValue.apply(Settings.EMPTY) == null) { throw new IllegalArgumentException("default value function must not return null"); } Function> parser = (s) -> parseableStringToList(s).stream().map(singleValueParser).collect(Collectors.toList()); - return new ListSetting<>(key, defaultStringValue, parser, properties); + return new ListSetting<>(key, fallbackSetting, defaultStringValue, parser, properties); } private static List parseableStringToList(String parsableString) { @@ -1266,6 +1263,51 @@ private static String arrayToParsableString(List array) { } } + private static class ListSetting extends Setting> { + + private final Function> defaultStringValue; + + private ListSetting( + final String key, + final @Nullable Setting> fallbackSetting, + final Function> defaultStringValue, + final Function> parser, + final Property... properties) { + super( + new ListKey(key), + fallbackSetting, + (s) -> Setting.arrayToParsableString(defaultStringValue.apply(s)), + parser, + (v,s) -> {}, + properties); + this.defaultStringValue = defaultStringValue; + } + + @Override + String innerGetRaw(final Settings settings) { + List array = settings.getAsList(getKey(), null); + return array == null ? defaultValue.apply(settings) : arrayToParsableString(array); + } + + @Override + boolean hasComplexMatcher() { + return true; + } + + @Override + public void diff(Settings.Builder builder, Settings source, Settings defaultSettings) { + if (exists(source) == false) { + List asList = defaultSettings.getAsList(getKey(), null); + if (asList == null) { + builder.putList(getKey(), defaultStringValue.apply(defaultSettings)); + } else { + builder.putList(getKey(), asList); + } + } + } + + } + static void logSettingUpdate(Setting setting, Settings current, Settings previous, Logger logger) { if (logger.isInfoEnabled()) { if (setting.isFiltered()) { 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 af2d874a67941..f0f8b6c417f2f 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -171,7 +171,7 @@ public void testDependentSettings() { 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()); + assertEquals("missing required setting [foo.test.name] for setting [foo.test.bar]", iae.getMessage()); service.validate(Settings.builder() .put("foo.test.name", "test") @@ -181,6 +181,34 @@ public void testDependentSettings() { service.validate(Settings.builder().put("foo.test.bar", 7).build(), false); } + public void testDependentSettingsWithFallback() { + Setting.AffixSetting nameFallbackSetting = + Setting.affixKeySetting("fallback.", "name", k -> Setting.simpleString(k, Property.Dynamic, Property.NodeScope)); + Setting.AffixSetting nameSetting = Setting.affixKeySetting( + "foo.", + "name", + k -> Setting.simpleString( + k, + "_na_".equals(k) + ? nameFallbackSetting.getConcreteSettingForNamespace(k) + : nameFallbackSetting.getConcreteSetting(k.replaceAll("^foo", "fallback")), + Property.Dynamic, + Property.NodeScope)); + Setting.AffixSetting barSetting = + Setting.affixKeySetting("foo.", "bar", k -> Setting.intSetting(k, 1, Property.Dynamic, Property.NodeScope), nameSetting); + + final AbstractScopedSettings service = + new ClusterSettings(Settings.EMPTY,new HashSet<>(Arrays.asList(nameFallbackSetting, nameSetting, barSetting))); + + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> service.validate(Settings.builder().put("foo.test.bar", 7).build(), true)); + assertThat(e, hasToString(containsString("missing required setting [foo.test.name] for setting [foo.test.bar]"))); + + service.validate(Settings.builder().put("foo.test.name", "test").put("foo.test.bar", 7).build(), true); + service.validate(Settings.builder().put("fallback.test.name", "test").put("foo.test.bar", 7).build(), true); + } + public void testTupleAffixUpdateConsumer() { String prefix = randomAlphaOfLength(3) + "foo."; String intSuffix = randomAlphaOfLength(3); diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index d82b620660249..b13988b705059 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -856,4 +856,30 @@ public void testAffixNamespacesWithGroupSetting() { assertThat(affixSetting.getNamespaces(Settings.builder().put("prefix.infix.suffix", "anything").build()), hasSize(1)); assertThat(affixSetting.getNamespaces(Settings.builder().put("prefix.infix.suffix.anything", "anything").build()), hasSize(1)); } + + public void testExists() { + final Setting fooSetting = Setting.simpleString("foo", Property.NodeScope); + assertFalse(fooSetting.exists(Settings.EMPTY)); + assertTrue(fooSetting.exists(Settings.builder().put("foo", "bar").build())); + } + + public void testExistsWithFallback() { + final int count = randomIntBetween(1, 16); + Setting current = Setting.simpleString("fallback0", Property.NodeScope); + for (int i = 1; i < count; i++) { + final Setting next = + new Setting<>(new Setting.SimpleKey("fallback" + i), current, Function.identity(), Property.NodeScope); + current = next; + } + final Setting fooSetting = new Setting<>(new Setting.SimpleKey("foo"), current, Function.identity(), Property.NodeScope); + assertFalse(fooSetting.exists(Settings.EMPTY)); + if (randomBoolean()) { + assertTrue(fooSetting.exists(Settings.builder().put("foo", "bar").build())); + } else { + final String setting = "fallback" + randomIntBetween(0, count - 1); + assertFalse(fooSetting.exists(Settings.builder().put(setting, "bar").build())); + assertTrue(fooSetting.existsOrFallbackExists(Settings.builder().put(setting, "bar").build())); + } + } + } diff --git a/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java b/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java index fbf1dcd5b33ec..33e9af91501d8 100644 --- a/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java @@ -129,18 +129,18 @@ 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()); + 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()); + 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()); + 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() @@ -149,7 +149,7 @@ public void testUpdateDependentClusterSettings() { 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()); + 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(); @@ -161,7 +161,7 @@ public void testUpdateDependentClusterSettings() { 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()); + 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") @@ -173,7 +173,7 @@ public void testUpdateDependentClusterSettings() { 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()); + 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++) { @@ -192,7 +192,7 @@ public void testUpdateDependentIndexSettings() { .put("index.acc.test.pw", "asdf")) .execute() .actionGet()); - assertEquals("Missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); // user has no dependency client() @@ -227,7 +227,7 @@ public void testUpdateDependentIndexSettings() { .putNull("index.acc.test.user")) .execute() .actionGet()); - assertEquals("Missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); // now we are consistent client() diff --git a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java index 0abde8839b44b..9732edb42276e 100644 --- a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java @@ -714,7 +714,7 @@ public void testRemoteClusterSkipIfDisconnectedSetting() { { Settings settings = Settings.builder().put("cluster.remote.foo.skip_unavailable", randomBoolean()).build(); IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> service.validate(settings, true)); - assertEquals("Missing required setting [cluster.remote.foo.seeds] for setting [cluster.remote.foo.skip_unavailable]", + assertEquals("missing required setting [cluster.remote.foo.seeds] for setting [cluster.remote.foo.skip_unavailable]", iae.getMessage()); } { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index e8bcf42233a7b..363cc7bb8827d 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -257,9 +257,11 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw static final Setting> AUDIT_OUTPUTS_SETTING = Setting.listSetting(SecurityField.setting("audit.outputs"), - s -> s.keySet().contains(SecurityField.setting("audit.outputs")) ? - Collections.emptyList() : Collections.singletonList(LoggingAuditTrail.NAME), - Function.identity(), Property.NodeScope); + Function.identity(), + s -> s.keySet().contains(SecurityField.setting("audit.outputs")) + ? Collections.emptyList() + : Collections.singletonList(LoggingAuditTrail.NAME), + Property.NodeScope); private final Settings settings; private final Environment env;