From 2aeae3956073412dc461e45ac641097a02ed9736 Mon Sep 17 00:00:00 2001 From: Antony Denyer Date: Thu, 23 Sep 2021 11:59:05 +0100 Subject: [PATCH] Allow the use of BESU_CONFIG_FILE env var (#2784) * Allow the use of BESU_CONFIG_FILE env var to specify config TOML * Throw an exception when BESU_CONFIG_FILE and --config-file specified * Switch ExecutionException for ParameterException Signed-off-by: Antony Denyer --- CHANGELOG.md | 1 + .../org/hyperledger/besu/cli/BesuCommand.java | 3 +- .../util/ConfigOptionSearchAndRunHandler.java | 31 ++++++++--- .../ConfigOptionSearchAndRunHandlerTest.java | 53 +++++++++++++++++-- 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 201b118de7..8eb715a548 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Low level performance improvements changes to cut worst-case EVM performance in half. [#2796](https://github.com/hyperledger/besu/pull/2796) ### Bug Fixes +* Allow BESU_CONFIG_FILE environment to specify TOML file [#2455](https://github.com/hyperledger/besu/issues/2455) ### Early Access Features diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 3d76e8b0b2..4dd8f06bdc 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -1342,8 +1342,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { // and eventually it will run regular parsing of the remaining options. final ConfigOptionSearchAndRunHandler configParsingHandler = - new ConfigOptionSearchAndRunHandler( - resultHandler, exceptionHandler, CONFIG_FILE_OPTION_NAME, environment); + new ConfigOptionSearchAndRunHandler(resultHandler, exceptionHandler, environment); ParseArgsHelper.getLauncherOptions(unstableLauncherOptions, args); if (unstableLauncherOptions.isLauncherMode() diff --git a/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java b/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java index b1b18ba4d2..179ad590e3 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java @@ -23,25 +23,22 @@ import java.util.Optional; import com.google.common.annotations.VisibleForTesting; import picocli.CommandLine; import picocli.CommandLine.AbstractParseResultHandler; -import picocli.CommandLine.ExecutionException; import picocli.CommandLine.IDefaultValueProvider; import picocli.CommandLine.Model.OptionSpec; +import picocli.CommandLine.ParameterException; import picocli.CommandLine.ParseResult; public class ConfigOptionSearchAndRunHandler extends AbstractParseResultHandler> { private final AbstractParseResultHandler> resultHandler; private final CommandLine.IExceptionHandler2> exceptionHandler; - private final String configFileOptionName; private final Map environment; public ConfigOptionSearchAndRunHandler( final AbstractParseResultHandler> resultHandler, final CommandLine.IExceptionHandler2> exceptionHandler, - final String configFileOptionName, final Map environment) { this.resultHandler = resultHandler; this.exceptionHandler = exceptionHandler; - this.configFileOptionName = configFileOptionName; this.environment = environment; // use the same output as the regular options handler to ensure that outputs are all going // in the same place. No need to do this for the exception handler as we reuse it directly. @@ -49,7 +46,7 @@ public class ConfigOptionSearchAndRunHandler extends AbstractParseResultHandler< } @Override - public List handle(final ParseResult parseResult) throws ExecutionException { + public List handle(final ParseResult parseResult) throws ParameterException { final CommandLine commandLine = parseResult.asCommandLineList().get(0); final Optional configFile = findConfigFile(parseResult, commandLine); commandLine.setDefaultValueProvider(createDefaultValueProvider(commandLine, configFile)); @@ -60,13 +57,31 @@ public class ConfigOptionSearchAndRunHandler extends AbstractParseResultHandler< private Optional findConfigFile( final ParseResult parseResult, final CommandLine commandLine) { - if (parseResult.hasMatchedOption(configFileOptionName)) { - final OptionSpec configFileOption = parseResult.matchedOption(configFileOptionName); + if (parseResult.hasMatchedOption("--config-file") + && environment.containsKey("BESU_CONFIG_FILE")) { + throw new ParameterException( + commandLine, + String.format( + "TOML file specified using BESU_CONFIG_FILE=%s and --config-file %s", + environment.get("BESU_CONFIG_FILE"), + parseResult.matchedOption("--config-file").stringValues())); + } else if (parseResult.hasMatchedOption("--config-file")) { + final OptionSpec configFileOption = parseResult.matchedOption("--config-file"); try { return Optional.of(configFileOption.getter().get()); } catch (final Exception e) { - throw new ExecutionException(commandLine, e.getMessage(), e); + throw new ParameterException(commandLine, e.getMessage(), e); } + } else if (environment.containsKey("BESU_CONFIG_FILE")) { + final File toml = new File(environment.get("BESU_CONFIG_FILE")); + if (!toml.exists()) { + throw new ParameterException( + commandLine, + String.format( + "TOML file %s specified in environment variable BESU_CONFIG_FILE not found", + environment.get("BESU_CONFIG_FILE"))); + } + return Optional.of(toml); } return Optional.empty(); diff --git a/besu/src/test/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandlerTest.java b/besu/src/test/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandlerTest.java index 348e52fe41..686c3fe82b 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandlerTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandlerTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.when; import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.IOException; import java.io.PrintStream; import java.util.ArrayList; import java.util.List; @@ -51,7 +52,7 @@ import picocli.CommandLine.RunLast; @RunWith(MockitoJUnitRunner.class) public class ConfigOptionSearchAndRunHandlerTest { - private static final String CONFIG_FILE_OPTION_NAME = "--config"; + private static final String CONFIG_FILE_OPTION_NAME = "--config-file"; @Rule public final TemporaryFolder temp = new TemporaryFolder(); @Rule public ExpectedException exceptionRule = ExpectedException.none(); @@ -67,8 +68,7 @@ public class ConfigOptionSearchAndRunHandlerTest { new DefaultExceptionHandler>().useErr(errPrintStream).useAnsi(Ansi.OFF); private final Map environment = singletonMap("BESU_LOGGING", "ERROR"); private final ConfigOptionSearchAndRunHandler configParsingHandler = - new ConfigOptionSearchAndRunHandler( - resultHandler, exceptionHandler, CONFIG_FILE_OPTION_NAME, environment); + new ConfigOptionSearchAndRunHandler(resultHandler, exceptionHandler, environment); @Mock ParseResult mockParseResult; @Mock CommandLine mockCommandLine; @@ -89,7 +89,7 @@ public class ConfigOptionSearchAndRunHandlerTest { } @Test - public void handle() throws Exception { + public void handleWithCommandLineOption() throws Exception { when(mockConfigOptionGetter.get()).thenReturn(temp.newFile()); final List result = configParsingHandler.handle(mockParseResult); verify(mockCommandLine).setDefaultValueProvider(any(IDefaultValueProvider.class)); @@ -98,7 +98,22 @@ public class ConfigOptionSearchAndRunHandlerTest { } @Test - public void handleShouldRaiseExceptionIfNoFileParam() throws Exception { + public void handleWithEnvironmentVariable() throws IOException { + when(mockParseResult.hasMatchedOption(CONFIG_FILE_OPTION_NAME)).thenReturn(false); + + final ConfigOptionSearchAndRunHandler environmentConfigFileParsingHandler = + new ConfigOptionSearchAndRunHandler( + resultHandler, + exceptionHandler, + singletonMap("BESU_CONFIG_FILE", temp.newFile().getAbsolutePath())); + + when(mockParseResult.hasMatchedOption(CONFIG_FILE_OPTION_NAME)).thenReturn(false); + + environmentConfigFileParsingHandler.handle(mockParseResult); + } + + @Test + public void handleWithCommandLineOptionShouldRaiseExceptionIfNoFileParam() throws Exception { exceptionRule.expect(Exception.class); final String error_message = "an error occurred during get"; exceptionRule.expectMessage(error_message); @@ -106,6 +121,18 @@ public class ConfigOptionSearchAndRunHandlerTest { configParsingHandler.handle(mockParseResult); } + @Test + public void handleWithEnvironmentVariableOptionShouldRaiseExceptionIfNoFileParam() { + exceptionRule.expect(CommandLine.ParameterException.class); + final ConfigOptionSearchAndRunHandler environmentConfigFileParsingHandler = + new ConfigOptionSearchAndRunHandler( + resultHandler, exceptionHandler, singletonMap("BESU_CONFIG_FILE", "not_found.toml")); + + when(mockParseResult.hasMatchedOption(CONFIG_FILE_OPTION_NAME)).thenReturn(false); + + environmentConfigFileParsingHandler.handle(mockParseResult); + } + @Test public void selfMustReturnTheHandler() { assertThat(configParsingHandler.self()).isSameAs(configParsingHandler); @@ -127,4 +154,20 @@ public class ConfigOptionSearchAndRunHandlerTest { final String value = defaultValueProvider.defaultValue(OptionSpec.builder("--logging").build()); assertThat(value).isEqualTo("ERROR"); } + + @Test + public void handleThrowsErrorWithWithEnvironmentVariableAndCommandLineSpecified() + throws IOException { + exceptionRule.expect(CommandLine.ParameterException.class); + + final ConfigOptionSearchAndRunHandler environmentConfigFileParsingHandler = + new ConfigOptionSearchAndRunHandler( + resultHandler, + exceptionHandler, + singletonMap("BESU_CONFIG_FILE", temp.newFile().getAbsolutePath())); + + when(mockParseResult.hasMatchedOption(CONFIG_FILE_OPTION_NAME)).thenReturn(true); + + environmentConfigFileParsingHandler.handle(mockParseResult); + } }