AwsSecretsManager allows loading multiple secrets from same name (#175)

* AwsSecretsManager allows loading multiple secrets from same name
* Added keystore common module
* Updated logging message to be more descriptive
* Use guava Streams mapWithIndex
* Changelog

CHANGELOG.md

@@ -1,8 +1,11 @@
 # Changelog
 
+## 2.2.5
+- AWS Secrets Manager - multiple values can be parsed from single secret name (line terminated values).
+
 ## 2.2.4
 - Various dependency upgrades
-
+---
 ## 2.2.3
 - Added support for prefix filter for AWS secrets manager
 

gradle/versions.gradle

@@ -137,5 +137,8 @@ dependencyManagement {
        \-- org.apache.tuweni:tuweni-net:2.3.1
      */
     dependency 'commons-net:commons-net:3.9.0'
+
+    // used in tests to assert log message
+    dependency 'de.neuland-bfi:assertj-logging-log4j:0.5.0'
   }
 }

keystorage/aws/build.gradle

@@ -36,6 +36,7 @@ dependencies {
   implementation 'software.amazon.awssdk:secretsmanager'
   implementation 'com.google.guava:guava'
   implementation 'org.apache.logging.log4j:log4j-api'
+  implementation project(':keystorage:common')
   runtimeOnly 'software.amazon.awssdk:sts'
 
   testImplementation 'org.assertj:assertj-core'

keystorage/aws/src/main/java/tech/pegasys/signers/aws/AwsSecretsManager.java

@@ -12,6 +12,8 @@
  */
 package tech.pegasys.signers.aws;
 
+import static tech.pegasys.signers.common.SecretValueMapperUtil.mapSecretValue;
+
 import java.io.Closeable;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -120,17 +122,11 @@ public <R> Collection<R> mapSecrets(
                           final Optional<String> secretValue = fetchSecret(secretEntry.name());
                           if (secretValue.isEmpty()) {
                             LOG.warn(
-                                "Failed to fetch secret value '{}', and was discarded",
+                                "Failed to fetch secret name '{}', and was discarded",
                                 secretEntry.name());
                           } else {
-                            final R obj = mapper.apply(secretEntry.name(), secretValue.get());
-                            if (obj == null) {
-                              LOG.warn(
-                                  "Mapped '{}' to a null object, and was discarded",
-                                  secretEntry.name());
-                            } else {
-                              result.add(obj);
-                            }
+                            result.addAll(
+                                mapSecretValue(mapper, secretEntry.name(), secretValue.get()));
                           }
                         } catch (final Exception e) {
                           LOG.warn(

keystorage/common/build.gradle

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2020 ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+
+apply plugin: 'java-library'
+
+jar {
+  archiveBaseName = calculateJarName(project)
+  manifest {
+    attributes(
+      'Specification-Title': archiveBaseName.get(),
+      'Specification-Version': rootProject.version,
+      'Implementation-Title': archiveBaseName.get(),
+      'Implementation-Version': calculateVersion()
+      )
+  }
+}
+
+dependencies {
+  implementation 'org.apache.logging.log4j:log4j-api'
+  implementation 'com.google.guava:guava'
+  runtimeOnly 'org.apache.logging.log4j:log4j-core'
+  runtimeOnly 'org.apache.logging.log4j:log4j-slf4j-impl'
+
+  testImplementation 'org.assertj:assertj-core'
+  testImplementation 'de.neuland-bfi:assertj-logging-log4j'
+  testImplementation 'org.junit.jupiter:junit-jupiter-engine'
+  testImplementation 'org.junit.jupiter:junit-jupiter-api'
+  testImplementation 'org.junit.jupiter:junit-jupiter-params'
+
+  testRuntimeOnly 'org.apache.logging.log4j:log4j-core'
+  testRuntimeOnly 'org.apache.logging.log4j:log4j-slf4j-impl'
+}

keystorage/common/src/main/java/tech/pegasys/signers/common/SecretValueMapperUtil.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2023 ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package tech.pegasys.signers.common;
+
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.BiFunction;
+import java.util.stream.Collectors;
+
+import com.google.common.collect.Streams;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+public class SecretValueMapperUtil {
+  private static final Logger LOG = LogManager.getLogger();
+
+  public static <R> Set<R> mapSecretValue(
+      BiFunction<String, String, R> mapper, String secretName, String secretValue) {
+    return Streams.mapWithIndex(
+            secretValue.lines(),
+            (value, index) -> {
+              final R obj = mapper.apply(secretName, value);
+              if (obj == null) {
+                LOG.warn(
+                    "Value from secret name {} at index {} was not mapped and discarded.",
+                    secretName,
+                    index);
+              }
+              return obj;
+            })
+        .filter(Objects::nonNull)
+        .collect(Collectors.toSet());
+  }
+}

keystorage/common/src/test/java/tech/pegasys/signers/common/SecretValueMapperUtilTest.java

@@ -0,0 +1,92 @@
+/*
+ * Copyright 2023 ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package tech.pegasys.signers.common;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static tech.pegasys.signers.common.SecretValueMapperUtil.mapSecretValue;
+
+import java.util.Set;
+
+import de.neuland.assertj.logging.ExpectedLogging;
+import de.neuland.assertj.logging.ExpectedLoggingAssertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+class SecretValueMapperUtilTest {
+  @RegisterExtension
+  private final ExpectedLogging logging = ExpectedLogging.forSource(SecretValueMapperUtil.class);
+
+  @Test
+  void singleValueIsMapped() {
+    Set<String> mappedValues = mapSecretValue((k, v) -> v, "key", "value");
+    assertThat(mappedValues).containsOnly("value");
+  }
+
+  @Test
+  void newlinesValuesAreMapped() {
+    Set<String> mappedValues = mapSecretValue((k, v) -> v, "key", "value1\nvalue2");
+    assertThat(mappedValues).containsOnly("value1", "value2");
+
+    Set<String> mappedValues2 = mapSecretValue((k, v) -> v, "key", "value1\nvalue2\n");
+    assertThat(mappedValues2).containsOnly("value1", "value2");
+  }
+
+  @Test
+  void emptyStringResultsEmptyCollection() {
+    Set<String> mappedValues = mapSecretValue((k, v) -> v, "key", "");
+    assertThat(mappedValues).isEmpty();
+  }
+
+  @Test
+  void emptyLineTerminationsReturnsEmptyStrings() {
+    assertThat(mapSecretValue((k, v) -> v, "key", "\n")).containsOnly("");
+    assertThat(mapSecretValue((k, v) -> v, "key", "\nok\n\n")).containsOnly("", "ok");
+  }
+
+  @Test
+  void nullMappedIsNotReturned() {
+    Set<String> mappedValues =
+        mapSecretValue(
+            (k, v) -> {
+              if (v.startsWith("err")) {
+                return null;
+              }
+              return v;
+            },
+            "0xabc",
+            "ok1\nerr1\nerr2\nok2");
+
+    assertThat(mappedValues).containsOnly("ok1", "ok2");
+
+    ExpectedLoggingAssertions.assertThat(logging)
+        .hasWarningMessage("Value from secret name 0xabc at index 1 was not mapped and discarded.");
+    ExpectedLoggingAssertions.assertThat(logging)
+        .hasWarningMessage("Value from secret name 0xabc at index 2 was not mapped and discarded.");
+  }
+
+  @Test
+  void sameValuesAreMappedOnce() {
+    Set<String> mappedValues =
+        mapSecretValue(
+            (k, v) -> {
+              if (v.startsWith("err")) {
+                return null;
+              }
+              return v;
+            },
+            "key",
+            "ok\nerr1\nerr2\nok\nok1");
+
+    assertThat(mappedValues).containsOnly("ok", "ok1");
+  }
+}

settings.gradle

@@ -13,6 +13,7 @@
 
 rootProject.name='signers'
 include 'bls-keystore'
+include 'keystorage:common'
 include 'keystorage:hashicorp'
 include 'keystorage:azure'
 include 'keystorage:yubihsm2'