Skip to content

Commit 2299c70

Browse files
s1monws1monw
authored
Allow affix settings to specify dependencies (elastic#27161)
We use affix settings to group settings / values under a certain namespace. In some cases like login information for instance a setting is only valid if one or more other settings are present. For instance `x.test.user` is only valid if there is an `x.test.passwd` present and vice versa. This change allows to specify such a dependency to prevent settings updates that leave settings in an inconsistent state.
1 parent 08037ee commit 2299c70

File tree

16 files changed

+309
-83
lines changed

16 files changed

+309
-83
lines changed

‎core/src/main/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdater.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,23 @@ synchronized ClusterState updateSettings(final ClusterState currentState, Settin
5454
transientSettings.put(currentState.metaData().transientSettings());
5555
changed |= clusterSettings.updateDynamicSettings(transientToApply, transientSettings, transientUpdates, "transient");
5656

57+
5758
Settings.Builder persistentSettings = Settings.builder();
5859
persistentSettings.put(currentState.metaData().persistentSettings());
5960
changed |= clusterSettings.updateDynamicSettings(persistentToApply, persistentSettings, persistentUpdates, "persistent");
6061

6162
final ClusterState clusterState;
6263
if (changed) {
64+
Settings transientFinalSettings = transientSettings.build();
65+
Settings persistentFinalSettings = persistentSettings.build();
66+
// both transient and persistent settings must be consistent by itself we can't allow dependencies to be
67+
// in either of them otherwise a full cluster restart will break the settings validation
68+
clusterSettings.validate(transientFinalSettings, true);
69+
clusterSettings.validate(persistentFinalSettings, true);
70+
6371
MetaData.Builder metaData = MetaData.builder(currentState.metaData())
64-
.persistentSettings(persistentSettings.build())
65-
.transientSettings(transientSettings.build());
72+
.persistentSettings(persistentFinalSettings)
73+
.transientSettings(transientFinalSettings);
6674

6775
ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
6876
boolean updatedReadOnly = MetaData.SETTING_READ_ONLY_SETTING.get(metaData.persistentSettings())

‎core/src/main/java/org/elasticsearch/action/admin/indices/template/put/TransportPutIndexTemplateAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ protected void masterOperation(final PutIndexTemplateRequest request, final Clus
7777
}
7878
final Settings.Builder templateSettingsBuilder = Settings.builder();
7979
templateSettingsBuilder.put(request.settings()).normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX);
80-
indexScopedSettings.validate(templateSettingsBuilder);
80+
indexScopedSettings.validate(templateSettingsBuilder.build(), true); // templates must be consistent with regards to dependencies
8181
indexTemplateService.putTemplate(new MetaDataIndexTemplateService.PutRequest(cause, request.name())
8282
.patterns(request.patterns())
8383
.order(request.order())

‎core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,9 @@ public void createIndex(final CreateIndexClusterStateUpdateRequest request,
220220
private void onlyCreateIndex(final CreateIndexClusterStateUpdateRequest request,
221221
final ActionListener<ClusterStateUpdateResponse> listener) {
222222
Settings.Builder updatedSettingsBuilder = Settings.builder();
223-
updatedSettingsBuilder.put(request.settings()).normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX);
224-
indexScopedSettings.validate(updatedSettingsBuilder);
225-
request.settings(updatedSettingsBuilder.build());
226-
223+
Settings build = updatedSettingsBuilder.put(request.settings()).normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX).build();
224+
indexScopedSettings.validate(build, true); // we do validate here - index setting must be consistent
225+
request.settings(build);
227226
clusterService.submitStateUpdateTask("create-index [" + request.index() + "], cause [" + request.cause() + "]",
228227
new IndexCreationTask(logger, allocationService, request, listener, indicesService, aliasValidator, xContentRegistry, settings,
229228
this::validate));
@@ -420,7 +419,6 @@ public ClusterState execute(ClusterState currentState) throws Exception {
420419
tmpImdBuilder.primaryTerm(shardId, primaryTerm);
421420
}
422421
}
423-
424422
// Set up everything, now locally create the index to see that things are ok, and apply
425423
final IndexMetaData tmpImd = tmpImdBuilder.build();
426424
ActiveShardCount waitForActiveShards = request.waitForActiveShards();

‎core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ private void validate(PutRequest request) {
276276
}
277277

278278
try {
279-
indexScopedSettings.validate(request.settings);
279+
indexScopedSettings.validate(request.settings, true); // templates must be consistent with regards to dependencies
280280
} catch (IllegalArgumentException iae) {
281281
validationErrors.add(iae.getMessage());
282282
for (Throwable t : iae.getSuppressed()) {

‎core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import java.util.Locale;
5555
import java.util.Map;
5656
import java.util.Set;
57+
import java.util.function.Predicate;
5758

5859
import static org.elasticsearch.action.support.ContextPreservingActionListener.wrapPreservingContext;
5960

@@ -163,7 +164,7 @@ public void updateSettings(final UpdateSettingsClusterStateUpdateRequest request
163164
Settings.Builder settingsForOpenIndices = Settings.builder();
164165
final Set<String> skippedSettings = new HashSet<>();
165166

166-
indexScopedSettings.validate(normalizedSettings);
167+
indexScopedSettings.validate(normalizedSettings, false); // don't validate dependencies here we check it below
167168
// never allow to change the number of shards
168169
for (String key : normalizedSettings.keySet()) {
169170
Setting setting = indexScopedSettings.get(key);
@@ -240,7 +241,9 @@ public ClusterState execute(ClusterState currentState) {
240241
if (preserveExisting) {
241242
indexSettings.put(indexMetaData.getSettings());
242243
}
243-
metaDataBuilder.put(IndexMetaData.builder(indexMetaData).settings(indexSettings));
244+
Settings finalSettings = indexSettings.build();
245+
indexScopedSettings.validate(finalSettings.filter(k -> indexScopedSettings.isPrivateSetting(k) == false), true);
246+
metaDataBuilder.put(IndexMetaData.builder(indexMetaData).settings(finalSettings));
244247
}
245248
}
246249
}
@@ -254,7 +257,9 @@ public ClusterState execute(ClusterState currentState) {
254257
if (preserveExisting) {
255258
indexSettings.put(indexMetaData.getSettings());
256259
}
257-
metaDataBuilder.put(IndexMetaData.builder(indexMetaData).settings(indexSettings));
260+
Settings finalSettings = indexSettings.build();
261+
indexScopedSettings.validate(finalSettings.filter(k -> indexScopedSettings.isPrivateSetting(k) == false), true);
262+
metaDataBuilder.put(IndexMetaData.builder(indexMetaData).settings(finalSettings));
258263
}
259264
}
260265
}

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

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -264,33 +264,28 @@ public synchronized <T> void addSettingsUpdateConsumer(Setting<T> setting, Consu
264264
}
265265

266266
/**
267-
* Validates that all settings in the builder are registered and valid
267+
* Validates that all given settings are registered and valid
268+
* @param settings the settings to validate
269+
* @param validateDependencies if <code>true</code> settings dependencies are validated as well.
270+
* @see Setting#getSettingsDependencies(String)
268271
*/
269-
public final void validate(Settings.Builder settingsBuilder) {
270-
validate(settingsBuilder.build());
271-
}
272-
273-
/**
274-
* * Validates that all given settings are registered and valid
275-
*/
276-
public final void validate(Settings settings) {
272+
public final void validate(Settings settings, boolean validateDependencies) {
277273
List<RuntimeException> exceptions = new ArrayList<>();
278274
for (String key : settings.keySet()) { // settings iterate in deterministic fashion
279275
try {
280-
validate(key, settings);
276+
validate(key, settings, validateDependencies);
281277
} catch (RuntimeException ex) {
282278
exceptions.add(ex);
283279
}
284280
}
285281
ExceptionsHelper.rethrowAndSuppress(exceptions);
286282
}
287283

288-
289284
/**
290285
* Validates that the setting is valid
291286
*/
292-
public final void validate(String key, Settings settings) {
293-
Setting setting = get(key);
287+
void validate(String key, Settings settings, boolean validateDependencies) {
288+
Setting setting = getRaw(key);
294289
if (setting == null) {
295290
LevensteinDistance ld = new LevensteinDistance();
296291
List<Tuple<Float, String>> scoredKeys = new ArrayList<>();
@@ -315,6 +310,20 @@ public final void validate(String key, Settings settings) {
315310
"settings";
316311
}
317312
throw new IllegalArgumentException(msg);
313+
} else {
314+
Set<String> settingsDependencies = setting.getSettingsDependencies(key);
315+
if (setting.hasComplexMatcher()) {
316+
setting = setting.getConcreteSetting(key);
317+
}
318+
if (validateDependencies && settingsDependencies.isEmpty() == false) {
319+
Set<String> settingKeys = settings.keySet();
320+
for (String requiredSetting : settingsDependencies) {
321+
if (settingKeys.contains(requiredSetting) == false) {
322+
throw new IllegalArgumentException("Missing required setting ["
323+
+ requiredSetting + "] for setting [" + setting.getKey() + "]");
324+
}
325+
}
326+
}
318327
}
319328
setting.get(settings);
320329
}
@@ -375,15 +384,27 @@ default Runnable updater(Settings current, Settings previous) {
375384
/**
376385
* Returns the {@link Setting} for the given key or <code>null</code> if the setting can not be found.
377386
*/
378-
public Setting<?> get(String key) {
387+
public final Setting<?> get(String key) {
388+
Setting<?> raw = getRaw(key);
389+
if (raw == null) {
390+
return null;
391+
} if (raw.hasComplexMatcher()) {
392+
return raw.getConcreteSetting(key);
393+
} else {
394+
return raw;
395+
}
396+
}
397+
398+
private Setting<?> getRaw(String key) {
379399
Setting<?> setting = keySettings.get(key);
380400
if (setting != null) {
381401
return setting;
382402
}
383403
for (Map.Entry<String, Setting<?>> entry : complexMatchers.entrySet()) {
384404
if (entry.getValue().match(key)) {
385405
assert assertMatcher(key, 1);
386-
return entry.getValue().getConcreteSetting(key);
406+
assert entry.getValue().hasComplexMatcher();
407+
return entry.getValue();
387408
}
388409
}
389410
return null;
@@ -513,7 +534,7 @@ private boolean updateSettings(Settings toApply, Settings.Builder target, Settin
513534
} else if (get(key) == null) {
514535
throw new IllegalArgumentException(type + " setting [" + key + "], not recognized");
515536
} else if (isNull == false && canUpdate.test(key)) {
516-
validate(key, toApply);
537+
validate(key, toApply, false); // we might not have a full picture here do to a dependency validation
517538
settingsBuilder.copy(key, toApply);
518539
updates.copy(key, toApply);
519540
changed = true;
@@ -654,7 +675,7 @@ public String setValue(String value) {
654675
* representation. Otherwise <code>false</code>
655676
*/
656677
// TODO this should be replaced by Setting.Property.HIDDEN or something like this.
657-
protected boolean isPrivateSetting(String key) {
678+
public boolean isPrivateSetting(String key) {
658679
return false;
659680
}
660681
}

‎core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ protected void validateSettingKey(Setting setting) {
191191
}
192192

193193
@Override
194-
protected boolean isPrivateSetting(String key) {
194+
public boolean isPrivateSetting(String key) {
195195
switch (key) {
196196
case IndexMetaData.SETTING_CREATION_DATE:
197197
case IndexMetaData.SETTING_INDEX_UUID:

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

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import java.util.Collections;
4343
import java.util.EnumSet;
4444
import java.util.HashMap;
45+
import java.util.HashSet;
4546
import java.util.IdentityHashMap;
4647
import java.util.Iterator;
4748
import java.util.List;
@@ -126,7 +127,7 @@ public enum Property {
126127
private static final EnumSet<Property> EMPTY_PROPERTIES = EnumSet.noneOf(Property.class);
127128

128129
private Setting(Key key, @Nullable Setting<T> fallbackSetting, Function<Settings, String> defaultValue, Function<String, T> parser,
129-
Validator<T> validator, Property... properties) {
130+
Validator<T> validator, Property... properties) {
130131
assert this instanceof SecureSetting || this.isGroupSetting() || parser.apply(defaultValue.apply(Settings.EMPTY)) != null
131132
: "parser returned null";
132133
this.key = key;
@@ -457,6 +458,14 @@ public Setting<T> getConcreteSetting(String key) {
457458
return this;
458459
}
459460

461+
/**
462+
* Returns a set of settings that are required at validation time. Unless all of the dependencies are present in the settings
463+
* object validation of setting must fail.
464+
*/
465+
public Set<String> getSettingsDependencies(String key) {
466+
return Collections.emptySet();
467+
}
468+
460469
/**
461470
* Build a new updater with a noop validator.
462471
*/
@@ -519,11 +528,13 @@ public String toString() {
519528
public static class AffixSetting<T> extends Setting<T> {
520529
private final AffixKey key;
521530
private final Function<String, Setting<T>> delegateFactory;
531+
private final Set<AffixSetting> dependencies;
522532

523-
public AffixSetting(AffixKey key, Setting<T> delegate, Function<String, Setting<T>> delegateFactory) {
533+
public AffixSetting(AffixKey key, Setting<T> delegate, Function<String, Setting<T>> delegateFactory, AffixSetting... dependencies) {
524534
super(key, delegate.defaultValue, delegate.parser, delegate.properties.toArray(new Property[0]));
525535
this.key = key;
526536
this.delegateFactory = delegateFactory;
537+
this.dependencies = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(dependencies)));
527538
}
528539

529540
boolean isGroupSetting() {
@@ -534,6 +545,15 @@ private Stream<String> matchStream(Settings settings) {
534545
return settings.keySet().stream().filter((key) -> match(key)).map(settingKey -> key.getConcreteString(settingKey));
535546
}
536547

548+
public Set<String> getSettingsDependencies(String settingsKey) {
549+
if (dependencies.isEmpty()) {
550+
return Collections.emptySet();
551+
} else {
552+
String namespace = key.getNamespace(settingsKey);
553+
return dependencies.stream().map(s -> s.key.toConcreteKey(namespace).key).collect(Collectors.toSet());
554+
}
555+
}
556+
537557
AbstractScopedSettings.SettingUpdater<Map<AbstractScopedSettings.SettingUpdater<T>, T>> newAffixUpdater(
538558
BiConsumer<String, T> consumer, Logger logger, BiConsumer<String, T> validator) {
539559
return new AbstractScopedSettings.SettingUpdater<Map<AbstractScopedSettings.SettingUpdater<T>, T>>() {
@@ -659,6 +679,13 @@ public Stream<Setting<T>> getAllConcreteSettings(Settings settings) {
659679
return matchStream(settings).distinct().map(this::getConcreteSetting);
660680
}
661681

682+
/**
683+
* Returns distinct namespaces for the given settings
684+
*/
685+
public Set<String> getNamespaces(Settings settings) {
686+
return settings.keySet().stream().filter(this::match).map(key::getNamespace).collect(Collectors.toSet());
687+
}
688+
662689
/**
663690
* Returns a map of all namespaces to it's values give the provided settings
664691
*/
@@ -1184,13 +1211,15 @@ public static <T> AffixSetting<T> prefixKeySetting(String prefix, Function<Strin
11841211
* storage.${backend}.enable=[true|false] can easily be added with this setting. Yet, affix key settings don't support updaters
11851212
* out of the box unless {@link #getConcreteSetting(String)} is used to pull the updater.
11861213
*/
1187-
public static <T> AffixSetting<T> affixKeySetting(String prefix, String suffix, Function<String, Setting<T>> delegateFactory) {
1188-
return affixKeySetting(new AffixKey(prefix, suffix), delegateFactory);
1214+
public static <T> AffixSetting<T> affixKeySetting(String prefix, String suffix, Function<String, Setting<T>> delegateFactory,
1215+
AffixSetting... dependencies) {
1216+
return affixKeySetting(new AffixKey(prefix, suffix), delegateFactory, dependencies);
11891217
}
11901218

1191-
private static <T> AffixSetting<T> affixKeySetting(AffixKey key, Function<String, Setting<T>> delegateFactory) {
1219+
private static <T> AffixSetting<T> affixKeySetting(AffixKey key, Function<String, Setting<T>> delegateFactory,
1220+
AffixSetting... dependencies) {
11921221
Setting<T> delegate = delegateFactory.apply("_na_");
1193-
return new AffixSetting<>(key, delegate, delegateFactory);
1222+
return new AffixSetting<>(key, delegate, delegateFactory, dependencies);
11941223
};
11951224

11961225

‎core/src/main/java/org/elasticsearch/common/settings/SettingsModule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public SettingsModule(Settings settings, List<Setting<?>> additionalSettings, Li
132132
}
133133
}
134134
// by now we are fully configured, lets check node level settings for unregistered index settings
135-
clusterSettings.validate(settings);
135+
clusterSettings.validate(settings, true);
136136
this.settingsFilter = new SettingsFilter(settings, settingsFilterPattern);
137137
}
138138

0 commit comments

Comments
 (0)