Introduce backward compatibility flag

CHANGELOG.md

@@ -1,5 +1,14 @@
 # Changelog
 
+## Next Release
+### Breaking Changes
+- The behavior of reload API endpoint has been changed. It 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 during reload API call. 
+To keep the old behavior `--reload-keep-stale-keys=true` option can be used which will not remove stale keys during 
+reload API call.
+
+---
 ## 24.12.0
 
 ### Breaking Changes

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 removedConfigFilePublicKeyIsRemovedAfterReload(final KeyType keyType) {
     final String[] prvKeys = privateKeys(keyType);
     final String[] keys = createKeys(keyType, true, prvKeys);
 
@@ -161,7 +161,7 @@ public void alreadyLoadedPublicKeysAreNotRemovedAfterReload(final KeyType keyTyp
     // reload API call
     signer.callReload().then().statusCode(200);
 
-    validateApiResponse(signer.callApiPublicKeys(keyType), containsInAnyOrder(keys));
+    validateApiResponse(signer.callApiPublicKeys(keyType), contains(keys[0]));
   }
 
   @ParameterizedTest

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

@@ -690,7 +690,10 @@ protected List<ArtifactSignerProvider> createArtifactSignerProvider(
         final Vertx vertx, final MetricsSystem metricsSystem) {
       return List.of(
           new DefaultArtifactSignerProvider(
-              Collections::emptyList, Optional.empty(), Optional.empty()));
+              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();
@@ -149,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",

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

@@ -157,6 +157,7 @@ protected List<ArtifactSignerProvider> createArtifactSignerProvider(
       final Vertx vertx, final MetricsSystem metricsSystem) {
     return List.of(
         new DefaultArtifactSignerProvider(
+            baseConfig.reloadKeepStaleKeys(),
             createArtifactSignerSupplier(vertx, metricsSystem),
             slashingProtectionContext.map(PostLoadingValidatorsProcessor::new),
             Optional.of(commitBoostApiParameters)));
@@ -201,7 +202,8 @@ private MappedResults<ArtifactSigner> loadSignersFromKeyConfigFiles(
               azureKeyVaultFactory);
 
       final MappedResults<ArtifactSigner> results =
-          new SignerLoader(baseConfig.keystoreParallelProcessingEnabled())
+          new SignerLoader(
+                  baseConfig.keystoreParallelProcessingEnabled(), baseConfig.reloadKeepStaleKeys())
               .load(
                   baseConfig.getKeyConfigPath(),
                   "yaml",

core/src/main/java/tech/pegasys/web3signer/core/config/BaseConfig.java

@@ -64,4 +64,6 @@ public interface BaseConfig {
   boolean keystoreParallelProcessingEnabled();
 
   int getVertxWorkerPoolSize();
+
+  boolean reloadKeepStaleKeys();
 }

signing/src/main/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProvider.java

@@ -45,6 +45,8 @@
 public class DefaultArtifactSignerProvider implements ArtifactSignerProvider {
 
   private static final Logger LOG = LogManager.getLogger();
+
+  private final boolean reloadKeepStaleKeys;
   private final Supplier<Collection<ArtifactSigner>> artifactSignerCollectionSupplier;
   private final Optional<BiConsumer<Set<String>, Set<String>>> postLoadingCallback;
   private final Optional<KeystoresParameters> commitBoostKeystoresParameters;
@@ -54,9 +56,11 @@ public class DefaultArtifactSignerProvider implements ArtifactSignerProvider {
   private final ExecutorService executorService = Executors.newSingleThreadExecutor();
 
   public DefaultArtifactSignerProvider(
+      final boolean reloadKeepStaleKeys,
       final Supplier<Collection<ArtifactSigner>> artifactSignerCollectionSupplier,
       final Optional<BiConsumer<Set<String>, Set<String>>> postLoadingCallback,
       final Optional<KeystoresParameters> commitBoostKeystoresParameters) {
+    this.reloadKeepStaleKeys = reloadKeepStaleKeys;
     this.artifactSignerCollectionSupplier = artifactSignerCollectionSupplier;
     this.postLoadingCallback = postLoadingCallback;
     this.commitBoostKeystoresParameters = commitBoostKeystoresParameters;
@@ -70,7 +74,9 @@ public Future<Void> load() {
           // step 1: Create copy of current signers
           final Map<String, ArtifactSigner> oldSigners = new HashMap<>(signers);
           // step 2: Clear current signers and then load them via ArtifactSignerCollectionSupplier
-          signers.clear();
+          if (!reloadKeepStaleKeys) {
+            signers.clear();
+          }
           signers.putAll(
               artifactSignerCollectionSupplier.get().stream()
                   .collect(
@@ -84,14 +90,21 @@ public Future<Void> load() {
                           })));
 
           // step 3: Collect all stale keys that are no longer valid
-          final Set<String> staleKeys = new HashSet<>(oldSigners.keySet());
-          staleKeys.removeAll(signers.keySet());
+          final Set<String> staleKeys;
+          if (reloadKeepStaleKeys) {
+            staleKeys = new HashSet<>();
+          } else {
+            staleKeys = new HashSet<>(oldSigners.keySet());
+            staleKeys.removeAll(signers.keySet());
+          }
 
           // step 4: callback to register new keys and disable stale keys in slashing database
           postLoadingCallback.ifPresent(callback -> callback.accept(signers.keySet(), staleKeys));
 
           // step 5: for each loaded signer, load commit boost proxy signers (if any)
-          proxySigners.clear();
+          if (!reloadKeepStaleKeys) {
+            proxySigners.clear();
+          }
           commitBoostKeystoresParameters
               .filter(KeystoresParameters::isEnabled)
               .ifPresent(

signing/src/main/java/tech/pegasys/web3signer/signing/config/SignerLoader.java

@@ -51,15 +51,14 @@ public class SignerLoader {
   private static final long FILES_PROCESSED_TO_REPORT = 10;
   // enable or disable parallel streams to convert and load private keys from metadata files
   private final boolean useParallelStreams;
+  // if false, reload metadata files even if they are not modified
+  private final boolean reloadKeepStaleFiles;
 
   private static final Map<Path, FileTime> metadataConfigFilesPathCache = new HashMap<>();
 
-  public SignerLoader(final boolean useParallelStreams) {
+  public SignerLoader(final boolean useParallelStreams, final boolean reloadKeepStaleFiles) {
     this.useParallelStreams = useParallelStreams;
-  }
-
-  public SignerLoader() {
-    this(true);
+    this.reloadKeepStaleFiles = reloadKeepStaleFiles;
   }
 
   public MappedResults<ArtifactSigner> load(
@@ -173,7 +172,8 @@ private boolean isNewOrModifiedMetadataFile(final Path path) {
     try {
       final FileTime lastModifiedTime = Files.getLastModifiedTime(path);
       if (metadataConfigFilesPathCache.containsKey(path)) {
-        if (metadataConfigFilesPathCache.get(path).compareTo(lastModifiedTime) == 0) {
+        if (reloadKeepStaleFiles
+            && metadataConfigFilesPathCache.get(path).compareTo(lastModifiedTime) == 0) {
           return false;
         }
       }

signing/src/test/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProviderTest.java

@@ -69,7 +69,7 @@ void signerReturnedForMatchingIdentifier() {
 
     signerProvider =
         new DefaultArtifactSignerProvider(
-            () -> List.of(mockSigner), Optional.empty(), Optional.empty());
+            false, () -> List.of(mockSigner), Optional.empty(), Optional.empty());
     assertThatCode(() -> signerProvider.load().get()).doesNotThrowAnyException();
 
     final Optional<ArtifactSigner> signer = signerProvider.getSigner(PUBLIC_KEY1);
@@ -86,7 +86,7 @@ void signerProviderOnlyHasSingleEntryIfPassedInListHasMultipleMatchingSigners()
 
     signerProvider =
         new DefaultArtifactSignerProvider(
-            () -> List.of(mockSigner1, mockSigner2), Optional.empty(), Optional.empty());
+            false, () -> List.of(mockSigner1, mockSigner2), Optional.empty(), Optional.empty());
     assertThatCode(() -> signerProvider.load().get()).doesNotThrowAnyException();
 
     assertThat(signerProvider.availableIdentifiers()).hasSize(1);
@@ -102,7 +102,7 @@ void signerProviderCanMapInTwoSigners() {
 
     signerProvider =
         new DefaultArtifactSignerProvider(
-            () -> List.of(mockSigner1, mockSigner2), Optional.empty(), Optional.empty());
+            false, () -> List.of(mockSigner1, mockSigner2), Optional.empty(), Optional.empty());
     assertThatCode(() -> signerProvider.load().get()).doesNotThrowAnyException();
     assertThat(signerProvider.availableIdentifiers()).hasSize(2);
     assertThat(signerProvider.availableIdentifiers()).containsOnly(PUBLIC_KEY1, PUBLIC_KEY2);
@@ -131,6 +131,7 @@ void proxySignersAreLoadedCorrectly() throws IOException {
 
     signerProvider =
         new DefaultArtifactSignerProvider(
+            false,
             () -> List.of(mockSigner1, mockSigner2),
             Optional.empty(),
             Optional.of(commitBoostParameters));
@@ -171,6 +172,7 @@ void emptyProxySignersAreLoadedSuccessfully() {
 
     signerProvider =
         new DefaultArtifactSignerProvider(
+            false,
             () -> List.of(mockSigner1, mockSigner2),
             Optional.empty(),
             Optional.of(commitBoostParameters));

signing/src/test/java/tech/pegasys/web3signer/signing/config/SignerLoaderTest.java

@@ -108,7 +108,7 @@ public void setup() {
     signerParser =
         new YamlSignerParser(
             List.of(blsArtifactSignerFactory), YamlMapperFactory.createYamlMapper());
-    signerLoader = new SignerLoader();
+    signerLoader = new SignerLoader(true, true);
   }
 
   @ParameterizedTest(name = "{index} - Signer created for file name {0}")