From 4db92abc4dd93056e51ddb31aeddd50c5f67192d Mon Sep 17 00:00:00 2001 From: Usman Saleem Date: Thu, 9 Nov 2023 16:20:47 +1000 Subject: [PATCH] Validate vertx pool size options (#947) * Validate vertx pool size options. The deprecated worker pool size and new vertx worker pool size cli options are mutually exclusive. --- .../commandline/Web3SignerBaseCommand.java | 26 +++++++-- .../commandline/CommandlineParserTest.java | 58 +++++++++++++++++++ 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/commandline/src/main/java/tech/pegasys/web3signer/commandline/Web3SignerBaseCommand.java b/commandline/src/main/java/tech/pegasys/web3signer/commandline/Web3SignerBaseCommand.java index 6cbdede94..c504f8399 100644 --- a/commandline/src/main/java/tech/pegasys/web3signer/commandline/Web3SignerBaseCommand.java +++ b/commandline/src/main/java/tech/pegasys/web3signer/commandline/Web3SignerBaseCommand.java @@ -66,7 +66,7 @@ subcommands = {HelpCommand.class}, footer = "Web3Signer is licensed under the Apache License 2.0") public class Web3SignerBaseCommand implements BaseConfig, Runnable { - + private static final int VERTX_WORKER_POOL_SIZE_DEFAULT = 20; @Spec private CommandLine.Model.CommandSpec spec; // injected by picocli public static final String KEY_STORE_CONFIG_FILE_SIZE_OPTION_NAME = "--key-store-config-file-max-size"; @@ -207,9 +207,11 @@ public class Web3SignerBaseCommand implements BaseConfig, Runnable { @Option( names = "--vertx-worker-pool-size", description = - "Configure the Vert.x worker pool size used for processing requests. (default: ${DEFAULT-VALUE})", + "Configure the Vert.x worker pool size used for processing requests. (default: " + + VERTX_WORKER_POOL_SIZE_DEFAULT + + ")", paramLabel = INTEGER_FORMAT_HELP) - private int vertxWorkerPoolSize = 20; + private Integer vertxWorkerPoolSize = null; @Deprecated @Option(names = "--Xworker-pool-size", hidden = true) @@ -321,10 +323,20 @@ public boolean keystoreParallelProcessingEnabled() { @Override public int getVertxWorkerPoolSize() { + // both values are not allowed on cli, they will be verified in validateArgs() ... + if (vertxWorkerPoolSize != null && deprecatedWorkerPoolSize != null) { + return -1; + } + + if (vertxWorkerPoolSize != null) { + return vertxWorkerPoolSize; + } + if (deprecatedWorkerPoolSize != null) { return deprecatedWorkerPoolSize; } - return vertxWorkerPoolSize; + + return VERTX_WORKER_POOL_SIZE_DEFAULT; } @Override @@ -374,6 +386,12 @@ public void validateArgs() { "--metrics-enabled option and --metrics-push-enabled option can't be used at the same " + "time. Please refer to CLI reference for more details about this constraint."); } + + if (vertxWorkerPoolSize != null && deprecatedWorkerPoolSize != null) { + throw new CommandLine.MutuallyExclusiveArgsException( + spec.commandLine(), + "--vertx-worker-pool-size option and --Xworker-pool-size option can't be used at the same time."); + } } public static class Web3signerMetricCategoryConverter extends MetricCategoryConverter { diff --git a/commandline/src/test/java/tech/pegasys/web3signer/commandline/CommandlineParserTest.java b/commandline/src/test/java/tech/pegasys/web3signer/commandline/CommandlineParserTest.java index a6c4cf025..d3edf8e41 100644 --- a/commandline/src/test/java/tech/pegasys/web3signer/commandline/CommandlineParserTest.java +++ b/commandline/src/test/java/tech/pegasys/web3signer/commandline/CommandlineParserTest.java @@ -557,6 +557,60 @@ void awsWithoutModeDefaultsToSpecified() { .contains("v1", "v2", "v3"); } + @Test + void vertxWorkerPoolSizeWithWorkerPoolSizeFailsToParse() { + String cmdline = validBaseCommandOptions(); + cmdline += + "--vertx-worker-pool-size=30 --Xworker-pool-size=40 eth2 --slashing-protection-enabled=false"; + + parser.registerSubCommands(new MockEth2SubCommand()); + final int result = parser.parseCommandLine(cmdline.split(" ")); + + assertThat(result).isNotZero(); + assertThat(commandError.toString()) + .contains( + "Error parsing parameters: --vertx-worker-pool-size option and --Xworker-pool-size option can't be used at the same time."); + } + + @Test + void vertxWorkerPoolSizeDefaultParsesSuccessfully() { + String cmdline = validBaseCommandOptions(); + cmdline += "eth2 --slashing-protection-enabled=false"; + + MockEth2SubCommand mockEth2SubCommand = new MockEth2SubCommand(); + parser.registerSubCommands(mockEth2SubCommand); + final int result = parser.parseCommandLine(cmdline.split(" ")); + + assertThat(result).isZero(); + assertThat(mockEth2SubCommand.getConfig().getVertxWorkerPoolSize()).isEqualTo(20); + } + + @Test + void vertxWorkerPoolSizeDeprecatedParsesSuccessfully() { + String cmdline = validBaseCommandOptions(); + cmdline += "--Xworker-pool-size=40 eth2 --slashing-protection-enabled=false"; + + MockEth2SubCommand mockEth2SubCommand = new MockEth2SubCommand(); + parser.registerSubCommands(mockEth2SubCommand); + final int result = parser.parseCommandLine(cmdline.split(" ")); + + assertThat(result).isZero(); + assertThat(mockEth2SubCommand.getConfig().getVertxWorkerPoolSize()).isEqualTo(40); + } + + @Test + void vertxWorkerPoolSizeParsesSuccessfully() { + String cmdline = validBaseCommandOptions(); + cmdline += "--vertx-worker-pool-size=40 eth2 --slashing-protection-enabled=false"; + + MockEth2SubCommand mockEth2SubCommand = new MockEth2SubCommand(); + parser.registerSubCommands(mockEth2SubCommand); + final int result = parser.parseCommandLine(cmdline.split(" ")); + + assertThat(result).isZero(); + assertThat(mockEth2SubCommand.getConfig().getVertxWorkerPoolSize()).isEqualTo(40); + } + private void missingOptionalParameterIsValidAndMeetsDefault( final String paramToRemove, final Supplier actualValueGetter, final T expectedValue) { @@ -573,6 +627,10 @@ public static class MockEth2SubCommand extends Eth2SubCommand { public Runner createRunner() { return new NoOpRunner(config); } + + public BaseConfig getConfig() { + return config; + } } public static class NoOpRunner extends Runner {