Do not reference values for filtered settings (#48066) (#48518)

This commit is contained in:
Dan Hermann 2019-10-25 16:22:11 -05:00 committed by GitHub
parent 5228956ecc
commit 2e3db518c9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 212 additions and 33 deletions

View File

@ -449,11 +449,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);
}
}
@ -957,8 +959,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;
}
@ -1012,8 +1015,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);
}
}
}
@ -1036,32 +1043,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) {
@ -1102,24 +1118,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;
}
@ -1137,15 +1168,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) {
@ -1406,7 +1449,7 @@ public class Setting<T> implements ToXContentObject {
simpleKey,
fallbackSetting,
fallbackSetting::getRaw,
minTimeValueParser(key, minValue),
minTimeValueParser(key, minValue, isFiltered(properties)),
v -> {},
properties);
}
@ -1414,23 +1457,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);
@ -1440,14 +1494,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);
@ -1488,10 +1551,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);

View File

@ -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;