honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-22 21:05:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

Validate exporter type is HTTP for HTTP exporter (#49992)

Browse Source 
Today the HTTP exporter settings without the exporter type having been
configured to HTTP. When it is time to initialize the exporter, we can
blow up. Since this initialization happens on the cluster state applier
thread, it is quite problematic that we do not reject settings updates
where the type is not configured to HTTP, but there are HTTP exporter
settings configured. This commit addresses this by validating that the
exporter type is not only set, but is set to HTTP.
This commit is contained in:
Jason Tedor 2019-12-12 19:17:59 -05:00
parent 5b9fce48c0
commit 29526d0dfe
No known key found for this signature in database
GPG Key ID: FA89F05560F16BC5
3 changed files with 77 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -76,6 +76,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).
*/
@ -109,9 +123,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;
@ -127,7 +138,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;
@ -151,26 +162,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.
*/
@ -180,12 +196,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));
@ -200,6 +216,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 + "]");
}
}
}
@ -208,7 +229,9 @@ public class HttpExporter extends Exporter {
final String namespace =
HttpExporter.AUTH_USERNAME_SETTING.getNamespace(
HttpExporter.AUTH_USERNAME_SETTING.getConcreteSetting(key));
final List<Setting<?>> settings = Collections.singletonList(
final List<Setting<?>> settings = Arrays.asList(
Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace),
HttpExporter.AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace));
return settings.iterator();
}
@ -216,7 +239,8 @@ public class HttpExporter extends Exporter {
},
Property.Dynamic,
Property.NodeScope,
Property.Filtered));
Property.Filtered),
TYPE_DEPENDENCY);
/**
* Password for basic auth.
*/
@ -260,15 +284,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.
*/
@ -287,19 +315,37 @@ 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.
*/
public static final Setting.AffixSetting<Settings> HEADERS_SETTING =
Setting.affixKeySetting("xpack.monitoring.exporters.","headers",
(key) -> Setting.groupSetting(key + ".", Property.Dynamic, Property.NodeScope));
(key) -> Setting.groupSetting(
key + ".",
settings -> {
final Set<String> names = settings.names();
for (String name : names) {
final String fullSetting = key + "." + name;
if (HttpExporter.BLACKLISTED_HEADERS.contains(name)) {
throw new SettingsException("header cannot be overwritten via [" + fullSetting + "]");
}
final List<String> values = settings.getAsList(name);
if (values.isEmpty()) {
throw new SettingsException("headers must have values, missing for setting [" + fullSetting + "]");
}
}
},
Property.Dynamic,
Property.NodeScope),
TYPE_DEPENDENCY);
/**
* Blacklist of headers that the user is not allowed to set.
* <p>
@ -311,19 +357,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).

2
x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterSslIT.java
View File

@ -178,6 +178,8 @@ public class HttpExporterSslIT extends MonitoringIntegTestCase {
private ActionFuture<ClusterUpdateSettingsResponse> setVerificationMode(String name, VerificationMode mode) {
final ClusterUpdateSettingsRequest updateSettings = new ClusterUpdateSettingsRequest();
final Settings settings = Settings.builder()
.put("xpack.monitoring.exporters." + name + ".type", HttpExporter.TYPE)
.put("xpack.monitoring.exporters." + name + ".host", "https://" + webServer.getHostName() + ":" + webServer.getPort())
.put("xpack.monitoring.exporters." + name + ".ssl.verification_mode", mode.name())
.build();
updateSettings.transientSettings(settings);

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

@ -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;
@ -37,6 +38,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
@ -134,14 +136,10 @@ public class HttpExporterTests extends ESTestCase {
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]")));
final ClusterSettings clusterSettings =
new ClusterSettings(settings, new HashSet<>(Arrays.asList(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() {