diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginInfo.java b/core/src/main/java/org/elasticsearch/plugins/PluginInfo.java index 6129f649460..184835d7405 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginInfo.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginInfo.java @@ -116,6 +116,12 @@ public class PluginInfo implements Streamable, ToXContent { throw new IllegalArgumentException("Property [classname] is missing for jvm plugin [" + name + "]"); } } + + if (site) { + if (!Files.exists(dir.resolve("_site"))) { + throw new IllegalArgumentException("Plugin [" + name + "] is a site plugin but has no _site"); + } + } return new PluginInfo(name, description, site, version, jvm, classname, isolated); } diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java index b6f88b3e02f..b67be105399 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java @@ -118,7 +118,7 @@ public class PluginManager { } private Path download(PluginHandle pluginHandle, Terminal terminal) throws IOException { - Path pluginFile = pluginHandle.distroFile(environment); + Path pluginFile = pluginHandle.newDistroFile(environment); HttpDownloadHelper downloadHelper = new HttpDownloadHelper(); boolean downloaded = false; @@ -345,17 +345,6 @@ public class PluginManager { } removed = true; } - pluginToDelete = pluginHandle.distroFile(environment); - if (Files.exists(pluginToDelete)) { - terminal.println(VERBOSE, "Removing: %s", pluginToDelete); - try { - Files.delete(pluginToDelete); - } catch (Exception ex) { - throw new IOException("Unable to remove " + pluginHandle.name + ". Check file permissions on " + - pluginToDelete.toString(), ex); - } - removed = true; - } Path binLocation = pluginHandle.binDir(environment); if (Files.exists(binLocation)) { terminal.println(VERBOSE, "Removing: %s", binLocation); @@ -464,8 +453,8 @@ public class PluginManager { } } - Path distroFile(Environment env) { - return env.tmpFile().resolve(name + ".zip"); + Path newDistroFile(Environment env) throws IOException { + return Files.createTempFile(env.tmpFile(), name, ".zip"); } Path extractedDir(Environment env) { diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java index 9f365ecdc67..b740ecbcb9f 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java @@ -38,7 +38,7 @@ import static org.hamcrest.Matchers.contains; public class PluginInfoTests extends ElasticsearchTestCase { - void writeProperties(Path pluginDir, String... stringProps) throws IOException { + static void writeProperties(Path pluginDir, String... stringProps) throws IOException { assert stringProps.length % 2 == 0; Files.createDirectories(pluginDir); Path propertiesFile = pluginDir.resolve(PluginInfo.ES_PLUGIN_PROPERTIES); @@ -166,6 +166,7 @@ public class PluginInfoTests extends ElasticsearchTestCase { public void testReadFromPropertiesSitePlugin() throws Exception { Path pluginDir = createTempDir().resolve("fake-plugin"); + Files.createDirectories(pluginDir.resolve("_site")); writeProperties(pluginDir, "description", "fake desc", "version", "1.0", @@ -176,6 +177,21 @@ public class PluginInfoTests extends ElasticsearchTestCase { assertFalse(info.isJvm()); assertEquals("NA", info.getClassname()); } + + public void testReadFromPropertiesSitePluginWithoutSite() throws Exception { + Path pluginDir = createTempDir().resolve("fake-plugin"); + writeProperties(pluginDir, + "description", "fake desc", + "version", "1.0", + "elasticsearch.version", Version.CURRENT.toString(), + "site", "true"); + try { + PluginInfo.readFromProperties(pluginDir); + fail("didn't get expected exception"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("site plugin but has no _site")); + } + } public void testPluginListSorted() { PluginsInfo pluginsInfo = new PluginsInfo(5); diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginManagerTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginManagerTests.java index 98ba4b6fbc1..a5924f8312c 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginManagerTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginManagerTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.plugins; import org.apache.http.impl.client.HttpClients; import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.Version; import org.elasticsearch.common.cli.CliTool; import org.elasticsearch.common.cli.CliToolTestCase.CaptureOutputTerminal; import org.elasticsearch.common.collect.Tuple; @@ -37,12 +38,20 @@ import org.junit.Test; import java.io.IOException; import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.nio.file.DirectoryStream; +import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.PosixFilePermission; import java.util.Locale; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; +import java.util.zip.ZipOutputStream; import static org.elasticsearch.common.cli.CliTool.ExitStatus.USAGE; import static org.elasticsearch.common.cli.CliToolTestCase.args; @@ -52,6 +61,7 @@ import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertDirectoryExists; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFileExists; import static org.hamcrest.Matchers.*; +import static org.elasticsearch.plugins.PluginInfoTests.writeProperties; @ClusterScope(scope = Scope.TEST, numDataNodes = 0, transportClientRatio = 0.0) @LuceneTestCase.SuppressFileSystems("*") // TODO: clean up this test to allow extra files @@ -81,22 +91,58 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest { public void clearPathHome() { System.clearProperty("es.default.path.home"); } + + /** creates a plugin .zip and returns the url for testing */ + private String createPlugin(final Path structure, String... properties) throws IOException { + writeProperties(structure, properties); + 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())); + Files.copy(file, stream); + return FileVisitResult.CONTINUE; + } + }); + } + return zip.toUri().toURL().toString(); + } @Test public void testThatPluginNameMustBeSupplied() throws IOException { - String pluginUrl = getPluginUrlForResource("plugin_with_bin_and_config.zip"); + Path pluginDir = createTempDir().resolve("fake-plugin"); + String pluginUrl = createPlugin(pluginDir, + "description", "fake desc", + "version", "1.0", + "elasticsearch.version", Version.CURRENT.toString(), + "jvm", "true", + "classname", "FakePlugin"); assertStatus("install --url " + pluginUrl, USAGE); } @Test public void testLocalPluginInstallWithBinAndConfig() throws Exception { - String pluginName = "plugin-test"; + String pluginName = "fake-plugin"; + Path pluginDir = createTempDir().resolve(pluginName); + // create bin/tool and config/file + Files.createDirectories(pluginDir.resolve("bin")); + Files.createFile(pluginDir.resolve("bin").resolve("tool")); + Files.createDirectories(pluginDir.resolve("config")); + Files.createFile(pluginDir.resolve("config").resolve("file")); + + String pluginUrl = createPlugin(pluginDir, + "description", "fake desc", + "version", "1.0", + "elasticsearch.version", Version.CURRENT.toString(), + "jvm", "true", + "classname", "FakePlugin"); + Environment env = initialSettings.v2(); Path binDir = env.homeFile().resolve("bin"); Path pluginBinDir = binDir.resolve(pluginName); Path pluginConfigDir = env.configFile().resolve(pluginName); - String pluginUrl = getPluginUrlForResource("plugin_with_bin_and_config.zip"); assertStatusOk("install " + pluginName + " --url " + pluginUrl + " --verbose"); terminal.getTerminalOutput().clear(); @@ -123,23 +169,35 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest { */ @Test public void testLocalPluginInstallWithBinAndConfigInAlreadyExistingConfigDir_7890() throws Exception { - String pluginName = "plugin-test"; + String pluginName = "fake-plugin"; + Path pluginDir = createTempDir().resolve(pluginName); + // create config/test.txt with contents 'version1' + Files.createDirectories(pluginDir.resolve("config")); + Files.write(pluginDir.resolve("config").resolve("test.txt"), "version1".getBytes(StandardCharsets.UTF_8)); + + String pluginUrl = createPlugin(pluginDir, + "description", "fake desc", + "version", "1.0", + "elasticsearch.version", Version.CURRENT.toString(), + "jvm", "true", + "classname", "FakePlugin"); + Environment env = initialSettings.v2(); Path pluginConfigDir = env.configFile().resolve(pluginName); - assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, getPluginUrlForResource("plugin_with_config_v1.zip"))); + assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, pluginUrl)); /* First time, our plugin contains: - config/test.txt (version1) */ - assertFileContent(pluginConfigDir, "test.txt", "version1\n"); + assertFileContent(pluginConfigDir, "test.txt", "version1"); // We now remove the plugin assertStatusOk("remove " + pluginName); // We should still have test.txt - assertFileContent(pluginConfigDir, "test.txt", "version1\n"); + assertFileContent(pluginConfigDir, "test.txt", "version1"); // Installing a new plugin version /* @@ -148,19 +206,30 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest { - config/dir/testdir.txt (version1) - config/dir/subdir/testsubdir.txt (version1) */ - assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, getPluginUrlForResource("plugin_with_config_v2.zip"))); + Files.write(pluginDir.resolve("config").resolve("test.txt"), "version2".getBytes(StandardCharsets.UTF_8)); + Files.createDirectories(pluginDir.resolve("config").resolve("dir").resolve("subdir")); + Files.write(pluginDir.resolve("config").resolve("dir").resolve("testdir.txt"), "version1".getBytes(StandardCharsets.UTF_8)); + Files.write(pluginDir.resolve("config").resolve("dir").resolve("subdir").resolve("testsubdir.txt"), "version1".getBytes(StandardCharsets.UTF_8)); + pluginUrl = createPlugin(pluginDir, + "description", "fake desc", + "version", "2.0", + "elasticsearch.version", Version.CURRENT.toString(), + "jvm", "true", + "classname", "FakePlugin"); + + assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, pluginUrl)); - assertFileContent(pluginConfigDir, "test.txt", "version1\n"); - assertFileContent(pluginConfigDir, "test.txt.new", "version2\n"); - assertFileContent(pluginConfigDir, "dir/testdir.txt", "version1\n"); - assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt", "version1\n"); + assertFileContent(pluginConfigDir, "test.txt", "version1"); + assertFileContent(pluginConfigDir, "test.txt.new", "version2"); + assertFileContent(pluginConfigDir, "dir/testdir.txt", "version1"); + assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt", "version1"); // Removing assertStatusOk("remove " + pluginName); - assertFileContent(pluginConfigDir, "test.txt", "version1\n"); - assertFileContent(pluginConfigDir, "test.txt.new", "version2\n"); - assertFileContent(pluginConfigDir, "dir/testdir.txt", "version1\n"); - assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt", "version1\n"); + assertFileContent(pluginConfigDir, "test.txt", "version1"); + assertFileContent(pluginConfigDir, "test.txt.new", "version2"); + assertFileContent(pluginConfigDir, "dir/testdir.txt", "version1"); + assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt", "version1"); // Installing a new plugin version /* @@ -171,40 +240,54 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest { - config/dir/testdir2.txt (version1) - config/dir/subdir/testsubdir.txt (version2) */ - assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, getPluginUrlForResource("plugin_with_config_v3.zip"))); + Files.write(pluginDir.resolve("config").resolve("test.txt"), "version3".getBytes(StandardCharsets.UTF_8)); + Files.write(pluginDir.resolve("config").resolve("test2.txt"), "version1".getBytes(StandardCharsets.UTF_8)); + Files.write(pluginDir.resolve("config").resolve("dir").resolve("testdir.txt"), "version2".getBytes(StandardCharsets.UTF_8)); + Files.write(pluginDir.resolve("config").resolve("dir").resolve("testdir2.txt"), "version1".getBytes(StandardCharsets.UTF_8)); + Files.write(pluginDir.resolve("config").resolve("dir").resolve("subdir").resolve("testsubdir.txt"), "version2".getBytes(StandardCharsets.UTF_8)); + pluginUrl = createPlugin(pluginDir, + "description", "fake desc", + "version", "3.0", + "elasticsearch.version", Version.CURRENT.toString(), + "jvm", "true", + "classname", "FakePlugin"); - assertFileContent(pluginConfigDir, "test.txt", "version1\n"); - assertFileContent(pluginConfigDir, "test2.txt", "version1\n"); - assertFileContent(pluginConfigDir, "test.txt.new", "version3\n"); - assertFileContent(pluginConfigDir, "dir/testdir.txt", "version1\n"); - assertFileContent(pluginConfigDir, "dir/testdir.txt.new", "version2\n"); - assertFileContent(pluginConfigDir, "dir/testdir2.txt", "version1\n"); - assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt", "version1\n"); - assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt.new", "version2\n"); + assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, pluginUrl)); + + assertFileContent(pluginConfigDir, "test.txt", "version1"); + assertFileContent(pluginConfigDir, "test2.txt", "version1"); + assertFileContent(pluginConfigDir, "test.txt.new", "version3"); + assertFileContent(pluginConfigDir, "dir/testdir.txt", "version1"); + assertFileContent(pluginConfigDir, "dir/testdir.txt.new", "version2"); + assertFileContent(pluginConfigDir, "dir/testdir2.txt", "version1"); + assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt", "version1"); + assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt.new", "version2"); } // For #7152 @Test public void testLocalPluginInstallWithBinOnly_7152() throws Exception { - String pluginName = "plugin-test"; + String pluginName = "fake-plugin"; + Path pluginDir = createTempDir().resolve(pluginName); + // create bin/tool + Files.createDirectories(pluginDir.resolve("bin")); + Files.createFile(pluginDir.resolve("bin").resolve("tool"));; + String pluginUrl = createPlugin(pluginDir, + "description", "fake desc", + "version", "1.0", + "elasticsearch.version", Version.CURRENT.toString(), + "jvm", "true", + "classname", "FakePlugin"); + Environment env = initialSettings.v2(); Path binDir = env.homeFile().resolve("bin"); Path pluginBinDir = binDir.resolve(pluginName); - assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, getPluginUrlForResource("plugin_with_bin_only.zip"))); + assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, pluginUrl)); assertThatPluginIsListed(pluginName); assertDirectoryExists(pluginBinDir); } - @Test - public void testSitePluginWithSourceDoesNotInstall() throws Exception { - String pluginName = "plugin-with-source"; - String cmd = String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, getPluginUrlForResource("plugin_with_sourcefiles.zip")); - int status = new PluginManagerCliParser(terminal).execute(args(cmd)); - assertThat(status, is(USAGE.status())); - assertThat(terminal.getTerminalOutput(), hasItem(containsString("Plugin installation assumed to be site plugin, but contains source code, aborting installation"))); - } - @Test public void testListInstalledEmpty() throws IOException { assertStatusOk("list"); @@ -220,18 +303,32 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest { @Test public void testInstallPlugin() throws IOException { - String pluginName = "plugin-classfile"; - assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, getPluginUrlForResource("plugin_with_classfile.zip"))); - assertThatPluginIsListed("plugin-classfile"); + String pluginName = "fake-plugin"; + Path pluginDir = createTempDir().resolve(pluginName); + String pluginUrl = createPlugin(pluginDir, + "description", "fake desc", + "version", "1.0", + "elasticsearch.version", Version.CURRENT.toString(), + "jvm", "true", + "classname", "FakePlugin"); + assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, pluginUrl)); + assertThatPluginIsListed(pluginName); } @Test public void testInstallSitePlugin() throws IOException { - String pluginName = "plugin-site"; - assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, getPluginUrlForResource("plugin_without_folders.zip"))); + String pluginName = "fake-plugin"; + Path pluginDir = createTempDir().resolve(pluginName); + Files.createDirectories(pluginDir.resolve("_site")); + Files.createFile(pluginDir.resolve("_site").resolve("somefile")); + String pluginUrl = createPlugin(pluginDir, + "description", "fake desc", + "version", "1.0", + "site", "true"); + assertStatusOk(String.format(Locale.ROOT, "install %s --url %s --verbose", pluginName, pluginUrl)); assertThatPluginIsListed(pluginName); // We want to check that Plugin Manager moves content to _site - assertFileExists(initialSettings.v2().pluginsFile().resolve("plugin-site/_site")); + assertFileExists(initialSettings.v2().pluginsFile().resolve(pluginName).resolve("_site")); } @@ -313,14 +410,23 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest { @Test public void testRemovePlugin() throws Exception { + String pluginName = "plugintest"; + Path pluginDir = createTempDir().resolve(pluginName); + String pluginUrl = createPlugin(pluginDir, + "description", "fake desc", + "version", "1.0.0", + "elasticsearch.version", Version.CURRENT.toString(), + "jvm", "true", + "classname", "FakePlugin"); + // We want to remove plugin with plugin short name - singlePluginInstallAndRemove("plugintest", "plugintest", getPluginUrlForResource("plugin_without_folders.zip")); + singlePluginInstallAndRemove("plugintest", "plugintest", pluginUrl); // We want to remove plugin with groupid/artifactid/version form - singlePluginInstallAndRemove("groupid/plugintest/1.0.0", "plugintest", getPluginUrlForResource("plugin_without_folders.zip")); + singlePluginInstallAndRemove("groupid/plugintest/1.0.0", "plugintest", pluginUrl); // We want to remove plugin with groupid/artifactid form - singlePluginInstallAndRemove("groupid/plugintest", "plugintest", getPluginUrlForResource("plugin_without_folders.zip")); + singlePluginInstallAndRemove("groupid/plugintest", "plugintest", pluginUrl); } @Test @@ -370,18 +476,6 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest { } } - /** - * Retrieve a URL string that represents the resource with the given {@code resourceName}. - * @param resourceName The resource name relative to {@link PluginManagerTests}. - * @return Never {@code null}. - * @throws NullPointerException if {@code resourceName} does not point to a valid resource. - */ - private String getPluginUrlForResource(String resourceName) { - URI uri = URI.create(PluginManagerTests.class.getResource(resourceName).toString()); - - return "file://" + uri.getPath(); - } - private Tuple buildInitialSettings() throws IOException { Settings settings = settingsBuilder() .put("discovery.zen.ping.multicast.enabled", false) diff --git a/core/src/test/resources/org/elasticsearch/plugins/plugin_with_sourcefiles.zip b/core/src/test/resources/org/elasticsearch/plugins/plugin_with_sourcefiles.zip deleted file mode 100644 index ccd194fc737..00000000000 Binary files a/core/src/test/resources/org/elasticsearch/plugins/plugin_with_sourcefiles.zip and /dev/null differ