Support dependent validation for index settings

A setting validator can declare settings that the validation depends on,
but when updating index settings, we eagerly validate the settings
before submitting the cluster state update and here we do not know the
existing settings. With this commit, we ensure that the pre-validation
works with such validators, letting the validator validate syntax
(validate(value)) only.

Relates elastic#70141.

Author: henningandersen

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

@@ -454,7 +454,7 @@ public final void validate(
      * Validates that all settings are registered and valid.
      *
      * @param settings                       the settings
-     * @param validateDependencies           true if dependent settings should be validated
+     * @param validateDependencies           true if dependent settings and settings where validation has dependencies should be 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
@@ -489,7 +489,7 @@ 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 validateDependencies true if dependent settings and settings where validation has dependencies should be validated
      * @throws IllegalArgumentException if the setting is invalid
      */
     void validate(final String key, final Settings settings, final boolean validateDependencies) {
@@ -501,7 +501,7 @@ 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 validateDependencies           true if dependent settings and settings where validation has dependencies should be validated
      * @param validateInternalOrPrivateIndex true if internal index settings should be validated
      * @throws IllegalArgumentException if the setting is invalid
      */
@@ -564,7 +564,11 @@ void validate(
                 }
             }
         }
-        setting.get(settings);
+        if (validateDependencies) {
+            setting.get(settings);
+        } else {
+            setting.validateWithoutDependencies(settings);
+        }
     }
 
     /**

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

@@ -82,6 +82,12 @@ public T get(Settings settings) {
         }
     }
 
+    @Override
+    void validateWithoutDependencies(Settings settings) {
+        // secure settings have no Setting.Validator
+        get(settings);
+    }
+
     /**
      * Returns the digest of this secure setting's value or {@code null} if the setting is missing (inside the keystore). This method can be
      * called even after the {@code SecureSettings} have been closed, unlike {@code #get(Settings)}. The digest is used to check for changes

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,46 @@ public void testDependentSettingsWithFallback() {
         service.validate(Settings.builder().put("fallback.test.name", "test").put("foo.test.bar", 7).build(), true);
     }
 
+    public void testValidatorWithDependencies() {
+        final boolean nodeSetting = randomBoolean();
+        final String prefix = nodeSetting ? "" : "index.";
+        final Property scopeProperty = nodeSetting ? Property.NodeScope : Property.IndexScope;
+        final Setting<String> baseSetting = Setting.simpleString(prefix + "foo.base", Property.Dynamic, scopeProperty);
+        final Setting.Validator<String> validator = new Setting.Validator<String>() {
+            @Override
+            public void validate(String value) {
+            }
+
+            @Override
+            public void validate(String value, Map<Setting<?>, Object> settings, boolean isPresent) {
+                if (Objects.equals(value, settings.get(baseSetting)) == false) {
+                    throw new IllegalArgumentException("must have same value");
+                }
+            }
+
+            @Override
+            public Iterator<Setting<?>> settings() {
+                return List.<Setting<?>>of(baseSetting).iterator();
+            }
+        };
+        final Setting<String> dependingSetting = Setting.simpleString(prefix + "foo.depending", validator, 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"));
+    }
+
     public void testTupleAffixUpdateConsumer() {
         String prefix = randomAlphaOfLength(3) + "foo.";
         String intSuffix = randomAlphaOfLength(3);