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 0c38748d9b1ff..0a2a30a7c32fe 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -449,11 +449,13 @@ private T get(Settings settings, boolean validate) { } catch (ElasticsearchParseException ex) { throw new IllegalArgumentException(ex.getMessage(), ex); } catch (NumberFormatException ex) { - throw new IllegalArgumentException("Failed to parse value [" + value + "] for setting [" + getKey() + "]", ex); + String err = "Failed to parse value" + (isFiltered() ? "" : " [" + value + "]") + " for setting [" + getKey() + "]"; + throw new IllegalArgumentException(err, ex); } catch (IllegalArgumentException ex) { throw ex; } catch (Exception t) { - throw new IllegalArgumentException("Failed to parse value [" + value + "] for setting [" + getKey() + "]", t); + String err = "Failed to parse value" + (isFiltered() ? "" : " [" + value + "]") + " for setting [" + getKey() + "]"; + throw new IllegalArgumentException(err, t); } } @@ -957,8 +959,9 @@ public Settings getValue(Settings current, Settings previous) { try { validator.accept(currentSettings); } catch (Exception | AssertionError e) { - throw new IllegalArgumentException("illegal value can't update [" + key + "] from [" - + previousSettings + "] to [" + currentSettings+ "]", e); + String err = "illegal value can't update [" + key + "]" + + (isFiltered() ? "" : " from [" + previousSettings + "] to [" + currentSettings+ "]"); + throw new IllegalArgumentException(err, e); } return currentSettings; } @@ -1012,8 +1015,12 @@ public T getValue(Settings current, Settings previous) { accept.accept(inst); return inst; } catch (Exception | AssertionError e) { - throw new IllegalArgumentException("illegal value can't update [" + key + "] from [" + value + "] to [" + newValue + "]", - e); + if (isFiltered()) { + throw new IllegalArgumentException("illegal value can't update [" + key + "]"); + } else { + throw new IllegalArgumentException("illegal value can't update [" + key + "] from [" + value + "] to [" + + newValue + "]", e); + } } } @@ -1036,32 +1043,41 @@ public static Setting floatSetting(String key, float defaultValue, float return new Setting<>(key, (s) -> Float.toString(defaultValue), (s) -> { float value = Float.parseFloat(s); if (value < minValue) { - throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be >= " + minValue); + String err = "Failed to parse value" + + (isFiltered(properties) ? "" : " [" + s + "]") + " for setting [" + key + "] must be >= " + minValue; + throw new IllegalArgumentException(err); } return value; }, properties); } + private static boolean isFiltered(Property[] properties) { + return properties != null && Arrays.asList(properties).contains(Property.Filtered); + } + public static Setting intSetting(String key, int defaultValue, int minValue, int maxValue, Property... properties) { - return new Setting<>(key, (s) -> Integer.toString(defaultValue), (s) -> parseInt(s, minValue, maxValue, key), properties); + return new Setting<>(key, (s) -> Integer.toString(defaultValue), + (s) -> parseInt(s, minValue, maxValue, key, isFiltered(properties)), properties); } public static Setting intSetting(String key, int defaultValue, int minValue, Property... properties) { - return new Setting<>(key, (s) -> Integer.toString(defaultValue), (s) -> parseInt(s, minValue, key), properties); + return new Setting<>(key, (s) -> Integer.toString(defaultValue), (s) -> parseInt(s, minValue, key, isFiltered(properties)), + properties); } public static Setting intSetting(String key, Setting fallbackSetting, int minValue, Property... properties) { - return new Setting<>(key, fallbackSetting, (s) -> parseInt(s, minValue, key), properties); + return new Setting<>(key, fallbackSetting, (s) -> parseInt(s, minValue, key, isFiltered(properties)), properties); } public static Setting intSetting(String key, Setting fallbackSetting, int minValue, Validator validator, Property... properties) { - return new Setting<>(new SimpleKey(key), fallbackSetting, fallbackSetting::getRaw, (s) -> parseInt(s, minValue, key),validator, - properties); + return new Setting<>(new SimpleKey(key), fallbackSetting, fallbackSetting::getRaw, + (s) -> parseInt(s, minValue, key, isFiltered(properties)),validator, properties); } public static Setting longSetting(String key, long defaultValue, long minValue, Property... properties) { - return new Setting<>(key, (s) -> Long.toString(defaultValue), (s) -> parseLong(s, minValue, key), properties); + return new Setting<>(key, (s) -> Long.toString(defaultValue), (s) -> parseLong(s, minValue, key, isFiltered(properties)), + properties); } public static Setting simpleString(String key, Property... properties) { @@ -1102,24 +1118,39 @@ public static Setting simpleString(String key, String defaultValue, Prop } public static int parseInt(String s, int minValue, String key) { - return parseInt(s, minValue, Integer.MAX_VALUE, key); + return parseInt(s, minValue, Integer.MAX_VALUE, key, false); + } + + public static int parseInt(String s, int minValue, String key, boolean isFiltered) { + return parseInt(s, minValue, Integer.MAX_VALUE, key, isFiltered); } public static int parseInt(String s, int minValue, int maxValue, String key) { + return parseInt(s, minValue, maxValue, key, false); + } + + static int parseInt(String s, int minValue, int maxValue, String key, boolean isFiltered) { int value = Integer.parseInt(s); if (value < minValue) { - throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be >= " + minValue); + String err = "Failed to parse value" + (isFiltered ? "" : " [" + s + "]") + " for setting [" + key + "] must be >= " + minValue; + throw new IllegalArgumentException(err); } if (value > maxValue) { - throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be <= " + maxValue); + String err = "Failed to parse value" + (isFiltered ? "" : " [" + s + "]") + " for setting [" + key + "] must be <= " + maxValue; + throw new IllegalArgumentException(err); } return value; } public static long parseLong(String s, long minValue, String key) { + return parseLong(s, minValue, key, false); + } + + static long parseLong(String s, long minValue, String key, boolean isFiltered) { long value = Long.parseLong(s); if (value < minValue) { - throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be >= " + minValue); + String err = "Failed to parse value" + (isFiltered ? "" : " [" + s + "]") + " for setting [" + key + "] must be >= " + minValue; + throw new IllegalArgumentException(err); } return value; } @@ -1137,15 +1168,27 @@ public static Setting intSetting(String key, int defaultValue, Property } public static Setting boolSetting(String key, boolean defaultValue, Property... properties) { - return new Setting<>(key, (s) -> Boolean.toString(defaultValue), Booleans::parseBoolean, properties); + return new Setting<>(key, (s) -> Boolean.toString(defaultValue), b -> parseBoolean(b, key, isFiltered(properties)), properties); } public static Setting boolSetting(String key, Setting fallbackSetting, Property... properties) { - return new Setting<>(key, fallbackSetting, Booleans::parseBoolean, properties); + return new Setting<>(key, fallbackSetting, b -> parseBoolean(b, key, isFiltered(properties)), properties); } public static Setting boolSetting(String key, Function defaultValueFn, Property... properties) { - return new Setting<>(key, defaultValueFn, Booleans::parseBoolean, properties); + return new Setting<>(key, defaultValueFn, b -> parseBoolean(b, key, isFiltered(properties)), properties); + } + + static boolean parseBoolean(String b, String key, boolean isFiltered) { + try { + return Booleans.parseBoolean(b); + } catch (IllegalArgumentException ex) { + if (isFiltered) { + throw new IllegalArgumentException("Failed to parse value for setting [" + key + "]"); + } else { + throw ex; + } + } } public static Setting byteSizeSetting(String key, ByteSizeValue value, Property... properties) { @@ -1406,7 +1449,7 @@ public static Setting timeSetting( simpleKey, fallbackSetting, fallbackSetting::getRaw, - minTimeValueParser(key, minValue), + minTimeValueParser(key, minValue, isFiltered(properties)), v -> {}, properties); } @@ -1414,23 +1457,34 @@ public static Setting timeSetting( public static Setting timeSetting( final String key, Function defaultValue, final TimeValue minValue, final Property... properties) { final SimpleKey simpleKey = new SimpleKey(key); - return new Setting<>(simpleKey, s -> defaultValue.apply(s).getStringRep(), minTimeValueParser(key, minValue), properties); + return new Setting<>(simpleKey, s -> defaultValue.apply(s).getStringRep(), + minTimeValueParser(key, minValue, isFiltered(properties)), properties); } public static Setting timeSetting( final String key, TimeValue defaultValue, final TimeValue minValue, final TimeValue maxValue, final Property... properties) { final SimpleKey simpleKey = new SimpleKey(key); - return new Setting<>(simpleKey, s -> defaultValue.getStringRep(), minMaxTimeValueParser(key, minValue, maxValue), properties); + return new Setting<>(simpleKey, s -> defaultValue.getStringRep(), + minMaxTimeValueParser(key, minValue, maxValue, isFiltered(properties)), properties); } - private static Function minTimeValueParser(final String key, final TimeValue minValue) { + private static Function minTimeValueParser(final String key, final TimeValue minValue, boolean isFiltered) { return s -> { - final TimeValue value = TimeValue.parseTimeValue(s, null, key); + TimeValue value; + try { + value = TimeValue.parseTimeValue(s, null, key); + } catch (RuntimeException ex) { + if (isFiltered) { + throw new IllegalArgumentException("failed to parse value for setting [" + key + "] as a time value"); + } else { + throw ex; + } + } if (value.millis() < minValue.millis()) { final String message = String.format( Locale.ROOT, - "failed to parse value [%s] for setting [%s], must be >= [%s]", - s, + "failed to parse value%s for setting [%s], must be >= [%s]", + isFiltered ? "" : " [" + s + "]", key, minValue.getStringRep()); throw new IllegalArgumentException(message); @@ -1440,14 +1494,23 @@ private static Function minTimeValueParser(final String key, } private static Function minMaxTimeValueParser( - final String key, final TimeValue minValue, final TimeValue maxValue) { + final String key, final TimeValue minValue, final TimeValue maxValue, boolean isFiltered) { return s -> { - final TimeValue value = minTimeValueParser(key, minValue).apply(s); + TimeValue value; + try { + value = minTimeValueParser(key, minValue, isFiltered).apply(s); + } catch (RuntimeException ex) { + if (isFiltered) { + throw new IllegalArgumentException("failed to parse value for setting [" + key + "] as a time value"); + } else { + throw ex; + } + } if (value.millis() > maxValue.millis()) { final String message = String.format( Locale.ROOT, - "failed to parse value [%s] for setting [%s], must be <= [%s]", - s, + "failed to parse value%s for setting [%s], must be <= [%s]", + isFiltered ? "" : " [" + s + "]", key, maxValue.getStringRep()); throw new IllegalArgumentException(message); @@ -1488,10 +1551,14 @@ public static Setting doubleSetting(String key, double defaultValue, dou return new Setting<>(key, (s) -> Double.toString(defaultValue), (s) -> { final double d = Double.parseDouble(s); if (d < minValue) { - throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be >= " + minValue); + String err = "Failed to parse value" + (isFiltered(properties) ? "" : " [" + s + "]") + " for setting [" + key + + "] must be >= " + minValue; + throw new IllegalArgumentException(err); } if (d > maxValue) { - throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be <= " + maxValue); + String err = "Failed to parse value" + (isFiltered(properties) ? "" : " [" + s + "]") + " for setting [" + key + + "] must be <= " + maxValue; + throw new IllegalArgumentException(err); } return d; }, properties); 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 12cb590de3acc..86387f2104bcf 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -182,6 +182,17 @@ public void testSimpleUpdate() { hasToString(containsString("Failed to parse value [I am not a boolean] as only [true] or [false] are allowed."))); } } + public void testSimpleUpdateOfFilteredSetting() { + Setting booleanSetting = Setting.boolSetting("foo.bar", false, Property.Dynamic, Property.Filtered); + AtomicReference atomicBoolean = new AtomicReference<>(null); + ClusterSettings.SettingUpdater settingUpdater = booleanSetting.newUpdater(atomicBoolean::set, logger); + + // try update bogus value + Settings build = Settings.builder().put("foo.bar", "I am not a boolean").build(); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> settingUpdater.apply(build, Settings.EMPTY)); + assertThat(ex, hasToString(equalTo("java.lang.IllegalArgumentException: illegal value can't update [foo.bar]"))); + assertNull(ex.getCause()); + } @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/33135") public void testValidateStringSetting() { @@ -241,6 +252,96 @@ public void testValidator() { assertTrue(FooBarValidator.invokedWithDependencies); } + public void testValidatorForFilteredStringSetting() { + final Setting filteredStringSetting = new Setting<>( + "foo.bar", + "foobar", + Function.identity(), + value -> { + throw new SettingsException("validate always fails"); + }, + Property.Filtered); + + final Settings settings = Settings.builder() + .put(filteredStringSetting.getKey(), filteredStringSetting.getKey() + " value") + .build(); + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> filteredStringSetting.get(settings)); + assertThat(e, hasToString(containsString("Failed to parse value for setting [" + filteredStringSetting.getKey() + "]"))); + assertThat(e.getCause(), instanceOf(SettingsException.class)); + assertThat(e.getCause(), hasToString(containsString("validate always fails"))); + } + + public void testFilteredFloatSetting() { + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> + Setting.floatSetting("foo", 42.0f, 43.0f, Property.Filtered)); + assertThat(e, hasToString(containsString("Failed to parse value for setting [foo] must be >= 43.0"))); + } + + public void testFilteredDoubleSetting() { + final IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, () -> + Setting.doubleSetting("foo", 42.0, 43.0, Property.Filtered)); + assertThat(e1, hasToString(containsString("Failed to parse value for setting [foo] must be >= 43.0"))); + + final IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, () -> + Setting.doubleSetting("foo", 45.0, 43.0, 44.0, Property.Filtered)); + assertThat(e2, hasToString(containsString("Failed to parse value for setting [foo] must be <= 44.0"))); + } + + public void testFilteredIntSetting() { + final IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, () -> + Setting.intSetting("foo", 42, 43, 44, Property.Filtered)); + assertThat(e1, hasToString(containsString("Failed to parse value for setting [foo] must be >= 43"))); + + final IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, () -> + Setting.intSetting("foo", 45, 43, 44, Property.Filtered)); + assertThat(e2, hasToString(containsString("Failed to parse value for setting [foo] must be <= 44"))); + } + + public void testFilteredLongSetting() { + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> + Setting.longSetting("foo", 42L, 43L, Property.Filtered)); + assertThat(e, hasToString(containsString("Failed to parse value for setting [foo] must be >= 43"))); + } + + public void testFilteredTimeSetting() { + final IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, () -> + Setting.timeSetting("foo", TimeValue.timeValueHours(1), TimeValue.timeValueHours(2), Property.Filtered)); + assertThat(e1, hasToString(containsString("failed to parse value for setting [foo], must be >= [2h]"))); + + final IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, () -> + Setting.timeSetting("foo", TimeValue.timeValueHours(4), TimeValue.timeValueHours(2), TimeValue.timeValueHours(3), + Property.Filtered)); + assertThat(e2, hasToString(containsString("failed to parse value for setting [foo], must be <= [3h]"))); + + final Setting minSetting = Setting.timeSetting("foo", TimeValue.timeValueHours(3), TimeValue.timeValueHours(2), Property.Filtered); + final Settings minSettings = Settings.builder() + .put("foo", "not a time value") + .build(); + final IllegalArgumentException e3 = expectThrows(IllegalArgumentException.class, () -> minSetting.get(minSettings)); + assertThat(e3, hasToString(containsString("failed to parse value for setting [foo] as a time value"))); + assertNull(e3.getCause()); + + final Setting maxSetting = Setting.timeSetting("foo", TimeValue.timeValueHours(3), TimeValue.timeValueHours(2), + TimeValue.timeValueHours(4), Property.Filtered); + final Settings maxSettings = Settings.builder() + .put("foo", "not a time value") + .build(); + final IllegalArgumentException e4 = expectThrows(IllegalArgumentException.class, () -> maxSetting.get(maxSettings)); + assertThat(e4, hasToString(containsString("failed to parse value for setting [foo] as a time value"))); + assertNull(e4.getCause()); + } + + public void testFilteredBooleanSetting() { + Setting setting = Setting.boolSetting("foo", false, Property.Filtered); + final Settings settings = Settings.builder() + .put("foo", "not a boolean value") + .build(); + + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> setting.get(settings)); + assertThat(e, hasToString(containsString("Failed to parse value for setting [foo]"))); + assertNull(e.getCause()); + } + public void testUpdateNotDynamic() { Setting booleanSetting = Setting.boolSetting("foo.bar", false, Property.NodeScope); assertFalse(booleanSetting.isGroupSetting()); @@ -390,6 +491,17 @@ public void testGroups() { } } + public void testFilteredGroups() { + AtomicReference ref = new AtomicReference<>(null); + Setting setting = Setting.groupSetting("foo.bar.", Property.Filtered, Property.Dynamic); + + ClusterSettings.SettingUpdater predicateSettingUpdater = setting.newUpdater(ref::set, logger, (s) -> assertFalse(true)); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, + () -> predicateSettingUpdater.apply(Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").build(), + Settings.EMPTY)); + assertEquals("illegal value can't update [foo.bar.]", ex.getMessage()); + } + public static class ComplexType { final String foo;