Merge pull request #16453 from jimferenczi/packaging_plugin_zip

Pack all the plugin files into a single folder named elasticsearch at the root of the plugin zip.

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)
 

buildSrc/src/main/resources/plugin-descriptor.properties

@@ -1,13 +1,14 @@
 # Elasticsearch plugin descriptor file
-# This file must exist as 'plugin-descriptor.properties' at
+# This file must exist as 'plugin-descriptor.properties' in a folder named `elasticsearch`
-# the root directory of all plugins.
+# inside all plugins.
 #
 ### example plugin for "foo"
 #
 # foo.zip <-- zip file for the plugin, with this structure:
-#   <arbitrary name1>.jar <-- classes, resources, dependencies
+#|____elasticsearch/
-#   <arbitrary nameN>.jar <-- any number of jars
+#| |____   <arbitrary name1>.jar <-- classes, resources, dependencies
-#   plugin-descriptor.properties <-- example contents below:
+#| |____   <arbitrary nameN>.jar <-- any number of jars
+#| |____   plugin-descriptor.properties <-- example contents below:
 #
 # classname=foo.bar.BazPlugin
 # description=My cool plugin

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;
     }
 

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
+All plugins must contain a file called `plugin-descriptor.properties` in the folder named `elasticsearch`. The format
-`plugin-descriptor.properties` in the root directory. The format for this file
+for this file is described in detail here:
-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]

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
 

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<Path>() {
                 @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...