From 8860364f72f479be218d2beacc35731e9d63a546 Mon Sep 17 00:00:00 2001 From: jaymode Date: Mon, 14 Sep 2015 16:46:46 -0400 Subject: [PATCH] update settings for tribes to fail if shield is not enabled or mandatory In 2.0, plugins cannot specify mandatory settings, they can only specify a default additional set of settings. For tribe nodes, we require shield to be enabled and be a mandatory plugin. If the settings specified by the user conflict with this, we now throw an exception and fail startup. Closes elastic/elasticsearch#426 Original commit: elastic/x-pack-elasticsearch@db4d6d79234ba3f954f0a4f7a996c6bfbdda5723 --- .../elasticsearch/shield/ShieldPlugin.java | 25 +++++--- .../shield/ShieldPluginSettingsTests.java | 61 ++++++++++--------- 2 files changed, 47 insertions(+), 39 deletions(-) diff --git a/shield/src/main/java/org/elasticsearch/shield/ShieldPlugin.java b/shield/src/main/java/org/elasticsearch/shield/ShieldPlugin.java index 2c5ad5082df..8c3a3175611 100644 --- a/shield/src/main/java/org/elasticsearch/shield/ShieldPlugin.java +++ b/shield/src/main/java/org/elasticsearch/shield/ShieldPlugin.java @@ -60,11 +60,11 @@ import java.util.Map; public class ShieldPlugin extends Plugin { public static final String NAME = "shield"; - public static final String ENABLED_SETTING_NAME = NAME + ".enabled"; - public static final String OPT_OUT_QUERY_CACHE = "opt_out_cache"; + private static final boolean DEFAULT_ENABLED_SETTING = true; + private final Settings settings; private final boolean enabled; private final boolean clientMode; @@ -250,15 +250,20 @@ public class ShieldPlugin extends Plugin { settingsBuilder.putArray(tribePrefix + "plugin.mandatory", NAME); } else { if (!isShieldMandatory(existingMandatoryPlugins)) { - String[] updatedMandatoryPlugins = new String[existingMandatoryPlugins.length + 1]; - System.arraycopy(existingMandatoryPlugins, 0, updatedMandatoryPlugins, 0, existingMandatoryPlugins.length); - updatedMandatoryPlugins[updatedMandatoryPlugins.length - 1] = NAME; - //shield is mandatory on every tribe if installed and enabled on the tribe node - settingsBuilder.putArray(tribePrefix + "plugin.mandatory", updatedMandatoryPlugins); + throw new IllegalStateException("when [plugin.mandatory] is explicitly configured, [" + NAME + "] must be included in this list"); } } - //shield must be enabled on every tribe if it's enabled on the tribe node - settingsBuilder.put(tribePrefix + ENABLED_SETTING_NAME, true); + + final String tribeEnabledSetting = tribePrefix + ENABLED_SETTING_NAME; + if (settings.get(tribeEnabledSetting) != null) { + boolean enabled = shieldEnabled(tribeSettings.getValue()); + if (!enabled) { + throw new IllegalStateException("tribe setting [" + tribeEnabledSetting + "] must be set to true but the value is [" + settings.get(tribeEnabledSetting) + "]"); + } + } else { + //shield must be enabled on every tribe if it's enabled on the tribe node + settingsBuilder.put(tribeEnabledSetting, true); + } } } @@ -294,7 +299,7 @@ public class ShieldPlugin extends Plugin { } public static boolean shieldEnabled(Settings settings) { - return settings.getAsBoolean(ENABLED_SETTING_NAME, true); + return settings.getAsBoolean(ENABLED_SETTING_NAME, DEFAULT_ENABLED_SETTING); } private void failIfShieldQueryCacheIsNotActive(Settings settings, boolean nodeSettings) { diff --git a/shield/src/test/java/org/elasticsearch/shield/ShieldPluginSettingsTests.java b/shield/src/test/java/org/elasticsearch/shield/ShieldPluginSettingsTests.java index 1fcf3ed9df6..d21a144cfa5 100644 --- a/shield/src/test/java/org/elasticsearch/shield/ShieldPluginSettingsTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/ShieldPluginSettingsTests.java @@ -9,6 +9,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.junit.Test; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.Matchers.arrayContaining; @@ -40,13 +41,13 @@ public class ShieldPluginSettingsTests extends ESTestCase { ShieldPlugin shieldPlugin = new ShieldPlugin(settings); //simulate what PluginsService#updatedSettings does to make sure we don't override existing mandatory plugins - Settings finalSettings = Settings.builder().put(settings).put(shieldPlugin.additionalSettings()).build(); - - String[] finalMandatoryPlugins = finalSettings.getAsArray("tribe.t1.plugin.mandatory", null); - assertThat(finalMandatoryPlugins, notNullValue()); - assertThat(finalMandatoryPlugins.length, equalTo(2)); - assertThat(finalMandatoryPlugins[0], equalTo("test_plugin")); - assertThat(finalMandatoryPlugins[1], equalTo(ShieldPlugin.NAME)); + try { + Settings.builder().put(settings).put(shieldPlugin.additionalSettings()).build(); + fail("shield cannot change the value of a setting that is already defined, so a exception should be thrown"); + } catch (IllegalStateException e) { + assertThat(e.getMessage(), containsString("shield")); + assertThat(e.getMessage(), containsString("plugin.mandatory")); + } } @Test @@ -67,9 +68,8 @@ public class ShieldPluginSettingsTests extends ESTestCase { } @Test - public void testShieldAlwaysEnabledOnTribes() { + public void testShieldIsEnabledByDefaultOnTribes() { Settings settings = Settings.builder().put("tribe.t1.cluster.name", "non_existing") - .put(TRIBE_T1_SHIELD_ENABLED, false) .put("tribe.t2.cluster.name", "non_existing").build(); ShieldPlugin shieldPlugin = new ShieldPlugin(settings); @@ -78,15 +78,26 @@ public class ShieldPluginSettingsTests extends ESTestCase { assertThat(additionalSettings.getAsBoolean(TRIBE_T1_SHIELD_ENABLED, null), equalTo(true)); assertThat(additionalSettings.getAsBoolean(TRIBE_T2_SHIELD_ENABLED, null), equalTo(true)); - - //simulate what PluginsService#updatedSettings does to make sure additional settings override existing settings with same name - Settings finalSettings = Settings.builder().put(settings).put(shieldPlugin.additionalSettings()).build(); - assertThat(finalSettings.getAsBoolean(TRIBE_T1_SHIELD_ENABLED, null), equalTo(true)); - assertThat(finalSettings.getAsBoolean(TRIBE_T2_SHIELD_ENABLED, null), equalTo(true)); } @Test - public void testShieldAlwaysEnabledOnTribesShieldAlreadyMandatory() { + public void testShieldDisabledOnATribe() { + Settings settings = Settings.builder().put("tribe.t1.cluster.name", "non_existing") + .put(TRIBE_T1_SHIELD_ENABLED, false) + .put("tribe.t2.cluster.name", "non_existing").build(); + + ShieldPlugin shieldPlugin = new ShieldPlugin(settings); + + try { + shieldPlugin.additionalSettings(); + fail("shield cannot change the value of a setting that is already defined, so a exception should be thrown"); + } catch (IllegalStateException e) { + assertThat(e.getMessage(), containsString(TRIBE_T1_SHIELD_ENABLED)); + } + } + + @Test + public void testShieldDisabledOnTribesShieldAlreadyMandatory() { Settings settings = Settings.builder().put("tribe.t1.cluster.name", "non_existing") .put(TRIBE_T1_SHIELD_ENABLED, false) .put("tribe.t2.cluster.name", "non_existing") @@ -94,19 +105,11 @@ public class ShieldPluginSettingsTests extends ESTestCase { ShieldPlugin shieldPlugin = new ShieldPlugin(settings); - Settings additionalSettings = shieldPlugin.additionalSettings(); - - assertThat(additionalSettings.getAsBoolean(TRIBE_T1_SHIELD_ENABLED, null), equalTo(true)); - assertThat(additionalSettings.getAsBoolean(TRIBE_T2_SHIELD_ENABLED, null), equalTo(true)); - - //simulate what PluginsService#updatedSettings does to make sure additional settings override existing settings with same name - Settings finalSettings = Settings.builder().put(settings).put(shieldPlugin.additionalSettings()).build(); - assertThat(finalSettings.getAsBoolean(TRIBE_T1_SHIELD_ENABLED, null), equalTo(true)); - assertThat(finalSettings.getAsBoolean(TRIBE_T2_SHIELD_ENABLED, null), equalTo(true)); - String[] finalMandatoryPlugins = finalSettings.getAsArray("tribe.t1.plugin.mandatory", null); - assertThat(finalMandatoryPlugins, notNullValue()); - assertThat(finalMandatoryPlugins.length, equalTo(2)); - assertThat(finalMandatoryPlugins[0], equalTo("test_plugin")); - assertThat(finalMandatoryPlugins[1], equalTo(ShieldPlugin.NAME)); + try { + shieldPlugin.additionalSettings(); + fail("shield cannot change the value of a setting that is already defined, so a exception should be thrown"); + } catch (IllegalStateException e) { + assertThat(e.getMessage(), containsString(TRIBE_T1_SHIELD_ENABLED)); + } } }