Skip to content

Commit 4f212dd

Browse files
jasontedorjasontedor
authored
Validate exporter type is HTTP for HTTP exporter (#49992)
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.
1 parent 3050172 commit 4f212dd

File tree

3 files changed

+59
-30
lines changed

3 files changed

+59
-30
lines changed

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

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,20 @@ public class HttpExporter extends Exporter {
7878

7979
public static final String TYPE = "http";
8080

81+
private static Setting.AffixSettingDependency TYPE_DEPENDENCY = new Setting.AffixSettingDependency() {
82+
@Override
83+
public Setting.AffixSetting getSetting() {
84+
return Exporter.TYPE_SETTING;
85+
}
86+
87+
@Override
88+
public void validate(final String key, final Object value, final Object dependency) {
89+
if (TYPE.equals(dependency) == false) {
90+
throw new SettingsException("[" + key + "] is set but type is [" + dependency + "]");
91+
}
92+
}
93+
};
94+
8195
/**
8296
* A string array representing the Elasticsearch node(s) to communicate with over HTTP(S).
8397
*/
@@ -111,9 +125,6 @@ public void validate(final List<String> hosts, final Map<Setting<?>, Object> set
111125
} else {
112126
throw new SettingsException("host list for [" + key + "] is empty but type is [" + type + "]");
113127
}
114-
} else if ("http".equals(type) == false) {
115-
// the hosts can only be non-empty if the type is "http"
116-
throw new SettingsException("host list for [" + key + "] is set but type is [" + type + "]");
117128
}
118129

119130
boolean httpHostFound = false;
@@ -129,7 +140,7 @@ public void validate(final List<String> hosts, final Map<Setting<?>, Object> set
129140
throw new SettingsException("[" + key + "] invalid host: [" + host + "]", e);
130141
}
131142

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

153164
},
154165
Property.Dynamic,
155-
Property.NodeScope));
166+
Property.NodeScope),
167+
TYPE_DEPENDENCY);
156168

157169
/**
158170
* Master timeout associated with bulk requests.
159171
*/
160172
public static final Setting.AffixSetting<TimeValue> BULK_TIMEOUT_SETTING =
161173
Setting.affixKeySetting("xpack.monitoring.exporters.","bulk.timeout",
162-
(key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope));
174+
(key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
163175
/**
164176
* Timeout used for initiating a connection.
165177
*/
166178
public static final Setting.AffixSetting<TimeValue> CONNECTION_TIMEOUT_SETTING =
167-
Setting.affixKeySetting("xpack.monitoring.exporters.","connection.timeout",
168-
(key) -> Setting.timeSetting(key, TimeValue.timeValueSeconds(6), Property.Dynamic, Property.NodeScope));
179+
Setting.affixKeySetting(
180+
"xpack.monitoring.exporters.",
181+
"connection.timeout",
182+
(key) -> Setting.timeSetting(key, TimeValue.timeValueSeconds(6), Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
169183
/**
170184
* Timeout used for reading from the connection.
171185
*/
172186
public static final Setting.AffixSetting<TimeValue> CONNECTION_READ_TIMEOUT_SETTING =
173-
Setting.affixKeySetting("xpack.monitoring.exporters.","connection.read_timeout",
174-
(key) -> Setting.timeSetting(key, TimeValue.timeValueSeconds(60), Property.Dynamic, Property.NodeScope));
187+
Setting.affixKeySetting(
188+
"xpack.monitoring.exporters.",
189+
"connection.read_timeout",
190+
(key) -> Setting.timeSetting(key, TimeValue.timeValueSeconds(60), Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
175191
/**
176192
* Username for basic auth.
177193
*/
@@ -181,12 +197,12 @@ public Iterator<Setting<?>> settings() {
181197
key,
182198
new Setting.Validator<String>() {
183199
@Override
184-
public void validate(String password) {
200+
public void validate(final String password) {
185201
// no username validation that is independent of other settings
186202
}
187203

188204
@Override
189-
public void validate(String username, Map<Setting<?>, Object> settings) {
205+
public void validate(final String username, final Map<Setting<?>, Object> settings) {
190206
final String namespace =
191207
HttpExporter.AUTH_USERNAME_SETTING.getNamespace(
192208
HttpExporter.AUTH_USERNAME_SETTING.getConcreteSetting(key));
@@ -201,6 +217,11 @@ public void validate(String username, Map<Setting<?>, Object> settings) {
201217
"but [" + AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace).getKey() + "] is " +
202218
"missing");
203219
}
220+
final String type =
221+
(String) settings.get(Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace));
222+
if ("http".equals(type) == false) {
223+
throw new SettingsException("username for [" + key + "] is set but type is [" + type + "]");
224+
}
204225
}
205226
}
206227

@@ -209,15 +230,18 @@ public Iterator<Setting<?>> settings() {
209230
final String namespace =
210231
HttpExporter.AUTH_USERNAME_SETTING.getNamespace(
211232
HttpExporter.AUTH_USERNAME_SETTING.getConcreteSetting(key));
233+
212234
final List<Setting<?>> settings = List.of(
235+
Exporter.TYPE_SETTING.getConcreteSettingForNamespace(namespace),
213236
HttpExporter.AUTH_PASSWORD_SETTING.getConcreteSettingForNamespace(namespace));
214237
return settings.iterator();
215238
}
216239

217240
},
218241
Property.Dynamic,
219242
Property.NodeScope,
220-
Property.Filtered));
243+
Property.Filtered),
244+
TYPE_DEPENDENCY);
221245
/**
222246
* Password for basic auth.
223247
*/
@@ -261,15 +285,19 @@ public Iterator<Setting<?>> settings() {
261285
},
262286
Property.Dynamic,
263287
Property.NodeScope,
264-
Property.Filtered));
288+
Property.Filtered),
289+
TYPE_DEPENDENCY);
265290
/**
266291
* The SSL settings.
267292
*
268293
* @see SSLService
269294
*/
270295
public static final Setting.AffixSetting<Settings> SSL_SETTING =
271-
Setting.affixKeySetting("xpack.monitoring.exporters.","ssl",
272-
(key) -> Setting.groupSetting(key + ".", Property.Dynamic, Property.NodeScope, Property.Filtered));
296+
Setting.affixKeySetting(
297+
"xpack.monitoring.exporters.",
298+
"ssl",
299+
(key) -> Setting.groupSetting(key + ".", Property.Dynamic, Property.NodeScope, Property.Filtered),
300+
TYPE_DEPENDENCY);
273301
/**
274302
* Proxy setting to allow users to send requests to a remote cluster that requires a proxy base path.
275303
*/
@@ -288,13 +316,14 @@ public Iterator<Setting<?>> settings() {
288316
}
289317
},
290318
Property.Dynamic,
291-
Property.NodeScope));
319+
Property.NodeScope),
320+
TYPE_DEPENDENCY);
292321
/**
293322
* A boolean setting to enable or disable sniffing for extra connections.
294323
*/
295324
public static final Setting.AffixSetting<Boolean> SNIFF_ENABLED_SETTING =
296325
Setting.affixKeySetting("xpack.monitoring.exporters.","sniff.enabled",
297-
(key) -> Setting.boolSetting(key, false, Property.Dynamic, Property.NodeScope));
326+
(key) -> Setting.boolSetting(key, false, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
298327
/**
299328
* A parent setting to header key/value pairs, whose names are user defined.
300329
*/
@@ -316,7 +345,8 @@ public Iterator<Setting<?>> settings() {
316345
}
317346
},
318347
Property.Dynamic,
319-
Property.NodeScope));
348+
Property.NodeScope),
349+
TYPE_DEPENDENCY);
320350
/**
321351
* Blacklist of headers that the user is not allowed to set.
322352
* <p>
@@ -328,19 +358,19 @@ public Iterator<Setting<?>> settings() {
328358
*/
329359
public static final Setting.AffixSetting<TimeValue> TEMPLATE_CHECK_TIMEOUT_SETTING =
330360
Setting.affixKeySetting("xpack.monitoring.exporters.","index.template.master_timeout",
331-
(key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope));
361+
(key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
332362
/**
333363
* A boolean setting to enable or disable whether to create placeholders for the old templates.
334364
*/
335365
public static final Setting.AffixSetting<Boolean> TEMPLATE_CREATE_LEGACY_VERSIONS_SETTING =
336366
Setting.affixKeySetting("xpack.monitoring.exporters.","index.template.create_legacy_templates",
337-
(key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope));
367+
(key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
338368
/**
339369
* ES level timeout used when checking and writing pipelines (used to speed up tests)
340370
*/
341371
public static final Setting.AffixSetting<TimeValue> PIPELINE_CHECK_TIMEOUT_SETTING =
342372
Setting.affixKeySetting("xpack.monitoring.exporters.","index.pipeline.master_timeout",
343-
(key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope));
373+
(key) -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Property.Dynamic, Property.NodeScope), TYPE_DEPENDENCY);
344374

345375
/**
346376
* Minimum supported version of the remote monitoring cluster (same major).

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,8 @@ private Exporter getExporter(String name) {
177177
private ActionFuture<ClusterUpdateSettingsResponse> setVerificationMode(String name, VerificationMode mode) {
178178
final ClusterUpdateSettingsRequest updateSettings = new ClusterUpdateSettingsRequest();
179179
final Settings settings = Settings.builder()
180+
.put("xpack.monitoring.exporters." + name + ".type", HttpExporter.TYPE)
181+
.put("xpack.monitoring.exporters." + name + ".host", "https://" + webServer.getHostName() + ":" + webServer.getPort())
180182
.put("xpack.monitoring.exporters." + name + ".ssl.verification_mode", mode.name())
181183
.build();
182184
updateSettings.transientSettings(settings);

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.elasticsearch.cluster.metadata.MetaData;
1818
import org.elasticsearch.cluster.node.DiscoveryNodes;
1919
import org.elasticsearch.cluster.service.ClusterService;
20+
import org.elasticsearch.common.settings.ClusterSettings;
2021
import org.elasticsearch.common.settings.Settings;
2122
import org.elasticsearch.common.settings.SettingsException;
2223
import org.elasticsearch.common.unit.TimeValue;
@@ -38,6 +39,7 @@
3839
import java.util.HashMap;
3940
import java.util.List;
4041
import java.util.Map;
42+
import java.util.Set;
4143
import java.util.concurrent.CountDownLatch;
4244
import java.util.concurrent.TimeUnit;
4345
import java.util.stream.Collectors;
@@ -133,14 +135,9 @@ public void testHostListIsRejectedIfTypeIsNotHttp() {
133135
final Settings.Builder builder = Settings.builder().put(prefix + ".type", "local");
134136
builder.putList(prefix + ".host", List.of("https://example.com:443"));
135137
final Settings settings = builder.build();
136-
final IllegalArgumentException e = expectThrows(
137-
IllegalArgumentException.class,
138-
() -> HttpExporter.HOST_SETTING.getConcreteSetting(prefix + ".host").get(settings));
139-
assertThat(
140-
e,
141-
hasToString(containsString("Failed to parse value [[\"https://example.com:443\"]] for setting [" + prefix + ".host]")));
142-
assertThat(e.getCause(), instanceOf(SettingsException.class));
143-
assertThat(e.getCause(), hasToString(containsString("host list for [" + prefix + ".host] is set but type is [local]")));
138+
final ClusterSettings clusterSettings = new ClusterSettings(settings, Set.of(HttpExporter.HOST_SETTING, Exporter.TYPE_SETTING));
139+
final SettingsException e = expectThrows(SettingsException.class, () -> clusterSettings.validate(settings, true));
140+
assertThat(e, hasToString(containsString("[" + prefix + ".host] is set but type is [local]")));
144141
}
145142

146143
public void testInvalidHost() {

0 commit comments

Comments
 (0)