From 4fad18d3bc6b0227d51e0d8b88a32c0373f525b5 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 19 Mar 2016 11:26:40 -0400 Subject: [PATCH 1/5] Fix line-length violations in InstallPluginCommand This commit fixes the line-length checkstyle violations in InstallPluginCommand.java and removes this from the list of files for which the line-length check is suppressed. --- .../resources/checkstyle_suppressions.xml | 1 - .../plugins/InstallPluginCommand.java | 24 ++++++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index 87c049ae0b1..eee82afe116 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -668,7 +668,6 @@ - diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 18e996f6f37..c8fa426b6a4 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -182,11 +182,18 @@ class InstallPluginCommand extends Command { final String version = Version.CURRENT.toString(); final String url; if (System.getProperty(PROPERTY_SUPPORT_STAGING_URLS, "false").equals("true")) { - url = String.format(Locale.ROOT, "https://download.elastic.co/elasticsearch/staging/%1$s-%2$s/org/elasticsearch/plugin/%3$s/%1$s/%3$s-%1$s.zip", - version, Build.CURRENT.shortHash(), pluginId); + url = String.format( + Locale.ROOT, + "https://download.elastic.co/elasticsearch/staging/%1$s-%2$s/org/elasticsearch/plugin/%3$s/%1$s/%3$s-%1$s.zip", + version, + Build.CURRENT.shortHash(), + pluginId); } else { - url = String.format(Locale.ROOT, "https://download.elastic.co/elasticsearch/release/org/elasticsearch/plugin/%1$s/%2$s/%1$s-%2$s.zip", - pluginId, version); + url = String.format( + Locale.ROOT, + "https://download.elastic.co/elasticsearch/release/org/elasticsearch/plugin/%1$s/%2$s/%1$s-%2$s.zip", + pluginId, + version); } terminal.println("-> Downloading " + pluginId + " from elastic"); return downloadZipAndChecksum(url, tmpDir); @@ -366,7 +373,10 @@ class InstallPluginCommand extends Command { final Path destination = env.pluginsFile().resolve(info.getName()); if (Files.exists(destination)) { - throw new UserError(ExitCodes.USAGE, "plugin directory " + destination.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command"); + throw new UserError( + ExitCodes.USAGE, + "plugin directory " + destination.toAbsolutePath() + + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command"); } Path tmpBinDir = tmpRoot.resolve("bin"); @@ -419,7 +429,9 @@ class InstallPluginCommand extends Command { try (DirectoryStream stream = Files.newDirectoryStream(tmpBinDir)) { for (Path srcFile : stream) { if (Files.isDirectory(srcFile)) { - throw new UserError(ExitCodes.DATA_ERROR, "Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName()); + throw new UserError( + ExitCodes.DATA_ERROR, + "Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName()); } Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile)); From 554eb8aa87fbd7d516b8b30b88d5008b04b91796 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 19 Mar 2016 11:32:06 -0400 Subject: [PATCH 2/5] Refactor POSIX handling when installing plugins This commit refactors the handling of POSIX filesystem attributes when install plugins. --- .../plugins/InstallPluginCommand.java | 79 ++++++++++--------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index c8fa426b6a4..c1212e46238 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -21,7 +21,6 @@ package org.elasticsearch.plugins; import joptsimple.OptionSet; import joptsimple.OptionSpec; -import org.apache.lucene.util.Constants; import org.apache.lucene.util.IOUtils; import org.elasticsearch.Build; import org.elasticsearch.Version; @@ -56,6 +55,8 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; +import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -249,21 +250,8 @@ class InstallPluginCommand extends Command { private Path unzip(Path zip, Path pluginsDir) throws IOException, UserError { // unzip plugin to a staging temp dir - final Path target; - if (Constants.WINDOWS) { - target = Files.createTempDirectory(pluginsDir, ".installing-"); - } else { - Set perms = new HashSet<>(); - perms.add(PosixFilePermission.OWNER_EXECUTE); - perms.add(PosixFilePermission.OWNER_READ); - perms.add(PosixFilePermission.OWNER_WRITE); - perms.add(PosixFilePermission.GROUP_READ); - perms.add(PosixFilePermission.GROUP_EXECUTE); - perms.add(PosixFilePermission.OTHERS_READ); - perms.add(PosixFilePermission.OTHERS_EXECUTE); - target = Files.createTempDirectory(pluginsDir, ".installing-", PosixFilePermissions.asFileAttribute(perms)); - } - Files.createDirectories(target); + + final Path target = stagingDirectory(pluginsDir); boolean hasEsDir = false; // TODO: we should wrap this in a try/catch and try deleting the target dir on failure? @@ -308,6 +296,22 @@ class InstallPluginCommand extends Command { return target; } + private Path stagingDirectory(Path pluginsDir) throws IOException { + try { + Set perms = new HashSet<>(); + perms.add(PosixFilePermission.OWNER_EXECUTE); + perms.add(PosixFilePermission.OWNER_READ); + perms.add(PosixFilePermission.OWNER_WRITE); + perms.add(PosixFilePermission.GROUP_READ); + perms.add(PosixFilePermission.GROUP_EXECUTE); + perms.add(PosixFilePermission.OTHERS_READ); + perms.add(PosixFilePermission.OTHERS_EXECUTE); + return Files.createTempDirectory(pluginsDir, ".installing-", PosixFilePermissions.asFileAttribute(perms)); + } catch (UnsupportedOperationException e) { + return Files.createTempDirectory(pluginsDir, ".installing-"); + } + } + /** Load information about the plugin, and verify it can be installed with no errors. */ private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch) throws Exception { // read and validate the plugin descriptor @@ -413,17 +417,15 @@ class InstallPluginCommand extends Command { } Files.createDirectory(destBinDir); - Set perms = new HashSet<>(); - if (Constants.WINDOWS == false) { - // setup file attributes for the installed files to those of the parent dir - PosixFileAttributeView binAttrs = Files.getFileAttributeView(destBinDir.getParent(), PosixFileAttributeView.class); - if (binAttrs != null) { - perms = new HashSet<>(binAttrs.readAttributes().permissions()); - // setting execute bits, since this just means "the file is executable", and actual execution requires read - perms.add(PosixFilePermission.OWNER_EXECUTE); - perms.add(PosixFilePermission.GROUP_EXECUTE); - perms.add(PosixFilePermission.OTHERS_EXECUTE); - } + // setup file attributes for the installed files to those of the parent dir + final Set perms = new HashSet<>(); + final PosixFileAttributeView binAttributeView = Files.getFileAttributeView(destBinDir.getParent(), PosixFileAttributeView.class); + if (binAttributeView != null) { + perms.addAll(binAttributeView.readAttributes().permissions()); + // setting execute bits, since this just means "the file is executable", and actual execution requires read + perms.add(PosixFilePermission.OWNER_EXECUTE); + perms.add(PosixFilePermission.GROUP_EXECUTE); + perms.add(PosixFilePermission.OTHERS_EXECUTE); } try (DirectoryStream stream = Files.newDirectoryStream(tmpBinDir)) { @@ -437,8 +439,8 @@ class InstallPluginCommand extends Command { Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile)); Files.copy(srcFile, destFile); - if (perms.isEmpty() == false) { - PosixFileAttributeView view = Files.getFileAttributeView(destFile, PosixFileAttributeView.class); + final PosixFileAttributeView view = Files.getFileAttributeView(destFile, PosixFileAttributeView.class); + if (view != null) { view.setPermissions(perms); } } @@ -457,15 +459,12 @@ class InstallPluginCommand extends Command { // create the plugin's config dir "if necessary" Files.createDirectories(destConfigDir); - - final PosixFileAttributes destConfigDirAttributes; - if (Constants.WINDOWS) { - destConfigDirAttributes = null; - } else { - destConfigDirAttributes = - Files.getFileAttributeView(destConfigDir.getParent(), PosixFileAttributeView.class).readAttributes(); + final PosixFileAttributeView destConfigDirAttributesView = + Files.getFileAttributeView(destConfigDir.getParent(), PosixFileAttributeView.class); + final PosixFileAttributes destConfigDirAttributes = + destConfigDirAttributesView != null ? destConfigDirAttributesView.readAttributes() : null; + if (destConfigDirAttributes != null) { setOwnerGroup(destConfigDir, destConfigDirAttributes); - } try (DirectoryStream stream = Files.newDirectoryStream(tmpConfigDir)) { @@ -477,7 +476,7 @@ class InstallPluginCommand extends Command { Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile)); if (Files.exists(destFile) == false) { Files.copy(srcFile, destFile); - if (Constants.WINDOWS == false) { + if (destConfigDirAttributes != null) { setOwnerGroup(destFile, destConfigDirAttributes); } } @@ -486,8 +485,10 @@ class InstallPluginCommand extends Command { IOUtils.rm(tmpConfigDir); // clean up what we just copied } - private static void setOwnerGroup(Path path, PosixFileAttributes attributes) throws IOException { + private static void setOwnerGroup(final Path path, final PosixFileAttributes attributes) throws IOException { + Objects.requireNonNull(attributes); PosixFileAttributeView fileAttributeView = Files.getFileAttributeView(path, PosixFileAttributeView.class); + assert fileAttributeView != null; fileAttributeView.setOwner(attributes.owner()); fileAttributeView.setGroup(attributes.group()); } From 6db6c15d0662f6fc310a3f97eddddd3b9c432aec Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 20 Mar 2016 17:47:39 -0400 Subject: [PATCH 3/5] Add tests of POSIX handling for installing plugins This commit refactors the unit tests for installing plugins to test against mock filesystems (as well as the native filesystem) for better test coverage. This commit also adds tests that cover the POSIX attributes handling when installing plugins (e.g., ensuring that the plugins directory has the right permissions, the bin directory has execute permissions, and the config directory has the same owner and group as its parent). --- .../elasticsearch/common/io/PathUtils.java | 36 ++- .../common/settings/Setting.java | 1 + .../org/elasticsearch/env/Environment.java | 15 +- .../plugins/InstallPluginCommand.java | 4 +- .../InternalSettingsPreparerTests.java | 1 + .../plugins/InstallPluginCommandTests.java | 243 ++++++++++++------ 6 files changed, 213 insertions(+), 87 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/io/PathUtils.java b/core/src/main/java/org/elasticsearch/common/io/PathUtils.java index 0002c1e5b5f..ab5b46f3596 100644 --- a/core/src/main/java/org/elasticsearch/common/io/PathUtils.java +++ b/core/src/main/java/org/elasticsearch/common/io/PathUtils.java @@ -27,7 +27,7 @@ import java.nio.file.FileSystems; import java.nio.file.Path; import java.nio.file.Paths; -/** +/** * Utilities for creating a Path from names, * or accessing the default FileSystem. *

@@ -39,14 +39,14 @@ import java.nio.file.Paths; public final class PathUtils { /** no instantiation */ private PathUtils() {} - + /** the actual JDK default */ static final FileSystem ACTUAL_DEFAULT = FileSystems.getDefault(); - + /** can be changed by tests */ static volatile FileSystem DEFAULT = ACTUAL_DEFAULT; - - /** + + /** * Returns a {@code Path} from name components. *

* This works just like {@code Paths.get()}. @@ -57,10 +57,30 @@ public final class PathUtils { * a path against an existing one! */ public static Path get(String first, String... more) { - return DEFAULT.getPath(first, more); + return get(DEFAULT, first, more); } - - /** + + /** + * Returns a {@code Path} from name components against the given + * {@code FileSystem}. + *

+ * This works just like {@code Paths.get()}. + * Remember: just like {@code Paths.get()} this is NOT A STRING CONCATENATION + * UTILITY FUNCTION. + *

+ * Remember: this should almost never be used. Usually resolve + * a path against an existing one! + * + * @param fs the given {@code FileSystem} + * @param first the first path component + * @param more the remaining path components + * @return a path + */ + public static Path get(FileSystem fs, String first, String... more) { + return fs.getPath(first, more); + } + + /** * Returns a {@code Path} from a URI *

* This works just like {@code Paths.get()}. diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java index f0e1b2e64ea..e600f845d86 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -38,6 +38,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; +import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; diff --git a/core/src/main/java/org/elasticsearch/env/Environment.java b/core/src/main/java/org/elasticsearch/env/Environment.java index e022ce6ad2f..ca7363a32ee 100644 --- a/core/src/main/java/org/elasticsearch/env/Environment.java +++ b/core/src/main/java/org/elasticsearch/env/Environment.java @@ -31,6 +31,7 @@ import java.net.MalformedURLException; import java.net.URISyntaxException; import java.net.URL; import java.nio.file.FileStore; +import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -108,28 +109,32 @@ public class Environment { } public Environment(Settings settings) { + this(PathUtils.getDefaultFileSystem(), settings); + } + + public Environment(FileSystem fs, Settings settings) { this.settings = settings; final Path homeFile; if (PATH_HOME_SETTING.exists(settings)) { - homeFile = PathUtils.get(cleanPath(PATH_HOME_SETTING.get(settings))); + homeFile = PathUtils.get(fs, PATH_HOME_SETTING.get(settings)); } else { throw new IllegalStateException(PATH_HOME_SETTING.getKey() + " is not configured"); } if (PATH_CONF_SETTING.exists(settings)) { - configFile = PathUtils.get(cleanPath(PATH_CONF_SETTING.get(settings))); + configFile = PathUtils.get(fs, cleanPath(PATH_CONF_SETTING.get(settings))); } else { configFile = homeFile.resolve("config"); } if (PATH_SCRIPTS_SETTING.exists(settings)) { - scriptsFile = PathUtils.get(cleanPath(PATH_SCRIPTS_SETTING.get(settings))); + scriptsFile = PathUtils.get(fs, cleanPath(PATH_SCRIPTS_SETTING.get(settings))); } else { scriptsFile = configFile.resolve("scripts"); } if (PATH_PLUGINS_SETTING.exists(settings)) { - pluginsFile = PathUtils.get(cleanPath(PATH_PLUGINS_SETTING.get(settings))); + pluginsFile = PathUtils.get(fs, cleanPath(PATH_PLUGINS_SETTING.get(settings))); } else { pluginsFile = homeFile.resolve("plugins"); } @@ -139,7 +144,7 @@ public class Environment { dataFiles = new Path[dataPaths.size()]; dataWithClusterFiles = new Path[dataPaths.size()]; for (int i = 0; i < dataPaths.size(); i++) { - dataFiles[i] = PathUtils.get(dataPaths.get(i)); + dataFiles[i] = PathUtils.get(fs, dataPaths.get(i)); dataWithClusterFiles[i] = dataFiles[i].resolve(ClusterName.clusterNameFromSettings(settings).value()); } } else { diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index c1212e46238..e822021e0b6 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -307,7 +307,7 @@ class InstallPluginCommand extends Command { perms.add(PosixFilePermission.OTHERS_READ); perms.add(PosixFilePermission.OTHERS_EXECUTE); return Files.createTempDirectory(pluginsDir, ".installing-", PosixFilePermissions.asFileAttribute(perms)); - } catch (UnsupportedOperationException e) { + } catch (IllegalArgumentException | UnsupportedOperationException e) { return Files.createTempDirectory(pluginsDir, ".installing-"); } } @@ -338,7 +338,7 @@ class InstallPluginCommand extends Command { } /** check a candidate plugin for jar hell before installing it */ - private void jarHellCheck(Path candidate, Path pluginsDir, boolean isolated) throws Exception { + void jarHellCheck(Path candidate, Path pluginsDir, boolean isolated) throws Exception { // create list of current jars in classpath final List jars = new ArrayList<>(); jars.addAll(Arrays.asList(JarHell.parseClassPath())); diff --git a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java index c979b2f4013..8f12ebc4597 100644 --- a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java +++ b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java @@ -21,6 +21,7 @@ package org.elasticsearch.node.internal; import java.io.IOException; import java.io.InputStream; +import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; 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 fb69c817f3a..7c516863637 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 @@ -19,6 +19,19 @@ package org.elasticsearch.plugins; +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; +import com.google.common.jimfs.Configuration; +import com.google.common.jimfs.Jimfs; +import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.Version; +import org.elasticsearch.cli.MockTerminal; +import org.elasticsearch.cli.UserError; +import org.elasticsearch.common.io.PathUtils; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.PosixPermissionsResetter; + import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; @@ -26,6 +39,7 @@ import java.net.URL; import java.nio.charset.StandardCharsets; import java.nio.file.DirectoryStream; import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileSystem; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.NoSuchFileException; @@ -36,44 +50,85 @@ 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.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; -import org.apache.lucene.util.LuceneTestCase; -import org.elasticsearch.Version; -import org.elasticsearch.cli.MockTerminal; -import org.elasticsearch.cli.UserError; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.PosixPermissionsResetter; -import org.junit.BeforeClass; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.Matchers.containsInAnyOrder; @LuceneTestCase.SuppressFileSystems("*") public class InstallPluginCommandTests extends ESTestCase { - private static boolean isPosix; + private final Function temp; - @BeforeClass - public static void checkPosix() throws IOException { - isPosix = Files.getFileAttributeView(createTempFile(), PosixFileAttributeView.class) != null; + private final FileSystem fs; + private final boolean isPosix; + private final boolean isReal; + + public InstallPluginCommandTests(FileSystem fs, Function temp) { + this.fs = fs; + this.temp = temp; + this.isPosix = fs.supportedFileAttributeViews().contains("posix"); + this.isReal = fs == PathUtils.getDefaultFileSystem(); + } + + @ParametersFactory + public static Iterable parameters() { + class Parameter { + private final FileSystem fileSystem; + private final Function temp; + + public Parameter(FileSystem fileSystem, Supplier root) { + this(fileSystem, root, s -> { + try { + return Files.createTempDirectory(fileSystem.getPath(root.get()), s); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } + + public Parameter(FileSystem fileSystem, Supplier root, Function temp) { + this.fileSystem = fileSystem; + this.temp = temp; + } + } + List parameters = new ArrayList<>(); + parameters.add(new Parameter(Jimfs.newFileSystem(Configuration.windows()), () -> "c:\\")); + parameters.add(new Parameter(Jimfs.newFileSystem(toPosix(Configuration.osX())), () -> "/")); + parameters.add(new Parameter(Jimfs.newFileSystem(toPosix(Configuration.unix())), () -> "/")); + parameters.add(new Parameter(PathUtils.getDefaultFileSystem(), () -> createTempDir().toString(), LuceneTestCase::createTempDir )); + return parameters.stream().map(p -> new Object[] { p.fileSystem, p.temp }).collect(Collectors.toList()); + } + + private static Configuration toPosix(Configuration configuration) { + return configuration.toBuilder().setAttributeViews("basic", "owner", "posix", "unix").build(); } /** Creates a test environment with bin, config and plugins directories. */ - static Environment createEnv() throws IOException { - Path home = createTempDir(); + static Environment createEnv(FileSystem fs, Function temp) throws IOException { + Path home = temp.apply("install-plugin-command-tests"); Files.createDirectories(home.resolve("bin")); Files.createFile(home.resolve("bin").resolve("elasticsearch")); Files.createDirectories(home.resolve("config")); Files.createFile(home.resolve("config").resolve("elasticsearch.yml")); - Files.createDirectories(home.resolve("plugins")); + Path plugins = Files.createDirectories(home.resolve("plugins")); + assertTrue(Files.exists(plugins)); Settings settings = Settings.builder() .put("path.home", home) .build(); - return new Environment(settings); + return new Environment(fs, settings); + } + + static Path createPluginDir(Function temp) throws IOException { + return temp.apply("pluginDir"); } /** creates a fake jar file with empty class files */ @@ -115,14 +170,40 @@ public class InstallPluginCommandTests extends ESTestCase { } static MockTerminal installPlugin(String pluginUrl, Environment env) throws Exception { + return installPlugin(pluginUrl, env, false); + } + + static MockTerminal installPlugin(String pluginUrl, Environment env, boolean jarHellCheck) throws Exception { MockTerminal terminal = new MockTerminal(); - new InstallPluginCommand(env).execute(terminal, pluginUrl, true); + new InstallPluginCommand(env) { + @Override + void jarHellCheck(Path candidate, Path pluginsDir, boolean isolated) throws Exception { + if (jarHellCheck) { + super.jarHellCheck(candidate, pluginsDir, isolated); + } + } + }.execute(terminal, pluginUrl, true); return terminal; } void assertPlugin(String name, Path original, Environment env) throws IOException { Path got = env.pluginsFile().resolve(name); assertTrue("dir " + name + " exists", Files.exists(got)); + + if (isPosix) { + Set perms = Files.getPosixFilePermissions(got); + assertThat( + perms, + containsInAnyOrder( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE, + PosixFilePermission.GROUP_READ, + PosixFilePermission.GROUP_EXECUTE, + PosixFilePermission.OTHERS_READ, + PosixFilePermission.OTHERS_EXECUTE)); + } + assertTrue("jar was copied", Files.exists(got.resolve("plugin.jar"))); assertFalse("bin was not copied", Files.exists(got.resolve("bin"))); assertFalse("config was not copied", Files.exists(got.resolve("config"))); @@ -152,6 +233,16 @@ public class InstallPluginCommandTests extends ESTestCase { Path configDir = env.configFile().resolve(name); assertTrue("config dir exists", Files.exists(configDir)); assertTrue("config is a dir", Files.isDirectory(configDir)); + + if (isPosix) { + Path configRoot = env.configFile(); + PosixFileAttributes configAttributes = + Files.getFileAttributeView(configRoot, PosixFileAttributeView.class).readAttributes(); + PosixFileAttributes attributes = Files.getFileAttributeView(configDir, PosixFileAttributeView.class).readAttributes(); + assertThat(attributes.owner(), equalTo(configAttributes.owner())); + assertThat(attributes.group(), equalTo(configAttributes.group())); + } + try (DirectoryStream stream = Files.newDirectoryStream(configDir)) { for (Path file : stream) { assertFalse("not a dir", Files.isDirectory(file)); @@ -172,16 +263,16 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testSomethingWorks() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); String pluginZip = createPlugin("fake", pluginDir); installPlugin(pluginZip, env); assertPlugin("fake", pluginDir, env); } public void testSpaceInUrl() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); String pluginZip = createPlugin("fake", pluginDir); Path pluginZipWithSpaces = createTempFile("foo bar", ".zip"); try (InputStream in = new URL(pluginZip).openStream()) { @@ -192,28 +283,30 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testMalformedUrlNotMaven() throws Exception { + Environment env = createEnv(fs, temp); // has two colons, so it appears similar to maven coordinates MalformedURLException e = expectThrows(MalformedURLException.class, () -> { - installPlugin("://host:1234", createEnv()); + installPlugin("://host:1234", env); }); assertTrue(e.getMessage(), e.getMessage().contains("no protocol")); } public void testPluginsDirMissing() throws Exception { - Environment env = createEnv(); + Environment env = createEnv(fs, temp); Files.delete(env.pluginsFile()); - Path pluginDir = createTempDir(); + Path pluginDir = createPluginDir(temp); String pluginZip = createPlugin("fake", pluginDir); installPlugin(pluginZip, env); assertPlugin("fake", pluginDir, env); } public void testPluginsDirReadOnly() throws Exception { - assumeTrue("posix filesystem", isPosix); - Environment env = createEnv(); + assumeTrue("posix and filesystem", isPosix && isReal); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); try (PosixPermissionsResetter pluginsAttrs = new PosixPermissionsResetter(env.pluginsFile())) { pluginsAttrs.setPermissions(new HashSet<>()); - String pluginZip = createPlugin("fake", createTempDir()); + String pluginZip = createPlugin("fake", pluginDir); IOException e = expectThrows(IOException.class, () -> { installPlugin(pluginZip, env); }); @@ -223,8 +316,9 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testBuiltinModule() throws Exception { - Environment env = createEnv(); - String pluginZip = createPlugin("lang-groovy", createTempDir()); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); + String pluginZip = createPlugin("lang-groovy", pluginDir); UserError e = expectThrows(UserError.class, () -> { installPlugin(pluginZip, env); }); @@ -233,24 +327,26 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testJarHell() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); - writeJar(pluginDir.resolve("other.jar"), "FakePlugin"); - String pluginZip = createPlugin("fake", pluginDir); // adds plugin.jar with FakePlugin + // jar hell test needs a real filesystem + assumeTrue("real filesystem", isReal); + Environment environment = createEnv(fs, temp); + Path pluginDirectory = createPluginDir(temp); + writeJar(pluginDirectory.resolve("other.jar"), "FakePlugin"); + String pluginZip = createPlugin("fake", pluginDirectory); // adds plugin.jar with FakePlugin IllegalStateException e = expectThrows(IllegalStateException.class, () -> { - installPlugin(pluginZip, env); + installPlugin(pluginZip, environment, true); }); assertTrue(e.getMessage(), e.getMessage().contains("jar hell")); - assertInstallCleaned(env); + assertInstallCleaned(environment); } public void testIsolatedPlugins() throws Exception { - Environment env = createEnv(); + Environment env = createEnv(fs, temp); // these both share the same FakePlugin class - Path pluginDir1 = createTempDir(); + Path pluginDir1 = createPluginDir(temp); String pluginZip1 = createPlugin("fake1", pluginDir1); installPlugin(pluginZip1, env); - Path pluginDir2 = createTempDir(); + Path pluginDir2 = createPluginDir(temp); String pluginZip2 = createPlugin("fake2", pluginDir2); installPlugin(pluginZip2, env); assertPlugin("fake1", pluginDir1, env); @@ -258,8 +354,9 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testPurgatoryJarHell() throws Exception { - Environment env = createEnv(); - Path pluginDir1 = createTempDir(); + assumeTrue("real filesystem", isReal); + Environment environment = createEnv(fs, temp); + Path pluginDir1 = createPluginDir(temp); PluginTestUtil.writeProperties(pluginDir1, "description", "fake desc", "name", "fake1", @@ -270,9 +367,9 @@ public class InstallPluginCommandTests extends ESTestCase { "isolated", "false"); writeJar(pluginDir1.resolve("plugin.jar"), "FakePlugin"); String pluginZip1 = writeZip(pluginDir1, "elasticsearch"); - installPlugin(pluginZip1, env); + installPlugin(pluginZip1, environment); - Path pluginDir2 = createTempDir(); + Path pluginDir2 = createPluginDir(temp); PluginTestUtil.writeProperties(pluginDir2, "description", "fake desc", "name", "fake2", @@ -284,15 +381,16 @@ public class InstallPluginCommandTests extends ESTestCase { writeJar(pluginDir2.resolve("plugin.jar"), "FakePlugin"); String pluginZip2 = writeZip(pluginDir2, "elasticsearch"); IllegalStateException e = expectThrows(IllegalStateException.class, () -> { - installPlugin(pluginZip2, env); + installPlugin(pluginZip2, environment, true); }); assertTrue(e.getMessage(), e.getMessage().contains("jar hell")); - assertInstallCleaned(env); + assertInstallCleaned(environment); } public void testExistingPlugin() throws Exception { - Environment env = createEnv(); - String pluginZip = createPlugin("fake", createTempDir()); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); + String pluginZip = createPlugin("fake", pluginDir); installPlugin(pluginZip, env); UserError e = expectThrows(UserError.class, () -> { installPlugin(pluginZip, env); @@ -302,8 +400,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testBin() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Path binDir = pluginDir.resolve("bin"); Files.createDirectory(binDir); Files.createFile(binDir.resolve("somescript")); @@ -313,8 +411,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testBinNotDir() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Path binDir = pluginDir.resolve("bin"); Files.createFile(binDir); String pluginZip = createPlugin("fake", pluginDir); @@ -326,8 +424,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testBinContainsDir() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Path dirInBinDir = pluginDir.resolve("bin").resolve("foo"); Files.createDirectories(dirInBinDir); Files.createFile(dirInBinDir.resolve("somescript")); @@ -340,8 +438,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testBinConflict() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Path binDir = pluginDir.resolve("bin"); Files.createDirectory(binDir); Files.createFile(binDir.resolve("somescript")); @@ -355,8 +453,8 @@ public class InstallPluginCommandTests extends ESTestCase { public void testBinPermissions() throws Exception { assumeTrue("posix filesystem", isPosix); - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Path binDir = pluginDir.resolve("bin"); Files.createDirectory(binDir); Files.createFile(binDir.resolve("somescript")); @@ -372,8 +470,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testConfig() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Path configDir = pluginDir.resolve("config"); Files.createDirectory(configDir); Files.createFile(configDir.resolve("custom.yaml")); @@ -383,11 +481,11 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testExistingConfig() throws Exception { - Environment env = createEnv(); + Environment env = createEnv(fs, temp); Path envConfigDir = env.configFile().resolve("fake"); Files.createDirectories(envConfigDir); Files.write(envConfigDir.resolve("custom.yaml"), "existing config".getBytes(StandardCharsets.UTF_8)); - Path pluginDir = createTempDir(); + Path pluginDir = createPluginDir(temp); Path configDir = pluginDir.resolve("config"); Files.createDirectory(configDir); Files.write(configDir.resolve("custom.yaml"), "new config".getBytes(StandardCharsets.UTF_8)); @@ -402,8 +500,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testConfigNotDir() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Path configDir = pluginDir.resolve("config"); Files.createFile(configDir); String pluginZip = createPlugin("fake", pluginDir); @@ -415,8 +513,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testConfigContainsDir() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Path dirInConfigDir = pluginDir.resolve("config").resolve("foo"); Files.createDirectories(dirInConfigDir); Files.createFile(dirInConfigDir.resolve("myconfig.yml")); @@ -429,8 +527,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testConfigConflict() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Path configDir = pluginDir.resolve("config"); Files.createDirectory(configDir); Files.createFile(configDir.resolve("myconfig.yml")); @@ -443,8 +541,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testMissingDescriptor() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Files.createFile(pluginDir.resolve("fake.yml")); String pluginZip = writeZip(pluginDir, "elasticsearch"); NoSuchFileException e = expectThrows(NoSuchFileException.class, () -> { @@ -455,8 +553,8 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testMissingDirectory() throws Exception { - Environment env = createEnv(); - Path pluginDir = createTempDir(); + Environment env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); Files.createFile(pluginDir.resolve(PluginInfo.ES_PLUGIN_PROPERTIES)); String pluginZip = writeZip(pluginDir, null); UserError e = expectThrows(UserError.class, () -> { @@ -467,13 +565,14 @@ public class InstallPluginCommandTests extends ESTestCase { } public void testZipRelativeOutsideEntryName() throws Exception { + Environment env = createEnv(fs, temp); Path zip = createTempDir().resolve("broken.zip"); try (ZipOutputStream stream = new ZipOutputStream(Files.newOutputStream(zip))) { stream.putNextEntry(new ZipEntry("elasticsearch/../blah")); } String pluginZip = zip.toUri().toURL().toString(); IOException e = expectThrows(IOException.class, () -> { - installPlugin(pluginZip, createEnv()); + installPlugin(pluginZip, env); }); assertTrue(e.getMessage(), e.getMessage().contains("resolving outside of plugin directory")); } From e6eefcb1429e66dc23b49267e04bba1b4d33ec14 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 21 Mar 2016 10:16:02 -0400 Subject: [PATCH 4/5] Jimfs throws IAE when it should throw UOE This commit adds a hack to detect when Jimfs throws an IAE where it should be throwing an UOE. Namely, the method FileSystemProvider#createDirectory should be throwing an UOE if an attempt is made to set attributes that the filesystem does not support, but instead Jimfs violates this and throws an IAE. --- .../plugins/InstallPluginCommand.java | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index e822021e0b6..ce589acf844 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -56,7 +56,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -307,11 +306,28 @@ class InstallPluginCommand extends Command { perms.add(PosixFilePermission.OTHERS_READ); perms.add(PosixFilePermission.OTHERS_EXECUTE); return Files.createTempDirectory(pluginsDir, ".installing-", PosixFilePermissions.asFileAttribute(perms)); - } catch (IllegalArgumentException | UnsupportedOperationException e) { - return Files.createTempDirectory(pluginsDir, ".installing-"); + } catch (IllegalArgumentException e) { + // Jimfs throws an IAE where it should throw an UOE + // remove when google/jimfs#30 is integrated into Jimfs + // and the Jimfs test dependency is upgraded to include + // this pull request + final StackTraceElement[] elements = e.getStackTrace(); + if (elements.length >= 1 && + elements[0].getClassName().equals("com.google.common.jimfs.AttributeService") && + elements[0].getMethodName().equals("setAttributeInternal")) { + return stagingDirectoryWithoutPosixPermissions(pluginsDir); + } else { + throw e; + } + } catch (UnsupportedOperationException e) { + return stagingDirectoryWithoutPosixPermissions(pluginsDir); } } + private Path stagingDirectoryWithoutPosixPermissions(Path pluginsDir) throws IOException { + return Files.createTempDirectory(pluginsDir, ".installing-"); + } + /** Load information about the plugin, and verify it can be installed with no errors. */ private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch) throws Exception { // read and validate the plugin descriptor From 5dc48e71d04ff749a697df96023ebf9352ce91c6 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 22 Mar 2016 09:48:06 -0400 Subject: [PATCH 5/5] Use mock filesystem during install plugins tests This commit sets up the default filesystem used during install plugins tests. A hack is neeeded to handle the temporary directory because the system property "java.io.tmpdir" will have been initialized to a value that is sensible for the default filesystem, but not necessarily to a value that makes sense for the mock filesystem in use during the tests. This property is restored after each test. --- .../elasticsearch/common/io/PathUtils.java | 36 +++++-------------- .../common/settings/Setting.java | 1 - .../org/elasticsearch/env/Environment.java | 15 +++----- .../InternalSettingsPreparerTests.java | 1 - .../plugins/InstallPluginCommandTests.java | 34 +++++++++++++----- 5 files changed, 38 insertions(+), 49 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/io/PathUtils.java b/core/src/main/java/org/elasticsearch/common/io/PathUtils.java index ab5b46f3596..0002c1e5b5f 100644 --- a/core/src/main/java/org/elasticsearch/common/io/PathUtils.java +++ b/core/src/main/java/org/elasticsearch/common/io/PathUtils.java @@ -27,7 +27,7 @@ import java.nio.file.FileSystems; import java.nio.file.Path; import java.nio.file.Paths; -/** +/** * Utilities for creating a Path from names, * or accessing the default FileSystem. *

@@ -39,14 +39,14 @@ import java.nio.file.Paths; public final class PathUtils { /** no instantiation */ private PathUtils() {} - + /** the actual JDK default */ static final FileSystem ACTUAL_DEFAULT = FileSystems.getDefault(); - + /** can be changed by tests */ static volatile FileSystem DEFAULT = ACTUAL_DEFAULT; - - /** + + /** * Returns a {@code Path} from name components. *

* This works just like {@code Paths.get()}. @@ -57,30 +57,10 @@ public final class PathUtils { * a path against an existing one! */ public static Path get(String first, String... more) { - return get(DEFAULT, first, more); + return DEFAULT.getPath(first, more); } - - /** - * Returns a {@code Path} from name components against the given - * {@code FileSystem}. - *

- * This works just like {@code Paths.get()}. - * Remember: just like {@code Paths.get()} this is NOT A STRING CONCATENATION - * UTILITY FUNCTION. - *

- * Remember: this should almost never be used. Usually resolve - * a path against an existing one! - * - * @param fs the given {@code FileSystem} - * @param first the first path component - * @param more the remaining path components - * @return a path - */ - public static Path get(FileSystem fs, String first, String... more) { - return fs.getPath(first, more); - } - - /** + + /** * Returns a {@code Path} from a URI *

* This works just like {@code Paths.get()}. diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java index e600f845d86..f0e1b2e64ea 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -38,7 +38,6 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; -import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; diff --git a/core/src/main/java/org/elasticsearch/env/Environment.java b/core/src/main/java/org/elasticsearch/env/Environment.java index ca7363a32ee..e022ce6ad2f 100644 --- a/core/src/main/java/org/elasticsearch/env/Environment.java +++ b/core/src/main/java/org/elasticsearch/env/Environment.java @@ -31,7 +31,6 @@ import java.net.MalformedURLException; import java.net.URISyntaxException; import java.net.URL; import java.nio.file.FileStore; -import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -109,32 +108,28 @@ public class Environment { } public Environment(Settings settings) { - this(PathUtils.getDefaultFileSystem(), settings); - } - - public Environment(FileSystem fs, Settings settings) { this.settings = settings; final Path homeFile; if (PATH_HOME_SETTING.exists(settings)) { - homeFile = PathUtils.get(fs, PATH_HOME_SETTING.get(settings)); + homeFile = PathUtils.get(cleanPath(PATH_HOME_SETTING.get(settings))); } else { throw new IllegalStateException(PATH_HOME_SETTING.getKey() + " is not configured"); } if (PATH_CONF_SETTING.exists(settings)) { - configFile = PathUtils.get(fs, cleanPath(PATH_CONF_SETTING.get(settings))); + configFile = PathUtils.get(cleanPath(PATH_CONF_SETTING.get(settings))); } else { configFile = homeFile.resolve("config"); } if (PATH_SCRIPTS_SETTING.exists(settings)) { - scriptsFile = PathUtils.get(fs, cleanPath(PATH_SCRIPTS_SETTING.get(settings))); + scriptsFile = PathUtils.get(cleanPath(PATH_SCRIPTS_SETTING.get(settings))); } else { scriptsFile = configFile.resolve("scripts"); } if (PATH_PLUGINS_SETTING.exists(settings)) { - pluginsFile = PathUtils.get(fs, cleanPath(PATH_PLUGINS_SETTING.get(settings))); + pluginsFile = PathUtils.get(cleanPath(PATH_PLUGINS_SETTING.get(settings))); } else { pluginsFile = homeFile.resolve("plugins"); } @@ -144,7 +139,7 @@ public class Environment { dataFiles = new Path[dataPaths.size()]; dataWithClusterFiles = new Path[dataPaths.size()]; for (int i = 0; i < dataPaths.size(); i++) { - dataFiles[i] = PathUtils.get(fs, dataPaths.get(i)); + dataFiles[i] = PathUtils.get(dataPaths.get(i)); dataWithClusterFiles[i] = dataFiles[i].resolve(ClusterName.clusterNameFromSettings(settings).value()); } } else { diff --git a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java index 8f12ebc4597..c979b2f4013 100644 --- a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java +++ b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java @@ -21,7 +21,6 @@ package org.elasticsearch.node.internal; import java.io.IOException; import java.io.InputStream; -import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; 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 7c516863637..fdcfc44e989 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 @@ -23,14 +23,17 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; import org.apache.lucene.util.LuceneTestCase; +import org.apache.lucene.util.SuppressForbidden; import org.elasticsearch.Version; import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.cli.UserError; import org.elasticsearch.common.io.PathUtils; +import org.elasticsearch.common.io.PathUtilsForTesting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.PosixPermissionsResetter; +import org.junit.After; import java.io.IOException; import java.io.InputStream; @@ -71,12 +74,25 @@ public class InstallPluginCommandTests extends ESTestCase { private final FileSystem fs; private final boolean isPosix; private final boolean isReal; + private final String javaIoTmpdir; + @SuppressForbidden(reason = "sets java.io.tmpdir") public InstallPluginCommandTests(FileSystem fs, Function temp) { this.fs = fs; this.temp = temp; this.isPosix = fs.supportedFileAttributeViews().contains("posix"); this.isReal = fs == PathUtils.getDefaultFileSystem(); + PathUtilsForTesting.installMock(fs); + javaIoTmpdir = System.getProperty("java.io.tmpdir"); + System.setProperty("java.io.tmpdir", temp.apply("tmpdir").toString()); + } + + @After + @SuppressForbidden(reason = "resets java.io.tmpdir") + public void tearDown() throws Exception { + System.setProperty("java.io.tmpdir", javaIoTmpdir); + PathUtilsForTesting.teardown(); + super.tearDown(); } @ParametersFactory @@ -85,26 +101,26 @@ public class InstallPluginCommandTests extends ESTestCase { private final FileSystem fileSystem; private final Function temp; - public Parameter(FileSystem fileSystem, Supplier root) { - this(fileSystem, root, s -> { + public Parameter(FileSystem fileSystem, String root) { + this(fileSystem, s -> { try { - return Files.createTempDirectory(fileSystem.getPath(root.get()), s); + return Files.createTempDirectory(fileSystem.getPath(root), s); } catch (IOException e) { throw new RuntimeException(e); } }); } - public Parameter(FileSystem fileSystem, Supplier root, Function temp) { + public Parameter(FileSystem fileSystem, Function temp) { this.fileSystem = fileSystem; this.temp = temp; } } List parameters = new ArrayList<>(); - parameters.add(new Parameter(Jimfs.newFileSystem(Configuration.windows()), () -> "c:\\")); - parameters.add(new Parameter(Jimfs.newFileSystem(toPosix(Configuration.osX())), () -> "/")); - parameters.add(new Parameter(Jimfs.newFileSystem(toPosix(Configuration.unix())), () -> "/")); - parameters.add(new Parameter(PathUtils.getDefaultFileSystem(), () -> createTempDir().toString(), LuceneTestCase::createTempDir )); + parameters.add(new Parameter(Jimfs.newFileSystem(Configuration.windows()), "c:\\")); + parameters.add(new Parameter(Jimfs.newFileSystem(toPosix(Configuration.osX())), "/")); + parameters.add(new Parameter(Jimfs.newFileSystem(toPosix(Configuration.unix())), "/")); + parameters.add(new Parameter(PathUtils.getDefaultFileSystem(), LuceneTestCase::createTempDir )); return parameters.stream().map(p -> new Object[] { p.fileSystem, p.temp }).collect(Collectors.toList()); } @@ -124,7 +140,7 @@ public class InstallPluginCommandTests extends ESTestCase { Settings settings = Settings.builder() .put("path.home", home) .build(); - return new Environment(fs, settings); + return new Environment(settings); } static Path createPluginDir(Function temp) throws IOException {