Skip to content

Commit

Permalink
Forbid settings without a namespace (#45947)
Browse files Browse the repository at this point in the history
This commit forbids settings that are not in any namespace, all setting
names must now contain a dot.
  • Loading branch information
jasontedor authored Aug 30, 2019
1 parent bf838f6 commit 3179a0c
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,27 @@ public SettingsModule(
List<Setting<?>> additionalSettings,
List<String> settingsFilter,
Set<SettingUpgrader<?>> settingUpgraders) {
this(
settings,
additionalSettings,
settingsFilter,
settingUpgraders,
ClusterSettings.BUILT_IN_CLUSTER_SETTINGS,
IndexScopedSettings.BUILT_IN_INDEX_SETTINGS);
}

SettingsModule(
final Settings settings,
final List<Setting<?>> additionalSettings,
final List<String> settingsFilter,
final Set<SettingUpgrader<?>> settingUpgraders,
final Set<Setting<?>> registeredClusterSettings,
final Set<Setting<?>> registeredIndexSettings) {
this.settings = settings;
for (Setting<?> setting : ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) {
for (Setting<?> setting : registeredClusterSettings) {
registerSetting(setting);
}
for (Setting<?> setting : IndexScopedSettings.BUILT_IN_INDEX_SETTINGS) {
for (Setting<?> setting : registeredIndexSettings) {
registerSetting(setting);
}

Expand Down Expand Up @@ -143,7 +159,7 @@ public SettingsModule(
// by now we are fully configured, lets check node level settings for unregistered index settings
clusterSettings.validate(settings, true);
this.settingsFilter = new SettingsFilter(settingsFilterPattern);
}
}

@Override
public void configure(Binder binder) {
Expand All @@ -159,6 +175,9 @@ public void configure(Binder binder) {
* the setting during startup.
*/
private void registerSetting(Setting<?> setting) {
if (setting.getKey().contains(".") == false) {
throw new IllegalArgumentException("setting [" + setting.getKey() + "] is not in any namespace, its name must contain a dot");
}
if (setting.isFiltered()) {
if (settingsFilterPattern.contains(setting.getKey()) == false) {
registerSettingsFilter(setting.getKey());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@
import org.hamcrest.Matchers;

import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.function.Supplier;

import static java.util.Collections.emptySet;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasToString;
import static org.hamcrest.Matchers.is;

public class SettingsModuleTests extends ModuleTestCase {
Expand Down Expand Up @@ -185,4 +189,31 @@ public void testMutuallyExclusiveScopes() {
assertThat(e.getMessage(), containsString("Cannot register setting [foo.bar] twice"));
}
}

public void testPluginSettingWithoutNamespace() {
final String key = randomAlphaOfLength(8);
final Setting<String> setting = Setting.simpleString(key, Property.NodeScope);
runSettingWithoutNamespaceTest(
key, () -> new SettingsModule(Settings.EMPTY, List.of(setting), List.of(), Set.of(), Set.of(), Set.of()));
}

public void testClusterSettingWithoutNamespace() {
final String key = randomAlphaOfLength(8);
final Setting<String> setting = Setting.simpleString(key, Property.NodeScope);
runSettingWithoutNamespaceTest(
key, () -> new SettingsModule(Settings.EMPTY, List.of(), List.of(), Set.of(), Set.of(setting), Set.of()));
}

public void testIndexSettingWithoutNamespace() {
final String key = randomAlphaOfLength(8);
final Setting<String> setting = Setting.simpleString(key, Property.IndexScope);
runSettingWithoutNamespaceTest(
key, () -> new SettingsModule(Settings.EMPTY, List.of(), List.of(), Set.of(), Set.of(), Set.of(setting)));
}

private void runSettingWithoutNamespaceTest(final String key, final Supplier<SettingsModule> supplier) {
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, supplier::get);
assertThat(e, hasToString(containsString("setting [" + key + "] is not in any namespace, its name must contain a dot")));
}

}

0 comments on commit 3179a0c

Please sign in to comment.