Enable dependent settings values to be validated

[jasontedor]

Today settings can declare dependencies on another setting. This declaration is implemented so that if the declared setting is not set when the declaring setting is, settings validation fails. Yet, in some cases we want not only that the setting is set, but that it also has a specific value. For example, with the monitoring exporter settings, if xpack.monitoring.exporters.my_exporter.host is set, we not only want that xpack.monitoring.exporters.my_exporter.type is set, but that it is also set to local. This commit extends the settings infrastructure so that this declaration is possible. The use of this in the monitoring exporter settings will be implemented in a follow-up.

[elasticmachine]

Pinging @elastic/es-core-infra (:Core/Infra/Settings)

[jasontedor]

Here is an idea of how this change will be used in the monitoring exporter settings:

diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java
index dd9041ffd07..a73cee446c2 100644
--- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java
+++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java
@@ -78,6 +78,20 @@ public class HttpExporter extends Exporter {
 
     public static final String TYPE = "http";
 
+    private static Setting.AffixSettingDependency TYPE_DEPENDENCY = new Setting.AffixSettingDependency() {
+        @Override
+        public Setting.AffixSetting getSetting() {
+            return Exporter.TYPE_SETTING;
+        }
+
+        @Override
+        public void validate(final String key, final Object value, final Object dependency) {
+            if (TYPE.equals(dependency) == false) {
+                throw new SettingsException("[" + key + "] is set but type is [" + dependency + "]");
+            }
+        }
+    };
+
     /**
      * A string array representing the Elasticsearch node(s) to communicate with over HTTP(S).
      */
@@ -111,9 +125,6 @@ public class HttpExporter extends Exporter {
                                 } else {
                                     throw new SettingsException("host list for [" + key + "] is empty but type is [" + type + "]");
                                 }
-                            } else if ("http".equals(type) == false) {
-                                // the hosts can only be non-empty if the type is "http"
-                                throw new SettingsException("host list for [" + key + "] is set but type is [" + type + "]");
                             }
 
                             boolean httpHostFound = false;
@@ -129,7 +140,7 @@ public class HttpExporter extends Exporter {
                                     throw new SettingsException("[" + key + "] invalid host: [" + host + "]", e);
                                 }
 
-                                if ("http".equals(httpHost.getSchemeName())) {
+                                if (TYPE.equals(httpHost.getSchemeName())) {
                                     httpHostFound = true;
                                 } else {
                                     httpsHostFound = true;
@@ -152,26 +163,31 @@ public class HttpExporter extends Exporter {
 
                     },
                     Property.Dynamic,
-                    Property.NodeScope));
+                    Property.NodeScope),
+                TYPE_DEPENDENCY);
 
     /**
      * Master timeout associated with bulk requests.
      */
     public static final Setting.AffixSetting<TimeValue> BULK_TIMEOUT_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.","bulk.timeout",
-                    (key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope));
+                    (key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
     /**
      * Timeout used for initiating a connection.
      */
     public static final Setting.AffixSetting<TimeValue> CONNECTION_TIMEOUT_SETTING =
-            Setting.affixKeySetting("xpack.monitoring.exporters.","connection.timeout",
-                    (key) -> Setting.timeSetting(key, TimeValue.timeValueSeconds(6), Property.Dynamic, Property.NodeScope));
+            Setting.affixKeySetting(
+                "xpack.monitoring.exporters.",
+                "connection.timeout",
+                (key) -> Setting.timeSetting(key, TimeValue.timeValueSeconds(6), Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
     /**
      * Timeout used for reading from the connection.
      */
     public static final Setting.AffixSetting<TimeValue> CONNECTION_READ_TIMEOUT_SETTING =
-            Setting.affixKeySetting("xpack.monitoring.exporters.","connection.read_timeout",
-                    (key) -> Setting.timeSetting(key, TimeValue.timeValueSeconds(60), Property.Dynamic, Property.NodeScope));
+            Setting.affixKeySetting(
+                "xpack.monitoring.exporters.",
+                "connection.read_timeout",
+                (key) -> Setting.timeSetting(key, TimeValue.timeValueSeconds(60), Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
     /**
      * Username for basic auth.
      */
@@ -181,12 +197,12 @@ public class HttpExporter extends Exporter {
                         key,
                         new Setting.Validator<String>() {
                             @Override
-                            public void validate(String password) {
+                            public void validate(final String password) {
                                  // no username validation that is independent of other settings
                             }
 
                             @Override
-                            public void validate(String username, Map<Setting<?>, Object> settings) {
+                            public void validate(final String username, final Map<Setting<?>, Object> settings) {
                                 final String namespace =
                                     HttpExporter.AUTH_USERNAME_SETTING.getNamespace(
                                         HttpExporter.AUTH_USERNAME_SETTING.getConcreteSetting(key));
@@ -201,6 +217,11 @@ public class HttpExporter extends Exporter {
                                             "but [" + AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace).getKey() + "] is " +
                                             "missing");
                                     }
+                                    final String type =
+                                        (String) settings.get(Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace));
+                                    if ("http".equals(type) == false) {
+                                        throw new SettingsException("username for [" + key + "] is set but type is [" + type + "]");
+                                    }
                                 }
                             }
 
@@ -209,7 +230,9 @@ public class HttpExporter extends Exporter {
                                 final String namespace =
                                     HttpExporter.AUTH_USERNAME_SETTING.getNamespace(
                                         HttpExporter.AUTH_USERNAME_SETTING.getConcreteSetting(key));
+
                                 final List<Setting<?>> settings = List.of(
+                                    Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace),
                                     HttpExporter.AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace));
                                 return settings.iterator();
                             }
@@ -217,7 +240,8 @@ public class HttpExporter extends Exporter {
                         },
                         Property.Dynamic,
                         Property.NodeScope,
-                        Property.Filtered));
+                        Property.Filtered),
+                    TYPE_DEPENDENCY);
     /**
      * Password for basic auth.
      */
@@ -261,15 +285,19 @@ public class HttpExporter extends Exporter {
                         },
                         Property.Dynamic,
                         Property.NodeScope,
-                        Property.Filtered));
+                        Property.Filtered),
+                    TYPE_DEPENDENCY);
     /**
      * The SSL settings.
      *
      * @see SSLService
      */
     public static final Setting.AffixSetting<Settings> SSL_SETTING =
-            Setting.affixKeySetting("xpack.monitoring.exporters.","ssl",
-                    (key) -> Setting.groupSetting(key + ".", Property.Dynamic, Property.NodeScope, Property.Filtered));
+            Setting.affixKeySetting(
+                "xpack.monitoring.exporters.",
+                "ssl",
+                (key) -> Setting.groupSetting(key + ".", Property.Dynamic, Property.NodeScope, Property.Filtered),
+                TYPE_DEPENDENCY);
     /**
      * Proxy setting to allow users to send requests to a remote cluster that requires a proxy base path.
      */
@@ -288,13 +316,14 @@ public class HttpExporter extends Exporter {
                             }
                         },
                         Property.Dynamic,
-                        Property.NodeScope));
+                        Property.NodeScope),
+                    TYPE_DEPENDENCY);
     /**
      * A boolean setting to enable or disable sniffing for extra connections.
      */
     public static final Setting.AffixSetting<Boolean> SNIFF_ENABLED_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.","sniff.enabled",
-                    (key) -> Setting.boolSetting(key, false, Property.Dynamic, Property.NodeScope));
+                    (key) -> Setting.boolSetting(key, false, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
     /**
      * A parent setting to header key/value pairs, whose names are user defined.
      */
@@ -316,7 +345,8 @@ public class HttpExporter extends Exporter {
                             }
                         },
                         Property.Dynamic,
-                        Property.NodeScope));
+                        Property.NodeScope),
+                    TYPE_DEPENDENCY);
     /**
      * Blacklist of headers that the user is not allowed to set.
      * <p>
@@ -328,19 +358,19 @@ public class HttpExporter extends Exporter {
      */
     public static final Setting.AffixSetting<TimeValue> TEMPLATE_CHECK_TIMEOUT_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.","index.template.master_timeout",
-                    (key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope));
+                    (key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
     /**
      * A boolean setting to enable or disable whether to create placeholders for the old templates.
      */
     public static final Setting.AffixSetting<Boolean> TEMPLATE_CREATE_LEGACY_VERSIONS_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.","index.template.create_legacy_templates",
-                    (key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope));
+                    (key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
     /**
      * ES level timeout used when checking and writing pipelines (used to speed up tests)
      */
     public static final Setting.AffixSetting<TimeValue> PIPELINE_CHECK_TIMEOUT_SETTING =
             Setting.affixKeySetting("xpack.monitoring.exporters.","index.pipeline.master_timeout",
-                    (key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope));
+                    (key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
 
     /**
      * Minimum supported version of the remote monitoring cluster (same major).
diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java
index bfa1e16d860..a50839ffbad 100644
--- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java
+++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java
@@ -17,6 +17,7 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.common.unit.TimeValue;
@@ -38,6 +39,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
@@ -133,14 +135,9 @@ public class HttpExporterTests extends ESTestCase {
         final Settings.Builder builder = Settings.builder().put(prefix + ".type", "local");
         builder.putList(prefix + ".host", List.of("https://example.com:443"));
         final Settings settings = builder.build();
-        final IllegalArgumentException e = expectThrows(
-            IllegalArgumentException.class,
-            () -> HttpExporter.HOST_SETTING.getConcreteSetting(prefix + ".host").get(settings));
-        assertThat(
-            e,
-            hasToString(containsString("Failed to parse value [[\"https://example.com:443\"]] for setting [" + prefix + ".host]")));
-        assertThat(e.getCause(), instanceOf(SettingsException.class));
-        assertThat(e.getCause(), hasToString(containsString("host list for [" + prefix + ".host] is set but type is [local]")));
+        final ClusterSettings clusterSettings = new ClusterSettings(settings, Set.of(HttpExporter.HOST_SETTING, Exporter.TYPE_SETTING));
+        final SettingsException e = expectThrows(SettingsException.class, () -> clusterSettings.validate(settings, true));
+        assertThat(e, hasToString(containsString("[" + prefix + ".host] is set but type is [local]")));
     }
 
     public void testInvalidHost() {

[jaymode]

LGTM