diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/AddFileKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/AddFileKeyStoreCommandTests.java index 637214f3911d..a2fbd6c7575e 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/AddFileKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/AddFileKeyStoreCommandTests.java @@ -8,6 +8,8 @@ package org.elasticsearch.cli.keystore; +import joptsimple.OptionSet; + import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; @@ -20,7 +22,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.stream.Stream; import static org.hamcrest.Matchers.anyOf; @@ -31,7 +32,7 @@ public class AddFileKeyStoreCommandTests extends KeyStoreCommandTestCase { protected Command newCommand() { return new AddFileKeyStoreCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return env; } }; diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/AddStringKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/AddStringKeyStoreCommandTests.java index 2d62c00f6cea..a0027c5f1be1 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/AddStringKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/AddStringKeyStoreCommandTests.java @@ -8,6 +8,8 @@ package org.elasticsearch.cli.keystore; +import joptsimple.OptionSet; + import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; @@ -18,7 +20,6 @@ import java.io.ByteArrayInputStream; import java.io.CharArrayWriter; import java.io.InputStream; import java.nio.charset.StandardCharsets; -import java.util.Map; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; @@ -31,7 +32,7 @@ public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase { protected Command newCommand() { return new AddStringKeyStoreCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return env; } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ChangeKeyStorePasswordCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ChangeKeyStorePasswordCommandTests.java index dd59f90ccf5e..fc803bd0e1de 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ChangeKeyStorePasswordCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ChangeKeyStorePasswordCommandTests.java @@ -8,13 +8,13 @@ package org.elasticsearch.cli.keystore; +import joptsimple.OptionSet; + import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; -import java.util.Map; - import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; @@ -23,7 +23,7 @@ public class ChangeKeyStorePasswordCommandTests extends KeyStoreCommandTestCase protected Command newCommand() { return new ChangeKeyStorePasswordCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return env; } }; diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/CreateKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/CreateKeyStoreCommandTests.java index 410ec6e46961..f66f96aeda9e 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/CreateKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/CreateKeyStoreCommandTests.java @@ -8,6 +8,8 @@ package org.elasticsearch.cli.keystore; +import joptsimple.OptionSet; + import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; @@ -17,7 +19,6 @@ import org.elasticsearch.env.Environment; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Map; import static org.hamcrest.Matchers.containsString; @@ -27,7 +28,7 @@ public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase { protected Command newCommand() { return new CreateKeyStoreCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return env; } }; diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/HasPasswordKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/HasPasswordKeyStoreCommandTests.java index d423aa4436eb..bf64a79f1833 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/HasPasswordKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/HasPasswordKeyStoreCommandTests.java @@ -8,12 +8,12 @@ package org.elasticsearch.cli.keystore; +import joptsimple.OptionSet; + import org.elasticsearch.cli.Command; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; -import java.util.Map; - import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.emptyString; @@ -24,7 +24,7 @@ public class HasPasswordKeyStoreCommandTests extends KeyStoreCommandTestCase { protected Command newCommand() { return new HasPasswordKeyStoreCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return env; } }; diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ListKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ListKeyStoreCommandTests.java index 3d43eda7d493..dd5309151c77 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ListKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ListKeyStoreCommandTests.java @@ -8,13 +8,13 @@ package org.elasticsearch.cli.keystore; +import joptsimple.OptionSet; + import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; -import java.util.Map; - import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; @@ -24,7 +24,7 @@ public class ListKeyStoreCommandTests extends KeyStoreCommandTestCase { protected Command newCommand() { return new ListKeyStoreCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return env; } }; diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/RemoveSettingKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/RemoveSettingKeyStoreCommandTests.java index 01845c909984..b1a3a7803639 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/RemoveSettingKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/RemoveSettingKeyStoreCommandTests.java @@ -8,12 +8,13 @@ package org.elasticsearch.cli.keystore; +import joptsimple.OptionSet; + import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; -import java.util.Map; import java.util.Set; import static org.hamcrest.Matchers.anyOf; @@ -25,7 +26,7 @@ public class RemoveSettingKeyStoreCommandTests extends KeyStoreCommandTestCase { protected Command newCommand() { return new RemoveSettingKeyStoreCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return env; } }; diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ShowKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ShowKeyStoreCommandTests.java index 8cfdae2600fa..38ed03bcece7 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ShowKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/ShowKeyStoreCommandTests.java @@ -8,14 +8,14 @@ package org.elasticsearch.cli.keystore; +import joptsimple.OptionSet; + import org.elasticsearch.cli.Command; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; import org.elasticsearch.common.settings.KeyStoreWrapper; import org.elasticsearch.env.Environment; -import java.util.Map; - import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -26,7 +26,7 @@ public class ShowKeyStoreCommandTests extends KeyStoreCommandTestCase { protected Command newCommand() { return new ShowKeyStoreCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return env; } }; diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/UpgradeKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/UpgradeKeyStoreCommandTests.java index 755c753d8db0..f064425eab89 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/UpgradeKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/cli/keystore/UpgradeKeyStoreCommandTests.java @@ -8,6 +8,8 @@ package org.elasticsearch.cli.keystore; +import joptsimple.OptionSet; + import org.elasticsearch.cli.Command; import org.elasticsearch.cli.UserException; import org.elasticsearch.common.settings.KeyStoreWrapper; @@ -17,7 +19,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Map; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -29,12 +30,10 @@ public class UpgradeKeyStoreCommandTests extends KeyStoreCommandTestCase { @Override protected Command newCommand() { return new UpgradeKeyStoreCommand() { - @Override - protected Environment createEnv(final Map settings) { + protected Environment createEnv(OptionSet options) { return env; } - }; } diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/ListPluginsCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/ListPluginsCommandTests.java index 200f793edf94..2d83f471179c 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/ListPluginsCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/ListPluginsCommandTests.java @@ -8,6 +8,8 @@ package org.elasticsearch.plugins.cli; +import joptsimple.OptionSet; + import org.apache.lucene.tests.util.LuceneTestCase; import org.elasticsearch.Version; import org.elasticsearch.cli.Command; @@ -24,7 +26,6 @@ import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.Arrays; -import java.util.Map; import java.util.stream.Collectors; @LuceneTestCase.SuppressFileSystems("*") @@ -242,7 +243,7 @@ public class ListPluginsCommandTests extends CommandTestCase { protected Command newCommand() { return new ListPluginsCommand() { @Override - protected Environment createEnv(Map settings) { + protected Environment createEnv(OptionSet options) { return env; } diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/MockInstallPluginCommand.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/MockInstallPluginCommand.java index ab6b5d083dd5..92815855227d 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/MockInstallPluginCommand.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/MockInstallPluginCommand.java @@ -8,11 +8,11 @@ package org.elasticsearch.plugins.cli; +import joptsimple.OptionSet; + import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; -import java.util.Map; - public class MockInstallPluginCommand extends InstallPluginCommand { private final Environment env; @@ -25,8 +25,8 @@ public class MockInstallPluginCommand extends InstallPluginCommand { } @Override - protected Environment createEnv(Map settings) throws UserException { - return this.env != null ? this.env : super.createEnv(settings); + protected Environment createEnv(OptionSet options) throws UserException { + return this.env != null ? this.env : super.createEnv(options); } @Override diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/MockRemovePluginCommand.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/MockRemovePluginCommand.java index ce3555275652..7aa92f44b59d 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/MockRemovePluginCommand.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/MockRemovePluginCommand.java @@ -8,9 +8,9 @@ package org.elasticsearch.plugins.cli; -import org.elasticsearch.env.Environment; +import joptsimple.OptionSet; -import java.util.Map; +import org.elasticsearch.env.Environment; public class MockRemovePluginCommand extends RemovePluginCommand { final Environment env; @@ -20,7 +20,7 @@ public class MockRemovePluginCommand extends RemovePluginCommand { } @Override - protected Environment createEnv(Map settings) { + protected Environment createEnv(OptionSet options) { return env; } } diff --git a/libs/cli/src/main/java/org/elasticsearch/cli/Command.java b/libs/cli/src/main/java/org/elasticsearch/cli/Command.java index 1df7e9432f3a..bfa47f5f93cf 100644 --- a/libs/cli/src/main/java/org/elasticsearch/cli/Command.java +++ b/libs/cli/src/main/java/org/elasticsearch/cli/Command.java @@ -17,7 +17,13 @@ import java.io.Closeable; import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import java.util.Properties; +import java.util.stream.Collectors; /** * An action to execute within a cli. @@ -29,6 +35,13 @@ public abstract class Command implements Closeable { private final Runnable beforeMain; + // these are the system properties and env vars from the environment, + // but they can be overriden by tests. Really though Command should be stateless, + // so the signature of main should take them in, which can happen once the entrypoint + // is unified. + protected final Map sysprops; + protected final Map envVars; + /** The option parser for this command. */ protected final OptionParser parser = new OptionParser(); @@ -46,6 +59,8 @@ public abstract class Command implements Closeable { public Command(final String description, final Runnable beforeMain) { this.description = description; this.beforeMain = beforeMain; + this.sysprops = captureSystemProperties(); + this.envVars = captureEnvironmentVariables(); } private Thread shutdownHookThread; @@ -147,6 +162,21 @@ public abstract class Command implements Closeable { * Any runtime user errors (like an input file that does not exist), should throw a {@link UserException}. */ protected abstract void execute(Terminal terminal, OptionSet options) throws Exception; + // protected to allow for tests to override + @SuppressForbidden(reason = "capture system properties") + protected Map captureSystemProperties() { + Properties properties = AccessController.doPrivileged((PrivilegedAction) System::getProperties); + return properties.entrySet() + .stream() + .collect(Collectors.toUnmodifiableMap(e -> e.getKey().toString(), e -> e.getValue().toString())); + } + + // protected to allow for tests to override + @SuppressForbidden(reason = "capture environment variables") + protected Map captureEnvironmentVariables() { + return Collections.unmodifiableMap(System.getenv()); + } + /** * Return whether or not to install the shutdown hook to cleanup resources on exit. This method should only be overridden in test * classes. diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java deleted file mode 100644 index 6ed334ee2be4..000000000000 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.bootstrap; - -import org.elasticsearch.cli.ExitCodes; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.core.PathUtils; -import org.elasticsearch.core.SuppressForbidden; - -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.hasSize; - -public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase { - - @SuppressForbidden(reason = "manipulates system properties for testing") - public void testPathHome() throws Exception { - final String pathHome = System.getProperty("es.path.home"); - final String value = randomAlphaOfLength(16); - System.setProperty("es.path.home", value); - - runTest(ExitCodes.OK, true, (output, error) -> {}, (foreground, pidFile, quiet, esSettings) -> { - Settings settings = esSettings.settings(); - assertThat(settings.keySet(), hasSize(2)); - assertThat(settings.get("path.home"), equalTo(PathUtils.get(System.getProperty("user.dir")).resolve(value).toString())); - assertThat(settings.keySet(), hasItem("path.logs")); // added by env initialization - }); - - System.clearProperty("es.path.home"); - final String commandLineValue = randomAlphaOfLength(16); - runTest(ExitCodes.OK, true, (output, error) -> {}, (foreground, pidFile, quiet, esSettings) -> { - Settings settings = esSettings.settings(); - assertThat(settings.keySet(), hasSize(2)); - assertThat( - settings.get("path.home"), - equalTo(PathUtils.get(System.getProperty("user.dir")).resolve(commandLineValue).toString()) - ); - assertThat(settings.keySet(), hasItem("path.logs")); // added by env initialization - }, "-Epath.home=" + commandLineValue); - - if (pathHome != null) System.setProperty("es.path.home", pathHome); - else System.clearProperty("es.path.home"); - } - -} diff --git a/server/src/main/java/org/elasticsearch/common/cli/EnvironmentAwareCommand.java b/server/src/main/java/org/elasticsearch/common/cli/EnvironmentAwareCommand.java index d3b62416b802..4ef7650ffbb0 100644 --- a/server/src/main/java/org/elasticsearch/common/cli/EnvironmentAwareCommand.java +++ b/server/src/main/java/org/elasticsearch/common/cli/EnvironmentAwareCommand.java @@ -56,6 +56,11 @@ public abstract class EnvironmentAwareCommand extends Command { @Override protected void execute(Terminal terminal, OptionSet options) throws Exception { + execute(terminal, options, createEnv(options)); + } + + /** Create an {@link Environment} for the command to use. Overrideable for tests. */ + protected Environment createEnv(OptionSet options) throws UserException { final Map settings = new HashMap<>(); for (final KeyValuePair kvp : settingOption.values(options)) { if (kvp.value.isEmpty()) { @@ -74,30 +79,20 @@ public abstract class EnvironmentAwareCommand extends Command { settings.put(kvp.key, kvp.value); } - putSystemPropertyIfSettingIsMissing(settings, "path.data", "es.path.data"); - putSystemPropertyIfSettingIsMissing(settings, "path.home", "es.path.home"); - putSystemPropertyIfSettingIsMissing(settings, "path.logs", "es.path.logs"); + putSystemPropertyIfSettingIsMissing(sysprops, settings, "path.data", "es.path.data"); + putSystemPropertyIfSettingIsMissing(sysprops, settings, "path.home", "es.path.home"); + putSystemPropertyIfSettingIsMissing(sysprops, settings, "path.logs", "es.path.logs"); - execute(terminal, options, createEnv(settings)); - } - - /** Create an {@link Environment} for the command to use. Overrideable for tests. */ - protected Environment createEnv(final Map settings) throws UserException { - return createEnv(Settings.EMPTY, settings); - } - - /** Create an {@link Environment} for the command to use. Overrideable for tests. */ - protected static Environment createEnv(final Settings baseSettings, final Map settings) throws UserException { - final String esPathConf = System.getProperty("es.path.conf"); + final String esPathConf = sysprops.get("es.path.conf"); if (esPathConf == null) { throw new UserException(ExitCodes.CONFIG, "the system property [es.path.conf] must be set"); } return InternalSettingsPreparer.prepareEnvironment( - baseSettings, + Settings.EMPTY, settings, getConfigPath(esPathConf), // HOSTNAME is set by elasticsearch-env and elasticsearch-env.bat so it is always available - () -> System.getenv("HOSTNAME") + () -> envVars.get("HOSTNAME") ); } @@ -107,8 +102,13 @@ public abstract class EnvironmentAwareCommand extends Command { } /** Ensure the given setting exists, reading it from system properties if not already set. */ - private static void putSystemPropertyIfSettingIsMissing(final Map settings, final String setting, final String key) { - final String value = System.getProperty(key); + private static void putSystemPropertyIfSettingIsMissing( + final Map sysprops, + final Map settings, + final String setting, + final String key + ) { + final String value = sysprops.get(key); if (value != null) { if (settings.containsKey(setting)) { final String message = String.format( diff --git a/server/src/main/resources/org/elasticsearch/bootstrap/security.policy b/server/src/main/resources/org/elasticsearch/bootstrap/security.policy index 09d7ae188559..3726b5d6b784 100644 --- a/server/src/main/resources/org/elasticsearch/bootstrap/security.policy +++ b/server/src/main/resources/org/elasticsearch/bootstrap/security.policy @@ -56,6 +56,11 @@ grant codeBase "${codebase.elasticsearch-x-content}" { permission java.lang.RuntimePermission "createClassLoader"; }; +grant codeBase "${codebase.elasticsearch-cli}" { + // we don't actually use write, but it is needed to get the entire property map + permission java.util.PropertyPermission "*", "read,write"; +}; + grant codeBase "${codebase.jna}" { // for registering native methods permission java.lang.RuntimePermission "accessDeclaredMembers"; diff --git a/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java b/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java index f60a752f645f..b16625448c7f 100644 --- a/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java +++ b/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java @@ -9,197 +9,188 @@ package org.elasticsearch.bootstrap; import org.elasticsearch.Build; +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.CommandTestCase; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; import org.elasticsearch.monitor.jvm.JvmInfo; +import org.hamcrest.Matcher; +import org.junit.Before; import java.nio.file.Path; import java.util.Locale; -import java.util.function.BiConsumer; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.emptyString; +import static org.hamcrest.Matchers.hasItem; -public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { +public class ElasticsearchCliTests extends CommandTestCase { + private void assertOk(String... args) throws Exception { + assertOkWithOutput(emptyString(), args); + } + + private void assertOkWithOutput(Matcher matcher, String... args) throws Exception { + terminal.reset(); + int status = executeMain(args); + assertThat(status, equalTo(ExitCodes.OK)); + assertThat(terminal.getErrorOutput(), emptyString()); + assertThat(terminal.getOutput(), matcher); + } + + private void assertUsage(Matcher matcher, String... args) throws Exception { + terminal.reset(); + initCallback = FAIL_INIT; + int status = executeMain(args); + assertThat(status, equalTo(ExitCodes.USAGE)); + assertThat(terminal.getErrorOutput(), matcher); + } + + private void assertMutuallyExclusiveOptions(String... args) throws Exception { + assertUsage(allOf(containsString("ERROR:"), containsString("are unavailable given other options on the command line")), args); + } public void testVersion() throws Exception { - runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-d"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--daemonize"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-p", "/tmp/pid"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--pidfile", "/tmp/pid"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-d"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--daemonize"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-p", "/tmp/pid"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--pidfile", "/tmp/pid"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-q"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--quiet"); + assertMutuallyExclusiveOptions("-V", "-d"); + assertMutuallyExclusiveOptions("-V", "--daemonize"); + assertMutuallyExclusiveOptions("-V", "-p", "/tmp/pid"); + assertMutuallyExclusiveOptions("-V", "--pidfile", "/tmp/pid"); + assertMutuallyExclusiveOptions("--version", "-d"); + assertMutuallyExclusiveOptions("--version", "--daemonize"); + assertMutuallyExclusiveOptions("--version", "-p", "/tmp/pid"); + assertMutuallyExclusiveOptions("--version", "--pidfile", "/tmp/pid"); + assertMutuallyExclusiveOptions("--version", "-q"); + assertMutuallyExclusiveOptions("--version", "--quiet"); - runTestThatVersionIsReturned("-V"); - runTestThatVersionIsReturned("--version"); - } - - private void runTestThatVersionIsMutuallyExclusiveToOtherOptions(String... args) throws Exception { - runTestVersion( - ExitCodes.USAGE, - (output, error) -> assertThat( - error, - allOf(containsString("ERROR:"), containsString("are unavailable given other options on the command line")) - ), - args + final String expectedBuildOutput = String.format( + Locale.ROOT, + "Build: %s/%s/%s", + Build.CURRENT.type().displayName(), + Build.CURRENT.hash(), + Build.CURRENT.date() ); - } - - private void runTestThatVersionIsReturned(String... args) throws Exception { - runTestVersion(ExitCodes.OK, (output, error) -> { - assertThat(output, containsString("Version: " + Build.CURRENT.qualifiedVersion())); - final String expectedBuildOutput = String.format( - Locale.ROOT, - "Build: %s/%s/%s", - Build.CURRENT.type().displayName(), - Build.CURRENT.hash(), - Build.CURRENT.date() - ); - assertThat(output, containsString(expectedBuildOutput)); - assertThat(output, containsString("JVM: " + JvmInfo.jvmInfo().version())); - }, args); - } - - private void runTestVersion(int expectedStatus, BiConsumer outputConsumer, String... args) throws Exception { - runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, quiet, esSettings) -> {}, args); + Matcher versionOutput = allOf( + containsString("Version: " + Build.CURRENT.qualifiedVersion()), + containsString(expectedBuildOutput), + containsString("JVM: " + JvmInfo.jvmInfo().version()) + ); + assertOkWithOutput(versionOutput, "-V"); + assertOkWithOutput(versionOutput, "--version"); } public void testPositionalArgs() throws Exception { - runTest( - ExitCodes.USAGE, - false, - (output, error) -> assertThat(error, containsString("Positional arguments not allowed, found [foo]")), - (foreground, pidFile, quiet, esSettings) -> {}, - "foo" - ); - runTest( - ExitCodes.USAGE, - false, - (output, error) -> assertThat(error, containsString("Positional arguments not allowed, found [foo, bar]")), - (foreground, pidFile, quiet, esSettings) -> {}, - "foo", - "bar" - ); - runTest( - ExitCodes.USAGE, - false, - (output, error) -> assertThat(error, containsString("Positional arguments not allowed, found [foo]")), - (foreground, pidFile, quiet, esSettings) -> {}, - "-E", - "foo=bar", - "foo", - "-E", - "baz=qux" - ); + String prefix = "Positional arguments not allowed, found "; + assertUsage(containsString(prefix + "[foo]"), "foo"); + assertUsage(containsString(prefix + "[foo, bar]"), "foo", "bar"); + assertUsage(containsString(prefix + "[foo]"), "-E", "foo=bar", "foo", "-E", "baz=qux"); } - public void testThatPidFileCanBeConfigured() throws Exception { + public void testPidFile() throws Exception { Path tmpDir = createTempDir(); - Path pidFile = tmpDir.resolve("pid"); - runPidFileTest( - ExitCodes.USAGE, - false, - (output, error) -> assertThat(error, containsString("Option p/pidfile requires an argument")), - pidFile, - "-p" - ); - runPidFileTest(ExitCodes.OK, true, (output, error) -> {}, pidFile, "-p", pidFile.toString()); - runPidFileTest(ExitCodes.OK, true, (output, error) -> {}, pidFile, "--pidfile", tmpDir.toString() + "/pid"); + Path pidFileArg = tmpDir.resolve("pid"); + assertUsage(containsString("Option p/pidfile requires an argument"), "-p"); + initCallback = (daemonize, pidFile, quiet, env) -> { assertThat(pidFile.toString(), equalTo(pidFileArg.toString())); }; + terminal.reset(); + assertOk("-p", pidFileArg.toString()); + terminal.reset(); + assertOk("--pidfile", pidFileArg.toString()); } - private void runPidFileTest( - final int expectedStatus, - final boolean expectedInit, - BiConsumer outputConsumer, - Path expectedPidFile, - final String... args - ) throws Exception { - runTest( - expectedStatus, - expectedInit, - outputConsumer, - (foreground, pidFile, quiet, esSettings) -> assertThat(pidFile.toString(), equalTo(expectedPidFile.toString())), - args - ); + public void testDaemonize() throws Exception { + AtomicBoolean expectDaemonize = new AtomicBoolean(true); + initCallback = (d, p, q, e) -> assertThat(d, equalTo(expectDaemonize.get())); + assertOk("-d"); + assertOk("--daemonize"); + expectDaemonize.set(false); + assertOk(); } - public void testThatParsingDaemonizeWorks() throws Exception { - runDaemonizeTest(true, "-d"); - runDaemonizeTest(true, "--daemonize"); - runDaemonizeTest(false); - } - - private void runDaemonizeTest(final boolean expectedDaemonize, final String... args) throws Exception { - runTest( - ExitCodes.OK, - true, - (output, error) -> {}, - (foreground, pidFile, quiet, esSettings) -> assertThat(foreground, equalTo(expectedDaemonize == false)), - args - ); - } - - public void testThatParsingQuietOptionWorks() throws Exception { - runQuietTest(true, "-q"); - runQuietTest(true, "--quiet"); - runQuietTest(false); - } - - private void runQuietTest(final boolean expectedQuiet, final String... args) throws Exception { - runTest( - ExitCodes.OK, - true, - (output, error) -> {}, - (foreground, pidFile, quiet, esSettings) -> assertThat(quiet, equalTo(expectedQuiet)), - args - ); + public void testQuiet() throws Exception { + AtomicBoolean expectQuiet = new AtomicBoolean(true); + initCallback = (d, p, q, e) -> assertThat(q, equalTo(expectQuiet.get())); + assertOk("-q"); + assertOk("--quiet"); + expectQuiet.set(false); + assertOk(); } public void testElasticsearchSettings() throws Exception { - runTest(ExitCodes.OK, true, (output, error) -> {}, (foreground, pidFile, quiet, env) -> { - Settings settings = env.settings(); - assertEquals("bar", settings.get("foo")); - assertEquals("qux", settings.get("baz")); - }, "-Efoo=bar", "-E", "baz=qux"); + initCallback = (d, p, q, e) -> { + Settings settings = e.settings(); + assertThat(settings.get("foo"), equalTo("bar")); + assertThat(settings.get("baz"), equalTo("qux")); + }; + assertOk("-Efoo=bar", "-E", "baz=qux"); } public void testElasticsearchSettingCanNotBeEmpty() throws Exception { - runTest( - ExitCodes.USAGE, - false, - (output, error) -> assertThat(error, containsString("setting [foo] must not be empty")), - (foreground, pidFile, quiet, esSettings) -> {}, - "-E", - "foo=" - ); + assertUsage(containsString("setting [foo] must not be empty"), "-E", "foo="); } public void testElasticsearchSettingCanNotBeDuplicated() throws Exception { - runTest( - ExitCodes.USAGE, - false, - (output, error) -> assertThat(error, containsString("setting [foo] already set, saw [bar] and [baz]")), - (foreground, pidFile, quiet, initialEnv) -> {}, - "-E", - "foo=bar", - "-E", - "foo=baz" - ); + assertUsage(containsString("setting [foo] already set, saw [bar] and [baz]"), "-E", "foo=bar", "-E", "foo=baz"); } public void testUnknownOption() throws Exception { - runTest( - ExitCodes.USAGE, - false, - (output, error) -> assertThat(error, containsString("network.host is not a recognized option")), - (foreground, pidFile, quiet, esSettings) -> {}, - "--network.host" - ); + assertUsage(containsString("network.host is not a recognized option"), "--network.host"); } + public void testPathHome() throws Exception { + AtomicReference expectedHomeDir = new AtomicReference<>(); + expectedHomeDir.set(homeDir.toString()); + initCallback = (d, p, q, e) -> { + Settings settings = e.settings(); + assertThat(settings.get("path.home"), equalTo(expectedHomeDir.get())); + assertThat(settings.keySet(), hasItem("path.logs")); // added by env initialization + }; + assertOk(); + homeDir = null; + final String commandLineValue = createTempDir().toString(); + expectedHomeDir.set(commandLineValue); + assertOk("-Epath.home=" + commandLineValue); + } + + interface InitMethod { + void init(boolean daemonize, Path pidFile, boolean quiet, Environment initialEnv); + } + + Path homeDir; + InitMethod initCallback; + final InitMethod FAIL_INIT = (d, p, q, e) -> fail("Did not expect to run init"); + + @Before + public void resetCommand() { + homeDir = createTempDir(); + initCallback = null; + } + + @Override + protected Command newCommand() { + return new Elasticsearch() { + @Override + protected Map captureSystemProperties() { + if (homeDir == null) { + return Map.of("es.path.conf", createTempDir().toString()); + } + return mockSystemProperties(homeDir); + } + + @Override + void init(boolean daemonize, Path pidFile, boolean quiet, Environment initialEnv) { + if (initCallback != null) { + initCallback.init(daemonize, pidFile, quiet, initialEnv); + } + } + + @Override + public boolean addShutdownHook() { + return false; + } + }; + } } diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java index d3e0feea1551..e0621c2f8415 100644 --- a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java +++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java @@ -202,6 +202,7 @@ public class BootstrapForTesting { addClassCodebase(codebases, "elasticsearch-secure-sm", "org.elasticsearch.secure_sm.SecureSM"); addClassCodebase(codebases, "elasticsearch-rest-client", "org.elasticsearch.client.RestClient"); addClassCodebase(codebases, "elasticsearch-x-content", "org.elasticsearch.xcontent.XContent"); + addClassCodebase(codebases, "elasticsearch-cli", "org.elasticsearch.cli.Command"); return codebases; } diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java deleted file mode 100644 index e820bf3aa569..000000000000 --- a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.bootstrap; - -import org.elasticsearch.cli.MockTerminal; -import org.elasticsearch.cli.UserException; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; -import org.elasticsearch.test.ESTestCase; - -import java.nio.file.Path; -import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.BiConsumer; - -import static org.hamcrest.CoreMatchers.equalTo; - -abstract class ESElasticsearchCliTestCase extends ESTestCase { - - interface InitConsumer { - void accept(boolean foreground, Path pidFile, boolean quiet, Environment initialEnv); - } - - void runTest( - final int expectedStatus, - final boolean expectedInit, - final BiConsumer outputConsumer, - final InitConsumer initConsumer, - final String... args - ) throws Exception { - final MockTerminal terminal = new MockTerminal(); - final Path home = createTempDir(); - try { - final AtomicBoolean init = new AtomicBoolean(); - final int status = Elasticsearch.main(args, new Elasticsearch() { - @Override - protected Environment createEnv(final Map settings) throws UserException { - Settings.Builder builder = Settings.builder().put("path.home", home); - settings.forEach((k, v) -> builder.put(k, v)); - final Settings realSettings = builder.build(); - return new Environment(realSettings, home.resolve("config")); - } - - @Override - void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv) { - init.set(true); - initConsumer.accept(daemonize == false, pidFile, quiet, initialEnv); - } - - @Override - protected boolean addShutdownHook() { - return false; - } - }, terminal); - assertThat(status, equalTo(expectedStatus)); - assertThat(init.get(), equalTo(expectedInit)); - outputConsumer.accept(terminal.getOutput(), terminal.getErrorOutput()); - } catch (Exception e) { - // if an unexpected exception is thrown, we log - // terminal output to aid debugging - logger.info("Stdout:\n" + terminal.getOutput()); - logger.info("Stderr:\n" + terminal.getErrorOutput()); - // rethrow so the test fails - throw e; - } - } - -} diff --git a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java index 61091f8af02e..785b20df99da 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java @@ -11,6 +11,9 @@ package org.elasticsearch.cli; import org.elasticsearch.test.ESTestCase; import org.junit.Before; +import java.nio.file.Path; +import java.util.Map; + /** * A base test case for cli tools. */ @@ -25,6 +28,10 @@ public abstract class CommandTestCase extends ESTestCase { terminal.setVerbosity(Terminal.Verbosity.NORMAL); } + protected static Map mockSystemProperties(Path homeDir) { + return Map.of("es.path.home", homeDir.toString(), "es.path.conf", homeDir.resolve("config").toString()); + } + /** Creates a Command to test execution. */ protected abstract Command newCommand(); diff --git a/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/AutoConfigureNode.java b/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/AutoConfigureNode.java index a3c1d887ab9c..c102ed3e1196 100644 --- a/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/AutoConfigureNode.java +++ b/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/AutoConfigureNode.java @@ -206,7 +206,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand { if (false == inEnrollmentMode) { throw new UserException(ExitCodes.USAGE, "enrollment-token is a mandatory parameter when reconfiguring the node"); } - env = possiblyReconfigureNode(env, terminal); + env = possiblyReconfigureNode(env, terminal, options); } // only perform auto-configuration if the existing configuration is not conflicting (eg Security already enabled) @@ -874,7 +874,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand { return false; } - private Environment possiblyReconfigureNode(Environment env, Terminal terminal) throws UserException { + private Environment possiblyReconfigureNode(Environment env, Terminal terminal, OptionSet options) throws UserException { // We remove the existing auto-configuration stanza from elasticsearch.yml, the elastisearch.keystore and // the directory with the auto-configured TLS key material, and then proceed as if elasticsearch is started // with --enrolment-token token, in the first place. @@ -917,7 +917,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand { ); } // rebuild the environment after removing the settings that were added in auto-configuration. - return createEnv(Map.of("path.home", env.settings().get("path.home"))); + return createEnv(options); } else { throw new UserException( ExitCodes.USAGE, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java index 6a548cfa2e6a..916213c2fca2 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java @@ -16,10 +16,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.settings.KeyStoreWrapper; import org.elasticsearch.common.settings.SecureString; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.Maps; -import org.elasticsearch.env.Environment; -import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.protocol.xpack.XPackInfoResponse; import org.elasticsearch.protocol.xpack.XPackInfoResponse.FeatureSetsInfo; import org.elasticsearch.protocol.xpack.XPackInfoResponse.FeatureSetsInfo.FeatureSet; @@ -72,7 +69,6 @@ import static org.mockito.Mockito.when; public class SetupPasswordToolTests extends CommandTestCase { - private final String pathHomeParameter = "-Epath.home=" + createTempDir(); private SecureString bootstrapPassword; private CommandLineHttpClient httpClient; private List usersInSetOrder; @@ -197,10 +193,10 @@ public class SetupPasswordToolTests extends CommandTestCase { public void testAutoSetup() throws Exception { URL url = new URL(httpClient.getDefaultURL()); if (randomBoolean()) { - execute("auto", pathHomeParameter, "-b", "true"); + execute("auto", "-b", "true"); } else { terminal.addTextInput("Y"); - execute("auto", pathHomeParameter); + execute("auto"); } if (usedKeyStore.hasPassword()) { // SecureString is already closed (zero-filled) and keystore-password is 17 char long @@ -262,7 +258,7 @@ public class SetupPasswordToolTests extends CommandTestCase { ).thenReturn(httpResponse); try { - execute(randomBoolean() ? "auto" : "interactive", pathHomeParameter); + execute(randomBoolean() ? "auto" : "interactive"); fail("Should have thrown exception"); } catch (UserException e) { assertEquals(ExitCodes.CONFIG, e.exitCode); @@ -310,7 +306,7 @@ public class SetupPasswordToolTests extends CommandTestCase { thrown.expect(UserException.class); thrown.expectMessage("X-Pack is not available on this Elasticsearch node."); - execute(randomBoolean() ? "auto" : "interactive", pathHomeParameter); + execute(randomBoolean() ? "auto" : "interactive"); } public void testErrorMessagesWhenXPackIsAvailableWithCorrectLicenseAndIsEnabledButStillFailedForUnknown() throws Exception { @@ -356,7 +352,7 @@ public class SetupPasswordToolTests extends CommandTestCase { thrown.expect(UserException.class); thrown.expectMessage("Unknown error"); - execute(randomBoolean() ? "auto" : "interactive", pathHomeParameter); + execute(randomBoolean() ? "auto" : "interactive"); } @@ -402,7 +398,7 @@ public class SetupPasswordToolTests extends CommandTestCase { thrown.expect(UserException.class); thrown.expectMessage("X-Pack Security is not available."); - execute(randomBoolean() ? "auto" : "interactive", pathHomeParameter); + execute(randomBoolean() ? "auto" : "interactive"); } @@ -448,7 +444,7 @@ public class SetupPasswordToolTests extends CommandTestCase { thrown.expect(UserException.class); thrown.expectMessage("X-Pack Security is disabled by configuration."); - execute(randomBoolean() ? "auto" : "interactive", pathHomeParameter); + execute(randomBoolean() ? "auto" : "interactive"); } public void testWrongServer() throws Exception { @@ -458,7 +454,7 @@ public class SetupPasswordToolTests extends CommandTestCase { .execute(eq("GET"), eq(authnURL), eq(ElasticUser.NAME), any(SecureString.class), anyCheckedSupplier(), anyCheckedFunction()); try { - execute(randomBoolean() ? "auto" : "interactive", pathHomeParameter); + execute(randomBoolean() ? "auto" : "interactive"); fail("Should have thrown exception"); } catch (UserException e) { assertEquals(ExitCodes.CONFIG, e.exitCode); @@ -501,7 +497,7 @@ public class SetupPasswordToolTests extends CommandTestCase { terminal.addTextInput("n"); try { - execute(randomBoolean() ? "auto" : "interactive", pathHomeParameter); + execute(randomBoolean() ? "auto" : "interactive"); fail("Should have thrown exception"); } catch (UserException e) { assertEquals(ExitCodes.OK, e.exitCode); @@ -511,7 +507,7 @@ public class SetupPasswordToolTests extends CommandTestCase { public void testUrlOption() throws Exception { URL url = new URL("http://localhost:9202" + randomFrom("", "/", "//", "/smth", "//smth/", "//x//x/")); - execute("auto", pathHomeParameter, "-u", url.toString(), "-b"); + execute("auto", "-u", url.toString(), "-b"); InOrder inOrder = Mockito.inOrder(httpClient); @@ -540,7 +536,7 @@ public class SetupPasswordToolTests extends CommandTestCase { doThrow(new IOException()).when(httpClient) .execute(eq("PUT"), eq(userToFailURL), anyString(), any(SecureString.class), anyCheckedSupplier(), anyCheckedFunction()); try { - execute(randomBoolean() ? "auto" : "interactive", pathHomeParameter, "-b"); + execute(randomBoolean() ? "auto" : "interactive", "-b"); fail("Should have thrown exception"); } catch (UserException e) { assertEquals(ExitCodes.TEMP_FAILURE, e.exitCode); @@ -551,7 +547,7 @@ public class SetupPasswordToolTests extends CommandTestCase { URL url = new URL(httpClient.getDefaultURL()); terminal.addTextInput("Y"); - execute("interactive", pathHomeParameter); + execute("interactive"); InOrder inOrder = Mockito.inOrder(httpClient); @@ -605,7 +601,7 @@ public class SetupPasswordToolTests extends CommandTestCase { terminal.addSecretInput(user + "-password"); } - execute("interactive", pathHomeParameter); + execute("interactive"); InOrder inOrder = Mockito.inOrder(httpClient); @@ -635,10 +631,10 @@ public class SetupPasswordToolTests extends CommandTestCase { terminal.addSecretInput("wrong-password"); final UserException e = expectThrows(UserException.class, () -> { if (randomBoolean()) { - execute(commandWithPasswordProtectedKeystore, "auto", pathHomeParameter, "-b", "true"); + execute(commandWithPasswordProtectedKeystore, "auto", "-b", "true"); } else { terminal.addTextInput("Y"); - execute(commandWithPasswordProtectedKeystore, "auto", pathHomeParameter); + execute(commandWithPasswordProtectedKeystore, "auto"); } }); assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); @@ -674,10 +670,8 @@ public class SetupPasswordToolTests extends CommandTestCase { protected AutoSetup newAutoSetup() { return new AutoSetup() { @Override - protected Environment createEnv(Map settings) throws UserException { - Settings.Builder builder = Settings.builder(); - settings.forEach((k, v) -> builder.put(k, v)); - return TestEnvironment.newEnvironment(builder.build()); + protected Map captureSystemProperties() { + return mockSystemProperties(createTempDir()); } }; } @@ -686,10 +680,8 @@ public class SetupPasswordToolTests extends CommandTestCase { protected InteractiveSetup newInteractiveSetup() { return new InteractiveSetup() { @Override - protected Environment createEnv(Map settings) throws UserException { - Settings.Builder builder = Settings.builder(); - settings.forEach((k, v) -> builder.put(k, v)); - return TestEnvironment.newEnvironment(builder.build()); + protected Map captureSystemProperties() { + return mockSystemProperties(createTempDir()); } }; } diff --git a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/ResetPasswordToolTests.java b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/ResetPasswordToolTests.java index 943b2770172a..25b83c855d54 100644 --- a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/ResetPasswordToolTests.java +++ b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/ResetPasswordToolTests.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.security.authc.esnative.tool; +import joptsimple.OptionSet; + import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; @@ -66,7 +68,7 @@ public class ResetPasswordToolTests extends CommandTestCase { protected Command newCommand() { return new ResetPasswordTool(environment -> client, environment -> keyStoreWrapper) { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(ResetPasswordToolTests.this.settings, confDir); } }; diff --git a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java index ccce1f97292c..4511a98ae3f0 100644 --- a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java +++ b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java @@ -6,6 +6,8 @@ */ package org.elasticsearch.xpack.security.authc.file.tool; +import joptsimple.OptionSet; + import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; @@ -38,7 +40,6 @@ import java.nio.file.Path; import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Objects; import static org.elasticsearch.test.SecurityIntegTestCase.getFastStoredHashAlgoForTests; @@ -118,7 +119,7 @@ public class UsersToolTests extends CommandTestCase { protected AddUserCommand newAddUserCommand() { return new AddUserCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(UsersToolTests.this.settings, confDir); } }; @@ -128,7 +129,7 @@ public class UsersToolTests extends CommandTestCase { protected DeleteUserCommand newDeleteUserCommand() { return new DeleteUserCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(UsersToolTests.this.settings, confDir); } }; @@ -138,7 +139,7 @@ public class UsersToolTests extends CommandTestCase { protected PasswordCommand newPasswordCommand() { return new PasswordCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(UsersToolTests.this.settings, confDir); } }; @@ -148,7 +149,7 @@ public class UsersToolTests extends CommandTestCase { protected RolesCommand newRolesCommand() { return new RolesCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(UsersToolTests.this.settings, confDir); } }; @@ -158,7 +159,7 @@ public class UsersToolTests extends CommandTestCase { protected ListCommand newListCommand() { return new ListCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(UsersToolTests.this.settings, confDir); } }; diff --git a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/service/FileTokensToolTests.java b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/service/FileTokensToolTests.java index 70ca85d84f27..da37e8513713 100644 --- a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/service/FileTokensToolTests.java +++ b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/service/FileTokensToolTests.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.security.authc.service; +import joptsimple.OptionSet; + import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; @@ -33,7 +35,6 @@ import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; -import java.util.Map; import static org.elasticsearch.test.SecurityIntegTestCase.getFastStoredHashAlgoForTests; import static org.hamcrest.Matchers.containsString; @@ -103,7 +104,7 @@ public class FileTokensToolTests extends CommandTestCase { protected CreateFileTokenCommand newCreateFileTokenCommand() { return new CreateFileTokenCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(FileTokensToolTests.this.settings, confDir); } }; @@ -113,7 +114,7 @@ public class FileTokensToolTests extends CommandTestCase { protected DeleteFileTokenCommand newDeleteFileTokenCommand() { return new DeleteFileTokenCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(FileTokensToolTests.this.settings, confDir); } }; @@ -123,7 +124,7 @@ public class FileTokensToolTests extends CommandTestCase { protected ListFileTokenCommand newListFileTokenCommand() { return new ListFileTokenCommand() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(FileTokensToolTests.this.settings, confDir); } }; diff --git a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/crypto/tool/SystemKeyToolTests.java b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/crypto/tool/SystemKeyToolTests.java index 889748acd5bd..b9e6718c0be7 100644 --- a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/crypto/tool/SystemKeyToolTests.java +++ b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/crypto/tool/SystemKeyToolTests.java @@ -6,12 +6,13 @@ */ package org.elasticsearch.xpack.security.crypto.tool; +import joptsimple.OptionSet; + import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; import org.elasticsearch.cli.Command; import org.elasticsearch.cli.CommandTestCase; -import org.elasticsearch.cli.UserException; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.PathUtilsForTesting; import org.elasticsearch.core.internal.io.IOUtils; @@ -23,19 +24,19 @@ import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.PosixFilePermission; -import java.util.Map; import java.util.Set; public class SystemKeyToolTests extends CommandTestCase { private FileSystem jimfs; + private Path homeDir; - private Path initFileSystem(boolean needsPosix) throws Exception { + private void initFileSystem(boolean needsPosix) throws Exception { String view = needsPosix ? "posix" : randomFrom("basic", "posix"); Configuration conf = Configuration.unix().toBuilder().setAttributeViews(view).build(); jimfs = Jimfs.newFileSystem(conf); PathUtilsForTesting.installMock(jimfs); - return jimfs.getPath("eshome"); + homeDir = jimfs.getPath("eshome"); } @After @@ -47,24 +48,23 @@ public class SystemKeyToolTests extends CommandTestCase { @Override protected Command newCommand() { return new SystemKeyTool() { - @Override - protected Environment createEnv(Map settings) throws UserException { - Settings.Builder builder = Settings.builder(); - settings.forEach((k, v) -> builder.put(k, v)); - return TestEnvironment.newEnvironment(builder.build()); + protected Environment createEnv(OptionSet options) { + // it would be better to mock the system properties here, but our generated file permissions + // do not play nice with jimfs... + Settings settings = Settings.builder().put("path.home", homeDir.toString()).build(); + return TestEnvironment.newEnvironment(settings); } - }; } public void testGenerate() throws Exception { - final Path homeDir = initFileSystem(true); + initFileSystem(true); Path path = jimfs.getPath(randomAlphaOfLength(10)).resolve("key"); Files.createDirectory(path.getParent()); - execute("-Epath.home=" + homeDir, path.toString()); + execute(path.toString()); byte[] bytes = Files.readAllBytes(path); // TODO: maybe we should actually check the key is...i dunno...valid? assertEquals(SystemKeyTool.KEY_SIZE / 8, bytes.length); @@ -76,31 +76,31 @@ public class SystemKeyToolTests extends CommandTestCase { } public void testGeneratePathInSettings() throws Exception { - final Path homeDir = initFileSystem(false); + initFileSystem(false); Path xpackConf = homeDir.resolve("config"); Files.createDirectories(xpackConf); - execute("-Epath.home=" + homeDir.toString()); + execute(); byte[] bytes = Files.readAllBytes(xpackConf.resolve("system_key")); assertEquals(SystemKeyTool.KEY_SIZE / 8, bytes.length); } public void testGenerateDefaultPath() throws Exception { - final Path homeDir = initFileSystem(false); + initFileSystem(false); Path keyPath = homeDir.resolve("config/system_key"); Files.createDirectories(keyPath.getParent()); - execute("-Epath.home=" + homeDir.toString()); + execute(); byte[] bytes = Files.readAllBytes(keyPath); assertEquals(SystemKeyTool.KEY_SIZE / 8, bytes.length); } public void testThatSystemKeyMayOnlyBeReadByOwner() throws Exception { - final Path homeDir = initFileSystem(true); + initFileSystem(true); Path path = jimfs.getPath(randomAlphaOfLength(10)).resolve("key"); Files.createDirectories(path.getParent()); - execute("-Epath.home=" + homeDir, path.toString()); + execute(path.toString()); Set perms = Files.getPosixFilePermissions(path); assertTrue(perms.toString(), perms.contains(PosixFilePermission.OWNER_READ)); assertTrue(perms.toString(), perms.contains(PosixFilePermission.OWNER_WRITE)); diff --git a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/AutoConfigGenerateElasticPasswordHashTests.java b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/AutoConfigGenerateElasticPasswordHashTests.java index fc84bfde31ac..41f60352f301 100644 --- a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/AutoConfigGenerateElasticPasswordHashTests.java +++ b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/AutoConfigGenerateElasticPasswordHashTests.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.security.enrollment.tool; +import joptsimple.OptionSet; + import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; @@ -28,7 +30,6 @@ import java.io.IOException; import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Map; import static org.elasticsearch.test.SecurityIntegTestCase.getFastStoredHashAlgoForTests; import static org.elasticsearch.xpack.security.authc.esnative.ReservedRealm.AUTOCONFIG_ELASTIC_PASSWORD_HASH; @@ -86,7 +87,7 @@ public class AutoConfigGenerateElasticPasswordHashTests extends CommandTestCase protected Command newCommand() { return new AutoConfigGenerateElasticPasswordHash() { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return env; } }; diff --git a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/BaseRunAsSuperuserCommandTests.java b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/BaseRunAsSuperuserCommandTests.java index 66986cf160dc..e4f8ffd44c66 100644 --- a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/BaseRunAsSuperuserCommandTests.java +++ b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/BaseRunAsSuperuserCommandTests.java @@ -79,7 +79,7 @@ public class BaseRunAsSuperuserCommandTests extends CommandTestCase { protected Command newCommand() { return new DummyRunAsSuperuserCommand(environment -> client, environment -> keyStoreWrapper) { @Override - protected Environment createEnv(Map settings) throws UserException { + protected Environment createEnv(OptionSet options) throws UserException { return new Environment(BaseRunAsSuperuserCommandTests.this.settings, confDir); } }; diff --git a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/CreateEnrollmentTokenToolTests.java b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/CreateEnrollmentTokenToolTests.java index d322ae2cfdc5..d4a6e698101d 100644 --- a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/CreateEnrollmentTokenToolTests.java +++ b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/enrollment/tool/CreateEnrollmentTokenToolTests.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.security.enrollment.tool; +import joptsimple.OptionSet; + import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; @@ -72,7 +74,7 @@ public class CreateEnrollmentTokenToolTests extends CommandTestCase { environment -> externalEnrollmentTokenGenerator ) { @Override - protected Environment createEnv(Map settings) { + protected Environment createEnv(OptionSet options) { return new Environment(CreateEnrollmentTokenToolTests.this.settings, confDir); } };