From 20cd47826c9958ee856a9a951d70e8d2fc723dc1 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 25 Aug 2019 12:20:11 -0400 Subject: [PATCH 1/3] Forbid settings without a namespace This commit forbids settings that are not in any namespace, all setting names must now contain a dot. --- .../common/settings/SettingsModule.java | 25 +++++++++++++++-- .../common/settings/SettingsModuleTests.java | 28 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java index 094ce5349b862..adef9fec912f8 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SettingsModule.java @@ -63,11 +63,27 @@ public SettingsModule( List> additionalSettings, List settingsFilter, Set> settingUpgraders) { + this( + settings, + additionalSettings, + settingsFilter, + settingUpgraders, + ClusterSettings.BUILT_IN_CLUSTER_SETTINGS, + IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); + } + + SettingsModule( + final Settings settings, + final List> additionalSettings, + final List settingsFilter, + final Set> settingUpgraders, + final Set> registeredClusterSettings, + final Set> 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); } @@ -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) { @@ -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()); diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index 42c846d01f5a5..bdb09a1993dcb 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -24,9 +24,12 @@ import org.hamcrest.Matchers; import java.util.Arrays; +import java.util.List; +import java.util.Set; 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 { @@ -185,4 +188,29 @@ public void testMutuallyExclusiveScopes() { assertThat(e.getMessage(), containsString("Cannot register setting [foo.bar] twice")); } } + + public void testPluginSettingWithoutNamespace() { + final Setting setting = Setting.simpleString("key", Property.NodeScope); + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> new SettingsModule(Settings.EMPTY, List.of(setting), List.of(), Set.of(), Set.of(), Set.of())); + assertThat(e, hasToString(containsString("setting [key] is not in any namespace, its name must contain a dot"))); + } + + public void testClusterSettingWithoutNamespace() { + final Setting setting = Setting.simpleString("key", Property.NodeScope); + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> new SettingsModule(Settings.EMPTY, List.of(), List.of(), Set.of(), Set.of(setting), Set.of())); + assertThat(e, hasToString(containsString("setting [key] is not in any namespace, its name must contain a dot"))); + } + + public void testIndexSettingWithoutNamespace() { + final Setting setting = Setting.simpleString("key", Property.IndexScope); + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> new SettingsModule(Settings.EMPTY, List.of(), List.of(), Set.of(), Set.of(), Set.of(setting))); + assertThat(e, hasToString(containsString("setting [key] is not in any namespace, its name must contain a dot"))); + } + } From 43ddf00e79e4e1ca478be38ac8252adb55278649 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 25 Aug 2019 12:26:14 -0400 Subject: [PATCH 2/3] Refactor tests --- .../common/settings/SettingsModuleTests.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index bdb09a1993dcb..787437022dbd3 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -26,6 +26,7 @@ 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; @@ -190,27 +191,26 @@ public void testMutuallyExclusiveScopes() { } public void testPluginSettingWithoutNamespace() { - final Setting setting = Setting.simpleString("key", Property.NodeScope); - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, - () -> new SettingsModule(Settings.EMPTY, List.of(setting), List.of(), Set.of(), Set.of(), Set.of())); - assertThat(e, hasToString(containsString("setting [key] is not in any namespace, its name must contain a dot"))); + final String key = randomAlphaOfLength(8); + final Setting 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 Setting setting = Setting.simpleString("key", Property.NodeScope); - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, - () -> new SettingsModule(Settings.EMPTY, List.of(), List.of(), Set.of(), Set.of(setting), Set.of())); - assertThat(e, hasToString(containsString("setting [key] is not in any namespace, its name must contain a dot"))); + final String key = randomAlphaOfLength(8); + final Setting 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 Setting setting = Setting.simpleString("key", Property.IndexScope); - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, - () -> new SettingsModule(Settings.EMPTY, List.of(), List.of(), Set.of(), Set.of(), Set.of(setting))); - assertThat(e, hasToString(containsString("setting [key] is not in any namespace, its name must contain a dot"))); + final String key = randomAlphaOfLength(8); + final Setting 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 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"))); } } From 08827f75f3b763355b5d2be4678f2ee46a0ef2b0 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 25 Aug 2019 13:29:01 -0400 Subject: [PATCH 3/3] Fix checkstyle --- .../common/settings/SettingsModuleTests.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index 787437022dbd3..9f2ed63b6da61 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -193,19 +193,22 @@ public void testMutuallyExclusiveScopes() { public void testPluginSettingWithoutNamespace() { final String key = randomAlphaOfLength(8); final Setting setting = Setting.simpleString(key, Property.NodeScope); - runSettingWithoutNamespaceTest(key, () -> new SettingsModule(Settings.EMPTY, List.of(setting), List.of(), Set.of(), Set.of(), Set.of())); + 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 setting = Setting.simpleString(key, Property.NodeScope); - runSettingWithoutNamespaceTest(key, () -> new SettingsModule(Settings.EMPTY, List.of(), List.of(), Set.of(), Set.of(setting), Set.of())); + 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 setting = Setting.simpleString(key, Property.IndexScope); - runSettingWithoutNamespaceTest(key, () -> new SettingsModule(Settings.EMPTY, List.of(), List.of(), Set.of(), Set.of(), Set.of(setting))); + 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 supplier) {