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.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.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 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.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.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 void testExistingIncompatiblePlugin() throws Exception {
     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 MockInstallPluginCommand() {
     }
 
     @Override
-    protected Environment createEnv(Map<String, String> 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

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 MockRemovePluginCommand(final Environment env) {
     }
 
     @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.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 @@ protected static void exit(int status) {
      * 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.