Do not reference values for filtered settings (#48066)

server/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -450,11 +450,13 @@ public class Setting<T> implements ToXContentObject {
         } 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);
         }
     }
 
@@ -958,8 +960,9 @@ public class Setting<T> implements ToXContentObject {
                     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;
                 }
@@ -1013,8 +1016,12 @@ public class Setting<T> implements ToXContentObject {
                 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);
+                }
             }
         }
 
@@ -1037,32 +1044,41 @@ public class Setting<T> implements ToXContentObject {
         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<Integer> 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<Integer> 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<Integer> intSetting(String key, Setting<Integer> 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<Integer> intSetting(String key, Setting<Integer> fallbackSetting, int minValue, Validator<Integer> 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<Long> 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<String> simpleString(String key, Property... properties) {
@@ -1103,24 +1119,39 @@ public class Setting<T> implements ToXContentObject {
     }
 
     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;
     }
@@ -1138,15 +1169,27 @@ public class Setting<T> implements ToXContentObject {
     }
 
     public static Setting<Boolean> 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<Boolean> boolSetting(String key, Setting<Boolean> 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<Boolean> boolSetting(String key, Function<Settings, String> 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<ByteSizeValue> byteSizeSetting(String key, ByteSizeValue value, Property... properties) {
@@ -1407,7 +1450,7 @@ public class Setting<T> implements ToXContentObject {
                 simpleKey,
                 fallbackSetting,
                 fallbackSetting::getRaw,
-                minTimeValueParser(key, minValue),
+                minTimeValueParser(key, minValue, isFiltered(properties)),
                 v -> {},
                 properties);
     }
@@ -1415,23 +1458,34 @@ public class Setting<T> implements ToXContentObject {
     public static Setting<TimeValue> timeSetting(
             final String key, Function<Settings, TimeValue> 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<TimeValue> 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<String, TimeValue> minTimeValueParser(final String key, final TimeValue minValue) {
+    private static Function<String, TimeValue> 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);
@@ -1441,14 +1495,23 @@ public class Setting<T> implements ToXContentObject {
     }
 
     private static Function<String, TimeValue> 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);
@@ -1489,10 +1552,14 @@ public class Setting<T> implements ToXContentObject {
         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);

server/src/test/java/org/elasticsearch/common/settings/SettingTests.java

@@ -182,6 +182,17 @@ public class SettingTests extends ESTestCase {
                     hasToString(containsString("Failed to parse value [I am not a boolean] as only [true] or [false] are allowed.")));
         }
     }
+    public void testSimpleUpdateOfFilteredSetting() {
+        Setting<Boolean> booleanSetting = Setting.boolSetting("foo.bar", false, Property.Dynamic, Property.Filtered);
+        AtomicReference<Boolean> atomicBoolean = new AtomicReference<>(null);
+        ClusterSettings.SettingUpdater<Boolean> 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 class SettingTests extends ESTestCase {
         assertTrue(FooBarValidator.invokedWithDependencies);
     }
 
+    public void testValidatorForFilteredStringSetting() {
+        final Setting<String> 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<Boolean> booleanSetting = Setting.boolSetting("foo.bar", false, Property.NodeScope);
         assertFalse(booleanSetting.isGroupSetting());
@@ -390,6 +491,17 @@ public class SettingTests extends ESTestCase {
         }
     }
 
+    public void testFilteredGroups() {
+        AtomicReference<Settings> ref = new AtomicReference<>(null);
+        Setting<Settings> setting = Setting.groupSetting("foo.bar.", Property.Filtered, Property.Dynamic);
+
+        ClusterSettings.SettingUpdater<Settings> 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;