diff --git a/plugins/examples/custom-settings/src/main/java/org/elasticsearch/example/customsettings/ExampleCustomSettingsConfig.java b/plugins/examples/custom-settings/src/main/java/org/elasticsearch/example/customsettings/ExampleCustomSettingsConfig.java index fafe3615f63..ffc7b4366b5 100644 --- a/plugins/examples/custom-settings/src/main/java/org/elasticsearch/example/customsettings/ExampleCustomSettingsConfig.java +++ b/plugins/examples/custom-settings/src/main/java/org/elasticsearch/example/customsettings/ExampleCustomSettingsConfig.java @@ -49,7 +49,7 @@ public class ExampleCustomSettingsConfig { /** * A string setting that can be dynamically updated and that is validated by some logic */ - static final Setting VALIDATED_SETTING = Setting.simpleString("custom.validated", (value, settings) -> { + static final Setting VALIDATED_SETTING = Setting.simpleString("custom.validated", value -> { if (value != null && value.contains("forbidden")) { throw new IllegalArgumentException("Setting must not contain [forbidden]"); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java index 9361c877d38..73e3c6b67ec 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java @@ -157,6 +157,10 @@ public class IndexMetaData implements Diffable, ToXContentFragmen public static final Setting INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING = Setting.intSetting("index.number_of_routing_shards", INDEX_NUMBER_OF_SHARDS_SETTING, 1, new Setting.Validator() { + @Override + public void validate(Integer value) { + } + @Override public void validate(Integer numRoutingShards, Map, Integer> settings) { Integer numShards = settings.get(INDEX_NUMBER_OF_SHARDS_SETTING); @@ -223,14 +227,14 @@ public class IndexMetaData implements Diffable, ToXContentFragmen public static final String INDEX_ROUTING_INCLUDE_GROUP_PREFIX = "index.routing.allocation.include"; public static final String INDEX_ROUTING_EXCLUDE_GROUP_PREFIX = "index.routing.allocation.exclude"; public static final Setting.AffixSetting INDEX_ROUTING_REQUIRE_GROUP_SETTING = - Setting.prefixKeySetting(INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".", (key) -> - Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope)); + Setting.prefixKeySetting(INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".", key -> + Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope)); public static final Setting.AffixSetting INDEX_ROUTING_INCLUDE_GROUP_SETTING = - Setting.prefixKeySetting(INDEX_ROUTING_INCLUDE_GROUP_PREFIX + ".", (key) -> - Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope)); + Setting.prefixKeySetting(INDEX_ROUTING_INCLUDE_GROUP_PREFIX + ".", key -> + Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope)); public static final Setting.AffixSetting INDEX_ROUTING_EXCLUDE_GROUP_SETTING = - Setting.prefixKeySetting(INDEX_ROUTING_EXCLUDE_GROUP_PREFIX + ".", (key) -> - Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope)); + Setting.prefixKeySetting(INDEX_ROUTING_EXCLUDE_GROUP_PREFIX + ".", key -> + Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope)); public static final Setting.AffixSetting INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING = Setting.prefixKeySetting("index.routing.allocation.initial_recovery.", key -> Setting.simpleString(key)); // this is only setable internally not a registered setting!! diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java index ccd64827b32..b8d234e9f10 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java @@ -93,6 +93,10 @@ public class DiskThresholdSettings { static final class LowDiskWatermarkValidator implements Setting.Validator { + @Override + public void validate(String value) { + } + @Override public void validate(String value, Map, String> settings) { final String highWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING); @@ -112,6 +116,10 @@ public class DiskThresholdSettings { static final class HighDiskWatermarkValidator implements Setting.Validator { + @Override + public void validate(String value) { + } + @Override public void validate(String value, Map, String> settings) { final String lowWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING); @@ -131,6 +139,10 @@ public class DiskThresholdSettings { static final class FloodStageValidator implements Setting.Validator { + @Override + public void validate(String value) { + } + @Override public void validate(String value, Map, String> settings) { final String lowWatermarkRaw = settings.get(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING); diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java index 053d696f676..7d24d463185 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java @@ -72,14 +72,14 @@ public class FilterAllocationDecider extends AllocationDecider { private static final String CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX = "cluster.routing.allocation.include"; private static final String CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX = "cluster.routing.allocation.exclude"; public static final Setting.AffixSetting CLUSTER_ROUTING_REQUIRE_GROUP_SETTING = - Setting.prefixKeySetting(CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX + ".", (key) -> - Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope)); + Setting.prefixKeySetting(CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX + ".", key -> + Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope)); public static final Setting.AffixSetting CLUSTER_ROUTING_INCLUDE_GROUP_SETTING = - Setting.prefixKeySetting(CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX + ".", (key) -> - Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope)); + Setting.prefixKeySetting(CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX + ".", key -> + Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope)); public static final Setting.AffixSettingCLUSTER_ROUTING_EXCLUDE_GROUP_SETTING = - Setting.prefixKeySetting(CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX + ".", (key) -> - Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope)); + Setting.prefixKeySetting(CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX + ".", key -> + Setting.simpleString(key, value -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope)); /** * The set of {@link RecoverySource.Type} values for which the 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 752a9d5aba1..b49f0f82250 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -723,7 +723,7 @@ public abstract class AbstractScopedSettings { } else if (get(key) == null) { throw new IllegalArgumentException(type + " setting [" + key + "], not recognized"); } else if (isDelete == false && canUpdate.test(key)) { - validate(key, toApply, false); // we might not have a full picture here do to a dependency validation + get(key).validateWithoutDependencies(toApply); // we might not have a full picture here do to a dependency validation settingsBuilder.copy(key, toApply); updates.copy(key, toApply); changed |= toApply.get(key).equals(target.get(key)) == false; 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 127f06da1a4..9c3762f857e 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -186,7 +186,7 @@ public class Setting implements ToXContentObject { * @param properties properties for this setting like scope, filtering... */ public Setting(Key key, Function defaultValue, Function parser, Property... properties) { - this(key, defaultValue, parser, (v, s) -> {}, properties); + this(key, defaultValue, parser, v -> {}, properties); } /** @@ -246,7 +246,7 @@ public class Setting implements ToXContentObject { * @param properties properties for this setting like scope, filtering... */ public Setting(Key key, Setting fallbackSetting, Function parser, Property... properties) { - this(key, fallbackSetting, fallbackSetting::getRaw, parser, (v, m) -> {}, properties); + this(key, fallbackSetting, fallbackSetting::getRaw, parser, v -> {}, properties); } /** @@ -354,6 +354,14 @@ public class Setting implements ToXContentObject { return isGroupSetting(); } + /** + * Validate the current setting value only without dependencies with {@link Setting.Validator#validate(Object)}. + * @param settings a settings object for settings that has a default value depending on another setting if available + */ + void validateWithoutDependencies(Settings settings) { + validator.validate(get(settings, false)); + } + /** * Returns the default value string representation for this setting. * @param settings a settings object for settings that has a default value depending on another setting if available @@ -414,6 +422,7 @@ public class Setting implements ToXContentObject { } else { map = Collections.emptyMap(); } + validator.validate(parsed); validator.validate(parsed, map); } return parsed; @@ -805,8 +814,10 @@ public class Setting implements ToXContentObject { } /** - * Represents a validator for a setting. The {@link #validate(Object, Map)} method is invoked with the value of this setting and a map - * from the settings specified by {@link #settings()}} to their values. All these values come from the same {@link Settings} instance. + * Represents a validator for a setting. The {@link #validate(Object)} method is invoked early in the update setting process with the + * value of this setting for a fail-fast validation. Later on, the {@link #validate(Object, Map)} method is invoked with the value of + * this setting and a map from the settings specified by {@link #settings()}} to their values. All these values come from the same + * {@link Settings} instance. * * @param the type of the {@link Setting} */ @@ -814,17 +825,28 @@ public class Setting implements ToXContentObject { public interface Validator { /** - * The validation routine for this validator. + * Validate this setting's value in isolation. + * + * @param value the value of this setting + */ + void validate(T value); + + /** + * Validate this setting against its dependencies, specified by {@link #settings()}. The default implementation does nothing, + * accepting any value as valid as long as it passes the validation in {@link #validate(Object)}. * * @param value the value of this setting * @param settings a map from the settings specified by {@link #settings()}} to their values */ - void validate(T value, Map, T> settings); + default void validate(T value, Map, T> settings) { + } /** - * The settings needed by this validator. + * The settings on which the validity of this setting depends. The values of the specified settings are passed to + * {@link #validate(Object, Map)}. By default this returns an empty iterator, indicating that this setting does not depend on any + * other settings. * - * @return the settings needed to validate; these can be used for cross-settings validation + * @return the settings on which the validity of this setting depends. */ default Iterator> settings() { return Collections.emptyIterator(); @@ -1021,8 +1043,8 @@ public class Setting implements ToXContentObject { return new Setting<>(key, s -> "", Function.identity(), properties); } - public static Setting simpleString(String key, Function parser, Property... properties) { - return new Setting<>(key, s -> "", parser, properties); + public static Setting simpleString(String key, Validator validator, Property... properties) { + return new Setting<>(new SimpleKey(key), null, s -> "", Function.identity(), validator, properties); } public static Setting simpleString(String key, Setting fallback, Property... properties) { @@ -1037,10 +1059,6 @@ public class Setting implements ToXContentObject { return new Setting<>(key, fallback, parser, properties); } - public static Setting simpleString(String key, Validator validator, Property... properties) { - return new Setting<>(new SimpleKey(key), null, s -> "", Function.identity(), validator, properties); - } - /** * Creates a new Setting instance with a String value * @@ -1279,9 +1297,9 @@ public class Setting implements ToXContentObject { super( new ListKey(key), fallbackSetting, - (s) -> Setting.arrayToParsableString(defaultStringValue.apply(s)), + s -> Setting.arrayToParsableString(defaultStringValue.apply(s)), parser, - (v,s) -> {}, + v -> {}, properties); this.defaultStringValue = defaultStringValue; } @@ -1339,7 +1357,7 @@ public class Setting implements ToXContentObject { fallbackSetting, fallbackSetting::getRaw, minTimeValueParser(key, minValue), - (v, s) -> {}, + v -> {}, properties); } diff --git a/server/src/main/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilder.java b/server/src/main/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilder.java index 45f53006ecd..bc17d52bcc2 100644 --- a/server/src/main/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilder.java +++ b/server/src/main/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilder.java @@ -75,8 +75,12 @@ public final class AutoQueueAdjustingExecutorBuilder extends ExecutorBuilder( minSizeKey, Integer.toString(minQueueSize), - (s) -> Setting.parseInt(s, 0, minSizeKey), + s -> Setting.parseInt(s, 0, minSizeKey), new Setting.Validator() { + @Override + public void validate(Integer value) { + } + @Override public void validate(Integer value, Map, Integer> settings) { if (value > settings.get(tempMaxQueueSizeSetting)) { @@ -94,8 +98,12 @@ public final class AutoQueueAdjustingExecutorBuilder extends ExecutorBuilder( maxSizeKey, Integer.toString(maxQueueSize), - (s) -> Setting.parseInt(s, 0, maxSizeKey), + s -> Setting.parseInt(s, 0, maxSizeKey), new Setting.Validator() { + @Override + public void validate(Integer value) { + } + @Override public void validate(Integer value, Map, Integer> settings) { if (value < settings.get(tempMinQueueSizeSetting)) { diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java index 5a874ba61a2..9b9243b612b 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java @@ -121,7 +121,6 @@ public abstract class RemoteClusterAware { if (Strings.hasLength(s)) { parsePort(s); } - return s; }, Setting.Property.Deprecated, Setting.Property.Dynamic, diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java index c345e34d20c..6786a630d86 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java @@ -29,12 +29,16 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import java.util.ArrayList; +import java.util.HashSet; +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.stream.Collectors; import java.util.stream.Stream; +import static java.util.Arrays.asList; import static org.elasticsearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; @@ -221,24 +225,11 @@ public class SettingsUpdaterTests extends ESTestCase { // these are invalid settings that exist as either persistent or transient settings final int numberOfInvalidSettings = randomIntBetween(0, 7); - final List> invalidSettings = new ArrayList<>(numberOfInvalidSettings); - for (int i = 0; i < numberOfInvalidSettings; i++) { - final Setting invalidSetting = Setting.simpleString( - "invalid.setting" + i, - (value, settings) -> { - throw new IllegalArgumentException("invalid"); - }, - Property.NodeScope); - invalidSettings.add(invalidSetting); - } + final List> invalidSettings = invalidSettings(numberOfInvalidSettings); // these are unknown settings that exist as either persistent or transient settings final int numberOfUnknownSettings = randomIntBetween(0, 7); - final List> unknownSettings = new ArrayList<>(numberOfUnknownSettings); - for (int i = 0; i < numberOfUnknownSettings; i++) { - final Setting unknownSetting = Setting.simpleString("unknown.setting" + i, Property.NodeScope); - unknownSettings.add(unknownSetting); - } + final List> unknownSettings = unknownSettings(numberOfUnknownSettings); final Settings.Builder existingPersistentSettings = Settings.builder(); final Settings.Builder existingTransientSettings = Settings.builder(); @@ -393,24 +384,11 @@ public class SettingsUpdaterTests extends ESTestCase { // these are invalid settings that exist as either persistent or transient settings final int numberOfInvalidSettings = randomIntBetween(0, 7); - final List> invalidSettings = new ArrayList<>(numberOfInvalidSettings); - for (int i = 0; i < numberOfInvalidSettings; i++) { - final Setting invalidSetting = Setting.simpleString( - "invalid.setting" + i, - (value, settings) -> { - throw new IllegalArgumentException("invalid"); - }, - Property.NodeScope); - invalidSettings.add(invalidSetting); - } + final List> invalidSettings = invalidSettings(numberOfInvalidSettings); // these are unknown settings that exist as either persistent or transient settings final int numberOfUnknownSettings = randomIntBetween(0, 7); - final List> unknownSettings = new ArrayList<>(numberOfUnknownSettings); - for (int i = 0; i < numberOfUnknownSettings; i++) { - final Setting unknownSetting = Setting.simpleString("unknown.setting" + i, Property.NodeScope); - unknownSettings.add(unknownSetting); - } + final List> unknownSettings = unknownSettings(numberOfUnknownSettings); final Settings.Builder existingPersistentSettings = Settings.builder(); final Settings.Builder existingTransientSettings = Settings.builder(); @@ -511,4 +489,120 @@ public class SettingsUpdaterTests extends ESTestCase { } } + private static List> unknownSettings(int numberOfUnknownSettings) { + final List> unknownSettings = new ArrayList<>(numberOfUnknownSettings); + for (int i = 0; i < numberOfUnknownSettings; i++) { + unknownSettings.add(Setting.simpleString("unknown.setting" + i, Property.NodeScope)); + } + return unknownSettings; + } + + private static List> invalidSettings(int numberOfInvalidSettings) { + final List> invalidSettings = new ArrayList<>(numberOfInvalidSettings); + for (int i = 0; i < numberOfInvalidSettings; i++) { + invalidSettings.add(randomBoolean() ? invalidInIsolationSetting(i) : invalidWithDependenciesSetting(i)); + } + return invalidSettings; + } + + private static Setting invalidInIsolationSetting(int index) { + return Setting.simpleString("invalid.setting" + index, + new Setting.Validator() { + @Override + public void validate(String value) { + throw new IllegalArgumentException("Invalid in isolation setting"); + } + + @Override + public void validate(String value, Map, String> settings) { + } + }, + Property.NodeScope); + } + + private static Setting invalidWithDependenciesSetting(int index) { + return Setting.simpleString("invalid.setting" + index, + new Setting.Validator() { + @Override + public void validate(String value) { + } + + @Override + public void validate(String value, Map, String> settings) { + throw new IllegalArgumentException("Invalid with dependencies setting"); + } + }, + Property.NodeScope); + } + + private static class FooLowSettingValidator implements Setting.Validator { + @Override + public void validate(Integer value) { + } + + @Override + public void validate(Integer low, Map, Integer> settings) { + if (settings.containsKey(SETTING_FOO_HIGH) && low > settings.get(SETTING_FOO_HIGH)) { + throw new IllegalArgumentException("[low]=" + low + " is higher than [high]=" + settings.get(SETTING_FOO_HIGH)); + } + } + + @Override + public Iterator> settings() { + return asList(SETTING_FOO_LOW, SETTING_FOO_HIGH).iterator(); + } + } + + private static class FooHighSettingValidator implements Setting.Validator { + @Override + public void validate(Integer value) { + } + + @Override + public void validate(Integer high, Map, Integer> settings) { + if (settings.containsKey(SETTING_FOO_LOW) && high < settings.get(SETTING_FOO_LOW)) { + throw new IllegalArgumentException("[high]=" + high + " is lower than [low]=" + settings.get(SETTING_FOO_LOW)); + } + } + + @Override + public Iterator> settings() { + return asList(SETTING_FOO_LOW, SETTING_FOO_HIGH).iterator(); + } + } + + private static final Setting SETTING_FOO_LOW = new Setting<>("foo.low", "10", + Integer::valueOf, new FooLowSettingValidator(), Property.Dynamic, Setting.Property.NodeScope); + private static final Setting SETTING_FOO_HIGH = new Setting<>("foo.high", "100", + Integer::valueOf, new FooHighSettingValidator(), Property.Dynamic, Setting.Property.NodeScope); + + public void testUpdateOfValidationDependentSettings() { + final ClusterSettings settings = new ClusterSettings(Settings.EMPTY, new HashSet<>(asList(SETTING_FOO_LOW, SETTING_FOO_HIGH))); + final SettingsUpdater updater = new SettingsUpdater(settings); + final MetaData.Builder metaData = MetaData.builder().persistentSettings(Settings.EMPTY).transientSettings(Settings.EMPTY); + + ClusterState cluster = ClusterState.builder(new ClusterName("cluster")).metaData(metaData).build(); + + cluster = updater.updateSettings(cluster, Settings.builder().put(SETTING_FOO_LOW.getKey(), 20).build(), Settings.EMPTY, logger); + assertThat(cluster.getMetaData().settings().get(SETTING_FOO_LOW.getKey()), equalTo("20")); + + cluster = updater.updateSettings(cluster, Settings.builder().put(SETTING_FOO_HIGH.getKey(), 40).build(), Settings.EMPTY, logger); + assertThat(cluster.getMetaData().settings().get(SETTING_FOO_LOW.getKey()), equalTo("20")); + assertThat(cluster.getMetaData().settings().get(SETTING_FOO_HIGH.getKey()), equalTo("40")); + + cluster = updater.updateSettings(cluster, Settings.builder().put(SETTING_FOO_LOW.getKey(), 5).build(), Settings.EMPTY, logger); + assertThat(cluster.getMetaData().settings().get(SETTING_FOO_LOW.getKey()), equalTo("5")); + assertThat(cluster.getMetaData().settings().get(SETTING_FOO_HIGH.getKey()), equalTo("40")); + + cluster = updater.updateSettings(cluster, Settings.builder().put(SETTING_FOO_HIGH.getKey(), 8).build(), Settings.EMPTY, logger); + assertThat(cluster.getMetaData().settings().get(SETTING_FOO_LOW.getKey()), equalTo("5")); + assertThat(cluster.getMetaData().settings().get(SETTING_FOO_HIGH.getKey()), equalTo("8")); + + final ClusterState finalCluster = cluster; + Exception exception = expectThrows(IllegalArgumentException.class, () -> + updater.updateSettings(finalCluster, Settings.builder().put(SETTING_FOO_HIGH.getKey(), 2).build(), Settings.EMPTY, logger)); + + assertThat(exception.getMessage(), equalTo("[high]=2 is lower than [low]=5")); + } + } diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettingsTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettingsTests.java index 342fcea7dde..d9e157187d5 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettingsTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.test.ESTestCase; import java.util.Locale; +import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.instanceOf; @@ -203,4 +204,50 @@ public class DiskThresholdSettingsTests extends ESTestCase { assertThat(cause, hasToString(containsString("low disk watermark [85%] more than high disk watermark [75%]"))); } + public void testSequenceOfUpdates() { + final ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + new DiskThresholdSettings(Settings.EMPTY, clusterSettings); // this has the effect of registering the settings updater + + final Settings.Builder target = Settings.builder(); + + { + final Settings settings = Settings.builder() + .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey(), "99%") + .build(); + final Settings.Builder updates = Settings.builder(); + assertTrue(clusterSettings.updateSettings(settings, target, updates, "transient")); + assertNull(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey())); + assertNull(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey())); + assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey()), + equalTo("99%")); + } + + { + final Settings settings = Settings.builder() + .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "97%") + .build(); + final Settings.Builder updates = Settings.builder(); + assertTrue(clusterSettings.updateSettings(settings, target, updates, "transient")); + assertNull(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey())); + assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey()), + equalTo("97%")); + assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey()), + equalTo("99%")); + } + + { + final Settings settings = Settings.builder() + .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "95%") + .build(); + final Settings.Builder updates = Settings.builder(); + assertTrue(clusterSettings.updateSettings(settings, target, updates, "transient")); + assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey()), + equalTo("95%")); + assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey()), + equalTo("97%")); + assertThat(target.get(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey()), + equalTo("99%")); + } + } + } 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 9194a60382d..fc732fbd88e 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -37,7 +37,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -51,9 +50,7 @@ import java.util.stream.Collectors; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.startsWith; -import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.sameInstance; @@ -514,94 +511,6 @@ public class ScopedSettingsTests extends ESTestCase { assertEquals(15, bC.get()); } - private static final Setting FOO_BAR_LOW_SETTING = new Setting<>( - "foo.bar.low", - "1", - Integer::parseInt, - new FooBarLowValidator(), - Property.Dynamic, - Property.NodeScope); - - private static final Setting FOO_BAR_HIGH_SETTING = new Setting<>( - "foo.bar.high", - "2", - Integer::parseInt, - new FooBarHighValidator(), - Property.Dynamic, - Property.NodeScope); - - static class FooBarLowValidator implements Setting.Validator { - @Override - public void validate(Integer value, Map, Integer> settings) { - final int high = settings.get(FOO_BAR_HIGH_SETTING); - if (value > high) { - throw new IllegalArgumentException("low [" + value + "] more than high [" + high + "]"); - } - } - - @Override - public Iterator> settings() { - return Collections.singletonList(FOO_BAR_HIGH_SETTING).iterator(); - } - } - - static class FooBarHighValidator implements Setting.Validator { - @Override - public void validate(Integer value, Map, Integer> settings) { - final int low = settings.get(FOO_BAR_LOW_SETTING); - if (value < low) { - throw new IllegalArgumentException("high [" + value + "] less than low [" + low + "]"); - } - } - - @Override - public Iterator> settings() { - return Collections.singletonList(FOO_BAR_LOW_SETTING).iterator(); - } - } - - public void testValidator() { - final AbstractScopedSettings service = - new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(FOO_BAR_LOW_SETTING, FOO_BAR_HIGH_SETTING))); - - final AtomicInteger consumerLow = new AtomicInteger(); - final AtomicInteger consumerHigh = new AtomicInteger(); - - service.addSettingsUpdateConsumer(FOO_BAR_LOW_SETTING, consumerLow::set); - - service.addSettingsUpdateConsumer(FOO_BAR_HIGH_SETTING, consumerHigh::set); - - final Settings newSettings = Settings.builder().put("foo.bar.low", 17).put("foo.bar.high", 13).build(); - { - final IllegalArgumentException e = - expectThrows( - IllegalArgumentException.class, - () -> service.validateUpdate(newSettings)); - assertThat(e, hasToString(containsString("illegal value can't update [foo.bar.low] from [1] to [17]"))); - assertNotNull(e.getCause()); - assertThat(e.getCause(), instanceOf(IllegalArgumentException.class)); - final IllegalArgumentException cause = (IllegalArgumentException) e.getCause(); - assertThat(cause, hasToString(containsString("low [17] more than high [13]"))); - assertThat(e.getSuppressed(), arrayWithSize(1)); - assertThat(e.getSuppressed()[0], instanceOf(IllegalArgumentException.class)); - final IllegalArgumentException suppressed = (IllegalArgumentException) e.getSuppressed()[0]; - assertThat(suppressed, hasToString(containsString("illegal value can't update [foo.bar.high] from [2] to [13]"))); - assertNotNull(suppressed.getCause()); - assertThat(suppressed.getCause(), instanceOf(IllegalArgumentException.class)); - final IllegalArgumentException suppressedCause = (IllegalArgumentException) suppressed.getCause(); - assertThat(suppressedCause, hasToString(containsString("high [13] less than low [17]"))); - assertThat(consumerLow.get(), equalTo(0)); - assertThat(consumerHigh.get(), equalTo(0)); - } - - { - final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> service.applySettings(newSettings)); - assertThat(e, hasToString(containsString("illegal value can't update [foo.bar.low] from [1] to [17]"))); - assertThat(consumerLow.get(), equalTo(0)); - assertThat(consumerHigh.get(), equalTo(0)); - } - } - public void testGet() { ClusterSettings settings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); // affix setting - complex matcher 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 750c7148946..220392a952c 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -204,12 +204,18 @@ public class SettingTests extends ESTestCase { static class FooBarValidator implements Setting.Validator { - public static boolean invoked; + public static boolean invokedInIsolation; + public static boolean invokedWithDependencies; + + @Override + public void validate(String value) { + invokedInIsolation = true; + assertThat(value, equalTo("foo.bar value")); + } @Override public void validate(String value, Map, String> settings) { - invoked = true; - assertThat(value, equalTo("foo.bar value")); + invokedWithDependencies = true; assertTrue(settings.keySet().contains(BAZ_QUX_SETTING)); assertThat(settings.get(BAZ_QUX_SETTING), equalTo("baz.qux value")); assertTrue(settings.keySet().contains(QUUX_QUUZ_SETTING)); @@ -230,7 +236,8 @@ public class SettingTests extends ESTestCase { .put("quux.quuz", "quux.quuz value") .build(); FOO_BAR_SETTING.get(settings); - assertTrue(FooBarValidator.invoked); + assertTrue(FooBarValidator.invokedInIsolation); + assertTrue(FooBarValidator.invokedWithDependencies); } public void testUpdateNotDynamic() { @@ -934,7 +941,7 @@ public class SettingTests extends ESTestCase { final Setting.AffixSetting affixSetting = Setting.prefixKeySetting("prefix" + ".", - (key) -> Setting.simpleString(key, (value, map) -> {}, Property.Dynamic, Property.NodeScope)); + key -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope)); final Consumer> consumer = (map) -> {}; final BiConsumer validator = (s1, s2) -> {}; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java index 111d8a9a68c..13cc4c121da 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java @@ -139,7 +139,7 @@ public class XPackSettings { * Do not allow insecure hashing algorithms to be used for password hashing */ public static final Setting PASSWORD_HASHING_ALGORITHM = new Setting<>( - "xpack.security.authc.password_hashing.algorithm", "bcrypt", Function.identity(), (v, s) -> { + "xpack.security.authc.password_hashing.algorithm", "bcrypt", Function.identity(), v -> { if (Hasher.getAvailableAlgoStoredHash().contains(v.toLowerCase(Locale.ROOT)) == false) { throw new IllegalArgumentException("Invalid algorithm: " + v + ". Valid values for password hashing are " + Hasher.getAvailableAlgoStoredHash().toString()); diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java index 85a6da15177..34c069adb2a 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java @@ -25,11 +25,11 @@ public abstract class Exporter implements AutoCloseable { private static final Setting.AffixSetting ENABLED_SETTING = Setting.affixKeySetting("xpack.monitoring.exporters.","enabled", - (key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope)); + key -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope)); private static final Setting.AffixSetting TYPE_SETTING = Setting.affixKeySetting("xpack.monitoring.exporters.","type", - (key) -> Setting.simpleString(key, (v, s) -> { + key -> Setting.simpleString(key, v -> { switch (v) { case "": case "http": @@ -47,13 +47,13 @@ public abstract class Exporter implements AutoCloseable { */ public static final Setting.AffixSetting USE_INGEST_PIPELINE_SETTING = Setting.affixKeySetting("xpack.monitoring.exporters.","use_ingest", - (key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope)); + key -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope)); /** * Every {@code Exporter} allows users to explicitly disable cluster alerts. */ public static final Setting.AffixSetting CLUSTER_ALERTS_MANAGEMENT_SETTING = Setting.affixKeySetting("xpack.monitoring.exporters.", "cluster_alerts.management.enabled", - (key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope)); + key -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope)); /** * Every {@code Exporter} allows users to explicitly disable specific cluster alerts. *

@@ -61,14 +61,14 @@ public abstract class Exporter implements AutoCloseable { */ public static final Setting.AffixSetting> CLUSTER_ALERTS_BLACKLIST_SETTING = Setting .affixKeySetting("xpack.monitoring.exporters.", "cluster_alerts.management.blacklist", - (key) -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), Property.Dynamic, Property.NodeScope)); + key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), Property.Dynamic, Property.NodeScope)); /** * Every {@code Exporter} allows users to use a different index time format. */ private static final Setting.AffixSetting INDEX_NAME_TIME_FORMAT_SETTING = Setting.affixKeySetting("xpack.monitoring.exporters.","index.name.time_format", - (key) -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope)); + key -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope)); private static final String INDEX_FORMAT = "YYYY.MM.dd"; diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpSettings.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpSettings.java index f4f97df1d4f..af4a20d596c 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpSettings.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpSettings.java @@ -34,7 +34,7 @@ public class HttpSettings { private static final String SSL_KEY_PREFIX = "xpack.http.ssl."; static final Setting PROXY_HOST = Setting.simpleString(PROXY_HOST_KEY, Property.NodeScope); - static final Setting PROXY_SCHEME = Setting.simpleString(PROXY_SCHEME_KEY, (v, s) -> Scheme.parse(v), Property.NodeScope); + static final Setting PROXY_SCHEME = Setting.simpleString(PROXY_SCHEME_KEY, Scheme::parse, Property.NodeScope); static final Setting PROXY_PORT = Setting.intSetting(PROXY_PORT_KEY, 0, 0, 0xFFFF, Property.NodeScope); static final Setting MAX_HTTP_RESPONSE_SIZE = Setting.byteSizeSetting("xpack.http.max_response_size",