Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Commit

Permalink
Fix Array Configurable CLI options (#514)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@shemnon
shemnon authored Jan 8, 2019
1 parent dcedfea commit 8d4048b
Show file tree
Hide file tree
Showing 6 changed files with 390 additions and 102 deletions.
15 changes: 7 additions & 8 deletions pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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),
Expand All @@ -341,6 +340,7 @@ private void setRefreshDelay(final Long refreshDelay) {
String.valueOf(DEFAULT_MAX_REFRESH_DELAY)));
}
this.refreshDelay = refreshDelay;
return refreshDelay;
}

@Option(
Expand All @@ -363,8 +363,7 @@ private void setRefreshDelay(final Long refreshDelay) {
paramLabel = "<hostname>",
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();

Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
86 changes: 47 additions & 39 deletions pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/CorsAllowedOriginsProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> {

private List<String> domains = Collections.emptyList();

public CorsAllowedOriginsProperty(final List<String> domains) {
this.domains = domains;
}
private final List<String> domains = new ArrayList<>();

public CorsAllowedOriginsProperty() {}

public List<String> getDomains() {
return domains;
@Override
@Nonnull
public Iterator<String> iterator() {
if (domains.size() == 1 && domains.get(0).equals("none")) {
return Collections.emptyIterator();
} else {
return domains.iterator();
}
}

public static class CorsAllowedOriginsPropertyConverter
implements ITypeConverter<CorsAllowedOriginsProperty> {
@Override
public int size() {
return domains.size();
}

@Override
public CorsAllowedOriginsProperty convert(final String value) throws IllegalArgumentException {
final List<String> 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<? extends String> 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;
}
}
79 changes: 45 additions & 34 deletions pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/JsonRPCWhitelistHostsProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> hostnamesWhitelist = Collections.emptyList();
public class JsonRPCWhitelistHostsProperty extends AbstractCollection<String> {

private JsonRPCWhitelistHostsProperty(final List<String> hostnamesWhitelist) {
this.hostnamesWhitelist = hostnamesWhitelist;
}
private final List<String> hostnamesWhitelist = new ArrayList<>();

public JsonRPCWhitelistHostsProperty() {}

public List<String> hostnamesWhitelist() {
return hostnamesWhitelist;
@Override
@Nonnull
public Iterator<String> iterator() {
if (hostnamesWhitelist.size() == 1 && hostnamesWhitelist.get(0).equals("none")) {
return Collections.emptyIterator();
} else {
return hostnamesWhitelist.iterator();
}
}

public static class JsonRPCWhitelistHostsConverter
implements CommandLine.ITypeConverter<JsonRPCWhitelistHostsProperty> {
@Override
public int size() {
return hostnamesWhitelist.size();
}

@Override
public JsonRPCWhitelistHostsProperty convert(final String value) {
final List<String> 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<? extends String> 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;
}
}
21 changes: 18 additions & 3 deletions pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -101,16 +102,30 @@ 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
pantheonCommand.parse(
new RunLast().useOut(outPrintStream).useAnsi(Ansi.OFF),
new DefaultExceptionHandler<List<Object>>().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);
}
}
}
Loading

0 comments on commit 8d4048b

Please sign in to comment.