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 <git@antonydenyer.co.uk>

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
 

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()

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<List<Object>> {
   private final AbstractParseResultHandler<List<Object>> resultHandler;
   private final CommandLine.IExceptionHandler2<List<Object>> exceptionHandler;
-  private final String configFileOptionName;
   private final Map<String, String> environment;
 
   public ConfigOptionSearchAndRunHandler(
       final AbstractParseResultHandler<List<Object>> resultHandler,
       final CommandLine.IExceptionHandler2<List<Object>> exceptionHandler,
-      final String configFileOptionName,
       final Map<String, String> 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<Object> handle(final ParseResult parseResult) throws ExecutionException {
+  public List<Object> handle(final ParseResult parseResult) throws ParameterException {
     final CommandLine commandLine = parseResult.asCommandLineList().get(0);
     final Optional<File> configFile = findConfigFile(parseResult, commandLine);
     commandLine.setDefaultValueProvider(createDefaultValueProvider(commandLine, configFile));
@@ -60,13 +57,31 @@ public class ConfigOptionSearchAndRunHandler extends AbstractParseResultHandler<
 
   private Optional<File> 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();

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<List<Object>>().useErr(errPrintStream).useAnsi(Ansi.OFF);
   private final Map<String, String> 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<Object> 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);
+  }
 }