feat: Remove stale validator keys during reload

CHANGELOG.md

@@ -1,5 +1,22 @@
 # Changelog
 
+## Next Release
+
+### Breaking Changes
+- The behavior of reload API endpoint has been modified due to issue [#1018][issue_1018] implemented by PR [#1054][pr_1054].
+The reload API call will remove all in-memory keys before loading them again using configuration files and bulk loading.
+This unfortunately results in behaviour change of local configuration files pointing to encrypted keystores. Previously 
+cached local configuration files were avoided being reloaded during reload API call, now they will be reloaded and 
+reparsed. To keep the old behavior `--reload-keep-stale-keys=true` option can be used which will not remove stale keys 
+during reload API call.
+
+[issue_1018]: https://github.com/Consensys/web3signer/issues/1018
+[pr_1054]: https://github.com/Consensys/web3signer/pull/1054
+
+### Features Added
+- Remove stale keys during reload API call. [#1018][issue_1018] [#1054][pr_1054]
+
+---
 ## 24.12.0
 
 ### Breaking Changes

acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/SignerConfiguration.java

@@ -83,6 +83,7 @@ public class SignerConfiguration {
 
   private final boolean signingExtEnabled;
   private Optional<Pair<Path, Path>> commitBoostParameters;
+  private final Optional<Boolean> reloadKeepStaleKeys;
 
   public SignerConfiguration(
       final String hostname,
@@ -131,7 +132,8 @@ public SignerConfiguration(
       final ChainIdProvider chainIdProvider,
       final Optional<KeystoresParameters> v3KeystoresBulkloadParameters,
       final boolean signingExtEnabled,
-      final Optional<Pair<Path, Path>> commitBoostParameters) {
+      final Optional<Pair<Path, Path>> commitBoostParameters,
+      final Optional<Boolean> reloadKeepStaleKeys) {
     this.hostname = hostname;
     this.logLevel = logLevel;
     this.httpRpcPort = httpRpcPort;
@@ -179,6 +181,7 @@ public SignerConfiguration(
     this.v3KeystoresBulkloadParameters = v3KeystoresBulkloadParameters;
     this.signingExtEnabled = signingExtEnabled;
     this.commitBoostParameters = commitBoostParameters;
+    this.reloadKeepStaleKeys = reloadKeepStaleKeys;
   }
 
   public String hostname() {
@@ -376,4 +379,8 @@ public boolean isSigningExtEnabled() {
   public Optional<Pair<Path, Path>> getCommitBoostParameters() {
     return commitBoostParameters;
   }
+
+  public Optional<Boolean> getReloadKeepStaleKeys() {
+    return reloadKeepStaleKeys;
+  }
 }

acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/SignerConfigurationBuilder.java

@@ -87,6 +87,7 @@ public class SignerConfigurationBuilder {
 
   private boolean signingExtEnabled;
   private Pair<Path, Path> commitBoostParameters;
+  private Optional<Boolean> reloadKeepStaleKeys = Optional.empty();
 
   public SignerConfigurationBuilder withLogLevel(final Level logLevel) {
     this.logLevel = logLevel;
@@ -339,6 +340,11 @@ public SignerConfigurationBuilder withCommitBoostParameters(
     return this;
   }
 
+  public SignerConfigurationBuilder withReloadKeepStaleKeys(final boolean reloadKeepStaleKeys) {
+    this.reloadKeepStaleKeys = Optional.of(reloadKeepStaleKeys);
+    return this;
+  }
+
   public SignerConfiguration build() {
     if (mode == null) {
       throw new IllegalArgumentException("Mode cannot be null");
@@ -390,6 +396,7 @@ public SignerConfiguration build() {
         chainIdProvider,
         Optional.ofNullable(v3KeystoresBulkloadParameters),
         signingExtEnabled,
-        Optional.ofNullable(commitBoostParameters));
+        Optional.ofNullable(commitBoostParameters),
+        reloadKeepStaleKeys);
   }
 }

acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsConfigFileImpl.java

@@ -123,6 +123,14 @@ public List<String> createCmdLineParams() {
 
     yamlConfig.append(createServerTlsArgs());
 
+    signerConfig
+        .getReloadKeepStaleKeys()
+        .ifPresent(
+            reloadKeepStaleKeys ->
+                yamlConfig.append(
+                    String.format(
+                        YAML_BOOLEAN_FMT, "reload-keep-stale-keys", reloadKeepStaleKeys)));
+
     params.add(signerConfig.getMode()); // sub-command .. it can't go to config file
 
     if (signerConfig.getMode().equals("eth2")) {

acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java

@@ -107,6 +107,11 @@ public List<String> createCmdLineParams() {
 
     params.addAll(createServerTlsArgs());
 
+    signerConfig
+        .getReloadKeepStaleKeys()
+        .ifPresent(
+            reloadKeepStaleKeys -> params.add("--reload-keep-stale-keys=" + reloadKeepStaleKeys));
+
     params.add(signerConfig.getMode());
 
     if (signerConfig.getMode().equals("eth2")) {

acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/publickeys/KeyIdentifiersAcceptanceTest.java

@@ -147,7 +147,7 @@ public void healthCheckReportsKeysLoadedAfterReloadInEth2Mode() {
 
   @ParameterizedTest
   @EnumSource(value = KeyType.class)
-  public void alreadyLoadedPublicKeysAreNotRemovedAfterReload(final KeyType keyType) {
+  public void publicKeysAreRemovedAfterReloadDefault(final KeyType keyType) {
     final String[] prvKeys = privateKeys(keyType);
     final String[] keys = createKeys(keyType, true, prvKeys);
 
@@ -161,7 +161,42 @@ public void alreadyLoadedPublicKeysAreNotRemovedAfterReload(final KeyType keyTyp
     // reload API call
     signer.callReload().then().statusCode(200);
 
+    // reload is async ... assert that the key is removed
+    Awaitility.await()
+        .atMost(5, SECONDS)
+        .untilAsserted(
+            () -> {
+              final List<String> publicKeysList =
+                  signer.callApiPublicKeys(keyType).jsonPath().getList(".");
+              assertThat(publicKeysList).containsOnly(keys[0]);
+            });
+  }
+
+  @ParameterizedTest
+  @EnumSource(value = KeyType.class)
+  public void publicKeysNotRemovedAfterReloadWithKeepStaleKeysTrue(final KeyType keyType) {
+    final String[] prvKeys = privateKeys(keyType);
+    final String[] keys = createKeys(keyType, true, prvKeys);
+
+    initAndStartSignerWithReloadKeepStaleKeys(calculateMode(keyType));
+
     validateApiResponse(signer.callApiPublicKeys(keyType), containsInAnyOrder(keys));
+
+    // remove one of the key config file
+    assertThat(testDirectory.resolve(keys[1] + ".yaml").toFile().delete()).isTrue();
+
+    // reload API call
+    signer.callReload().then().statusCode(200);
+
+    // reload is async ... assert that the keys are not removed
+    Awaitility.await()
+        .atMost(5, SECONDS)
+        .untilAsserted(
+            () -> {
+              final List<String> publicKeysList =
+                  signer.callApiPublicKeys(keyType).jsonPath().getList(".");
+              assertThat(publicKeysList).containsExactlyInAnyOrder(keys);
+            });
   }
 
   @ParameterizedTest

acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/publickeys/KeyIdentifiersAcceptanceTestBase.java

@@ -141,6 +141,15 @@ protected void initAndStartSigner(final String mode) {
     startSigner(builder.build());
   }
 
+  protected void initAndStartSignerWithReloadKeepStaleKeys(final String mode) {
+    startSigner(
+        new SignerConfigurationBuilder()
+            .withKeyStoreDirectory(testDirectory)
+            .withMode(mode)
+            .withReloadKeepStaleKeys(true)
+            .build());
+  }
+
   protected Response callApiPublicKeysWithoutOpenApiClientSideFilter(final KeyType keyType) {
     return given().baseUri(signer.getUrl()).accept("").get(Signer.publicKeysPath(keyType));
   }

commandline/src/main/java/tech/pegasys/web3signer/commandline/Web3SignerBaseCommand.java

@@ -213,9 +213,13 @@ public class Web3SignerBaseCommand implements BaseConfig, Runnable {
       paramLabel = INTEGER_FORMAT_HELP)
   private Integer vertxWorkerPoolSize = null;
 
-  @Deprecated(forRemoval = true)
-  @Option(names = "--Xworker-pool-size", hidden = true)
-  private Integer deprecatedWorkerPoolSize = null;
+  @Option(
+      names = "--reload-keep-stale-keys",
+      description =
+          "Set to true to keep stale keys loaded during reload API call. (default: ${DEFAULT-VALUE})",
+      paramLabel = "<BOOL>",
+      arity = "1")
+  private boolean reloadKeepStaleKeys = false;
 
   @CommandLine.Mixin private PicoCliTlsServerOptions picoCliTlsServerOptions;
 
@@ -323,22 +327,18 @@ 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 VERTX_WORKER_POOL_SIZE_DEFAULT;
   }
 
+  @Override
+  public boolean reloadKeepStaleKeys() {
+    return reloadKeepStaleKeys;
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)
@@ -386,12 +386,6 @@ 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 {

commandline/src/test/java/tech/pegasys/web3signer/commandline/CommandlineParserTest.java

@@ -559,21 +559,6 @@ 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();
@@ -587,19 +572,6 @@ void vertxWorkerPoolSizeDefaultParsesSuccessfully() {
     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();
@@ -688,7 +660,12 @@ public void run() {}
     @Override
     protected List<ArtifactSignerProvider> createArtifactSignerProvider(
         final Vertx vertx, final MetricsSystem metricsSystem) {
-      return List.of(new DefaultArtifactSignerProvider(Collections::emptyList, Optional.empty()));
+      return List.of(
+          new DefaultArtifactSignerProvider(
+              baseConfig.reloadKeepStaleKeys(),
+              Collections::emptyList,
+              Optional.empty(),
+              Optional.empty()));
     }
 
     @Override

core/src/integrationTest/java/tech/pegasys/web3signer/core/jsonrpcproxy/support/TestBaseConfig.java

@@ -141,4 +141,9 @@ public boolean keystoreParallelProcessingEnabled() {
   public int getVertxWorkerPoolSize() {
     return 20;
   }
+
+  @Override
+  public boolean reloadKeepStaleKeys() {
+    return false;
+  }
 }

core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java

@@ -92,6 +92,7 @@ protected List<ArtifactSignerProvider> createArtifactSignerProvider(
 
     final ArtifactSignerProvider signerProvider =
         new DefaultArtifactSignerProvider(
+            baseConfig.reloadKeepStaleKeys(),
             () -> {
               final List<ArtifactSigner> signers = new ArrayList<>();
               final AzureKeyVaultFactory azureKeyVaultFactory = new AzureKeyVaultFactory();
@@ -116,6 +117,7 @@ protected List<ArtifactSignerProvider> createArtifactSignerProvider(
                       .getValues());
               return signers;
             },
+            Optional.empty(),
             Optional.empty());
 
     // uses eth1 address as identifier
@@ -148,7 +150,8 @@ private MappedResults<ArtifactSigner> loadSignersFromKeyConfigFiles(
               awsKmsSignerFactory,
               true);
 
-      return new SignerLoader(baseConfig.keystoreParallelProcessingEnabled())
+      return new SignerLoader(
+              baseConfig.keystoreParallelProcessingEnabled(), baseConfig.reloadKeepStaleKeys())
           .load(
               baseConfig.getKeyConfigPath(),
               "yaml",