From 1088ef6ded05ba91ac9c7ead62f851a58d8c5dfc Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 14 Apr 2022 09:22:57 -0700 Subject: [PATCH] Capture system properties and env variables for cli tools to use (#85885) Currently any code needing to access system properties or environment variables does it with the static methods provided by Java. While this is ok in production since these are instantiated for the entire jvm once, it makes any code reading these properties difficult to test without mucking with the test jvm. This commit adds system properties and environment variables to the base Command class that our CLI tools use. While it does not propagate the properties and env down for all possible uses in the system, it is the first step, and it makes CLI testing a bit easier. --- .../keystore/AddFileKeyStoreCommandTests.java | 5 +- .../AddStringKeyStoreCommandTests.java | 5 +- .../ChangeKeyStorePasswordCommandTests.java | 6 +- .../keystore/CreateKeyStoreCommandTests.java | 5 +- .../HasPasswordKeyStoreCommandTests.java | 6 +- .../keystore/ListKeyStoreCommandTests.java | 6 +- .../RemoveSettingKeyStoreCommandTests.java | 5 +- .../keystore/ShowKeyStoreCommandTests.java | 6 +- .../keystore/UpgradeKeyStoreCommandTests.java | 7 +- .../plugins/cli/ListPluginsCommandTests.java | 5 +- .../plugins/cli/MockInstallPluginCommand.java | 8 +- .../plugins/cli/MockRemovePluginCommand.java | 6 +- .../java/org/elasticsearch/cli/Command.java | 30 ++ .../bootstrap/EvilElasticsearchCliTests.java | 51 --- .../common/cli/EnvironmentAwareCommand.java | 36 +-- .../elasticsearch/bootstrap/security.policy | 5 + .../bootstrap/ElasticsearchCliTests.java | 299 +++++++++--------- .../bootstrap/BootstrapForTesting.java | 1 + .../bootstrap/ESElasticsearchCliTestCase.java | 74 ----- .../elasticsearch/cli/CommandTestCase.java | 7 + .../xpack/security/cli/AutoConfigureNode.java | 6 +- .../esnative/tool/SetupPasswordToolTests.java | 46 ++- .../esnative/tool/ResetPasswordToolTests.java | 4 +- .../authc/file/tool/UsersToolTests.java | 13 +- .../authc/service/FileTokensToolTests.java | 9 +- .../crypto/tool/SystemKeyToolTests.java | 36 +-- ...onfigGenerateElasticPasswordHashTests.java | 5 +- .../tool/BaseRunAsSuperuserCommandTests.java | 2 +- .../tool/CreateEnrollmentTokenToolTests.java | 4 +- 29 files changed, 305 insertions(+), 393 deletions(-) delete mode 100644 qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java delete mode 100644 test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java 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); } };