diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java b/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java index 67593fb7d1..2f0ffc4fee 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java @@ -18,26 +18,26 @@ import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; import java.io.File; +import java.io.FileNotFoundException; import java.net.URI; -import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; import com.google.common.io.Resources; import net.consensys.cava.toml.TomlArray; import net.consensys.cava.toml.TomlParseResult; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; public class PermissioningConfigurationBuilder { - private static final Logger LOG = LogManager.getLogger(); + public static final String ACCOUNTS_WHITELIST = "accounts-whitelist"; + public static final String NODES_WHITELIST = "nodes-whitelist"; // will be used to reload the config from a file while node is running public static PermissioningConfiguration permissioningConfigurationFromToml( final String configFilePath, final boolean permissionedNodeEnabled, - final boolean permissionedAccountEnabled) { + final boolean permissionedAccountEnabled) + throws Exception { return permissioningConfiguration( configFilePath, permissionedNodeEnabled, permissionedAccountEnabled); @@ -46,14 +46,23 @@ public static PermissioningConfiguration permissioningConfigurationFromToml( public static PermissioningConfiguration permissioningConfiguration( final String configFilePath, final boolean permissionedNodeEnabled, - final boolean permissionedAccountEnabled) { + final boolean permissionedAccountEnabled) + throws Exception { - TomlParseResult permToml = - TomlConfigFileParser.loadConfiguration( - permissioningConfigTomlAsString(permissioningConfigFile(configFilePath))); + TomlParseResult permToml; + boolean foundValidOptions = false; - TomlArray accountWhitelistTomlArray = permToml.getArray("accounts-whitelist"); - TomlArray nodeWhitelistTomlArray = permToml.getArray("nodes-whitelist"); + try { + permToml = + TomlConfigFileParser.loadConfiguration( + permissioningConfigTomlAsString(permissioningConfigFile(configFilePath))); + } catch (Exception e) { + throw new Exception( + "Unable to read permissions TOML config file : " + configFilePath + " " + e.getMessage()); + } + + TomlArray accountWhitelistTomlArray = permToml.getArray(ACCOUNTS_WHITELIST); + TomlArray nodeWhitelistTomlArray = permToml.getArray(NODES_WHITELIST); final PermissioningConfiguration permissioningConfiguration = PermissioningConfiguration.createDefault(); @@ -68,7 +77,8 @@ public static PermissioningConfiguration permissioningConfiguration( .collect(Collectors.toList()); permissioningConfiguration.setAccountWhitelist(accountsWhitelistToml); } else { - permissioningConfiguration.setAccountWhitelist(new ArrayList<>()); + throw new Exception( + ACCOUNTS_WHITELIST + " config option missing in TOML config file " + configFilePath); } } if (permissionedNodeEnabled) { @@ -82,29 +92,25 @@ public static PermissioningConfiguration permissioningConfiguration( .collect(Collectors.toList()); permissioningConfiguration.setNodeWhitelist(nodesWhitelistToml); } else { - permissioningConfiguration.setNodeWhitelist(new ArrayList<>()); + throw new Exception( + NODES_WHITELIST + " config option missing in TOML config file " + configFilePath); } } return permissioningConfiguration; } - private static String permissioningConfigTomlAsString(final File file) { - try { - return Resources.toString(file.toURI().toURL(), UTF_8); - } catch (final Exception e) { - LOG.error("Unable to load permissioning config {}.", file, e); - return null; - } + private static String permissioningConfigTomlAsString(final File file) throws Exception { + return Resources.toString(file.toURI().toURL(), UTF_8); } - private static File permissioningConfigFile(final String filename) { + private static File permissioningConfigFile(final String filename) throws FileNotFoundException { final File permissioningConfigFile = new File(filename); if (permissioningConfigFile.exists()) { return permissioningConfigFile; } else { - LOG.error("File does not exist: permissioning config path: {}.", filename); - return null; + throw new FileNotFoundException( + "File does not exist: permissioning config path: " + filename); } } } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java b/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java index 65e3dd8f83..b24d84f288 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java @@ -17,29 +17,25 @@ import net.consensys.cava.toml.Toml; import net.consensys.cava.toml.TomlParseError; import net.consensys.cava.toml.TomlParseResult; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; public class TomlConfigFileParser { - private static final Logger LOG = LogManager.getLogger(); - private static TomlParseResult checkConfigurationValidity( - final TomlParseResult result, final String toml) { + final TomlParseResult result, final String toml) throws Exception { if (result == null || result.isEmpty()) { - LOG.error("Unable to read TOML configuration file %s", toml); + throw new Exception("Empty TOML result: " + toml); } return result; } - public static TomlParseResult loadConfiguration(final String toml) { + public static TomlParseResult loadConfiguration(final String toml) throws Exception { TomlParseResult result = Toml.parse(toml); if (result.hasErrors()) { final String errors = result.errors().stream().map(TomlParseError::toString).collect(Collectors.joining("%n")); ; - LOG.error("Invalid TOML configuration : %s", errors); + throw new Exception("Invalid TOML configuration : " + errors); } return checkConfigurationValidity(result, toml); diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index f2e88e1d9b..cec2ffd1ad 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -583,23 +583,27 @@ public void run() { } final EthNetworkConfig ethNetworkConfig = updateNetworkConfig(getNetwork()); - final Optional permissioningConfiguration = - permissioningConfiguration(); - permissioningConfiguration.ifPresent( - p -> ensureAllBootnodesAreInWhitelist(ethNetworkConfig, p)); - - synchronize( - buildController(), - p2pEnabled, - peerDiscoveryEnabled, - ethNetworkConfig.getBootNodes(), - maxPeers, - p2pHost, - p2pPort, - jsonRpcConfiguration(), - webSocketConfiguration(), - metricsConfiguration(), - permissioningConfiguration); + try { + final Optional permissioningConfiguration = + permissioningConfiguration(); + permissioningConfiguration.ifPresent( + p -> ensureAllBootnodesAreInWhitelist(ethNetworkConfig, p)); + + synchronize( + buildController(), + p2pEnabled, + peerDiscoveryEnabled, + ethNetworkConfig.getBootNodes(), + maxPeers, + p2pHost, + p2pPort, + jsonRpcConfiguration(), + webSocketConfiguration(), + metricsConfiguration(), + permissioningConfiguration); + } catch (Exception e) { + throw new ParameterException(new CommandLine(this), e.getMessage()); + } } private NetworkName getNetwork() { @@ -734,7 +738,7 @@ MetricsConfiguration metricsConfiguration() { return metricsConfiguration; } - private Optional permissioningConfiguration() { + private Optional permissioningConfiguration() throws Exception { if (!permissionsAccountsEnabled && !permissionsNodesEnabled) { return Optional.empty(); diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java index f4f8124225..600040e420 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java @@ -17,7 +17,6 @@ import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; -import java.io.IOException; import java.net.URI; import java.net.URL; import java.nio.file.Files; @@ -28,7 +27,7 @@ public class PermissioningConfigurationBuilderTest { - static final String PERMISSIONING_CONFIG_TOML = "permissioning_config.toml"; + static final String PERMISSIONING_CONFIG_VALID = "permissioning_config.toml"; static final String PERMISSIONING_CONFIG_ACCOUNT_WHITELIST_ONLY = "permissioning_config_account_whitelist_only.toml"; static final String PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY = @@ -39,17 +38,19 @@ public class PermissioningConfigurationBuilderTest { "permissioning_config_empty_whitelists.toml"; static final String PERMISSIONING_CONFIG_ABSENT_WHITELISTS = "permissioning_config_absent_whitelists.toml"; + static final String PERMISSIONING_CONFIG_UNRECOGNIZED_KEY = + "permissioning_config_unrecognized_key.toml"; private final String VALID_NODE_ID = "6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"; @Test - public void permissioningConfig() throws IOException { + public void permissioningConfig() throws Exception { final String uri = "enode://" + VALID_NODE_ID + "@192.168.0.9:4567"; final String uri2 = "enode://" + VALID_NODE_ID + "@192.169.0.9:4568"; - final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_TOML); + final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_VALID); final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); @@ -66,7 +67,7 @@ public void permissioningConfig() throws IOException { } @Test - public void permissioningConfigWithOnlyNodeWhitelistSet() throws IOException { + public void permissioningConfigWithOnlyNodeWhitelistSet() throws Exception { final String uri = "enode://" + VALID_NODE_ID + "@192.168.0.9:4567"; @@ -84,7 +85,7 @@ public void permissioningConfigWithOnlyNodeWhitelistSet() throws IOException { } @Test - public void permissioningConfigWithOnlyAccountWhitelistSet() throws IOException { + public void permissioningConfigWithOnlyAccountWhitelistSet() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ACCOUNT_WHITELIST_ONLY); final Path toml = Files.createTempFile("toml", ""); @@ -101,7 +102,7 @@ public void permissioningConfigWithOnlyAccountWhitelistSet() throws IOException } @Test - public void permissioningConfigWithInvalidEnode() throws IOException { + public void permissioningConfigWithInvalidEnode() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_INVALID_ENODE); final Path toml = Files.createTempFile("toml", ""); @@ -117,7 +118,7 @@ public void permissioningConfigWithInvalidEnode() throws IOException { } @Test - public void permissioningConfigWithEmptyWhitelistMustNotError() throws IOException { + public void permissioningConfigWithEmptyWhitelistMustNotError() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_EMPTY_WHITELISTS); final Path toml = Files.createTempFile("toml", ""); @@ -134,26 +135,59 @@ public void permissioningConfigWithEmptyWhitelistMustNotError() throws IOExcepti } @Test - public void permissioningConfigWithAbsentWhitelistMustSetValues() throws IOException { + public void permissioningConfigWithAbsentWhitelistMustThrowException() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ABSENT_WHITELISTS); final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); - PermissioningConfiguration permissioningConfiguration = - PermissioningConfigurationBuilder.permissioningConfiguration( - toml.toAbsolutePath().toString(), true, true); + try { + PermissioningConfigurationBuilder.permissioningConfiguration( + toml.toAbsolutePath().toString(), true, true); + fail("expected exception: no valid whitelists in the TOML file"); + } catch (Exception e) { + assertThat(e.getMessage().contains("Unexpected end of line")).isTrue(); + } + } - assertThat(permissioningConfiguration.isNodeWhitelistEnabled()).isTrue(); - assertThat(permissioningConfiguration.getNodeWhitelist()).isEmpty(); - assertThat(permissioningConfiguration.isAccountWhitelistEnabled()).isTrue(); - assertThat(permissioningConfiguration.getAccountWhitelist()).isEmpty(); + @Test + public void permissioningConfigWithUnrecognizedKeyMustThrowException() throws Exception { + + final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_UNRECOGNIZED_KEY); + final Path toml = Files.createTempFile("toml", ""); + Files.write(toml, Resources.toByteArray(configFile)); + + try { + PermissioningConfigurationBuilder.permissioningConfiguration( + toml.toAbsolutePath().toString(), true, true); + fail("expected exception: didn't find a recognized key in the TOML file"); + } catch (Exception e) { + assertThat(e.getMessage().contains("config option missing")).isTrue(); + assertThat(e.getMessage().contains(PermissioningConfigurationBuilder.ACCOUNTS_WHITELIST)) + .isTrue(); + } } @Test - public void permissioningConfigFromFileMustSetFilePath() throws IOException { + public void permissioningConfigWithEmptyFileMustThrowException() throws Exception { - final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ABSENT_WHITELISTS); + // write an empty file + final Path toml = Files.createTempFile("toml", ""); + + try { + PermissioningConfigurationBuilder.permissioningConfiguration( + toml.toAbsolutePath().toString(), true, true); + fail("expected exception: empty TOML file"); + + } catch (Exception e) { + assertThat(e.getMessage().contains("Empty TOML result")).isTrue(); + } + } + + @Test + public void permissioningConfigFromFileMustSetFilePath() throws Exception { + + final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_VALID); final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); @@ -162,10 +196,17 @@ public void permissioningConfigFromFileMustSetFilePath() throws IOException { toml.toString(), true, true); assertThat(permissioningConfiguration.getConfigurationFilePath()).isEqualTo(toml.toString()); + } - assertThat(permissioningConfiguration.isNodeWhitelistEnabled()).isTrue(); - assertThat(permissioningConfiguration.getNodeWhitelist()).isEmpty(); - assertThat(permissioningConfiguration.isAccountWhitelistEnabled()).isTrue(); - assertThat(permissioningConfiguration.getAccountWhitelist()).isEmpty(); + @Test + public void permissioningConfigFromNonexistentFileMustThrowException() { + + try { + PermissioningConfigurationBuilder.permissioningConfigurationFromToml( + "file-does-not-exist", true, true); + fail("expected exception: file does not exist"); + } catch (Exception e) { + assertThat(e.getMessage().contains("File does not exist")).isTrue(); + } } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index bf1f77e87b..75125d22cf 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -320,6 +320,17 @@ public void permissionsTomlPathWithoutOptionMustDisplayUsage() { assertThat(commandOutput.toString()).isEmpty(); } + @Test + public void permissionsEnabledWithNonexistentConfigFileMustError() { + parseCommand( + "--permissions-accounts-enabled", "--permissions-config-path", "file-does-not-exist"); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString()).contains("File does not exist"); + assertThat(commandOutput.toString()).isEmpty(); + } + @Test public void permissionsTomlPathMustUseOption() throws IOException { diff --git a/pantheon/src/test/resources/permissioning_config_unrecognized_key.toml b/pantheon/src/test/resources/permissioning_config_unrecognized_key.toml new file mode 100644 index 0000000000..aa54b4ab70 --- /dev/null +++ b/pantheon/src/test/resources/permissioning_config_unrecognized_key.toml @@ -0,0 +1,3 @@ +# Permissioning TOML file with typo in key + +perm-node-whitelist=[] \ No newline at end of file