From b146f3ecb3bc215b186903081b03c79c7d0840a2 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 4 Feb 2016 14:29:21 +0100 Subject: [PATCH] Pack all the plugin files into a single folder named elasticsearch at the root of the plugin zip. --- .../gradle/plugin/PluginBuildPlugin.groovy | 3 ++ .../resources/plugin-descriptor.properties | 11 +++--- .../plugins/InstallPluginCommand.java | 14 ++++++- docs/plugins/authors.asciidoc | 12 ++++-- docs/reference/migration/migrate_3_0.asciidoc | 2 + .../plugins/InstallPluginCommandTests.java | 37 ++++++++++++++++--- 6 files changed, 63 insertions(+), 16 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy index 67d0e167b8a..435008132c7 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy @@ -112,6 +112,9 @@ public class PluginBuildPlugin extends BuildPlugin { include 'config/**' include 'bin/**' } + if (project.path.startsWith(':modules:') == false) { + into('elasticsearch') + } } project.assemble.dependsOn(bundle) diff --git a/buildSrc/src/main/resources/plugin-descriptor.properties b/buildSrc/src/main/resources/plugin-descriptor.properties index e6a5f81882d..7659f11bd09 100644 --- a/buildSrc/src/main/resources/plugin-descriptor.properties +++ b/buildSrc/src/main/resources/plugin-descriptor.properties @@ -1,13 +1,14 @@ # Elasticsearch plugin descriptor file -# This file must exist as 'plugin-descriptor.properties' at -# the root directory of all plugins. +# This file must exist as 'plugin-descriptor.properties' in a folder named `elasticsearch` +# inside all plugins. # ### example plugin for "foo" # # foo.zip <-- zip file for the plugin, with this structure: -# .jar <-- classes, resources, dependencies -# .jar <-- any number of jars -# plugin-descriptor.properties <-- example contents below: +#|____elasticsearch/ +#| |____ .jar <-- classes, resources, dependencies +#| |____ .jar <-- any number of jars +#| |____ plugin-descriptor.properties <-- example contents below: # # classname=foo.bar.BazPlugin # description=My cool plugin diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index c0afcbbc118..aeff769accb 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -208,17 +208,23 @@ class InstallPluginCommand extends CliTool.Command { return zip; } - private Path unzip(Path zip, Path pluginsDir) throws IOException { + private Path unzip(Path zip, Path pluginsDir) throws IOException, UserError { // unzip plugin to a staging temp dir Path target = Files.createTempDirectory(pluginsDir, ".installing-"); Files.createDirectories(target); + boolean hasEsDir = false; // TODO: we should wrap this in a try/catch and try deleting the target dir on failure? try (ZipInputStream zipInput = new ZipInputStream(Files.newInputStream(zip))) { ZipEntry entry; byte[] buffer = new byte[8192]; while ((entry = zipInput.getNextEntry()) != null) { - Path targetFile = target.resolve(entry.getName()); + if (entry.getName().startsWith("elasticsearch/") == false) { + // only extract the elasticsearch directory + continue; + } + hasEsDir = true; + Path targetFile = target.resolve(entry.getName().substring("elasticsearch/".length())); // TODO: handle name being an absolute path // be on the safe side: do not rely on that directories are always extracted @@ -236,6 +242,10 @@ class InstallPluginCommand extends CliTool.Command { } } Files.delete(zip); + if (hasEsDir == false) { + IOUtils.rm(target); + throw new UserError(CliTool.ExitStatus.DATA_ERROR, "`elasticsearch` directory is missing in the plugin zip"); + } return target; } diff --git a/docs/plugins/authors.asciidoc b/docs/plugins/authors.asciidoc index 7d5ce489c17..d0a606987ae 100644 --- a/docs/plugins/authors.asciidoc +++ b/docs/plugins/authors.asciidoc @@ -10,12 +10,16 @@ These examples provide the bare bones needed to get started. For more information about how to write a plugin, we recommend looking at the plugins listed in this documentation for inspiration. +[float] +=== Plugin Structure + +All plugin files must be contained in a directory called `elasticsearch`. + [float] === Plugin descriptor file -All plugins, be they site or Java plugins, must contain a file called -`plugin-descriptor.properties` in the root directory. The format for this file -is described in detail here: +All plugins must contain a file called `plugin-descriptor.properties` in the folder named `elasticsearch`. The format +for this file is described in detail here: https://github.com/elastic/elasticsearch/blob/master/buildSrc/src/main/resources/plugin-descriptor.properties[`/buildSrc/src/main/resources/plugin-descriptor.properties`]. @@ -50,7 +54,7 @@ of nonnegative decimal integers separated by "."'s and may have leading zeros. |======================================================================= -Note that only jar files in the root directory are added to the classpath for the plugin! +Note that only jar files in the 'elasticsearch' directory are added to the classpath for the plugin! If you need other resources, package them into a resources jar. [IMPORTANT] diff --git a/docs/reference/migration/migrate_3_0.asciidoc b/docs/reference/migration/migrate_3_0.asciidoc index 705b23d1ea0..58d1eae2611 100644 --- a/docs/reference/migration/migrate_3_0.asciidoc +++ b/docs/reference/migration/migrate_3_0.asciidoc @@ -346,6 +346,8 @@ disable doc values is by using the `doc_values` property of mappings. === Plugin changes The command `bin/plugin` has been renamed to `bin/elasticsearch-plugin`. +The structure of the plugin has changed. All the plugin files must be contained in a directory called `elasticsearch`. +If you use the gradle build, this structure is automatically generated. ==== Site plugins removed diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index 727728f84ab..49a06d2fb44 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -27,6 +27,7 @@ import java.nio.file.DirectoryStream; import java.nio.file.FileAlreadyExistsException; import java.nio.file.FileVisitResult; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.StandardCopyOption; @@ -46,6 +47,7 @@ import org.elasticsearch.common.cli.CliTool; import org.elasticsearch.common.cli.CliToolTestCase; import org.elasticsearch.common.cli.Terminal; import org.elasticsearch.common.cli.UserError; +import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; @@ -102,13 +104,14 @@ public class InstallPluginCommandTests extends ESTestCase { } } - static String writeZip(Path structure) throws IOException { + static String writeZip(Path structure, Path prefix) throws IOException { Path zip = createTempDir().resolve(structure.getFileName() + ".zip"); try (ZipOutputStream stream = new ZipOutputStream(Files.newOutputStream(zip))) { Files.walkFileTree(structure, new SimpleFileVisitor() { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { - stream.putNextEntry(new ZipEntry(structure.relativize(file).toString())); + Path target = prefix.resolve(structure.relativize(file)); + stream.putNextEntry(new ZipEntry(target.toString())); Files.copy(file, stream); return FileVisitResult.CONTINUE; } @@ -127,7 +130,7 @@ public class InstallPluginCommandTests extends ESTestCase { "java.version", System.getProperty("java.specification.version"), "classname", "FakePlugin"); writeJar(structure.resolve("plugin.jar"), "FakePlugin"); - return writeZip(structure); + return writeZip(structure, PathUtils.get("elasticsearch")); } static CliToolTestCase.CaptureOutputTerminal installPlugin(String pluginUrl, Environment env) throws Exception { @@ -284,7 +287,7 @@ public class InstallPluginCommandTests extends ESTestCase { "classname", "FakePlugin", "isolated", "false"); writeJar(pluginDir1.resolve("plugin.jar"), "FakePlugin"); - String pluginZip1 = writeZip(pluginDir1); + String pluginZip1 = writeZip(pluginDir1, PathUtils.get("elasticsearch")); installPlugin(pluginZip1, env); Path pluginDir2 = createTempDir(); @@ -297,7 +300,7 @@ public class InstallPluginCommandTests extends ESTestCase { "classname", "FakePlugin", "isolated", "false"); writeJar(pluginDir2.resolve("plugin.jar"), "FakePlugin"); - String pluginZip2 = writeZip(pluginDir2); + String pluginZip2 = writeZip(pluginDir2, PathUtils.get("elasticsearch")); IllegalStateException e = expectThrows(IllegalStateException.class, () -> { installPlugin(pluginZip2, env); }); @@ -457,6 +460,30 @@ public class InstallPluginCommandTests extends ESTestCase { assertInstallCleaned(env); } + public void testMissingDescriptor() throws Exception { + Environment env = createEnv(); + Path pluginDir = createTempDir(); + Files.createFile(pluginDir.resolve("fake.yml")); + String pluginZip = writeZip(pluginDir, PathUtils.get("elasticsearch")); + NoSuchFileException e = expectThrows(NoSuchFileException.class, () -> { + installPlugin(pluginZip, env); + }); + assertTrue(e.getMessage(), e.getMessage().contains("plugin-descriptor.properties")); + assertInstallCleaned(env); + } + + public void testMissingDirectory() throws Exception { + Environment env = createEnv(); + Path pluginDir = createTempDir(); + Files.createFile(pluginDir.resolve(PluginInfo.ES_PLUGIN_PROPERTIES)); + String pluginZip = writeZip(pluginDir, PathUtils.get("")); + UserError e = expectThrows(UserError.class, () -> { + installPlugin(pluginZip, env); + }); + assertTrue(e.getMessage(), e.getMessage().contains("`elasticsearch` directory is missing in the plugin zip")); + assertInstallCleaned(env); + } + // TODO: test batch flag? // TODO: test checksum (need maven/official below) // TODO: test maven, official, and staging install...need tests with fixtures...