Validating monitoring hosts setting while parsing (#47571)

This commit lifts the validation of the monitoring hosts setting into
the setting itself, rather than when the setting is used. This prevents
a scenario where an invalid value for the setting is accepted, but then
later fails while applying a cluster state with the invalid setting.

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

@@ -1247,6 +1247,15 @@ public class Setting<T> implements ToXContentObject {
         return listSetting(key, null, singleValueParser, (s) -> defaultStringValue, properties);
     }
 
+    public static <T> Setting<List<T>> listSetting(
+        final String key,
+        final List<String> defaultStringValue,
+        final Function<String, T> singleValueParser,
+        final Validator<List<T>> validator,
+        final Property... properties) {
+        return listSetting(key, null, singleValueParser, (s) -> defaultStringValue, validator, properties);
+    }
+
     // TODO this one's two argument get is still broken
     public static <T> Setting<List<T>> listSetting(
             final String key,
@@ -1270,13 +1279,23 @@ public class Setting<T> implements ToXContentObject {
             final Function<String, T> singleValueParser,
             final Function<Settings, List<String>> defaultStringValue,
             final Property... properties) {
+        return listSetting(key, fallbackSetting, singleValueParser, defaultStringValue, v -> {}, properties);
+    }
+
+    static <T> Setting<List<T>> listSetting(
+        final String key,
+        final @Nullable Setting<List<T>> fallbackSetting,
+        final Function<String, T> singleValueParser,
+        final Function<Settings, List<String>> defaultStringValue,
+        final Validator<List<T>> validator,
+        final Property... properties) {
         if (defaultStringValue.apply(Settings.EMPTY) == null) {
             throw new IllegalArgumentException("default value function must not return null");
         }
         Function<String, List<T>> parser = (s) ->
-                parseableStringToList(s).stream().map(singleValueParser).collect(Collectors.toList());
+            parseableStringToList(s).stream().map(singleValueParser).collect(Collectors.toList());
 
-        return new ListSetting<>(key, fallbackSetting, defaultStringValue, parser, properties);
+        return new ListSetting<>(key, fallbackSetting, defaultStringValue, parser, validator, properties);
     }
 
     private static List<String> parseableStringToList(String parsableString) {
@@ -1323,13 +1342,14 @@ public class Setting<T> implements ToXContentObject {
                 final @Nullable Setting<List<T>> fallbackSetting,
                 final Function<Settings, List<String>> defaultStringValue,
                 final Function<String, List<T>> parser,
+                final Validator<List<T>> validator,
                 final Property... properties) {
             super(
                     new ListKey(key),
                     fallbackSetting,
                     s -> Setting.arrayToParsableString(defaultStringValue.apply(s)),
                     parser,
-                    v -> {},
+                    validator,
                     properties);
             this.defaultStringValue = defaultStringValue;
         }

x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java

@@ -13,11 +13,14 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.license.XPackLicenseState;
+import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter;
 
 import java.time.ZoneOffset;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Function;
 
@@ -27,19 +30,53 @@ public abstract class Exporter implements AutoCloseable {
             Setting.affixKeySetting("xpack.monitoring.exporters.","enabled",
                     key -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope));
 
-    private static final Setting.AffixSetting<String> TYPE_SETTING =
+    public static final Setting.AffixSetting<String> TYPE_SETTING = Setting.affixKeySetting(
-            Setting.affixKeySetting("xpack.monitoring.exporters.","type",
+        "xpack.monitoring.exporters.",
-                    key -> Setting.simpleString(key, v -> {
+        "type",
-                        switch (v) {
+        key -> Setting.simpleString(
-                            case "":
+            key,
-                            case "http":
+            new Setting.Validator<String>() {
-                            case "local":
+
-                                break;
+                @Override
-                            default:
+                public void validate(final String value) {
-                                throw new IllegalArgumentException("only exporter types [http] and [local] are allowed [" + v +
+
-                                        "] is invalid");
+                }
-                        }
+
-                    }, Property.Dynamic, Property.NodeScope));
+                @Override
+                public void validate(final String value, final Map<Setting<?>, Object> settings) {
+                    switch (value) {
+                        case "":
+                            break;
+                        case "http":
+                            // if the type is http, then hosts must be set
+                            final String namespace = TYPE_SETTING.getNamespace(TYPE_SETTING.getConcreteSetting(key));
+                            final Setting<List<String>> hostsSetting = HttpExporter.HOST_SETTING.getConcreteSettingForNamespace(namespace);
+                            @SuppressWarnings("unchecked") final List<String> hosts = (List<String>) settings.get(hostsSetting);
+                            if (hosts.isEmpty()) {
+                                throw new SettingsException("host list for [" + hostsSetting.getKey() + "] is empty");
+                            }
+                            break;
+                        case "local":
+                            break;
+                        default:
+                            throw new SettingsException(
+                                "type [" + value + "] for key [" + key + "] is invalid, only [http] and [local] are allowed");
+                    }
+
+                }
+
+                @Override
+                public Iterator<Setting<?>> settings() {
+                    final String namespace =
+                        Exporter.TYPE_SETTING.getNamespace(Exporter.TYPE_SETTING.getConcreteSetting(key));
+                    final List<Setting<?>> settings =
+                        Collections.singletonList(HttpExporter.HOST_SETTING.getConcreteSettingForNamespace(namespace));
+                    return settings.iterator();
+                }
+
+            },
+            Property.Dynamic,
+            Property.NodeScope));
     /**
      * Every {@code Exporter} adds the ingest pipeline to bulk requests, but they should, at the exporter level, allow that to be disabled.
      * <p>

x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java

@@ -47,6 +47,7 @@ import javax.net.ssl.SSLContext;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -79,9 +80,79 @@ public class HttpExporter extends Exporter {
      * A string array representing the Elasticsearch node(s) to communicate with over HTTP(S).
      */
     public static final Setting.AffixSetting<List<String>> HOST_SETTING =
-            Setting.affixKeySetting("xpack.monitoring.exporters.","host",
+            Setting.affixKeySetting(
-                    (key) -> Setting.listSetting(key, Collections.emptyList(), Function.identity(),
+                "xpack.monitoring.exporters.",
-                            Property.Dynamic, Property.NodeScope));
+                "host",
+                key -> Setting.listSetting(
+                    key,
+                    Collections.emptyList(),
+                    Function.identity(),
+                    new Setting.Validator<List<String>>() {
+
+                        @Override
+                        public void validate(final List<String> value) {
+
+                        }
+
+                        @Override
+                        public void validate(final List<String> hosts, final Map<Setting<?>, Object> settings) {
+                            final String namespace =
+                                HttpExporter.HOST_SETTING.getNamespace(HttpExporter.HOST_SETTING.getConcreteSetting(key));
+                            final String type = (String) settings.get(Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace));
+
+                            if (hosts.isEmpty()) {
+                                final String defaultType =
+                                    Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace).get(Settings.EMPTY);
+                                if (Objects.equals(type, defaultType)) {
+                                    // hosts can only be empty if the type is unset
+                                    return;
+                                } 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;
+                            boolean httpsHostFound = false;
+
+                            // every host must be configured
+                            for (final String host : hosts) {
+                                final HttpHost httpHost;
+
+                                try {
+                                    httpHost = HttpHostBuilder.builder(host).build();
+                                } catch (final IllegalArgumentException e) {
+                                    throw new SettingsException("[" + key + "] invalid host: [" + host + "]", e);
+                                }
+
+                                if ("http".equals(httpHost.getSchemeName())) {
+                                    httpHostFound = true;
+                                } else {
+                                    httpsHostFound = true;
+                                }
+
+                                // fail if we find them configuring the scheme/protocol in different ways
+                                if (httpHostFound && httpsHostFound) {
+                                    throw new SettingsException("[" + key + "] must use a consistent scheme: http or https");
+                                }
+                            }
+                        }
+
+                        @Override
+                        public Iterator<Setting<?>> settings() {
+                            final String namespace =
+                                HttpExporter.HOST_SETTING.getNamespace(HttpExporter.HOST_SETTING.getConcreteSetting(key));
+                            final List<Setting<?>> settings =
+                                Collections.singletonList(Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace));
+                            return settings.iterator();
+                        }
+
+                    },
+                    Property.Dynamic,
+                    Property.NodeScope));
+
     /**
      * Master timeout associated with bulk requests.
      */
@@ -380,43 +451,17 @@ public class HttpExporter extends Exporter {
      */
     private static HttpHost[] createHosts(final Config config) {
         final List<String> hosts = HOST_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings());
-        String configKey = HOST_SETTING.getConcreteSettingForNamespace(config.name()).getKey();
-
-        if (hosts.isEmpty()) {
-            throw new SettingsException("missing required setting [" + configKey + "]");
-        }
 
         final List<HttpHost> httpHosts = new ArrayList<>(hosts.size());
-        boolean httpHostFound = false;
-        boolean httpsHostFound = false;
 
-        // every host must be configured
         for (final String host : hosts) {
-            final HttpHost httpHost;
+            final HttpHost httpHost = HttpHostBuilder.builder(host).build();
-
-            try {
-                httpHost = HttpHostBuilder.builder(host).build();
-            } catch (IllegalArgumentException e) {
-                throw new SettingsException("[" + configKey + "] invalid host: [" + host + "]", e);
-            }
-
-            if ("http".equals(httpHost.getSchemeName())) {
-                httpHostFound = true;
-            } else {
-                httpsHostFound = true;
-            }
-
-            // fail if we find them configuring the scheme/protocol in different ways
-            if (httpHostFound && httpsHostFound) {
-                throw new SettingsException("[" + configKey + "] must use a consistent scheme: http or https");
-            }
-
             httpHosts.add(httpHost);
         }
 
         logger.debug("exporter [{}] using hosts {}", config.name(), hosts);
 
-        return httpHosts.toArray(new HttpHost[httpHosts.size()]);
+        return httpHosts.toArray(new HttpHost[0]);
     }
 
     /**

x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java

@@ -27,6 +27,7 @@ import org.elasticsearch.xpack.core.monitoring.MonitoredSystem;
 import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc;
 import org.elasticsearch.xpack.monitoring.MonitoringService;
 import org.elasticsearch.xpack.monitoring.cleaner.CleanerService;
+import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter;
 import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporter;
 import org.junit.Before;
 
@@ -53,6 +54,7 @@ import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
@@ -98,6 +100,17 @@ public class ExportersTests extends ESTestCase {
         exporters = new Exporters(Settings.EMPTY, factories, clusterService, licenseState, threadContext);
     }
 
+    public void testHostsMustBeSetIfTypeIsHttp() {
+        final String prefix = "xpack.monitoring.exporters.example";
+        final Settings settings  = Settings.builder().put(prefix + ".type", "http").build();
+        final IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> HttpExporter.TYPE_SETTING.getConcreteSetting(prefix + ".type").get(settings));
+        assertThat(e, hasToString(containsString("Failed to parse value [http] for setting [" + prefix + ".type]")));
+        assertThat(e.getCause(), instanceOf(SettingsException.class));
+        assertThat(e.getCause(), hasToString(containsString("host list for [" + prefix + ".host] is empty")));
+    }
+
     public void testExporterIndexPattern() {
         Exporter.Config config = mock(Exporter.Config.class);
         when(config.name()).thenReturn("anything");

x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java

@@ -113,25 +113,15 @@ public class HttpExporterIT extends MonitoringIntegTestCase {
         return true;
     }
 
-    @Override
-    protected Settings nodeSettings(int nodeOrdinal) {
-        // we create and disable the exporter to avoid the cluster actually using it (thus speeding up tests)
-        // we make an exporter on demand per test
-        return Settings.builder()
-                       .put(super.nodeSettings(nodeOrdinal))
-                       .put("xpack.monitoring.exporters._http.type", "http")
-                       .put("xpack.monitoring.exporters._http.ssl.truststore.password", "foobar") // ensure that ssl can be used by settings
-                       .put("xpack.monitoring.exporters._http.headers.ignored", "value") // ensure that headers can be used by settings
-                       .put("xpack.monitoring.exporters._http.enabled", false)
-                       .build();
-    }
-
     private Settings.Builder baseSettings() {
         return Settings.builder()
-                       .put("xpack.monitoring.exporters._http.type", "http")
+            .put("xpack.monitoring.exporters._http.enabled", false)
-                       .put("xpack.monitoring.exporters._http.host", getFormattedAddress(webServer))
+            .put("xpack.monitoring.exporters._http.type", "http")
-                       .putList("xpack.monitoring.exporters._http.cluster_alerts.management.blacklist", clusterAlertBlacklist)
+            .put("xpack.monitoring.exporters._http.ssl.truststore.password", "foobar") // ensure that ssl can be used by settings
-                       .put("xpack.monitoring.exporters._http.index.template.create_legacy_templates", includeOldTemplates);
+            .put("xpack.monitoring.exporters._http.headers.ignored", "value") // ensure that headers can be used by settings
+            .put("xpack.monitoring.exporters._http.host", getFormattedAddress(webServer))
+            .putList("xpack.monitoring.exporters._http.cluster_alerts.management.blacklist", clusterAlertBlacklist)
+            .put("xpack.monitoring.exporters._http.index.template.create_legacy_templates", includeOldTemplates);
     }
 
     public void testExport() throws Exception {

x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java

@@ -5,8 +5,6 @@
  */
 package org.elasticsearch.xpack.monitoring.exporter.http;
 
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
 import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy;
@@ -29,23 +27,30 @@ import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils;
 import org.elasticsearch.xpack.core.ssl.SSLService;
 import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil;
 import org.elasticsearch.xpack.monitoring.exporter.ExportBulk;
+import org.elasticsearch.xpack.monitoring.exporter.Exporter;
 import org.elasticsearch.xpack.monitoring.exporter.Exporter.Config;
 import org.junit.Before;
 import org.mockito.InOrder;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils.OLD_TEMPLATE_IDS;
 import static org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils.PIPELINE_IDS;
 import static org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils.TEMPLATE_IDS;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.hasToString;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.nullValue;
 import static org.mockito.Matchers.any;
@@ -82,6 +87,101 @@ public class HttpExporterTests extends ESTestCase {
         when(nodes.isLocalNodeElectedMaster()).thenReturn(true);
     }
 
+    public void testEmptyHostListDefault() {
+        runTestEmptyHostList(true);
+    }
+
+    public void testEmptyHostListExplicit() {
+        runTestEmptyHostList(false);
+    }
+
+    private void runTestEmptyHostList(final boolean useDefault) {
+        final String prefix = "xpack.monitoring.exporters.example";
+        final Settings.Builder builder = Settings.builder().put(prefix + ".type", "http");
+        if (useDefault == false) {
+            builder.putList(prefix + ".host", Collections.emptyList());
+        }
+        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 [[]] for setting [" + prefix + ".host]")));
+        assertThat(e.getCause(), instanceOf(SettingsException.class));
+        assertThat(e.getCause(), hasToString(containsString("host list for [" + prefix + ".host] is empty")));
+    }
+
+    public void testEmptyHostListOkayIfTypeNotSetDefault() {
+        runTestEmptyHostListOkayIfTypeNotSet(true);
+    }
+
+    public void testEmptyHostListOkayIfTypeNotSetExplicit() {
+        runTestEmptyHostListOkayIfTypeNotSet(true);
+    }
+
+    private void runTestEmptyHostListOkayIfTypeNotSet(final boolean useDefault) {
+        final String prefix = "xpack.monitoring.exporters.example";
+        final Settings.Builder builder = Settings.builder();
+        if (useDefault == false) {
+            builder.put(prefix + ".type", Exporter.TYPE_SETTING.getConcreteSettingForNamespace("example").get(Settings.EMPTY));
+        }
+        builder.putList(prefix + ".host", Collections.emptyList());
+        final Settings settings = builder.build();
+        HttpExporter.HOST_SETTING.getConcreteSetting(prefix + ".host").get(settings);
+    }
+
+    public void testHostListIsRejectedIfTypeIsNotHttp() {
+        final String prefix = "xpack.monitoring.exporters.example";
+        final Settings.Builder builder = Settings.builder().put(prefix + ".type", "local");
+        builder.putList(prefix + ".host", Collections.singletonList("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]")));
+    }
+
+    public void testInvalidHost() {
+        final String prefix = "xpack.monitoring.exporters.example";
+        final String host = "https://example.com:443/";
+        final Settings settings = Settings.builder()
+            .put(prefix + ".type", "http")
+            .put(prefix + ".host", host)
+            .build();
+        final IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> HttpExporter.HOST_SETTING.getConcreteSetting(prefix + ".host").get(settings));
+        assertThat(
+            e,
+            hasToString(containsString("Failed to parse value [[\"" + host + "\"]] for setting [" + prefix + ".host]")));
+        assertThat(e.getCause(), instanceOf(SettingsException.class));
+        assertThat(e.getCause(), hasToString(containsString("[" + prefix + ".host] invalid host: [" + host + "]")));
+        assertThat(e.getCause().getCause(), instanceOf(IllegalArgumentException.class));
+        assertThat(e.getCause().getCause(), hasToString(containsString("HttpHosts do not use paths [/].")));
+    }
+
+    public void testMixedSchemes() {
+        final String prefix = "xpack.monitoring.exporters.example";
+        final String httpHost = "http://example.com:443";
+        final String httpsHost = "https://example.com:443";
+        final Settings settings = Settings.builder()
+            .put(prefix + ".type", "http")
+            .putList(prefix + ".host", Arrays.asList(httpHost, httpsHost))
+            .build();
+        final IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> HttpExporter.HOST_SETTING.getConcreteSetting(prefix + ".host").get(settings));
+        assertThat(
+            e,
+            hasToString(containsString(
+                "Failed to parse value [[\"" + httpHost + "\",\"" + httpsHost + "\"]] for setting [" + prefix + ".host]")));
+        assertThat(e.getCause(), instanceOf(SettingsException.class));
+        assertThat(e.getCause(), hasToString(containsString("[" + prefix + ".host] must use a consistent scheme: http or https")));
+    }
+
     public void testExporterWithBlacklistedHeaders() {
         final String blacklistedHeader = randomFrom(HttpExporter.BLACKLISTED_HEADERS);
         final String expected = "header cannot be overwritten via [xpack.monitoring.exporters._http.headers." + blacklistedHeader + "]";
@@ -139,66 +239,6 @@ public class HttpExporterTests extends ESTestCase {
         assertThat(exception.getMessage(), equalTo(expected));
     }
 
-    public void testExporterWithMissingHost() {
-        // forgot host!
-        final Settings.Builder builder = Settings.builder()
-                .put("xpack.monitoring.exporters._http.type", HttpExporter.TYPE);
-
-        if (randomBoolean()) {
-            builder.put("xpack.monitoring.exporters._http.host", "");
-        } else if (randomBoolean()) {
-            builder.putList("xpack.monitoring.exporters._http.host");
-        } else if (randomBoolean()) {
-            builder.putNull("xpack.monitoring.exporters._http.host");
-        }
-
-        final Config config = createConfig(builder.build());
-
-        final SettingsException exception =
-                expectThrows(SettingsException.class, () -> new HttpExporter(config, sslService, threadContext));
-
-        assertThat(exception.getMessage(), equalTo("missing required setting [xpack.monitoring.exporters._http.host]"));
-    }
-
-    public void testExporterWithInconsistentSchemes() {
-        final Settings.Builder builder = Settings.builder()
-                .put("xpack.monitoring.exporters._http.type", HttpExporter.TYPE)
-                .putList("xpack.monitoring.exporters._http.host", "http://localhost:9200", "https://localhost:9201");
-
-        final Config config = createConfig(builder.build());
-
-        final SettingsException exception =
-                expectThrows(SettingsException.class, () -> new HttpExporter(config, sslService, threadContext));
-
-        assertThat(exception.getMessage(),
-                   equalTo("[xpack.monitoring.exporters._http.host] must use a consistent scheme: http or https"));
-    }
-
-    public void testExporterWithInvalidHost() {
-        final String invalidHost = randomFrom("://localhost:9200", "gopher!://xyz.my.com");
-
-        final Settings.Builder builder = Settings.builder()
-                .put("xpack.monitoring.exporters._http.type", HttpExporter.TYPE);
-
-        // sometimes add a valid URL with it
-        if (randomBoolean()) {
-            if (randomBoolean()) {
-                builder.putList("xpack.monitoring.exporters._http.host", "localhost:9200", invalidHost);
-            } else {
-                builder.putList("xpack.monitoring.exporters._http.host", invalidHost, "localhost:9200");
-            }
-        } else {
-            builder.put("xpack.monitoring.exporters._http.host", invalidHost);
-        }
-
-        final Config config = createConfig(builder.build());
-
-        final SettingsException exception =
-                expectThrows(SettingsException.class, () -> new HttpExporter(config, sslService, threadContext));
-
-        assertThat(exception.getMessage(), equalTo("[xpack.monitoring.exporters._http.host] invalid host: [" + invalidHost + "]"));
-    }
-
     public void testExporterWithUnknownBlacklistedClusterAlerts() {
         final SSLIOSessionStrategy sslStrategy = mock(SSLIOSessionStrategy.class);
         when(sslService.sslIOSessionStrategy(any(Settings.class))).thenReturn(sslStrategy);
@@ -277,18 +317,6 @@ public class HttpExporterTests extends ESTestCase {
         verifyZeroInteractions(client, listener);
     }
 
-    public void testCreateSnifferWithoutHosts() {
-        final Settings.Builder builder = Settings.builder()
-                .put("xpack.monitoring.exporters._http.type", "http")
-                .put("xpack.monitoring.exporters._http.sniff.enabled", true);
-
-        final Config config = createConfig(builder.build());
-        final RestClient client = mock(RestClient.class);
-        final NodeFailureListener listener = mock(NodeFailureListener.class);
-
-        expectThrows(IndexOutOfBoundsException.class, () -> HttpExporter.createSniffer(config, client, listener));
-    }
-
     public void testCreateSniffer() throws IOException {
         final Settings.Builder builder = Settings.builder()
                 .put("xpack.monitoring.exporters._http.type", "http")

x-pack/qa/rolling-upgrade/build.gradle

@@ -58,10 +58,6 @@ for (Version bwcVersion : bwcVersions.wireCompatible) {
             setting 'repositories.url.allowed_urls', 'http://snapshot.test*'
             setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}"
             setting 'http.content_type.required', 'true'
-            setting 'xpack.monitoring.exporters._http.type', 'http'
-            setting 'xpack.monitoring.exporters._http.enabled', 'false'
-            setting 'xpack.monitoring.exporters._http.auth.username', 'test_user'
-            setting 'xpack.monitoring.exporters._http.auth.password', 'x-pack-test-password'
             setting 'xpack.license.self_generated.type', 'trial'
             setting 'xpack.security.enabled', 'true'
             setting 'xpack.security.transport.ssl.enabled', 'true'

x-pack/qa/smoke-test-plugins-ssl/build.gradle

@@ -44,12 +44,6 @@ def pluginsCount = 0
 testClusters.integTest {
   testDistribution = 'DEFAULT'
   setting 'xpack.monitoring.collection.interval', '1s'
-  setting 'xpack.monitoring.exporters._http.type', 'http'
-  setting 'xpack.monitoring.exporters._http.enabled', 'false'
-  setting 'xpack.monitoring.exporters._http.auth.username', 'monitoring_agent'
-  setting 'xpack.monitoring.exporters._http.auth.password', 'x-pack-test-password'
-  setting 'xpack.monitoring.exporters._http.ssl.verification_mode', 'full'
-  setting 'xpack.monitoring.exporters._http.ssl.certificate_authorities', 'testnode.crt'
 
   setting 'xpack.license.self_generated.type', 'trial'
   setting 'xpack.security.enabled', 'true'

x-pack/qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java

@@ -96,9 +96,14 @@ public class SmokeTestMonitoringWithSecurityIT extends ESIntegTestCase {
     @Before
     public void enableExporter() throws Exception {
         Settings exporterSettings = Settings.builder()
-                .put("xpack.monitoring.collection.enabled", true)
+            .put("xpack.monitoring.collection.enabled", true)
-                .put("xpack.monitoring.exporters._http.enabled", true)
+            .put("xpack.monitoring.exporters._http.enabled", true)
-                .put("xpack.monitoring.exporters._http.host", "https://" + randomNodeHttpAddress())
+            .put("xpack.monitoring.exporters._http.type", "http")
+            .put("xpack.monitoring.exporters._http.host", "https://" + randomNodeHttpAddress())
+            .put("xpack.monitoring.exporters._http.auth.username", "monitoring_agent")
+            .put("xpack.monitoring.exporters._http.auth.password", "x-pack-test-password")
+            .put("xpack.monitoring.exporters._http.ssl.verification_mode", "full")
+            .put("xpack.monitoring.exporters._http.ssl.certificate_authorities", "testnode.crt")
                 .build();
         assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(exporterSettings));
     }
@@ -106,10 +111,15 @@ public class SmokeTestMonitoringWithSecurityIT extends ESIntegTestCase {
     @After
     public void disableExporter() {
         Settings exporterSettings = Settings.builder()
-                .putNull("xpack.monitoring.collection.enabled")
+            .putNull("xpack.monitoring.collection.enabled")
-                .putNull("xpack.monitoring.exporters._http.enabled")
+            .putNull("xpack.monitoring.exporters._http.enabled")
-                .putNull("xpack.monitoring.exporters._http.host")
+            .putNull("xpack.monitoring.exporters._http.type")
-                .build();
+            .putNull("xpack.monitoring.exporters._http.host")
+            .putNull("xpack.monitoring.exporters._http.auth.username")
+            .putNull("xpack.monitoring.exporters._http.auth.password")
+            .putNull("xpack.monitoring.exporters._http.ssl.verification_mode")
+            .putNull("xpack.monitoring.exporters._http.ssl.certificate_authorities")
+            .build();
         assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(exporterSettings));
     }
 
@@ -169,4 +179,5 @@ public class SmokeTestMonitoringWithSecurityIT extends ESIntegTestCase {
         }
         return NetworkAddress.format(randomFrom(httpAddresses));
     }
+
 }

x-pack/qa/smoke-test-plugins-ssl/src/test/resources/rest-api-spec/test/smoke_test_plugins_ssl/20_settings_filter.yml

@@ -1,20 +0,0 @@
-# Integration tests for smoke testing plugins
-#
-"Secret settings are correctly filtered":
-  - do:
-      cluster.state: {}
-
-  - set: {master_node: master}
-
-  - do:
-      nodes.info:
-        metric: [ settings ]
-
-  - is_true:  nodes
-  - is_true:  nodes.$master.settings.xpack.monitoring.exporters._http.type
-
-  - is_false: nodes.$master.settings.xpack.monitoring.exporters._http.auth.username
-  - is_false: nodes.$master.settings.xpack.monitoring.exporters._http.auth.password
-  - is_false: nodes.$master.settings.xpack.monitoring.exporters._http.ssl.truststore.path
-  - is_false: nodes.$master.settings.xpack.monitoring.exporters._http.ssl.truststore.password
-  - is_false: nodes.$master.settings.xpack.monitoring.exporters._http.ssl.verification_mode