diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java index 851267e995e1b..c6655b62e574f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java @@ -81,7 +81,7 @@ public void updateSettings(final UpdateSettingsClusterStateUpdateRequest request indexScopedSettings.validate( normalizedSettings.filter(s -> Regex.isSimpleMatchPattern(s) == false), // don't validate wildcards - false, // don't validate dependencies here we check it below never allow to change the number of shards + false, // don't validate values here we check it below never allow to change the number of shards true); // validate internal or private index settings for (String key : normalizedSettings.keySet()) { Setting setting = indexScopedSettings.get(key); 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 3958696139170..915e60037eaac 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -414,47 +414,47 @@ public synchronized void addSettingsUpdateConsumer(Setting setting, Consu * Validates that all settings are registered and valid. * * @param settings the settings to validate - * @param validateDependencies true if dependent settings should be validated + * @param validateValues true if values should be validated, otherwise only keys are validated * @see Setting#getSettingsDependencies(String) */ - public final void validate(final Settings settings, final boolean validateDependencies) { - validate(settings, validateDependencies, false, false); + public final void validate(final Settings settings, final boolean validateValues) { + validate(settings, validateValues, false, false); } /** * Validates that all settings are registered and valid. * * @param settings the settings to validate - * @param validateDependencies true if dependent settings should be validated + * @param validateValues true if values should be validated, otherwise only keys are validated * @param validateInternalOrPrivateIndex true if internal index settings should be validated * @see Setting#getSettingsDependencies(String) */ - public final void validate(final Settings settings, final boolean validateDependencies, final boolean validateInternalOrPrivateIndex) { - validate(settings, validateDependencies, false, false, validateInternalOrPrivateIndex); + public final void validate(final Settings settings, final boolean validateValues, final boolean validateInternalOrPrivateIndex) { + validate(settings, validateValues, false, false, validateInternalOrPrivateIndex); } /** * Validates that all settings are registered and valid. * * @param settings the settings - * @param validateDependencies true if dependent settings should be validated + * @param validateValues true if values should be validated, otherwise only keys are validated * @param ignorePrivateSettings true if private settings should be ignored during validation * @param ignoreArchivedSettings true if archived settings should be ignored during validation * @see Setting#getSettingsDependencies(String) */ public final void validate( final Settings settings, - final boolean validateDependencies, + final boolean validateValues, final boolean ignorePrivateSettings, final boolean ignoreArchivedSettings) { - validate(settings, validateDependencies, ignorePrivateSettings, ignoreArchivedSettings, false); + validate(settings, validateValues, ignorePrivateSettings, ignoreArchivedSettings, false); } /** * Validates that all settings are registered and valid. * * @param settings the settings - * @param validateDependencies true if dependent settings should be validated + * @param validateValues true if values should be validated, otherwise only keys are validated * @param ignorePrivateSettings true if private settings should be ignored during validation * @param ignoreArchivedSettings true if archived settings should be ignored during validation * @param validateInternalOrPrivateIndex true if index internal settings should be validated @@ -462,7 +462,7 @@ public final void validate( */ public final void validate( final Settings settings, - final boolean validateDependencies, + final boolean validateValues, final boolean ignorePrivateSettings, final boolean ignoreArchivedSettings, final boolean validateInternalOrPrivateIndex) { @@ -476,7 +476,7 @@ public final void validate( continue; } try { - validate(key, settings, validateDependencies, validateInternalOrPrivateIndex); + validate(key, settings, validateValues, validateInternalOrPrivateIndex); } catch (final RuntimeException ex) { exceptions.add(ex); } @@ -489,11 +489,11 @@ public final void validate( * * @param key the key of the setting to validate * @param settings the settings - * @param validateDependencies true if dependent settings should be validated + * @param validateValue true if value should be validated, otherwise only keys are validated * @throws IllegalArgumentException if the setting is invalid */ - void validate(final String key, final Settings settings, final boolean validateDependencies) { - validate(key, settings, validateDependencies, false); + void validate(final String key, final Settings settings, final boolean validateValue) { + validate(key, settings, validateValue, false); } /** @@ -501,12 +501,12 @@ void validate(final String key, final Settings settings, final boolean validateD * * @param key the key of the setting to validate * @param settings the settings - * @param validateDependencies true if dependent settings should be validated + * @param validateValue true if value should be validated, otherwise only keys are validated * @param validateInternalOrPrivateIndex true if internal index settings should be validated * @throws IllegalArgumentException if the setting is invalid */ void validate( - final String key, final Settings settings, final boolean validateDependencies, final boolean validateInternalOrPrivateIndex) { + final String key, final Settings settings, final boolean validateValue, final boolean validateInternalOrPrivateIndex) { Setting setting = getRaw(key); if (setting == null) { LevenshteinDistance ld = new LevenshteinDistance(); @@ -537,7 +537,7 @@ void validate( if (setting.hasComplexMatcher()) { setting = setting.getConcreteSetting(key); } - if (validateDependencies && settingsDependencies.isEmpty() == false) { + if (validateValue && settingsDependencies.isEmpty() == false) { for (final Setting.SettingDependency settingDependency : settingsDependencies) { final Setting dependency = settingDependency.getSetting(); // validate the dependent setting is set @@ -564,7 +564,9 @@ void validate( } } } - setting.get(settings); + if (validateValue) { + setting.get(settings); + } } /** 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 84d8ce8855842..58258c918b910 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -26,9 +26,11 @@ 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; +import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -259,6 +261,58 @@ public void testDependentSettingsWithFallback() { service.validate(Settings.builder().put("fallback.test.name", "test").put("foo.test.bar", 7).build(), true); } + public void testValidateValue() { + final boolean nodeSetting = randomBoolean(); + final String prefix = nodeSetting ? "" : "index."; + final Property scopeProperty = nodeSetting ? Property.NodeScope : Property.IndexScope; + final Setting.Validator baseValidator = s -> { + if (s.length() > 1) { + throw new IllegalArgumentException("too long"); + } + }; + final Setting baseSetting = Setting.simpleString(prefix + "foo.base", baseValidator, + Property.Dynamic, scopeProperty); + final Setting.Validator dependingValidator = new Setting.Validator() { + @Override + public void validate(String value) { + } + + @Override + public void validate(String value, Map, Object> settings, boolean isPresent) { + if (Objects.equals(value, settings.get(baseSetting)) == false) { + throw new IllegalArgumentException("must have same value"); + } + } + + @Override + public Iterator> settings() { + return List.>of(baseSetting).iterator(); + } + }; + final Setting dependingSetting = Setting.simpleString(prefix + "foo.depending", dependingValidator, scopeProperty); + + final AbstractScopedSettings service = nodeSetting ? new ClusterSettings(Settings.EMPTY, Set.of(baseSetting, dependingSetting)) : + new IndexScopedSettings(Settings.EMPTY, Set.of(baseSetting, dependingSetting)); + + service.validate(Settings.builder().put(baseSetting.getKey(), "1").put(dependingSetting.getKey(), 1).build(), true); + service.validate(Settings.builder().put(dependingSetting.getKey(), "1").build(), false); + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> service.validate(Settings.builder().put(dependingSetting.getKey(), "1").build(), true)); + assertThat(e.getMessage(), equalTo("must have same value")); + + final IllegalArgumentException e2 = expectThrows( + IllegalArgumentException.class, + () -> service.validate(Settings.builder().put(baseSetting.getKey(), "2").put(dependingSetting.getKey(), "1").build(), true)); + assertThat(e2.getMessage(), equalTo("must have same value")); + + service.validate(Settings.builder().put(baseSetting.getKey(), "22").build(), false); + final IllegalArgumentException e3 = expectThrows( + IllegalArgumentException.class, + () -> service.validate(Settings.builder().put(baseSetting.getKey(), "22").build(), true)); + assertThat(e3.getMessage(), equalTo("too long")); + } + public void testTupleAffixUpdateConsumer() { String prefix = randomAlphaOfLength(3) + "foo."; String intSuffix = randomAlphaOfLength(3); @@ -850,16 +904,16 @@ public void testValidate() { assertEquals("unknown setting [i.am.not.a.setting]" + unknownMsgSuffix, e.getMessage()); e = expectThrows(IllegalArgumentException.class, () -> - settings.validate(Settings.builder().put("index.store.type", "boom").put("index.number_of_replicas", true).build(), false)); + settings.validate(Settings.builder().put("index.store.type", "boom").put("index.number_of_replicas", true).build(), true)); assertEquals("Failed to parse value [true] for setting [index.number_of_replicas]", e.getMessage()); e = expectThrows(IllegalArgumentException.class, () -> - settings.validate("index.number_of_replicas", Settings.builder().put("index.number_of_replicas", "true").build(), false)); + settings.validate("index.number_of_replicas", Settings.builder().put("index.number_of_replicas", "true").build(), true)); assertEquals("Failed to parse value [true] for setting [index.number_of_replicas]", e.getMessage()); e = expectThrows(IllegalArgumentException.class, () -> settings.validate("index.similarity.boolean.type", Settings.builder().put("index.similarity.boolean.type", "mine").build(), - false)); + true)); assertEquals("illegal value for [index.similarity.boolean] cannot redefine built-in similarity", e.getMessage()); } @@ -967,7 +1021,7 @@ public void testLoggingUpdates() { IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, - () -> settings.validate(Settings.builder().put("logger._root", "boom").build(), false)); + () -> settings.validate(Settings.builder().put("logger._root", "boom").build(), true)); assertEquals("Unknown level constant [BOOM].", ex.getMessage()); assertEquals(level, LogManager.getRootLogger().getLevel()); settings.applySettings(Settings.builder().put("logger._root", "TRACE").build()); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java index b8b773ca2ee0d..6784d4358954b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java @@ -469,7 +469,7 @@ public void testPKIRealmSettingsPassValidation() throws Exception { List> settingList = new ArrayList<>(); settingList.addAll(InternalRealmsSettings.getSettings()); ClusterSettings clusterSettings = new ClusterSettings(settings, new HashSet<>(settingList)); - clusterSettings.validate(settings, false); + clusterSettings.validate(settings, true); assertSettingDeprecationsAndWarnings(new Setting[]{ PkiRealmSettings.LEGACY_TRUST_STORE_PASSWORD.getConcreteSettingForNamespace("pki1")