From 0f529e10a8ff4931cb12cb564ea7bf202e5acfaf Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 26 May 2016 09:18:20 -0400 Subject: [PATCH 1/3] Fix plugin command name in remove plugin command This commit fixes the name of the plugin command that is output when a user attempts to remove a plugin that does not exist. --- .../java/org/elasticsearch/plugins/RemovePluginCommand.java | 2 +- .../org/elasticsearch/plugins/RemovePluginCommandTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java index af48c1d8207..8279cc5c51f 100644 --- a/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java @@ -71,7 +71,7 @@ class RemovePluginCommand extends SettingCommand { Path pluginDir = env.pluginsFile().resolve(pluginName); if (Files.exists(pluginDir) == false) { - throw new UserError(ExitCodes.USAGE, "Plugin " + pluginName + " not found. Run 'plugin list' to get list of installed plugins."); + throw new UserError(ExitCodes.USAGE, "plugin " + pluginName + " not found; run 'elasticsearch-plugin list' to get list of installed plugins"); } List pluginPaths = new ArrayList<>(); diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java index 6528bbc0911..3a4639fa839 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java @@ -73,7 +73,7 @@ public class RemovePluginCommandTests extends ESTestCase { public void testMissing() throws Exception { UserError e = expectThrows(UserError.class, () -> removePlugin("dne", home)); - assertTrue(e.getMessage(), e.getMessage().contains("Plugin dne not found")); + assertTrue(e.getMessage(), e.getMessage().contains("plugin dne not found")); assertRemoveCleaned(env); } From d29844e59748c76475d7c434e987fffebf0f853a Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 26 May 2016 09:37:51 -0400 Subject: [PATCH 2/3] Remove custom plugins path This commit removes the ability to specify a custom plugins path. Instead, the plugins path will always be a subdirectory called "plugins" off of the home directory. --- .../org/elasticsearch/bootstrap/Security.java | 2 +- .../common/settings/ClusterSettings.java | 1 - .../org/elasticsearch/env/Environment.java | 7 +------ .../org/elasticsearch/tribe/TribeService.java | 3 --- .../tribe/TribeServiceTests.java | 2 -- docs/plugins/plugin-script.asciidoc | 13 +----------- .../migration/migrate_5_0/plugins.asciidoc | 5 +++++ docs/reference/modules/tribe.asciidoc | 1 - docs/reference/setup/install/deb.asciidoc | 1 - docs/reference/setup/install/rpm.asciidoc | 1 - docs/reference/setup/install/windows.asciidoc | 1 - .../setup/install/zip-targz.asciidoc | 1 - .../bootstrap/EvilSecurityTests.java | 1 - .../scripts/module_and_plugin_test_cases.bash | 21 ------------------- 14 files changed, 8 insertions(+), 52 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java index 4437097bb35..e1c6d320df6 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -244,7 +244,7 @@ final class Security { addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.binFile(), "read,readlink"); addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.libFile(), "read,readlink"); addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.modulesFile(), "read,readlink"); - addPath(policy, Environment.PATH_PLUGINS_SETTING.getKey(), environment.pluginsFile(), "read,readlink"); + addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.pluginsFile(), "read,readlink"); addPath(policy, Environment.PATH_CONF_SETTING.getKey(), environment.configFile(), "read,readlink"); addPath(policy, Environment.PATH_SCRIPTS_SETTING.getKey(), environment.scriptsFile(), "read,readlink"); // read-write dirs diff --git a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 36ee01484e6..b29e7604a98 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -332,7 +332,6 @@ public final class ClusterSettings extends AbstractScopedSettings { Environment.PATH_DATA_SETTING, Environment.PATH_HOME_SETTING, Environment.PATH_LOGS_SETTING, - Environment.PATH_PLUGINS_SETTING, Environment.PATH_REPO_SETTING, Environment.PATH_SCRIPTS_SETTING, Environment.PATH_SHARED_DATA_SETTING, diff --git a/core/src/main/java/org/elasticsearch/env/Environment.java b/core/src/main/java/org/elasticsearch/env/Environment.java index e022ce6ad2f..c44db111a5a 100644 --- a/core/src/main/java/org/elasticsearch/env/Environment.java +++ b/core/src/main/java/org/elasticsearch/env/Environment.java @@ -53,7 +53,6 @@ public class Environment { public static final Setting> PATH_DATA_SETTING = Setting.listSetting("path.data", Collections.emptyList(), Function.identity(), Property.NodeScope); public static final Setting PATH_LOGS_SETTING = Setting.simpleString("path.logs", Property.NodeScope); - public static final Setting PATH_PLUGINS_SETTING = Setting.simpleString("path.plugins", Property.NodeScope); public static final Setting> PATH_REPO_SETTING = Setting.listSetting("path.repo", Collections.emptyList(), Function.identity(), Property.NodeScope); public static final Setting PATH_SHARED_DATA_SETTING = Setting.simpleString("path.shared_data", Property.NodeScope); @@ -128,11 +127,7 @@ public class Environment { scriptsFile = configFile.resolve("scripts"); } - if (PATH_PLUGINS_SETTING.exists(settings)) { - pluginsFile = PathUtils.get(cleanPath(PATH_PLUGINS_SETTING.get(settings))); - } else { - pluginsFile = homeFile.resolve("plugins"); - } + pluginsFile = homeFile.resolve("plugins"); List dataPaths = PATH_DATA_SETTING.get(settings); if (dataPaths.isEmpty() == false) { diff --git a/core/src/main/java/org/elasticsearch/tribe/TribeService.java b/core/src/main/java/org/elasticsearch/tribe/TribeService.java index b66f428367c..50be6e39b07 100644 --- a/core/src/main/java/org/elasticsearch/tribe/TribeService.java +++ b/core/src/main/java/org/elasticsearch/tribe/TribeService.java @@ -220,9 +220,6 @@ public class TribeService extends AbstractLifecycleComponent { if (Environment.PATH_CONF_SETTING.exists(globalSettings)) { sb.put(Environment.PATH_CONF_SETTING.getKey(), Environment.PATH_CONF_SETTING.get(globalSettings)); } - if (Environment.PATH_PLUGINS_SETTING.exists(globalSettings)) { - sb.put(Environment.PATH_PLUGINS_SETTING.getKey(), Environment.PATH_PLUGINS_SETTING.get(globalSettings)); - } if (Environment.PATH_LOGS_SETTING.exists(globalSettings)) { sb.put(Environment.PATH_LOGS_SETTING.getKey(), Environment.PATH_LOGS_SETTING.get(globalSettings)); } diff --git a/core/src/test/java/org/elasticsearch/tribe/TribeServiceTests.java b/core/src/test/java/org/elasticsearch/tribe/TribeServiceTests.java index daeb80d194c..2bbedd8784b 100644 --- a/core/src/test/java/org/elasticsearch/tribe/TribeServiceTests.java +++ b/core/src/test/java/org/elasticsearch/tribe/TribeServiceTests.java @@ -43,13 +43,11 @@ public class TribeServiceTests extends ESTestCase { .put("node.name", "nodename") .put("path.home", "some/path") .put("path.conf", "conf/path") - .put("path.plugins", "plugins/path") .put("path.scripts", "scripts/path") .put("path.logs", "logs/path").build(); Settings clientSettings = TribeService.buildClientSettings("tribe1", globalSettings, Settings.EMPTY); assertEquals("some/path", clientSettings.get("path.home")); assertEquals("conf/path", clientSettings.get("path.conf")); - assertEquals("plugins/path", clientSettings.get("path.plugins")); assertEquals("scripts/path", clientSettings.get("path.scripts")); assertEquals("logs/path", clientSettings.get("path.logs")); diff --git a/docs/plugins/plugin-script.asciidoc b/docs/plugins/plugin-script.asciidoc index 08ad129f22f..1e21288e39c 100644 --- a/docs/plugins/plugin-script.asciidoc +++ b/docs/plugins/plugin-script.asciidoc @@ -180,18 +180,7 @@ set ES_JAVA_OPTS="-DproxyHost=host_name -DproxyPort=port_number" bin/elasticsearch-plugin install analysis-icu ----------------------------------- -=== Settings related to plugins - -[float] -=== Custom plugins directory - -The `plugins` directory can be changed from the default by adding the -following to the `elasticsearch.yml` config file: - -[source,yml] ---------------------- -path.plugins: /path/to/custom/plugins/dir ---------------------- +=== Plugins directory The default location of the `plugins` directory depends on which package you install: diff --git a/docs/reference/migration/migrate_5_0/plugins.asciidoc b/docs/reference/migration/migrate_5_0/plugins.asciidoc index ae1113caa48..1578ae074df 100644 --- a/docs/reference/migration/migrate_5_0/plugins.asciidoc +++ b/docs/reference/migration/migrate_5_0/plugins.asciidoc @@ -112,3 +112,8 @@ Previously, Java system properties could be passed to the plugin command by passing `-D` style arguments directly to the plugin script. This is no longer permitted and such system properties must be passed via ES_JAVA_OPTS. + +==== Custom plugins path + +The ability to specify a custom plugins path via `path.plugins` has +been removed. diff --git a/docs/reference/modules/tribe.asciidoc b/docs/reference/modules/tribe.asciidoc index 74c383eaa9e..a1240a28f84 100644 --- a/docs/reference/modules/tribe.asciidoc +++ b/docs/reference/modules/tribe.asciidoc @@ -87,7 +87,6 @@ configuration options are passed down from the tribe node to each node client: * `transport.publish_host` * `path.home` * `path.conf` -* `path.plugins` * `path.logs` * `path.scripts` * `shield.*` diff --git a/docs/reference/setup/install/deb.asciidoc b/docs/reference/setup/install/deb.asciidoc index 0feba56b0b6..e02d0f84a1d 100644 --- a/docs/reference/setup/install/deb.asciidoc +++ b/docs/reference/setup/install/deb.asciidoc @@ -174,7 +174,6 @@ locations for a Debian-based system: | plugins | Plugin files location. Each plugin will be contained in a subdirectory. | /usr/share/elasticsearch/plugins - | path.plugins | repo | Shared file system repository locations. Can hold multiple locations. A file system repository can be placed in to any subdirectory of any directory specified here. diff --git a/docs/reference/setup/install/rpm.asciidoc b/docs/reference/setup/install/rpm.asciidoc index 46a5c8739eb..d7f216670dc 100644 --- a/docs/reference/setup/install/rpm.asciidoc +++ b/docs/reference/setup/install/rpm.asciidoc @@ -160,7 +160,6 @@ locations for an RPM-based system: | plugins | Plugin files location. Each plugin will be contained in a subdirectory. | /usr/share/elasticsearch/plugins - | path.plugins | repo | Shared file system repository locations. Can hold multiple locations. A file system repository can be placed in to any subdirectory of any directory specified here. diff --git a/docs/reference/setup/install/windows.asciidoc b/docs/reference/setup/install/windows.asciidoc index 0d2e8bf04f6..edf91e08890 100644 --- a/docs/reference/setup/install/windows.asciidoc +++ b/docs/reference/setup/install/windows.asciidoc @@ -228,7 +228,6 @@ directory so that you do not delete important data later on. | plugins | Plugin files location. Each plugin will be contained in a subdirectory. | %ES_HOME%\plugins - | path.plugins | repo | Shared file system repository locations. Can hold multiple locations. A file system repository can be placed in to any subdirectory of any directory specified here. diff --git a/docs/reference/setup/install/zip-targz.asciidoc b/docs/reference/setup/install/zip-targz.asciidoc index 7fc41a0f3f8..680d698976d 100644 --- a/docs/reference/setup/install/zip-targz.asciidoc +++ b/docs/reference/setup/install/zip-targz.asciidoc @@ -147,7 +147,6 @@ directory so that you do not delete important data later on. | plugins | Plugin files location. Each plugin will be contained in a subdirectory. | $ES_HOME/plugins - | path.plugins | repo | Shared file system repository locations. Can hold multiple locations. A file system repository can be placed in to any subdirectory of any directory specified here. diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java index c213f18f333..df54cf87fa4 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java @@ -77,7 +77,6 @@ public class EvilSecurityTests extends ESTestCase { settingsBuilder.put(Environment.PATH_HOME_SETTING.getKey(), esHome.resolve("home").toString()); settingsBuilder.put(Environment.PATH_CONF_SETTING.getKey(), esHome.resolve("conf").toString()); settingsBuilder.put(Environment.PATH_SCRIPTS_SETTING.getKey(), esHome.resolve("scripts").toString()); - settingsBuilder.put(Environment.PATH_PLUGINS_SETTING.getKey(), esHome.resolve("plugins").toString()); settingsBuilder.putArray(Environment.PATH_DATA_SETTING.getKey(), esHome.resolve("data1").toString(), esHome.resolve("data2").toString()); settingsBuilder.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), esHome.resolve("custom").toString()); settingsBuilder.put(Environment.PATH_LOGS_SETTING.getKey(), esHome.resolve("logs").toString()); diff --git a/qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash b/qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash index e20959a82e1..19d9f69ece0 100644 --- a/qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash +++ b/qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash @@ -113,27 +113,6 @@ fi fi } -@test "[$GROUP] install jvm-example plugin with a custom path.plugins" { - # Clean up after the last time this test was run - rm -rf /tmp/plugins.* - - local oldPlugins="$ESPLUGINS" - export ESPLUGINS=$(mktemp -d -t 'plugins.XXXX') - - # Modify the path.plugins setting in configuration file - echo "path.plugins: $ESPLUGINS" >> "$ESCONFIG/elasticsearch.yml" - chown -R elasticsearch:elasticsearch "$ESPLUGINS" - - install_jvm_example - start_elasticsearch_service - # check that configuration was actually picked up - curl -s localhost:9200/_cat/configured_example | sed 's/ *$//' > /tmp/installed - echo "foo" > /tmp/expected - diff /tmp/installed /tmp/expected - stop_elasticsearch_service - remove_jvm_example -} - @test "[$GROUP] install jvm-example plugin with a custom CONFIG_DIR" { # Clean up after the last time we ran this test rm -rf /tmp/config.* From f16f65741e426b0e58bdf67673aad474cdd5bc7e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 26 May 2016 14:10:32 -0400 Subject: [PATCH 3/3] Fix when plugins directory is symlink This commit fixes an issue with the plugins directory being a symbolic link. Namely, the install plugins command attempts to always create the plugins directory just in case it does not exist. The JDK method used here guarantees that the directory is created, and an exception is not thrown if the directory could not be created because it already exists. The problem is that this JDK method does not respect symlinks so its internal existence checks fails, it proceeds to attempt to create the directory, but the directory creation fails because the symlink exists. This is documented as being not an issue. We work around this by checking if there is a symlink where we expect the plugins directory to be, and only attempt to create if not. We add a unit test that plugin installation to a symlinked plugins directory works as expected. --- .../plugins/InstallPluginCommand.java | 4 +++- .../scripts/module_and_plugin_test_cases.bash | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 95fc2373766..9e17eec2e93 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -304,7 +304,9 @@ class InstallPluginCommand extends SettingCommand { // be on the safe side: do not rely on that directories are always extracted // before their children (although this makes sense, but is it guaranteed?) - Files.createDirectories(targetFile.getParent()); + if (!Files.isSymbolicLink(targetFile.getParent())) { + Files.createDirectories(targetFile.getParent()); + } if (entry.isDirectory() == false) { try (OutputStream out = Files.newOutputStream(targetFile)) { int len; diff --git a/qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash b/qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash index 19d9f69ece0..32118fda389 100644 --- a/qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash +++ b/qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash @@ -113,6 +113,28 @@ fi fi } +@test "[$GROUP] install jvm-example plugin with a symlinked plugins path" { + # Clean up after the last time this test was run + rm -rf /tmp/plugins.* + rm -rf /tmp/old_plugins.* + + rm -rf "$ESPLUGINS" + local es_plugins=$(mktemp -d -t 'plugins.XXXX') + chown -R elasticsearch:elasticsearch "$es_plugins" + ln -s "$es_plugins" "$ESPLUGINS" + + install_jvm_example + start_elasticsearch_service + # check that symlinked plugin was actually picked up + curl -s localhost:9200/_cat/configured_example | sed 's/ *$//' > /tmp/installed + echo "foo" > /tmp/expected + diff /tmp/installed /tmp/expected + stop_elasticsearch_service + remove_jvm_example + + unlink "$ESPLUGINS" +} + @test "[$GROUP] install jvm-example plugin with a custom CONFIG_DIR" { # Clean up after the last time we ran this test rm -rf /tmp/config.*