Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validate exporter type is HTTP for HTTP exporter #49992

Merged
merged 3 commits into from
Dec 13, 2019
Merged

Validate exporter type is HTTP for HTTP exporter #49992

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d855010
Validate exporter type is HTTP for HTTP exporter
jasontedor Dec 6, 2019
425b96a
Fix tests
jasontedor Dec 12, 2019
2f9277b
Merge branch 'master' into http-exporter-type-http-dependency
elasticmachine Dec 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 52 additions & 22 deletions ...nitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
*/
Expand Down Expand Up @@ -111,9 +125,6 @@ public void validate(final List<String> hosts, final Map<Setting<?>, Object> set
} 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;
Expand All @@ -129,7 +140,7 @@ public void validate(final List<String> hosts, final Map<Setting<?>, Object> set
throw new SettingsException("[" + key + "] invalid host: [" + host + "]", e);
}

if ("http".equals(httpHost.getSchemeName())) {
if (TYPE.equals(httpHost.getSchemeName())) {
httpHostFound = true;
} else {
httpsHostFound = true;
Expand All @@ -152,26 +163,31 @@ public Iterator<Setting<?>> settings() {

},
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.
*/
Expand All @@ -181,12 +197,12 @@ public Iterator<Setting<?>> settings() {
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));
Expand All @@ -201,6 +217,11 @@ public void validate(String username, Map<Setting<?>, Object> settings) {
"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 + "]");
}
}
}

Expand All @@ -209,15 +230,18 @@ public Iterator<Setting<?>> settings() {
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();
}

},
Property.Dynamic,
Property.NodeScope,
Property.Filtered));
Property.Filtered),
TYPE_DEPENDENCY);
/**
* Password for basic auth.
*/
Expand Down Expand Up @@ -261,15 +285,19 @@ public Iterator<Setting<?>> settings() {
},
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.
*/
Expand All @@ -288,13 +316,14 @@ public Iterator<Setting<?>> settings() {
}
},
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.
*/
Expand All @@ -316,7 +345,8 @@ public Iterator<Setting<?>> settings() {
}
},
Property.Dynamic,
Property.NodeScope));
Property.NodeScope),
TYPE_DEPENDENCY);
/**
* Blacklist of headers that the user is not allowed to set.
* <p>
Expand All @@ -328,19 +358,19 @@ public Iterator<Setting<?>> settings() {
*/
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).
Expand Down
13 changes: 5 additions & 8 deletions ...ing/src/test/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
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;
Expand All @@ -38,6 +39,7 @@
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;
Expand Down Expand Up @@ -133,14 +135,9 @@ public void testHostListIsRejectedIfTypeIsNotHttp() {
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() {
Expand Down