From 8d4048b98802cfc0b018d706c12dc0236c637073 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Mon, 7 Jan 2019 19:45:38 -0700 Subject: [PATCH] Fix Array Configurable CLI options (#514) * Fix Array Configurable CLI options --host-whitelist and --rpc-cors-origins could not be configured as a TOML array. The underlying PicoCLI issues were resolved with revamped property types that act like Collections. A "configure everything" test is added that creates a TOML file that requires all CLI options to be configurable and configured in a new everything_config.toml test file. --- .../pegasys/pantheon/cli/PantheonCommand.java | 15 +- .../custom/CorsAllowedOriginsProperty.java | 86 ++++--- .../custom/JsonRPCWhitelistHostsProperty.java | 79 +++--- .../pantheon/cli/CommandTestAbstract.java | 21 +- .../pantheon/cli/PantheonCommandTest.java | 228 ++++++++++++++++-- .../src/test/resources/everything_config.toml | 63 +++++ 6 files changed, 390 insertions(+), 102 deletions(-) create mode 100644 pantheon/src/test/resources/everything_config.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 533ea44730..064cf82411 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -277,8 +277,7 @@ public static class RpcApisConversionException extends Exception { // A list of origins URLs that are accepted by the JsonRpcHttpServer (CORS) @Option( names = {"--rpc-cors-origins"}, - description = "Comma separated origin domain URLs for CORS validation (default: none)", - converter = CorsAllowedOriginsProperty.CorsAllowedOriginsPropertyConverter.class + description = "Comma separated origin domain URLs for CORS validation (default: none)" ) private final CorsAllowedOriginsProperty rpcCorsAllowedOrigins = new CorsAllowedOriginsProperty(); @@ -331,7 +330,7 @@ public static class RpcApisConversionException extends Exception { + "default: ${DEFAULT-VALUE}", defaultValue = "" + WebSocketConfiguration.DEFAULT_WEBSOCKET_REFRESH_DELAY ) - private void setRefreshDelay(final Long refreshDelay) { + private Long configureRefreshDelay(final Long refreshDelay) { if (refreshDelay < DEFAULT_MIN_REFRESH_DELAY || refreshDelay > DEFAULT_MAX_REFRESH_DELAY) { throw new ParameterException( new CommandLine(this), @@ -341,6 +340,7 @@ private void setRefreshDelay(final Long refreshDelay) { String.valueOf(DEFAULT_MAX_REFRESH_DELAY))); } this.refreshDelay = refreshDelay; + return refreshDelay; } @Option( @@ -363,8 +363,7 @@ private void setRefreshDelay(final Long refreshDelay) { paramLabel = "", description = "Comma separated list of hostnames to whitelist for RPC access. default: ${DEFAULT-VALUE}", - defaultValue = "localhost", - converter = JsonRPCWhitelistHostsProperty.JsonRPCWhitelistHostsConverter.class + defaultValue = "localhost" ) private final JsonRPCWhitelistHostsProperty hostsWhitelist = new JsonRPCWhitelistHostsProperty(); @@ -579,9 +578,9 @@ private JsonRpcConfiguration jsonRpcConfiguration() { jsonRpcConfiguration.setEnabled(isJsonRpcEnabled); jsonRpcConfiguration.setHost(rpcHostAndPort.getHost()); jsonRpcConfiguration.setPort(rpcHostAndPort.getPort()); - jsonRpcConfiguration.setCorsAllowedDomains(rpcCorsAllowedOrigins.getDomains()); + jsonRpcConfiguration.setCorsAllowedDomains(rpcCorsAllowedOrigins); jsonRpcConfiguration.setRpcApis(rpcApis); - jsonRpcConfiguration.setHostsWhitelist(hostsWhitelist.hostnamesWhitelist()); + jsonRpcConfiguration.setHostsWhitelist(hostsWhitelist); return jsonRpcConfiguration; } @@ -600,7 +599,7 @@ MetricsConfiguration metricsConfiguration() { metricsConfiguration.setEnabled(isMetricsEnabled); metricsConfiguration.setHost(metricsHostAndPort.getHost()); metricsConfiguration.setPort(metricsHostAndPort.getPort()); - metricsConfiguration.setHostsWhitelist(hostsWhitelist.hostnamesWhitelist()); + metricsConfiguration.setHostsWhitelist(hostsWhitelist); return metricsConfiguration; } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/CorsAllowedOriginsProperty.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/CorsAllowedOriginsProperty.java index f81008c1f6..ea3d934e79 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/CorsAllowedOriginsProperty.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/CorsAllowedOriginsProperty.java @@ -12,72 +12,80 @@ */ package tech.pegasys.pantheon.cli.custom; +import java.util.AbstractCollection; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.Iterator; import java.util.List; import java.util.StringJoiner; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; +import javax.annotation.Nonnull; -import com.google.common.collect.Lists; -import picocli.CommandLine.ITypeConverter; +import com.google.common.base.Splitter; +import com.google.common.base.Strings; -public class CorsAllowedOriginsProperty { +public class CorsAllowedOriginsProperty extends AbstractCollection { - private List domains = Collections.emptyList(); - - public CorsAllowedOriginsProperty(final List domains) { - this.domains = domains; - } + private final List domains = new ArrayList<>(); public CorsAllowedOriginsProperty() {} - public List getDomains() { - return domains; + @Override + @Nonnull + public Iterator iterator() { + if (domains.size() == 1 && domains.get(0).equals("none")) { + return Collections.emptyIterator(); + } else { + return domains.iterator(); + } } - public static class CorsAllowedOriginsPropertyConverter - implements ITypeConverter { + @Override + public int size() { + return domains.size(); + } - @Override - public CorsAllowedOriginsProperty convert(final String value) throws IllegalArgumentException { - final List domains; - if (value != null && !value.isEmpty()) { - domains = new ArrayList<>(Arrays.asList(value.split("\\s*,\\s*"))); - } else { - throw new IllegalArgumentException("Property can't be null/empty string"); - } + @Override + public boolean add(final String string) { + return addAll(Collections.singleton(string)); + } - if (domains.contains("none")) { - if (domains.size() > 1) { - throw new IllegalArgumentException("Value 'none' can't be used with other domains"); - } else { - return new CorsAllowedOriginsProperty(Collections.emptyList()); - } + @Override + public boolean addAll(final Collection collection) { + final int initialSize = domains.size(); + for (final String string : collection) { + if (Strings.isNullOrEmpty(string)) { + throw new IllegalArgumentException("Domain cannot be empty string or null string."); } - - if (domains.contains("all") || domains.contains("*")) { - if (domains.size() > 1) { - throw new IllegalArgumentException("Value 'all' can't be used with other domains"); + for (final String s : Splitter.onPattern("\\s*,+\\s*").split(string)) { + if ("all".equals(s)) { + domains.add("*"); } else { - return new CorsAllowedOriginsProperty(Lists.newArrayList("*")); + domains.add(s); } } + } + if (domains.contains("none")) { + if (domains.size() > 1) { + throw new IllegalArgumentException("Value 'none' can't be used with other domains"); + } + } else if (domains.contains("*")) { + if (domains.size() > 1) { + throw new IllegalArgumentException("Values '*' or 'all' can't be used with other domains"); + } + } else { try { final StringJoiner stringJoiner = new StringJoiner("|"); - domains.stream().filter(s -> !s.isEmpty()).forEach(stringJoiner::add); + domains.forEach(stringJoiner::add); Pattern.compile(stringJoiner.toString()); } catch (final PatternSyntaxException e) { throw new IllegalArgumentException("Domain values result in invalid regex pattern", e); } - - if (domains.size() > 0) { - return new CorsAllowedOriginsProperty(domains); - } else { - return new CorsAllowedOriginsProperty(); - } } + + return domains.size() != initialSize; } } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/JsonRPCWhitelistHostsProperty.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/JsonRPCWhitelistHostsProperty.java index bab8e33ce6..fdd39b057a 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/JsonRPCWhitelistHostsProperty.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/JsonRPCWhitelistHostsProperty.java @@ -12,59 +12,70 @@ */ package tech.pegasys.pantheon.cli.custom; +import java.util.AbstractCollection; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.Iterator; import java.util.List; +import javax.annotation.Nonnull; -import picocli.CommandLine; +import com.google.common.base.Splitter; +import com.google.common.base.Strings; -public class JsonRPCWhitelistHostsProperty { - private List hostnamesWhitelist = Collections.emptyList(); +public class JsonRPCWhitelistHostsProperty extends AbstractCollection { - private JsonRPCWhitelistHostsProperty(final List hostnamesWhitelist) { - this.hostnamesWhitelist = hostnamesWhitelist; - } + private final List hostnamesWhitelist = new ArrayList<>(); public JsonRPCWhitelistHostsProperty() {} - public List hostnamesWhitelist() { - return hostnamesWhitelist; + @Override + @Nonnull + public Iterator iterator() { + if (hostnamesWhitelist.size() == 1 && hostnamesWhitelist.get(0).equals("none")) { + return Collections.emptyIterator(); + } else { + return hostnamesWhitelist.iterator(); + } } - public static class JsonRPCWhitelistHostsConverter - implements CommandLine.ITypeConverter { + @Override + public int size() { + return hostnamesWhitelist.size(); + } - @Override - public JsonRPCWhitelistHostsProperty convert(final String value) { - final List hostNames; - if (value != null && !value.isEmpty()) { - hostNames = new ArrayList<>(Arrays.asList(value.split("\\s*,\\s*"))); - } else { - throw new IllegalArgumentException("Property can't be null/empty string"); - } + @Override + public boolean add(final String string) { + return addAll(Collections.singleton(string)); + } - if (hostNames.contains("all")) { - if (hostNames.size() > 1) { - throw new IllegalArgumentException("Value 'all' can't be used with other hostnames"); - } else { - return new JsonRPCWhitelistHostsProperty(Collections.singletonList("*")); - } + @Override + public boolean addAll(final Collection collection) { + final int initialSize = hostnamesWhitelist.size(); + for (final String string : collection) { + if (Strings.isNullOrEmpty(string)) { + throw new IllegalArgumentException("Hostname cannot be empty string or null string."); } - - if (hostNames.contains("*")) { - if (hostNames.size() > 1) { - throw new IllegalArgumentException("Value '*' can't be used with other hostnames"); + for (final String s : Splitter.onPattern("\\s*,+\\s*").split(string)) { + if ("all".equals(s)) { + hostnamesWhitelist.add("*"); } else { - return new JsonRPCWhitelistHostsProperty(Collections.singletonList("*")); + hostnamesWhitelist.add(s); } } + } - if (hostNames.size() > 0) { - return new JsonRPCWhitelistHostsProperty(hostNames); - } else { - return new JsonRPCWhitelistHostsProperty(); + if (hostnamesWhitelist.contains("none")) { + if (hostnamesWhitelist.size() > 1) { + throw new IllegalArgumentException("Value 'none' can't be used with other hostnames"); + } + } else if (hostnamesWhitelist.contains("*")) { + if (hostnamesWhitelist.size() > 1) { + throw new IllegalArgumentException( + "Values '*' or 'all' can't be used with other hostnames"); } } + + return hostnamesWhitelist.size() != initialSize; } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java index 9389118f6a..1b7cf3a35f 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java @@ -44,6 +44,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import picocli.CommandLine; import picocli.CommandLine.DefaultExceptionHandler; import picocli.CommandLine.Help.Ansi; import picocli.CommandLine.RunLast; @@ -101,10 +102,10 @@ public void displayOutput() { LOG.info("Standard error {}", commandErrorOutput.toString()); } - void parseCommand(final String... args) { + CommandLine.Model.CommandSpec parseCommand(final String... args) { - final PantheonCommand pantheonCommand = - new PantheonCommand( + final TestPantheonCommand pantheonCommand = + new TestPantheonCommand( mockBlockImporter, mockRunnerBuilder, mockControllerBuilder, mockSyncConfBuilder); // parse using Ansi.OFF to be able to assert on non formatted output results @@ -112,5 +113,19 @@ void parseCommand(final String... args) { new RunLast().useOut(outPrintStream).useAnsi(Ansi.OFF), new DefaultExceptionHandler>().useErr(errPrintStream).useAnsi(Ansi.OFF), args); + return pantheonCommand.spec; + } + + @CommandLine.Command + static class TestPantheonCommand extends PantheonCommand { + @CommandLine.Spec CommandLine.Model.CommandSpec spec; + + TestPantheonCommand( + final BlockImporter mockBlockImporter, + final RunnerBuilder mockRunnerBuilder, + final PantheonControllerBuilder mockControllerBuilder, + final SynchronizerConfiguration.Builder mockSyncConfBuilder) { + super(mockBlockImporter, mockRunnerBuilder, mockControllerBuilder, mockSyncConfBuilder); + } } } 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 276e38aae5..d66b95f154 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -50,13 +50,15 @@ import java.nio.file.Paths; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; -import com.google.common.collect.Lists; import com.google.common.io.Resources; +import net.consensys.cava.toml.Toml; +import net.consensys.cava.toml.TomlParseResult; import org.apache.commons.text.StringEscapeUtils; import org.junit.Before; import org.junit.Ignore; @@ -65,6 +67,7 @@ import org.junit.rules.TemporaryFolder; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatchers; +import picocli.CommandLine; public class PantheonCommandTest extends CommandTestAbstract { private final String VALID_NODE_ID = @@ -76,13 +79,13 @@ public class PantheonCommandTest extends CommandTestAbstract { private static final MetricsConfiguration defaultMetricsConfiguration; private static final String GENESIS_CONFIG_TESTDATA = "genesis_config"; - final String[] validENodeStrings = { + private final String[] validENodeStrings = { "enode://" + VALID_NODE_ID + "@192.168.0.1:4567", "enode://" + VALID_NODE_ID + "@192.168.0.2:4567", "enode://" + VALID_NODE_ID + "@192.168.0.3:4567" }; - final String[] ropstenBootnodes = { + private final String[] ropstenBootnodes = { "enode://6332792c4a00e3e4ee0926ed89e0d27ef985424d97b6a45bf0f23e51f0dcb5e66b875777506458aea7af6f9e4ffb69f43f3778ee73c81ed9d34c51c4b16b0b0f@52.232.243.152:30303", "enode://94c15d1b9e2fe7ce56e458b9a3b672ef11894ddedd0c6f247e0f1d3487f52b66208fb4aeb8179fce6e3a749ea93ed147c37976d67af557508d199d9594c35f09@192.81.208.223:30303" }; @@ -98,8 +101,7 @@ public class PantheonCommandTest extends CommandTestAbstract { websocketConf.addRpcApi(IbftRpcApis.IBFT); defaultWebSocketConfiguration = websocketConf; - final MetricsConfiguration metricsConf = MetricsConfiguration.createDefault(); - defaultMetricsConfiguration = metricsConf; + defaultMetricsConfiguration = MetricsConfiguration.createDefault(); } @Before @@ -339,6 +341,47 @@ public void overrideDefaultValuesIfKeyIsPresentInConfigFile() throws IOException assertThat(commandErrorOutput.toString()).isEmpty(); } + @Test + public void tomlThatConfiguresEverything() throws IOException { + assumeTrue(isFullInstantiation()); + + // Load a TOML that configures literally everything + final URL configFile = Resources.getResource("everything_config.toml"); + final Path toml = Files.createTempFile("toml", ""); + Files.write(toml, Resources.toByteArray(configFile)); + + // Parse it. + final CommandLine.Model.CommandSpec spec = parseCommand("--config", toml.toString()); + final TomlParseResult tomlResult = Toml.parse(toml); + + // Verify we configured everything + final HashSet options = new HashSet<>(spec.options()); + + // Except for meta-options + options.remove(spec.optionsMap().get("--config")); + options.remove(spec.optionsMap().get("--help")); + options.remove(spec.optionsMap().get("--version")); + + for (final String tomlKey : tomlResult.keySet()) { + final CommandLine.Model.OptionSpec optionSpec = spec.optionsMap().get("--" + tomlKey); + assertThat(optionSpec) + .describedAs("Option '%s' should be a configurable option.", tomlKey) + .isNotNull(); + // Verify TOML stores it by the appropriate type + if (optionSpec.type().equals(Boolean.class)) { + tomlResult.getBoolean(tomlKey); + } else if (optionSpec.isMultiValue() || optionSpec.arity().max > 1) { + tomlResult.getArray(tomlKey); + } else if (Number.class.isAssignableFrom(optionSpec.type())) { + tomlResult.getLong(tomlKey); + } else { + tomlResult.getString(tomlKey); + } + options.remove(optionSpec); + } + assertThat(options.stream().map(CommandLine.Model.OptionSpec::longestName)).isEmpty(); + } + @Test public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() throws IOException { assumeTrue(isFullInstantiation()); @@ -823,6 +866,21 @@ public void jsonRpcCorsOriginsTwoDomainsMustBuildListWithBothDomains() { assertThat(commandErrorOutput.toString()).isEmpty(); } + @Test + public void jsonRpcCorsOriginsDoubleCommaFilteredOut() { + final String[] origins = {"http://domain1.com", "https://domain2.com"}; + parseCommand("--rpc-cors-origins", String.join(",,", origins)); + + verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); + verify(mockRunnerBuilder).build(); + + assertThat(jsonRpcConfigArgumentCaptor.getValue().getCorsAllowedDomains().toArray()) + .isEqualTo(origins); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + @Test public void jsonRpcCorsOriginsWithWildcardMustBuildListWithWildcard() { final String[] origins = {"*"}; @@ -840,14 +898,12 @@ public void jsonRpcCorsOriginsWithWildcardMustBuildListWithWildcard() { @Test public void jsonRpcCorsOriginsWithAllMustBuildListWithWildcard() { - final String[] origins = {"all"}; - parseCommand("--rpc-cors-origins", String.join(",", origins)); + parseCommand("--rpc-cors-origins", "all"); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); - assertThat(jsonRpcConfigArgumentCaptor.getValue().getCorsAllowedDomains()) - .isEqualTo(Lists.newArrayList("*")); + assertThat(jsonRpcConfigArgumentCaptor.getValue().getCorsAllowedDomains()).containsExactly("*"); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); @@ -880,27 +936,59 @@ public void jsonRpcCorsOriginsNoneWithAnotherDomainMustFail() { } @Test - public void jsonRpcCorsOriginsAllWithAnotherDomainMustFail() { - final String[] origins = {"http://domain1.com", "all"}; + public void jsonRpcCorsOriginsNoneWithAnotherDomainMustFailNoneFirst() { + final String[] origins = {"none", "http://domain1.com"}; parseCommand("--rpc-cors-origins", String.join(",", origins)); verifyZeroInteractions(mockRunnerBuilder); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()) - .contains("Value 'all' can't be used with other domains"); + .contains("Value 'none' can't be used with other domains"); + } + + @Test + public void jsonRpcCorsOriginsAllWithAnotherDomainMustFail() { + parseCommand("--rpc-cors-origins=http://domain1.com,all"); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()) + .contains("Values '*' or 'all' can't be used with other domains"); + } + + @Test + public void jsonRpcCorsOriginsAllWithAnotherDomainMustFailAsFlags() { + parseCommand("--rpc-cors-origins=http://domain1.com", "--rpc-cors-origins=all"); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()) + .contains("Values '*' or 'all' can't be used with other domains"); } @Test public void jsonRpcCorsOriginsWildcardWithAnotherDomainMustFail() { - final String[] origins = {"http://domain1.com", "*"}; - parseCommand("--rpc-cors-origins", String.join(",", origins)); + parseCommand("--rpc-cors-origins=http://domain1.com,*"); verifyZeroInteractions(mockRunnerBuilder); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()) - .contains("Value 'all' can't be used with other domains"); + .contains("Values '*' or 'all' can't be used with other domains"); + } + + @Test + public void jsonRpcCorsOriginsWildcardWithAnotherDomainMustFailAsFlags() { + parseCommand("--rpc-cors-origins=http://domain1.com", "--rpc-cors-origins=*"); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()) + .contains("Values '*' or 'all' can't be used with other domains"); } @Test @@ -915,6 +1003,17 @@ public void jsonRpcCorsOriginsInvalidRegexShouldFail() { .contains("Domain values result in invalid regex pattern"); } + @Test + public void jsonRpcCorsOriginsEmtyValueFails() { + parseCommand("--rpc-cors-origins="); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()) + .contains("Domain cannot be empty string or null string."); + } + @Test public void jsonRpcHostWhitelistAcceptsSingleArgument() { parseCommand("--host-whitelist", "a"); @@ -947,6 +1046,38 @@ public void jsonRpcHostWhitelistAcceptsMultipleArguments() { assertThat(commandErrorOutput.toString()).isEmpty(); } + @Test + public void jsonRpcHostWhitelistAcceptsDoubleComma() { + parseCommand("--host-whitelist", "a,,b"); + + verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); + verify(mockRunnerBuilder).build(); + + assertThat(jsonRpcConfigArgumentCaptor.getValue().getHostsWhitelist().size()).isEqualTo(2); + assertThat(jsonRpcConfigArgumentCaptor.getValue().getHostsWhitelist()).contains("a", "b"); + assertThat(jsonRpcConfigArgumentCaptor.getValue().getHostsWhitelist()) + .doesNotContain("*", "localhost"); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + + @Test + public void jsonRpcHostWhitelistAcceptsMultipleFlags() { + parseCommand("--host-whitelist=a", "--host-whitelist=b"); + + verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); + verify(mockRunnerBuilder).build(); + + assertThat(jsonRpcConfigArgumentCaptor.getValue().getHostsWhitelist().size()).isEqualTo(2); + assertThat(jsonRpcConfigArgumentCaptor.getValue().getHostsWhitelist()).contains("a", "b"); + assertThat(jsonRpcConfigArgumentCaptor.getValue().getHostsWhitelist()) + .doesNotContain("*", "localhost"); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + @Test public void jsonRpcHostWhitelistStarWithAnotherHostnameMustFail() { final String[] origins = {"friend", "*"}; @@ -956,7 +1087,19 @@ public void jsonRpcHostWhitelistStarWithAnotherHostnameMustFail() { assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()) - .contains("Value '*' can't be used with other hostnames"); + .contains("Values '*' or 'all' can't be used with other hostnames"); + } + + @Test + public void jsonRpcHostWhitelistStarWithAnotherHostnameMustFailStarFirst() { + final String[] origins = {"*", "friend"}; + parseCommand("--host-whitelist", String.join(",", origins)); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()) + .contains("Values '*' or 'all' can't be used with other hostnames"); } @Test @@ -968,7 +1111,56 @@ public void jsonRpcHostWhitelistAllWithAnotherHostnameMustFail() { assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()) - .contains("Value 'all' can't be used with other hostnames"); + .contains("Values '*' or 'all' can't be used with other hostnames"); + } + + @Test + public void jsonRpcHostWhitelistWithNoneMustBuildEmptyList() { + final String[] origins = {"none"}; + parseCommand("--host-whitelist", String.join(",", origins)); + + verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); + verify(mockRunnerBuilder).build(); + + assertThat(jsonRpcConfigArgumentCaptor.getValue().getHostsWhitelist()).isEmpty(); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + + @Test + public void jsonRpcHostWhitelistNoneWithAnotherDomainMustFail() { + final String[] origins = {"http://domain1.com", "none"}; + parseCommand("--host-whitelist", String.join(",", origins)); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()) + .contains("Value 'none' can't be used with other hostnames"); + } + + @Test + public void jsonRpcHostWhitelistNoneWithAnotherDomainMustFailNoneFirst() { + final String[] origins = {"none", "http://domain1.com"}; + parseCommand("--host-whitelist", String.join(",", origins)); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()) + .contains("Value 'none' can't be used with other hostnames"); + } + + @Test + public void jsonRpcHostWhitelistEmptyValueFails() { + parseCommand("--host-whitelist="); + + verifyZeroInteractions(mockRunnerBuilder); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()) + .contains("Hostname cannot be empty string or null string."); } @Test @@ -1175,7 +1367,7 @@ public void goerliValuesAreUsed() throws Exception { } @Test - public void noSeveralNetworkOptions() throws Exception { + public void noSeveralNetworkOptions() { parseCommand("--goerli", "--rinkeby"); verifyZeroInteractions(mockRunnerBuilder); diff --git a/pantheon/src/test/resources/everything_config.toml b/pantheon/src/test/resources/everything_config.toml new file mode 100644 index 0000000000..a5634b15d7 --- /dev/null +++ b/pantheon/src/test/resources/everything_config.toml @@ -0,0 +1,63 @@ +# Every possible CLI should be in this file. +# The odds are you are reading this because you added a CLI and didn't add it +# here and a test broke. To fix the test add your CLI to this file. +# +# Please use a plausable value, Pantheon has to at least be able to parse it. +# If it is a multi-valued CLI make it a TOML array. +# If it is a number or boolean make it a number or boolean +# All other config options are strings, and must be quoted. +# Please provide some sensible grouping. + +# Node Information +datadir="~/pantheondata" +logging="INFO" +node-private-key="0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef" + +# P2P network +no-discovery=true +bootnodes=[ + "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567", + "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567", + "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567" +] +banned-nodeids=["0x6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"] +p2p-listen="1.2.3.4:1234" +max-peers=42 +max-trailing-peers=5 +host-whitelist=["all"] + +# chain +genesis="~/genesis.json" +#sync-mode="fast" +ottoman=false +ropsten=false +goerli=false +network-id=303 +rinkeby=false +dev-mode=false + +# JSON-RPC +rpc-enabled=false +rpc-listen="5.6.7.8:5678" +rpc-api=["DEBUG","ETH"] +rpc-cors-origins=["none"] + +# WebSockets API +ws-enabled=false +ws-api=["DEBUG","ETH"] +ws-listen="9.10.11.12:9101" +ws-refresh-delay=500 + +# Prometheus Metrics Endpoint +metrics-enabled=false +metrics-listen="8.6.7.5:309" + +# Mining +miner-enabled=false +miner-coinbase="0x0000000000000000000000000000000000000002" +miner-extraData="Protocol Engineering Group And SYStems" +miner-minTransactionGasPriceWei="1" + +# Permissioning +accounts-whitelist=["0x0000000000000000000000000000000000000009"] +nodes-whitelist=["all"]