Skip to content

Commit bfb2dc1

Browse files
jasontedorjasontedor
committed
Enable dependent settings values to be validated (elastic#49942)
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.
1 parent 48e7420 commit bfb2dc1

File tree

8 files changed

+139
-34
lines changed

8 files changed

+139
-34
lines changed

‎plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,29 +62,34 @@ final class AzureStorageSettings {
6262
public static final AffixSetting<Integer> MAX_RETRIES_SETTING =
6363
Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "max_retries",
6464
(key) -> Setting.intSetting(key, RetryPolicy.DEFAULT_CLIENT_RETRY_COUNT, Setting.Property.NodeScope),
65-
ACCOUNT_SETTING, KEY_SETTING);
65+
() -> ACCOUNT_SETTING, () -> KEY_SETTING);
6666
/**
6767
* Azure endpoint suffix. Default to core.windows.net (CloudStorageAccount.DEFAULT_DNS).
6868
*/
6969
public static final AffixSetting<String> ENDPOINT_SUFFIX_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "endpoint_suffix",
70-
key -> Setting.simpleString(key, Property.NodeScope), ACCOUNT_SETTING, KEY_SETTING);
70+
key -> Setting.simpleString(key, Property.NodeScope), () -> ACCOUNT_SETTING, () -> KEY_SETTING);
7171

7272
public static final AffixSetting<TimeValue> TIMEOUT_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "timeout",
73-
(key) -> Setting.timeSetting(key, TimeValue.timeValueMinutes(-1), Property.NodeScope), ACCOUNT_SETTING, KEY_SETTING);
73+
(key) -> Setting.timeSetting(key, TimeValue.timeValueMinutes(-1), Property.NodeScope), () -> ACCOUNT_SETTING, () -> KEY_SETTING);
7474

7575
/** The type of the proxy to connect to azure through. Can be direct (no proxy, default), http or socks */
7676
public static final AffixSetting<Proxy.Type> PROXY_TYPE_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "proxy.type",
7777
(key) -> new Setting<>(key, "direct", s -> Proxy.Type.valueOf(s.toUpperCase(Locale.ROOT)), Property.NodeScope)
78-
, ACCOUNT_SETTING, KEY_SETTING);
78+
, () -> ACCOUNT_SETTING, () -> KEY_SETTING);
7979

8080
/** The host name of a proxy to connect to azure through. */
8181
public static final AffixSetting<String> PROXY_HOST_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "proxy.host",
82-
(key) -> Setting.simpleString(key, Property.NodeScope), KEY_SETTING, ACCOUNT_SETTING, PROXY_TYPE_SETTING);
82+
(key) -> Setting.simpleString(key, Property.NodeScope), () -> KEY_SETTING, () -> ACCOUNT_SETTING, () -> PROXY_TYPE_SETTING);
8383

8484
/** The port of a proxy to connect to azure through. */
85-
public static final Setting<Integer> PROXY_PORT_SETTING = Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "proxy.port",
86-
(key) -> Setting.intSetting(key, 0, 0, 65535, Setting.Property.NodeScope), ACCOUNT_SETTING, KEY_SETTING, PROXY_TYPE_SETTING,
87-
PROXY_HOST_SETTING);
85+
public static final Setting<Integer> PROXY_PORT_SETTING = Setting.affixKeySetting(
86+
AZURE_CLIENT_PREFIX_KEY,
87+
"proxy.port",
88+
(key) -> Setting.intSetting(key, 0, 0, 65535, Setting.Property.NodeScope),
89+
() -> ACCOUNT_SETTING,
90+
() -> KEY_SETTING,
91+
() -> PROXY_TYPE_SETTING,
92+
() -> PROXY_HOST_SETTING);
8893

8994
private final String account;
9095
private final String connectString;

‎server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -529,20 +529,24 @@ void validate(
529529
}
530530
throw new IllegalArgumentException(msg);
531531
} else {
532-
Set<Setting<?>> settingsDependencies = setting.getSettingsDependencies(key);
532+
Set<Setting.SettingDependency> settingsDependencies = setting.getSettingsDependencies(key);
533533
if (setting.hasComplexMatcher()) {
534534
setting = setting.getConcreteSetting(key);
535535
}
536536
if (validateDependencies && settingsDependencies.isEmpty() == false) {
537-
for (final Setting<?> settingDependency : settingsDependencies) {
538-
if (settingDependency.existsOrFallbackExists(settings) == false) {
537+
for (final Setting.SettingDependency settingDependency : settingsDependencies) {
538+
final Setting<?> dependency = settingDependency.getSetting();
539+
// validate the dependent setting is set
540+
if (dependency.existsOrFallbackExists(settings) == false) {
539541
final String message = String.format(
540542
Locale.ROOT,
541543
"missing required setting [%s] for setting [%s]",
542-
settingDependency.getKey(),
544+
dependency.getKey(),
543545
setting.getKey());
544546
throw new IllegalArgumentException(message);
545547
}
548+
// validate the dependent setting value
549+
settingDependency.validate(setting.getKey(), setting.get(settings), dependency.get(settings));
546550
}
547551
}
548552
// the only time that validateInternalOrPrivateIndex should be true is if this call is coming via the update settings API

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

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -563,11 +563,37 @@ public Setting<T> getConcreteSetting(String key) {
563563
return this;
564564
}
565565

566+
/**
567+
* Allows a setting to declare a dependency on another setting being set. Optionally, a setting can validate the value of the dependent
568+
* setting.
569+
*/
570+
public interface SettingDependency {
571+
572+
/**
573+
* The setting to declare a dependency on.
574+
*
575+
* @return the setting
576+
*/
577+
Setting getSetting();
578+
579+
/**
580+
* Validates the dependent setting value.
581+
*
582+
* @param key the key for this setting
583+
* @param value the value of this setting
584+
* @param dependency the value of the dependent setting
585+
*/
586+
default void validate(String key, Object value, Object dependency) {
587+
588+
}
589+
590+
}
591+
566592
/**
567593
* Returns a set of settings that are required at validation time. Unless all of the dependencies are present in the settings
568594
* object validation of setting must fail.
569595
*/
570-
public Set<Setting<?>> getSettingsDependencies(String key) {
596+
public Set<SettingDependency> getSettingsDependencies(final String key) {
571597
return Collections.emptySet();
572598
}
573599

@@ -670,13 +696,23 @@ public String toString() {
670696
};
671697
}
672698

699+
/**
700+
* Allows an affix setting to declare a dependency on another affix setting.
701+
*/
702+
public interface AffixSettingDependency extends SettingDependency {
703+
704+
@Override
705+
AffixSetting getSetting();
706+
707+
}
708+
673709
public static class AffixSetting<T> extends Setting<T> {
674710
private final AffixKey key;
675711
private final BiFunction<String, String, Setting<T>> delegateFactory;
676-
private final Set<AffixSetting> dependencies;
712+
private final Set<AffixSettingDependency> dependencies;
677713

678714
public AffixSetting(AffixKey key, Setting<T> delegate, BiFunction<String, String, Setting<T>> delegateFactory,
679-
AffixSetting... dependencies) {
715+
AffixSettingDependency... dependencies) {
680716
super(key, delegate.defaultValue, delegate.parser, delegate.properties.toArray(new Property[0]));
681717
this.key = key;
682718
this.delegateFactory = delegateFactory;
@@ -692,12 +728,25 @@ private Stream<String> matchStream(Settings settings) {
692728
}
693729

694730
@Override
695-
public Set<Setting<?>> getSettingsDependencies(String settingsKey) {
731+
public Set<SettingDependency> getSettingsDependencies(String settingsKey) {
696732
if (dependencies.isEmpty()) {
697733
return Collections.emptySet();
698734
} else {
699735
String namespace = key.getNamespace(settingsKey);
700-
return dependencies.stream().map(s -> (Setting<?>)s.getConcreteSettingForNamespace(namespace)).collect(Collectors.toSet());
736+
return dependencies.stream()
737+
.map(s ->
738+
new SettingDependency() {
739+
@Override
740+
public Setting<Object> getSetting() {
741+
return s.getSetting().getConcreteSettingForNamespace(namespace);
742+
}
743+
744+
@Override
745+
public void validate(final String key, final Object value, final Object dependency) {
746+
s.validate(key, value, dependency);
747+
};
748+
})
749+
.collect(Collectors.toSet());
701750
}
702751
}
703752

@@ -1638,19 +1687,19 @@ public static <T> AffixSetting<T> prefixKeySetting(String prefix, Function<Strin
16381687
* out of the box unless {@link #getConcreteSetting(String)} is used to pull the updater.
16391688
*/
16401689
public static <T> AffixSetting<T> affixKeySetting(String prefix, String suffix, Function<String, Setting<T>> delegateFactory,
1641-
AffixSetting... dependencies) {
1690+
AffixSettingDependency... dependencies) {
16421691
BiFunction<String, String, Setting<T>> delegateFactoryWithNamespace = (ns, k) -> delegateFactory.apply(k);
16431692
return affixKeySetting(new AffixKey(prefix, suffix), delegateFactoryWithNamespace, dependencies);
16441693
}
16451694

16461695
public static <T> AffixSetting<T> affixKeySetting(String prefix, String suffix, BiFunction<String, String, Setting<T>> delegateFactory,
1647-
AffixSetting... dependencies) {
1696+
AffixSettingDependency... dependencies) {
16481697
Setting<T> delegate = delegateFactory.apply("_na_", "_na_");
16491698
return new AffixSetting<>(new AffixKey(prefix, suffix), delegate, delegateFactory, dependencies);
16501699
}
16511700

16521701
private static <T> AffixSetting<T> affixKeySetting(AffixKey key, BiFunction<String, String, Setting<T>> delegateFactory,
1653-
AffixSetting... dependencies) {
1702+
AffixSettingDependency... dependencies) {
16541703
Setting<T> delegate = delegateFactory.apply("_na_", "_na_");
16551704
return new AffixSetting<>(key, delegate, delegateFactory, dependencies);
16561705
}

‎server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public final class RemoteClusterService extends RemoteClusterAware implements Cl
121121
"search.remote.",
122122
"skip_unavailable",
123123
key -> boolSetting(key, false, Setting.Property.Deprecated, Setting.Property.Dynamic, Setting.Property.NodeScope),
124-
SniffConnectionStrategy.REMOTE_CLUSTER_SEEDS);
124+
() -> SniffConnectionStrategy.REMOTE_CLUSTER_SEEDS);
125125

126126
public static final SettingUpgrader<Boolean> SEARCH_REMOTE_CLUSTER_SKIP_UNAVAILABLE_UPGRADER = new SettingUpgrader<Boolean>() {
127127

@@ -149,19 +149,19 @@ public String getKey(final String key) {
149149
: SEARCH_REMOTE_CLUSTER_SKIP_UNAVAILABLE.getConcreteSetting(key.replaceAll("^cluster", "search")),
150150
Setting.Property.Dynamic,
151151
Setting.Property.NodeScope),
152-
SniffConnectionStrategy.REMOTE_CLUSTER_SEEDS);
152+
() -> SniffConnectionStrategy.REMOTE_CLUSTER_SEEDS);
153153

154154
public static final Setting.AffixSetting<TimeValue> REMOTE_CLUSTER_PING_SCHEDULE = Setting.affixKeySetting(
155155
"cluster.remote.",
156156
"transport.ping_schedule",
157157
key -> timeSetting(key, TransportSettings.PING_SCHEDULE, Setting.Property.Dynamic, Setting.Property.NodeScope),
158-
SniffConnectionStrategy.REMOTE_CLUSTER_SEEDS);
158+
() -> SniffConnectionStrategy.REMOTE_CLUSTER_SEEDS);
159159

160160
public static final Setting.AffixSetting<Boolean> REMOTE_CLUSTER_COMPRESS = Setting.affixKeySetting(
161161
"cluster.remote.",
162162
"transport.compress",
163163
key -> boolSetting(key, TransportSettings.TRANSPORT_COMPRESS, Setting.Property.Dynamic, Setting.Property.NodeScope),
164-
SniffConnectionStrategy.REMOTE_CLUSTER_SEEDS);
164+
() -> SniffConnectionStrategy.REMOTE_CLUSTER_SEEDS);
165165

166166
private final TransportService transportService;
167167
private final Map<String, RemoteClusterConnection> remoteClusters = ConcurrentCollections.newConcurrentMap();

‎server/src/main/java/org/elasticsearch/transport/SniffConnectionStrategy.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public String getKey(final String key) {
147147
Setting.Property.Deprecated,
148148
Setting.Property.Dynamic,
149149
Setting.Property.NodeScope),
150-
REMOTE_CLUSTER_SEEDS);
150+
() -> REMOTE_CLUSTER_SEEDS);
151151

152152
/**
153153
* A proxy address for the remote cluster. By default this is not set, meaning that Elasticsearch will connect directly to the nodes in
@@ -168,7 +168,7 @@ public String getKey(final String key) {
168168
SEARCH_REMOTE_CLUSTERS_PROXY.getConcreteSettingForNamespace(ns),
169169
Setting.Property.Dynamic,
170170
Setting.Property.NodeScope),
171-
REMOTE_CLUSTER_SEEDS);
171+
() -> REMOTE_CLUSTER_SEEDS);
172172

173173
public static final Setting<Integer> SEARCH_REMOTE_CONNECTIONS_PER_CLUSTER =
174174
intSetting("search.remote.connections_per_cluster", 3, 1, Setting.Property.NodeScope, Setting.Property.Deprecated);

‎server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,9 @@ public void testDependentSettings() {
179179
Setting.AffixSetting<String> stringSetting = Setting.affixKeySetting("foo.", "name",
180180
(k) -> Setting.simpleString(k, Property.Dynamic, Property.NodeScope));
181181
Setting.AffixSetting<Integer> intSetting = Setting.affixKeySetting("foo.", "bar",
182-
(k) -> Setting.intSetting(k, 1, Property.Dynamic, Property.NodeScope), stringSetting);
182+
(k) -> Setting.intSetting(k, 1, Property.Dynamic, Property.NodeScope), () -> stringSetting);
183183

184-
AbstractScopedSettings service = new ClusterSettings(Settings.EMPTY,new HashSet<>(Arrays.asList(intSetting, stringSetting)));
184+
AbstractScopedSettings service = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(intSetting, stringSetting)));
185185

186186
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
187187
() -> service.validate(Settings.builder().put("foo.test.bar", 7).build(), true));
@@ -195,6 +195,50 @@ public void testDependentSettings() {
195195
service.validate(Settings.builder().put("foo.test.bar", 7).build(), false);
196196
}
197197

198+
public void testDependentSettingsValidate() {
199+
Setting.AffixSetting<String> stringSetting = Setting.affixKeySetting(
200+
"foo.",
201+
"name",
202+
(k) -> Setting.simpleString(k, Property.Dynamic, Property.NodeScope));
203+
Setting.AffixSetting<Integer> intSetting = Setting.affixKeySetting(
204+
"foo.",
205+
"bar",
206+
(k) -> Setting.intSetting(k, 1, Property.Dynamic, Property.NodeScope),
207+
new Setting.AffixSettingDependency() {
208+
209+
@Override
210+
public Setting.AffixSetting getSetting() {
211+
return stringSetting;
212+
}
213+
214+
@Override
215+
public void validate(final String key, final Object value, final Object dependency) {
216+
if ("valid".equals(dependency) == false) {
217+
throw new SettingsException("[" + key + "] is set but [name] is [" + dependency + "]");
218+
}
219+
}
220+
});
221+
222+
AbstractScopedSettings service = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(intSetting, stringSetting)));
223+
224+
SettingsException iae = expectThrows(
225+
SettingsException.class,
226+
() -> service.validate(Settings.builder().put("foo.test.bar", 7).put("foo.test.name", "invalid").build(), true));
227+
assertEquals("[foo.test.bar] is set but [name] is [invalid]", iae.getMessage());
228+
229+
service.validate(Settings.builder()
230+
.put("foo.test.bar", 7)
231+
.put("foo.test.name", "valid")
232+
.build(),
233+
true);
234+
235+
service.validate(Settings.builder()
236+
.put("foo.test.bar", 7)
237+
.put("foo.test.name", "invalid")
238+
.build(),
239+
false);
240+
}
241+
198242
public void testDependentSettingsWithFallback() {
199243
Setting.AffixSetting<String> nameFallbackSetting =
200244
Setting.affixKeySetting("fallback.", "name", k -> Setting.simpleString(k, Property.Dynamic, Property.NodeScope));
@@ -208,8 +252,11 @@ public void testDependentSettingsWithFallback() {
208252
: nameFallbackSetting.getConcreteSetting(k.replaceAll("^foo", "fallback")),
209253
Property.Dynamic,
210254
Property.NodeScope));
211-
Setting.AffixSetting<Integer> barSetting =
212-
Setting.affixKeySetting("foo.", "bar", k -> Setting.intSetting(k, 1, Property.Dynamic, Property.NodeScope), nameSetting);
255+
Setting.AffixSetting<Integer> barSetting = Setting.affixKeySetting(
256+
"foo.",
257+
"bar",
258+
k -> Setting.intSetting(k, 1, Property.Dynamic, Property.NodeScope),
259+
() -> nameSetting);
213260

214261
final AbstractScopedSettings service =
215262
new ClusterSettings(Settings.EMPTY,new HashSet<>(Arrays.asList(nameFallbackSetting, nameSetting, barSetting)));

‎server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,12 @@ public static class DummySettingPlugin extends Plugin {
9393
public static final Setting.AffixSetting<String> DUMMY_ACCOUNT_USER = Setting.affixKeySetting("index.acc.", "user",
9494
k -> Setting.simpleString(k, Setting.Property.IndexScope, Setting.Property.Dynamic));
9595
public static final Setting<String> DUMMY_ACCOUNT_PW = Setting.affixKeySetting("index.acc.", "pw",
96-
k -> Setting.simpleString(k, Setting.Property.IndexScope, Setting.Property.Dynamic), DUMMY_ACCOUNT_USER);
96+
k -> Setting.simpleString(k, Setting.Property.IndexScope, Setting.Property.Dynamic), () -> DUMMY_ACCOUNT_USER);
9797

9898
public static final Setting.AffixSetting<String> DUMMY_ACCOUNT_USER_CLUSTER = Setting.affixKeySetting("cluster.acc.", "user",
9999
k -> Setting.simpleString(k, Setting.Property.NodeScope, Setting.Property.Dynamic));
100100
public static final Setting<String> DUMMY_ACCOUNT_PW_CLUSTER = Setting.affixKeySetting("cluster.acc.", "pw",
101-
k -> Setting.simpleString(k, Setting.Property.NodeScope, Setting.Property.Dynamic), DUMMY_ACCOUNT_USER_CLUSTER);
101+
k -> Setting.simpleString(k, Setting.Property.NodeScope, Setting.Property.Dynamic), () -> DUMMY_ACCOUNT_USER_CLUSTER);
102102

103103
@Override
104104
public void onIndexModule(IndexModule indexModule) {

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
/**
2222
* Provides a number of utility methods for interacting with {@link Settings} and {@link Setting} inside a {@link Realm}.
23-
* Settings for realms use an {@link Setting#affixKeySetting(String, String, Function, Setting.AffixSetting[]) affix} style,
23+
* Settings for realms use an {@link Setting#affixKeySetting(String, String, Function, Setting.AffixSettingDependency[]) affix} style,
2424
* where the <em>type</em> of the realm is part of the prefix, and name of the realm is the variable portion
2525
* (That is to set the order in a file realm named "file1", then full setting key would be
2626
* {@code xpack.security.authc.realms.file.file1.order}.
@@ -74,7 +74,7 @@ public static Setting.AffixSetting<SecureString> secureString(String realmType,
7474
* The {@code Function} takes the <em>realm-type</em> as an argument.
7575
* @param suffix The suffix of the setting (everything following the realm name in the affix setting)
7676
* @param delegateFactory A factory to produce the concrete setting.
77-
* See {@link Setting#affixKeySetting(String, String, Function, Setting.AffixSetting[])}
77+
* See {@link Setting#affixKeySetting(String, String, Function, Setting.AffixSettingDependency[])}
7878
*/
7979
public static <T> Function<String, Setting.AffixSetting<T>> affixSetting(String suffix, Function<String, Setting<T>> delegateFactory) {
8080
return realmType -> Setting.affixKeySetting(realmSettingPrefix(realmType), suffix, delegateFactory);

0 commit comments

Comments
 (0)