Skip to content

Commit

Permalink
Move cli sysprops and envVars to execute parameter (#86279)
Browse files Browse the repository at this point in the history
The sysprops and envVars members of Command provide cli implementations
with information about the jvm process that is running. This is
convenient for runtime, but difficult for tests to mock because they
must subclass the cli class.

This commit adds a ProcessInfo record, and plumbs it through the
main and execute methods. The new record includes system properties,
environment variables and the working directory. By having this be a
single new parameter, additional information can be added in the future
without again needing to modify the method signatures.

relates #85758
  • Loading branch information
rjernst committed Apr 29, 2022
1 parent 4e481c3 commit ed749fc
Show file tree
Hide file tree
Showing 74 changed files with 325 additions and 241 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@
import org.apache.logging.log4j.Level;
import org.elasticsearch.cli.CliToolProvider;
import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.common.logging.LogConfigurator;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.SuppressForbidden;

import java.util.Map;
import java.util.Properties;
import java.util.stream.Collectors;

/**
* A unified main method for Elasticsearch tools.
Expand All @@ -45,16 +44,16 @@ class CliToolLauncher {
* @throws Exception if the tool fails with an unknown error
*/
public static void main(String[] args) throws Exception {
Map<String, String> sysprops = getSystemProperties();
ProcessInfo pinfo = ProcessInfo.fromSystem();

// configure logging as early as possible
configureLoggingWithoutConfig(sysprops);
configureLoggingWithoutConfig(pinfo.sysprops());

String toolname = getToolName(sysprops);
String libs = sysprops.getOrDefault("cli.libs", "");
String toolname = getToolName(pinfo.sysprops());
String libs = pinfo.sysprops().getOrDefault("cli.libs", "");

Command command = CliToolProvider.load(toolname, libs).create();
exit(command.main(args, Terminal.DEFAULT));
exit(command.main(args, Terminal.DEFAULT, pinfo));
}

// package private for tests
Expand All @@ -73,12 +72,6 @@ static String getToolName(Map<String, String> sysprops) {
return toolname;
}

@SuppressForbidden(reason = "collect system properties")
private static Map<String, String> getSystemProperties() {
Properties props = System.getProperties();
return props.entrySet().stream().collect(Collectors.toMap(e -> e.getKey().toString(), e -> e.getValue().toString()));
}

@SuppressForbidden(reason = "System#exit")
private static void exit(int status) {
System.exit(status);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import joptsimple.OptionSpec;

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.common.hash.MessageDigests;
import org.elasticsearch.core.PathUtils;
Expand Down Expand Up @@ -54,7 +55,7 @@ public GeoIpCli() {
}

@Override
protected void execute(Terminal terminal, OptionSet options) throws Exception {
protected void execute(Terminal terminal, OptionSet options, ProcessInfo processInfo) throws Exception {
Path source = getPath(options.valueOf(sourceDirectory));
String targetString = options.valueOf(targetDirectory);
Path target = targetString != null ? getPath(targetString) : source;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import joptsimple.OptionSpec;

import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.cli.KeyStoreAwareCommand;
Expand All @@ -35,7 +36,7 @@ public BaseKeyStoreCommand(String description, boolean keyStoreMustExist) {
}

@Override
protected final void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
protected final void execute(Terminal terminal, OptionSet options, Environment env, ProcessInfo processInfo) throws Exception {
try {
final Path configFile = env.configFile();
keyStore = KeyStoreWrapper.load(configFile);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import joptsimple.OptionSpec;

import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.cli.KeyStoreAwareCommand;
Expand All @@ -36,7 +37,7 @@ class CreateKeyStoreCommand extends KeyStoreAwareCommand {
}

@Override
protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
protected void execute(Terminal terminal, OptionSet options, Environment env, ProcessInfo processInfo) throws Exception {
try (SecureString password = options.has(passwordOption) ? readPassword(terminal, true) : new SecureString(new char[0])) {
Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile());
if (Files.exists(keystoreFile)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import joptsimple.OptionSet;

import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.cli.KeyStoreAwareCommand;
Expand All @@ -29,7 +30,7 @@ public class HasPasswordKeyStoreCommand extends KeyStoreAwareCommand {
}

@Override
protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
protected void execute(Terminal terminal, OptionSet options, Environment env, ProcessInfo processInfo) throws Exception {
final Path configFile = env.configFile();
final KeyStoreWrapper keyStore = KeyStoreWrapper.load(configFile);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.KeyStoreWrapper;
import org.elasticsearch.core.Tuple;
Expand All @@ -32,7 +33,7 @@ public class AddFileKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new AddFileKeyStoreCommand() {
@Override
protected Environment createEnv(OptionSet options) throws UserException {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) throws UserException {
return env;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.KeyStoreWrapper;
import org.elasticsearch.env.Environment;
Expand All @@ -32,7 +33,7 @@ public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new AddStringKeyStoreCommand() {
@Override
protected Environment createEnv(OptionSet options) throws UserException {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) throws UserException {
return env;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.env.Environment;

Expand All @@ -23,7 +24,7 @@ public class ChangeKeyStorePasswordCommandTests extends KeyStoreCommandTestCase
protected Command newCommand() {
return new ChangeKeyStorePasswordCommand() {
@Override
protected Environment createEnv(OptionSet options) throws UserException {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) throws UserException {
return env;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.KeyStoreWrapper;
import org.elasticsearch.env.Environment;
Expand All @@ -28,7 +29,7 @@ public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new CreateKeyStoreCommand() {
@Override
protected Environment createEnv(OptionSet options) throws UserException {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) throws UserException {
return env;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import joptsimple.OptionSet;

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.env.Environment;

Expand All @@ -24,7 +25,7 @@ public class HasPasswordKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new HasPasswordKeyStoreCommand() {
@Override
protected Environment createEnv(OptionSet options) throws UserException {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) throws UserException {
return env;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.env.Environment;

Expand All @@ -24,7 +25,7 @@ public class ListKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new ListKeyStoreCommand() {
@Override
protected Environment createEnv(OptionSet options) throws UserException {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) throws UserException {
return env;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.env.Environment;

Expand All @@ -26,7 +27,7 @@ public class RemoveSettingKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new RemoveSettingKeyStoreCommand() {
@Override
protected Environment createEnv(OptionSet options) throws UserException {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) throws UserException {
return env;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.KeyStoreWrapper;
import org.elasticsearch.env.Environment;
Expand All @@ -26,7 +27,7 @@ public class ShowKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new ShowKeyStoreCommand() {
@Override
protected Environment createEnv(OptionSet options) throws UserException {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) throws UserException {
return env;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import joptsimple.OptionSet;

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.KeyStoreWrapper;
import org.elasticsearch.env.Environment;
Expand All @@ -31,7 +32,7 @@ public class UpgradeKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new UpgradeKeyStoreCommand() {
@Override
protected Environment createEnv(OptionSet options) {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) {
return env;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import joptsimple.OptionSet;
import joptsimple.OptionSpec;

import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.common.cli.EnvironmentAwareCommand;
import org.elasticsearch.env.Environment;
Expand Down Expand Up @@ -74,7 +75,7 @@ protected void printAdditionalHelp(Terminal terminal) {
}

@Override
protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
protected void execute(Terminal terminal, OptionSet options, Environment env, ProcessInfo processInfo) throws Exception {
SyncPluginsAction.ensureNoConfigFile(env);

List<PluginDescriptor> plugins = arguments.values(options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import joptsimple.OptionSet;

import org.elasticsearch.Version;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.common.cli.EnvironmentAwareCommand;
import org.elasticsearch.env.Environment;
Expand All @@ -36,7 +37,7 @@ class ListPluginsCommand extends EnvironmentAwareCommand {
}

@Override
protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
protected void execute(Terminal terminal, OptionSet options, Environment env, ProcessInfo processInfo) throws Exception {
if (Files.exists(env.pluginsFile()) == false) {
throw new IOException("Plugins directory missing: " + env.pluginsFile());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import joptsimple.OptionSet;
import joptsimple.OptionSpec;

import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.common.cli.EnvironmentAwareCommand;
import org.elasticsearch.env.Environment;
Expand All @@ -33,7 +34,8 @@ class RemovePluginCommand extends EnvironmentAwareCommand {
}

@Override
protected void execute(final Terminal terminal, final OptionSet options, final Environment env) throws Exception {
protected void execute(final Terminal terminal, final OptionSet options, final Environment env, ProcessInfo processInfo)
throws Exception {
SyncPluginsAction.ensureNoConfigFile(env);

final List<PluginDescriptor> plugins = arguments.values(options).stream().map(PluginDescriptor::new).collect(Collectors.toList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.Version;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.hash.MessageDigests;
Expand Down Expand Up @@ -87,6 +88,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.function.BiFunction;
Expand Down Expand Up @@ -794,7 +796,7 @@ public void testZipRelativeOutsideEntryName() throws Exception {

public void testOfficialPluginsHelpSortedAndMissingObviouslyWrongPlugins() throws Exception {
MockTerminal mockTerminal = MockTerminal.create();
new MockInstallPluginCommand().main(new String[] { "--help" }, mockTerminal);
new MockInstallPluginCommand().main(new String[] { "--help" }, mockTerminal, new ProcessInfo(Map.of(), Map.of(), createTempDir()));
try (BufferedReader reader = new BufferedReader(new StringReader(mockTerminal.getOutput()))) {
String line = reader.readLine();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.elasticsearch.Version;
import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.CommandTestCase;
import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.TestEnvironment;
Expand Down Expand Up @@ -243,7 +244,7 @@ public void testExistingIncompatiblePlugin() throws Exception {
protected Command newCommand() {
return new ListPluginsCommand() {
@Override
protected Environment createEnv(OptionSet options) {
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) {
return env;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import joptsimple.OptionSet;

import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.env.Environment;

Expand All @@ -25,8 +26,8 @@ public MockInstallPluginCommand() {
}

@Override
protected Environment createEnv(OptionSet options) throws UserException {
return this.env != null ? this.env : super.createEnv(options);
protected Environment createEnv(OptionSet options, ProcessInfo processInfo) throws UserException {
return this.env != null ? this.env : super.createEnv(options, processInfo);
}

@Override
Expand Down

0 comments on commit ed749fc

Please sign in to comment.