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.

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return env;
             }
         };

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return env;
             }
 

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return env;
             }
         };

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return env;
             }
         };

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return env;
             }
         };

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return env;
             }
         };

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return env;
             }
         };

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return env;
             }
         };

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<String, String> settings) {
+            protected Environment createEnv(OptionSet options) {
                 return env;
             }
-
         };
     }
 

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<String, String> settings) {
+            protected Environment createEnv(OptionSet options) {
                 return env;
             }
 

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<String, String> settings) throws UserException {
+    protected Environment createEnv(OptionSet options) throws UserException {
-        return this.env != null ? this.env : super.createEnv(settings);
+        return this.env != null ? this.env : super.createEnv(options);
     }
 
     @Override

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<String, String> settings) {
+    protected Environment createEnv(OptionSet options) {
         return env;
     }
 }

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<String, String> sysprops;
+    protected final Map<String, String> 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<String, String> captureSystemProperties() {
+        Properties properties = AccessController.doPrivileged((PrivilegedAction<Properties>) 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<String, String> 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.

qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java

@@ -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");
-    }
-
-}

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<String, String> 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(sysprops, settings, "path.data", "es.path.data");
-        putSystemPropertyIfSettingIsMissing(settings, "path.home", "es.path.home");
+        putSystemPropertyIfSettingIsMissing(sysprops, settings, "path.home", "es.path.home");
-        putSystemPropertyIfSettingIsMissing(settings, "path.logs", "es.path.logs");
+        putSystemPropertyIfSettingIsMissing(sysprops, settings, "path.logs", "es.path.logs");
 
-        execute(terminal, options, createEnv(settings));
+        final String esPathConf = sysprops.get("es.path.conf");
-    }
-
-    /** Create an {@link Environment} for the command to use. Overrideable for tests. */
-    protected Environment createEnv(final Map<String, String> 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<String, String> settings) throws UserException {
-        final String esPathConf = System.getProperty("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<String, String> settings, final String setting, final String key) {
+    private static void putSystemPropertyIfSettingIsMissing(
-        final String value = System.getProperty(key);
+        final Map<String, String> sysprops,
+        final Map<String, String> 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(

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

server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java

@@ -9,50 +9,64 @@
 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<String> 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<String> 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");
+        assertMutuallyExclusiveOptions("-V", "-d");
-        runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--daemonize");
+        assertMutuallyExclusiveOptions("-V", "--daemonize");
-        runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-p", "/tmp/pid");
+        assertMutuallyExclusiveOptions("-V", "-p", "/tmp/pid");
-        runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--pidfile", "/tmp/pid");
+        assertMutuallyExclusiveOptions("-V", "--pidfile", "/tmp/pid");
-        runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-d");
+        assertMutuallyExclusiveOptions("--version", "-d");
-        runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--daemonize");
+        assertMutuallyExclusiveOptions("--version", "--daemonize");
-        runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-p", "/tmp/pid");
+        assertMutuallyExclusiveOptions("--version", "-p", "/tmp/pid");
-        runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--pidfile", "/tmp/pid");
+        assertMutuallyExclusiveOptions("--version", "--pidfile", "/tmp/pid");
-        runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-q");
+        assertMutuallyExclusiveOptions("--version", "-q");
-        runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--quiet");
+        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
-        );
-    }
-
-    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",
@@ -60,146 +74,123 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase {
             Build.CURRENT.hash(),
             Build.CURRENT.date()
         );
-            assertThat(output, containsString(expectedBuildOutput));
+        Matcher<String> versionOutput = allOf(
-            assertThat(output, containsString("JVM: " + JvmInfo.jvmInfo().version()));
+            containsString("Version: " + Build.CURRENT.qualifiedVersion()),
-        }, args);
+            containsString(expectedBuildOutput),
-    }
+            containsString("JVM: " + JvmInfo.jvmInfo().version())
-
+        );
-    private void runTestVersion(int expectedStatus, BiConsumer<String, String> outputConsumer, String... args) throws Exception {
+        assertOkWithOutput(versionOutput, "-V");
-        runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, quiet, esSettings) -> {}, args);
+        assertOkWithOutput(versionOutput, "--version");
     }
 
     public void testPositionalArgs() throws Exception {
-        runTest(
+        String prefix = "Positional arguments not allowed, found ";
-            ExitCodes.USAGE,
+        assertUsage(containsString(prefix + "[foo]"), "foo");
-            false,
+        assertUsage(containsString(prefix + "[foo, bar]"), "foo", "bar");
-            (output, error) -> assertThat(error, containsString("Positional arguments not allowed, found [foo]")),
+        assertUsage(containsString(prefix + "[foo]"), "-E", "foo=bar", "foo", "-E", "baz=qux");
-            (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"
-        );
     }
 
-    public void testThatPidFileCanBeConfigured() throws Exception {
+    public void testPidFile() throws Exception {
         Path tmpDir = createTempDir();
-        Path pidFile = tmpDir.resolve("pid");
+        Path pidFileArg = tmpDir.resolve("pid");
-        runPidFileTest(
+        assertUsage(containsString("Option p/pidfile requires an argument"), "-p");
-            ExitCodes.USAGE,
+        initCallback = (daemonize, pidFile, quiet, env) -> { assertThat(pidFile.toString(), equalTo(pidFileArg.toString())); };
-            false,
+        terminal.reset();
-            (output, error) -> assertThat(error, containsString("Option p/pidfile requires an argument")),
+        assertOk("-p", pidFileArg.toString());
-            pidFile,
+        terminal.reset();
-            "-p"
+        assertOk("--pidfile", pidFileArg.toString());
-        );
-        runPidFileTest(ExitCodes.OK, true, (output, error) -> {}, pidFile, "-p", pidFile.toString());
-        runPidFileTest(ExitCodes.OK, true, (output, error) -> {}, pidFile, "--pidfile", tmpDir.toString() + "/pid");
     }
 
-    private void runPidFileTest(
+    public void testDaemonize() throws Exception {
-        final int expectedStatus,
+        AtomicBoolean expectDaemonize = new AtomicBoolean(true);
-        final boolean expectedInit,
+        initCallback = (d, p, q, e) -> assertThat(d, equalTo(expectDaemonize.get()));
-        BiConsumer<String, String> outputConsumer,
+        assertOk("-d");
-        Path expectedPidFile,
+        assertOk("--daemonize");
-        final String... args
+        expectDaemonize.set(false);
-    ) throws Exception {
+        assertOk();
-        runTest(
-            expectedStatus,
-            expectedInit,
-            outputConsumer,
-            (foreground, pidFile, quiet, esSettings) -> assertThat(pidFile.toString(), equalTo(expectedPidFile.toString())),
-            args
-        );
     }
 
-    public void testThatParsingDaemonizeWorks() throws Exception {
+    public void testQuiet() throws Exception {
-        runDaemonizeTest(true, "-d");
+        AtomicBoolean expectQuiet = new AtomicBoolean(true);
-        runDaemonizeTest(true, "--daemonize");
+        initCallback = (d, p, q, e) -> assertThat(q, equalTo(expectQuiet.get()));
-        runDaemonizeTest(false);
+        assertOk("-q");
-    }
+        assertOk("--quiet");
-
+        expectQuiet.set(false);
-    private void runDaemonizeTest(final boolean expectedDaemonize, final String... args) throws Exception {
+        assertOk();
-        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 testElasticsearchSettings() throws Exception {
-        runTest(ExitCodes.OK, true, (output, error) -> {}, (foreground, pidFile, quiet, env) -> {
+        initCallback = (d, p, q, e) -> {
-            Settings settings = env.settings();
+            Settings settings = e.settings();
-            assertEquals("bar", settings.get("foo"));
+            assertThat(settings.get("foo"), equalTo("bar"));
-            assertEquals("qux", settings.get("baz"));
+            assertThat(settings.get("baz"), equalTo("qux"));
-        }, "-Efoo=bar", "-E", "baz=qux");
+        };
+        assertOk("-Efoo=bar", "-E", "baz=qux");
     }
 
     public void testElasticsearchSettingCanNotBeEmpty() throws Exception {
-        runTest(
+        assertUsage(containsString("setting [foo] must not be empty"), "-E", "foo=");
-            ExitCodes.USAGE,
-            false,
-            (output, error) -> assertThat(error, containsString("setting [foo] must not be empty")),
-            (foreground, pidFile, quiet, esSettings) -> {},
-            "-E",
-            "foo="
-        );
     }
 
     public void testElasticsearchSettingCanNotBeDuplicated() throws Exception {
-        runTest(
+        assertUsage(containsString("setting [foo] already set, saw [bar] and [baz]"), "-E", "foo=bar", "-E", "foo=baz");
-            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"
-        );
     }
 
     public void testUnknownOption() throws Exception {
-        runTest(
+        assertUsage(containsString("network.host is not a recognized option"), "--network.host");
-            ExitCodes.USAGE,
-            false,
-            (output, error) -> assertThat(error, containsString("network.host is not a recognized option")),
-            (foreground, pidFile, quiet, esSettings) -> {},
-            "--network.host"
-        );
     }
 
+    public void testPathHome() throws Exception {
+        AtomicReference<String> 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<String, String> 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;
+            }
+        };
+    }
 }

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

test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java

@@ -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<String, String> 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<String, String> 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;
-        }
-    }
-
-}

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<String, String> 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();
 

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,

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<String> 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<String, String> settings) throws UserException {
+                    protected Map<String, String> captureSystemProperties() {
-                        Settings.Builder builder = Settings.builder();
+                        return mockSystemProperties(createTempDir());
-                        settings.forEach((k, v) -> builder.put(k, v));
-                        return TestEnvironment.newEnvironment(builder.build());
                     }
                 };
             }
@@ -686,10 +680,8 @@ public class SetupPasswordToolTests extends CommandTestCase {
             protected InteractiveSetup newInteractiveSetup() {
                 return new InteractiveSetup() {
                     @Override
-                    protected Environment createEnv(Map<String, String> settings) throws UserException {
+                    protected Map<String, String> captureSystemProperties() {
-                        Settings.Builder builder = Settings.builder();
+                        return mockSystemProperties(createTempDir());
-                        settings.forEach((k, v) -> builder.put(k, v));
-                        return TestEnvironment.newEnvironment(builder.build());
                     }
                 };
             }

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return new Environment(ResetPasswordToolTests.this.settings, confDir);
             }
         };

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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> settings) throws UserException {
+                    protected Environment createEnv(OptionSet options) throws UserException {
                         return new Environment(UsersToolTests.this.settings, confDir);
                     }
                 };

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<String, String> 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<String, String> 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<String, String> settings) throws UserException {
+                    protected Environment createEnv(OptionSet options) throws UserException {
                         return new Environment(FileTokensToolTests.this.settings, confDir);
                     }
                 };

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) {
-                Settings.Builder builder = Settings.builder();
+                // it would be better to mock the system properties here, but our generated file permissions
-                settings.forEach((k, v) -> builder.put(k, v));
+                // do not play nice with jimfs...
-                return TestEnvironment.newEnvironment(builder.build());
+                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<PosixFilePermission> perms = Files.getPosixFilePermissions(path);
         assertTrue(perms.toString(), perms.contains(PosixFilePermission.OWNER_READ));
         assertTrue(perms.toString(), perms.contains(PosixFilePermission.OWNER_WRITE));

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return env;
             }
         };

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<String, String> settings) throws UserException {
+            protected Environment createEnv(OptionSet options) throws UserException {
                 return new Environment(BaseRunAsSuperuserCommandTests.this.settings, confDir);
             }
         };

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<String, String> settings) {
+            protected Environment createEnv(OptionSet options) {
                 return new Environment(CreateEnrollmentTokenToolTests.this.settings, confDir);
             }
         };