From d193a4563d412a1d2eb81597caf7bdff7806690f Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 23 Sep 2024 14:17:28 +0200 Subject: [PATCH 1/8] Move aws sdk2 version and bom to vespa --- dependency-versions/pom.xml | 1 + parent/pom.xml | 43 +++++++++++++++++++++---------------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index 67d0dd415ba7..fb13d4706e0a 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -78,6 +78,7 @@ find zkfacade/src/main/java/org/apache/curator -name package-info.java | \ xargs perl -pi -e 's/major = [0-9]+, minor = [0-9]+, micro = [0-9]+/major = 5, minor = 3, micro = 0/g' --> + 2.27.4 1.78.1 1.14.18 3.38.0 diff --git a/parent/pom.xml b/parent/pom.xml index 05c5ad132653..d99534219cb9 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -496,6 +496,16 @@ expect4j 1.6 + + com.github.luben + zstd-jni + ${luben.zstd.vespa.version} + + + com.google.errorprone + error_prone_annotations + ${error-prone-annotations.vespa.version} + org.apache.zookeeper zookeeper @@ -519,11 +529,6 @@ - - com.github.luben - zstd-jni - ${luben.zstd.vespa.version} - org.vafer jdependency @@ -672,6 +677,11 @@ jjwt-jackson ${java-jjwt.vespa.version} + + jakarta.inject + jakarta.inject-api + ${jakarta.inject.vespa.version} + joda-time joda-time @@ -722,6 +732,11 @@ airline ${airline.vespa.version} + + io.dropwizard.metrics + metrics-core + ${dropwizard.metrics.vespa.version} + io.netty netty-buffer @@ -1308,9 +1323,11 @@ ${snappy.vespa.version} - io.dropwizard.metrics - metrics-core - ${dropwizard.metrics.vespa.version} + software.amazon.awssdk + bom + ${aws-sdk2.vespa.version} + pom + import xerces @@ -1328,16 +1345,6 @@ json ${org.json.vespa.version} - - com.google.errorprone - error_prone_annotations - ${error-prone-annotations.vespa.version} - - - jakarta.inject - jakarta.inject-api - ${jakarta.inject.vespa.version} - From 11f3ddafb9f8e0443a82ac27382f167c46957a10 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 23 Sep 2024 17:16:18 +0200 Subject: [PATCH 2/8] Update allowed maven dependencies after moving AsmSecretStore to jdisc-cloud-aws --- .../allowed-maven-dependencies.txt | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/vespa-dependencies-enforcer/allowed-maven-dependencies.txt b/vespa-dependencies-enforcer/allowed-maven-dependencies.txt index 186f28caad61..8fc614bab45e 100644 --- a/vespa-dependencies-enforcer/allowed-maven-dependencies.txt +++ b/vespa-dependencies-enforcer/allowed-maven-dependencies.txt @@ -76,6 +76,8 @@ io.jsonwebtoken:jjwt-api:${java-jjwt.vespa.version} io.jsonwebtoken:jjwt-impl:${java-jjwt.vespa.version} io.jsonwebtoken:jjwt-jackson:${java-jjwt.vespa.version} io.netty:netty-buffer:${netty.vespa.version} +io.netty:netty-codec-http2:${netty.vespa.version} +io.netty:netty-codec-http:${netty.vespa.version} io.netty:netty-codec:${netty.vespa.version} io.netty:netty-common:${netty.vespa.version} io.netty:netty-handler:${netty.vespa.version} @@ -223,6 +225,7 @@ org.ow2.asm:asm-tree:${asm.vespa.version} org.ow2.asm:asm-util:${asm.vespa.version} org.ow2.asm:asm:${asm.vespa.version} org.questdb:questdb:${questdb.vespa.version} +org.reactivestreams:reactive-streams:1.0.4 org.slf4j:jcl-over-slf4j:${slf4j.vespa.version} org.slf4j:log4j-over-slf4j:${slf4j.vespa.version} org.slf4j:slf4j-api:${slf4j.vespa.version} @@ -231,4 +234,31 @@ org.slf4j:slf4j-simple:${slf4j.vespa.version} org.tukaani:xz:1.9 org.wiremock:wiremock-standalone:${wiremock.vespa.version} org.xerial.snappy:snappy-java:${snappy.vespa.version} +software.amazon.awssdk:annotations:${aws-sdk2.vespa.version} +software.amazon.awssdk:apache-client:${aws-sdk2.vespa.version} +software.amazon.awssdk:auth:${aws-sdk2.vespa.version} +software.amazon.awssdk:aws-core:${aws-sdk2.vespa.version} +software.amazon.awssdk:aws-json-protocol:${aws-sdk2.vespa.version} +software.amazon.awssdk:checksums-spi:${aws-sdk2.vespa.version} +software.amazon.awssdk:checksums:${aws-sdk2.vespa.version} +software.amazon.awssdk:endpoints-spi:${aws-sdk2.vespa.version} +software.amazon.awssdk:http-auth-aws-eventstream:${aws-sdk2.vespa.version} +software.amazon.awssdk:http-auth-aws:${aws-sdk2.vespa.version} +software.amazon.awssdk:http-auth-spi:${aws-sdk2.vespa.version} +software.amazon.awssdk:http-auth:${aws-sdk2.vespa.version} +software.amazon.awssdk:http-client-spi:${aws-sdk2.vespa.version} +software.amazon.awssdk:identity-spi:${aws-sdk2.vespa.version} +software.amazon.awssdk:json-utils:${aws-sdk2.vespa.version} +software.amazon.awssdk:metrics-spi:${aws-sdk2.vespa.version} +software.amazon.awssdk:netty-nio-client:${aws-sdk2.vespa.version} +software.amazon.awssdk:profiles:${aws-sdk2.vespa.version} +software.amazon.awssdk:protocol-core:${aws-sdk2.vespa.version} +software.amazon.awssdk:regions:${aws-sdk2.vespa.version} +software.amazon.awssdk:retries-spi:${aws-sdk2.vespa.version} +software.amazon.awssdk:retries:${aws-sdk2.vespa.version} +software.amazon.awssdk:sdk-core:${aws-sdk2.vespa.version} +software.amazon.awssdk:secretsmanager:${aws-sdk2.vespa.version} +software.amazon.awssdk:third-party-jackson-core:${aws-sdk2.vespa.version} +software.amazon.awssdk:utils:${aws-sdk2.vespa.version} +software.amazon.eventstream:eventstream:1.0.1 xerces:xercesImpl:${xerces.vespa.version} From 21b71901e0d1cff066ff9f60625fe6d74d8c630c Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 23 Sep 2024 12:09:47 +0200 Subject: [PATCH 3/8] Move tests for secret model classes to container-disc --- .../java/ai/vespa/secret/model/KeyTest.java | 24 ++++++++ .../ai/vespa/secret/model/SecretNameTest.java | 27 +++++++++ .../ai/vespa/secret/model/SecretTest.java | 58 +++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 container-disc/src/test/java/ai/vespa/secret/model/KeyTest.java create mode 100644 container-disc/src/test/java/ai/vespa/secret/model/SecretNameTest.java create mode 100644 container-disc/src/test/java/ai/vespa/secret/model/SecretTest.java diff --git a/container-disc/src/test/java/ai/vespa/secret/model/KeyTest.java b/container-disc/src/test/java/ai/vespa/secret/model/KeyTest.java new file mode 100644 index 000000000000..6ca462af7462 --- /dev/null +++ b/container-disc/src/test/java/ai/vespa/secret/model/KeyTest.java @@ -0,0 +1,24 @@ +package ai.vespa.secret.model; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * @author gjoranv + */ +public class KeyTest { + + @Test + void string_can_be_converted_to_key() { + var vault = VaultName.of("vaultName"); + var secret = SecretName.of("secretName"); + + var expected = new Key(vault, secret); + assertEquals(expected, Key.fromString("vaultName/secretName")); + + assertThrows(IllegalArgumentException.class, () -> Key.fromString("vaultName")); + assertThrows(IllegalArgumentException.class, () -> Key.fromString("vaultName/secretName/extra")); + } +} diff --git a/container-disc/src/test/java/ai/vespa/secret/model/SecretNameTest.java b/container-disc/src/test/java/ai/vespa/secret/model/SecretNameTest.java new file mode 100644 index 000000000000..29b99e504e96 --- /dev/null +++ b/container-disc/src/test/java/ai/vespa/secret/model/SecretNameTest.java @@ -0,0 +1,27 @@ +package ai.vespa.secret.model; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * @author gjoranv + */ +public class SecretNameTest { + + @Test + void testSecretName() { + SecretName.of("foo-bar"); + SecretName.of("-"); + SecretName.of("0"); + SecretName.of("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde"); + assertThrows(IllegalArgumentException.class, () -> SecretName.of("")); + + // TODO: enable when all secrets are < 64 characters + //assertThrows(IllegalArgumentException.class, () -> SecretName.of("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")); + + for (char c : "+/$ {}[]()!\"@#?\\'".toCharArray()) + assertThrows(IllegalArgumentException.class, () -> SecretName.of("foo" + c + "bar")); + } + +} diff --git a/container-disc/src/test/java/ai/vespa/secret/model/SecretTest.java b/container-disc/src/test/java/ai/vespa/secret/model/SecretTest.java new file mode 100644 index 000000000000..3ba826fb6929 --- /dev/null +++ b/container-disc/src/test/java/ai/vespa/secret/model/SecretTest.java @@ -0,0 +1,58 @@ +package ai.vespa.secret.model; + +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static ai.vespa.secret.model.SecretVersionState.CURRENT; +import static ai.vespa.secret.model.SecretVersionState.DEPRECATED; +import static ai.vespa.secret.model.SecretVersionState.PENDING; +import static ai.vespa.secret.model.SecretVersionState.PREVIOUS; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author gjoranv + */ +public class SecretTest { + + @Test + void secrets_are_sorted_on_vault_then_name_then_state() { + + var s11pe = secret("vault1", "name1", PENDING); + var s11cu = secret("vault1", "name1", CURRENT); + var s12cu = secret("vault1", "name2", CURRENT); + var s21pe = secret("vault2", "name1", PENDING); + var s21cu = secret("vault2", "name1", CURRENT); + var s21pr = secret("vault2", "name1", PREVIOUS); + var s21de = secret("vault2", "name1", DEPRECATED); + + var secrets = List.of( s21pe, s11cu, s12cu, s11pe, s21de, s21pr, s21cu ); + + var expected = List.of( s11pe, s11cu, s12cu, s21pe, s21cu, s21pr, s21de ); + + assertEquals(expected, secrets.stream().sorted().toList()); + } + + // This is relevant for secrets from CKMS, which don't use state, but ascending version numbers. + @Test + void secrets_with_same_state_are_sorted_by_version_descending() { + var v1 = secretWithIntVersion(1); + var v2 = secretWithIntVersion(2); + var v3 = secretWithIntVersion(3); + + var secrets = List.of(v3, v1, v2); + var expected = List.of(v3, v2, v1); + assertEquals(expected, secrets.stream().sorted().toList()); + } + + private static Secret secretWithIntVersion(Integer version) { + return new Secret(new Key(VaultName.of("foo"), SecretName.of("bar")), new byte[0], + SecretVersionId.of(version.toString())); + } + + private static Secret secret(String vault, String name, SecretVersionState state) { + return new Secret(new Key(VaultName.of(vault), SecretName.of(name)), new byte[0], + SecretVersionId.of("0"), state); + } + +} From 32b74372626d0181b84c807e44634ed09556ae19 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 23 Sep 2024 13:16:54 +0200 Subject: [PATCH 4/8] Move TypedSecretStore to container-disc --- .../secret/internal/TypedSecretStore.java | 34 +++++++++++++++++++ .../vespa/secret/internal/package-info.java | 6 ++++ 2 files changed, 40 insertions(+) create mode 100644 container-disc/src/main/java/ai/vespa/secret/internal/TypedSecretStore.java create mode 100644 container-disc/src/main/java/ai/vespa/secret/internal/package-info.java diff --git a/container-disc/src/main/java/ai/vespa/secret/internal/TypedSecretStore.java b/container-disc/src/main/java/ai/vespa/secret/internal/TypedSecretStore.java new file mode 100644 index 000000000000..472bd78f2e3c --- /dev/null +++ b/container-disc/src/main/java/ai/vespa/secret/internal/TypedSecretStore.java @@ -0,0 +1,34 @@ +package ai.vespa.secret.internal; + +import ai.vespa.secret.model.Key; +import ai.vespa.secret.model.Secret; +import ai.vespa.secret.model.SecretVersionId; +import com.yahoo.container.jdisc.secretstore.SecretStore; + +import java.util.List; + +public interface TypedSecretStore extends SecretStore { + + enum Type { + PUBLIC, + TEST, + YAHOO + } + + Secret getSecret(Key key); + + Secret getSecret(Key key, SecretVersionId version); + + /** Lists the existing versions of this secret */ + default List listSecretVersions(Key key) { + throw new UnsupportedOperationException("Secret store does not support listing versions"); + } + + Type type(); + + // Do not use! Only for legacy compatibility + default Secret getSecret(Key k, int i) { + return getSecret(k, SecretVersionId.of(String.valueOf(i))); + } + +} diff --git a/container-disc/src/main/java/ai/vespa/secret/internal/package-info.java b/container-disc/src/main/java/ai/vespa/secret/internal/package-info.java new file mode 100644 index 000000000000..b3183d6a3b04 --- /dev/null +++ b/container-disc/src/main/java/ai/vespa/secret/internal/package-info.java @@ -0,0 +1,6 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +@ExportPackage +package ai.vespa.secret.internal; + +import com.yahoo.osgi.annotation.ExportPackage; From 79c9c5ace4f83cd36c3420c5aff6e077b79e9cee Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 23 Sep 2024 14:04:05 +0200 Subject: [PATCH 5/8] Move AsmSecretStore from cloud-common to jdisc-cloud-aws --- jdisc-cloud-aws/pom.xml | 23 +- .../ai/vespa/secret/aws/AsmSecretStore.java | 180 +++++++++++++ .../vespa/secret/aws/AsmSecretStoreBase.java | 84 ++++++ .../src/main/resources/asm-secret.def | 5 + .../vespa/secret/aws/AsmSecretStoreTest.java | 246 ++++++++++++++++++ 5 files changed, 537 insertions(+), 1 deletion(-) create mode 100644 jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStore.java create mode 100644 jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStoreBase.java create mode 100644 jdisc-cloud-aws/src/main/resources/asm-secret.def create mode 100644 jdisc-cloud-aws/src/test/java/ai/vespa/secret/aws/AsmSecretStoreTest.java diff --git a/jdisc-cloud-aws/pom.xml b/jdisc-cloud-aws/pom.xml index 55cc03be5624..a5e404821a4f 100644 --- a/jdisc-cloud-aws/pom.xml +++ b/jdisc-cloud-aws/pom.xml @@ -14,7 +14,6 @@ jdisc-cloud-aws 8-SNAPSHOT container-plugin - ${project.artifactId} @@ -29,6 +28,12 @@ ${project.version} provided + + com.yahoo.vespa + vespa-athenz + ${project.version} + provided + com.amazonaws aws-java-sdk-core @@ -51,6 +56,22 @@ com.amazonaws aws-java-sdk-ssm + + software.amazon.awssdk + secretsmanager + + + org.slf4j + * + + + org.apache.httpcomponents + * + + + + + org.junit.jupiter junit-jupiter-api diff --git a/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStore.java b/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStore.java new file mode 100644 index 000000000000..0ad1647c5109 --- /dev/null +++ b/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStore.java @@ -0,0 +1,180 @@ +package ai.vespa.secret.aws; + +import ai.vespa.secret.internal.TypedSecretStore; +import ai.vespa.secret.model.Key; +import ai.vespa.secret.model.Role; +import ai.vespa.secret.model.Secret; +import ai.vespa.secret.model.SecretVersionId; +import ai.vespa.secret.model.SecretVersionState; +import ai.vespa.secret.model.VaultName; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import com.yahoo.component.annotation.Inject; +import com.yahoo.vespa.athenz.api.AthenzDomain; +import com.yahoo.vespa.athenz.client.zts.DefaultZtsClient; +import com.yahoo.vespa.athenz.client.zts.ZtsClient; +import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; +import software.amazon.awssdk.services.secretsmanager.SecretsManagerClient; +import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueRequest; +import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueResponse; +import software.amazon.awssdk.services.secretsmanager.model.ListSecretVersionIdsRequest; +import software.amazon.awssdk.services.secretsmanager.model.ListSecretVersionIdsResponse; + +import javax.net.ssl.SSLContext; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.List; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.function.Function; + +/** + * A read-only client for AWS Secrets Manager, with caching of secrets. + * Based on ASMSecretStore in hosted-configserver. + * + * @author gjoranv + */ +public final class AsmSecretStore extends AsmSecretStoreBase implements TypedSecretStore { + + private static final Duration CACHE_EXPIRE = Duration.ofMinutes(30); + + private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(5); + + private final LoadingCache cache; + private final Runnable closeable; + + protected record VersionKey(Key key, SecretVersionId version) {} + + @Inject + public AsmSecretStore(AsmSecretConfig config, ServiceIdentityProvider identities) { + this(URI.create(config.ztsUri()), identities.getIdentitySslContext(), identities.identity().getDomain()); + } + + public AsmSecretStore(URI ztsUri, SSLContext sslContext, AthenzDomain systemDomain) { + this(new DefaultZtsClient.Builder(ztsUri).withSslContext(sslContext).build(), systemDomain); + } + + private AsmSecretStore(ZtsClient ztsClient, AthenzDomain systemDomain) { + super(ztsClient, Role.READER, systemDomain); + cache = initCache(); + closeable = ztsClient::close; + } + + // For testing + AsmSecretStore(Function clientAndCredentialsSupplier) { + super(clientAndCredentialsSupplier); + cache = initCache(); + closeable = () -> {}; + } + + + private LoadingCache initCache() { + return CacheBuilder.newBuilder() + .refreshAfterWrite(CACHE_EXPIRE) + // See documentation for refreshAfterWrite for why we use asyncReloading. + .build(CacheLoader.asyncReloading(new CacheLoader<>() { + @Override + public Secret load(VersionKey key) { + return retrieveSecret(key.key(), key.version()); + } + }, scheduler)); + } + + private Secret retrieveSecret(Key key, SecretVersionId version) { + var client = getClient(key.vaultName()); + var requestBuilder = GetSecretValueRequest.builder() + .secretId(awsSecretId(key)); + + if (version != null) { + requestBuilder.versionId(version.value()); + } else { + requestBuilder.versionStage(AWSCURRENT); + } + var request = requestBuilder.build(); + + GetSecretValueResponse secret = client.getSecretValue(request); + return new Secret(key, + secret.secretString().getBytes(StandardCharsets.UTF_8), + SecretVersionId.of(secret.versionId()), + toSecretVersionState(secret.versionStages())); + } + + private static SecretVersionState toSecretVersionState(List versionStages) { + if (versionStages.size() != 1) { + throw new IllegalArgumentException("Expected exactly one version stage, got: " + versionStages); + } + var state = versionStages.get(0); + return switch (state) { + case "AWSCURRENT" -> SecretVersionState.CURRENT; + case "AWSPENDING" -> SecretVersionState.PENDING; + case "AWSPREVIOUS" -> SecretVersionState.PREVIOUS; + default -> throw new IllegalArgumentException("Unknown secret version state: " + state); + }; + } + + /** + * If version is null, the version with label AWSCURRENT is returned. + */ + public Secret getSecret(Key key, SecretVersionId version) { + try { + return cache.getUnchecked(new VersionKey(key, version)); + } catch (Exception e) { + var msg = version == null ? + "Failed to retrieve current version of secret with key " + key + : "Failed to retrieve secret with key " + key + ", version: " + version.value(); + throw new IllegalArgumentException(msg, e); + } + } + + @Override + public Secret getSecret(Key key) { + return getSecret(key, null); + } + + /** + * List all versions of the given secret, sorted according to + * {@link Secret#compareTo(Secret)}. Always retrieves all versions from ASM, + * and refreshes the cache for each version. + */ + @Override + public List listSecretVersions(Key key) { + var client = getClient(key.vaultName()); + + ListSecretVersionIdsResponse response = client.listSecretVersionIds( + ListSecretVersionIdsRequest.builder() + .secretId(awsSecretId(key)).build()); + + var secretVersions = response.versions().stream() + .map(version -> getSecret(key, SecretVersionId.of(version.versionId()))) + .sorted().toList(); + + secretVersions.forEach(secret -> cache.put(new VersionKey(key, secret.version()), secret)); + + return secretVersions; + } + + @Override + public Type type() { + return Type.PUBLIC; + } + + @Override + public void close() { + scheduler.shutdown(); + closeable.run(); + super.close(); + } + + @Override + public String getSecret(String key) { + throw new UnsupportedOperationException("This secret store does not support String lookups."); + } + + @Override + public String getSecret(String key, int version) { + throw new UnsupportedOperationException("This secret store does not support String lookups."); + } + +} diff --git a/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStoreBase.java b/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStoreBase.java new file mode 100644 index 000000000000..72ddc8c8918b --- /dev/null +++ b/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStoreBase.java @@ -0,0 +1,84 @@ +package ai.vespa.secret.aws; + +import ai.vespa.secret.model.Key; +import ai.vespa.secret.model.Role; +import ai.vespa.secret.model.VaultName; +import com.yahoo.component.AbstractComponent; +import com.yahoo.vespa.athenz.api.AthenzDomain; +import com.yahoo.vespa.athenz.api.AwsRole; +import com.yahoo.vespa.athenz.api.AwsTemporaryCredentials; +import com.yahoo.vespa.athenz.aws.AwsCredentials; +import com.yahoo.vespa.athenz.client.zts.ZtsClient; +import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; +import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.secretsmanager.SecretsManagerClient; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; + +/** + * Base class for AWS Secrets Manager read or write clients. + * + * @author gjoranv + */ +public sealed abstract class AsmSecretStoreBase extends AbstractComponent + implements AutoCloseable + permits AsmSecretStore, AsmSecretStoreWriter { + + public static final String AWSCURRENT = "AWSCURRENT"; + + private final ConcurrentMap clientMap = new ConcurrentHashMap<>(); + private final Function clientAndCredentialsSupplier; + + + public AsmSecretStoreBase(ZtsClient ztsClient, Role role, AthenzDomain athenzDomain) { + this(vault -> SecretsManagerClient.builder().region(Region.US_EAST_1) + .credentialsProvider(getAwsSessionCredsProvider(ztsClient, athenzDomain, role, vault)) + .build()); + } + + AsmSecretStoreBase(Function clientAndCredentialsSupplier) { + this.clientAndCredentialsSupplier = clientAndCredentialsSupplier; + } + + + protected SecretsManagerClient getClient(VaultName vaultName) { + return clientMap.computeIfAbsent(vaultName, clientAndCredentialsSupplier); + } + + private static AwsCredentialsProvider getAwsSessionCredsProvider(ZtsClient ztsClient, AthenzDomain athenzDomain, Role role, VaultName vaultName) { + var awsRole = new AwsRole(role.forVault(vaultName)); + AwsCredentials credentials = new AwsCredentials(ztsClient, athenzDomain, awsRole); + + return () -> { + AwsTemporaryCredentials temporary = credentials.get(); + return AwsSessionCredentials.create(temporary.accessKeyId(), + temporary.secretAccessKey(), + temporary.sessionToken()); + }; + } + + protected String awsSecretId(Key key) { + return key.vaultName().value() + "/" + key.secretName(); + } + + @Override + public void close() { + clientMap.values().forEach(client -> { + try { + client.close(); + } catch (Exception e) { + throw new RuntimeException("Failed to close", e); + } + }); + } + + @Override + public void deconstruct() { + close(); + super.deconstruct(); + } + +} diff --git a/jdisc-cloud-aws/src/main/resources/asm-secret.def b/jdisc-cloud-aws/src/main/resources/asm-secret.def new file mode 100644 index 000000000000..0ec42b27601d --- /dev/null +++ b/jdisc-cloud-aws/src/main/resources/asm-secret.def @@ -0,0 +1,5 @@ +# Config for AsmSecretStore + +package=ai.vespa.secret.aws + +ztsUri string diff --git a/jdisc-cloud-aws/src/test/java/ai/vespa/secret/aws/AsmSecretStoreTest.java b/jdisc-cloud-aws/src/test/java/ai/vespa/secret/aws/AsmSecretStoreTest.java new file mode 100644 index 000000000000..3ca3d4087def --- /dev/null +++ b/jdisc-cloud-aws/src/test/java/ai/vespa/secret/aws/AsmSecretStoreTest.java @@ -0,0 +1,246 @@ +package ai.vespa.secret.aws; + +import ai.vespa.secret.model.Key; +import ai.vespa.secret.model.Secret; +import ai.vespa.secret.model.SecretName; +import ai.vespa.secret.model.SecretVersionId; +import ai.vespa.secret.model.SecretVersionState; +import ai.vespa.secret.model.VaultName; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.awscore.exception.AwsServiceException; +import software.amazon.awssdk.core.exception.SdkClientException; +import software.amazon.awssdk.services.secretsmanager.SecretsManagerClient; +import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueRequest; +import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueResponse; +import software.amazon.awssdk.services.secretsmanager.model.InternalServiceErrorException; +import software.amazon.awssdk.services.secretsmanager.model.InvalidNextTokenException; +import software.amazon.awssdk.services.secretsmanager.model.InvalidParameterException; +import software.amazon.awssdk.services.secretsmanager.model.ListSecretVersionIdsRequest; +import software.amazon.awssdk.services.secretsmanager.model.ListSecretVersionIdsResponse; +import software.amazon.awssdk.services.secretsmanager.model.ResourceNotFoundException; +import software.amazon.awssdk.services.secretsmanager.model.SecretVersionsListEntry; +import software.amazon.awssdk.services.secretsmanager.model.SecretsManagerException; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * @author gjoranv + */ +public class AsmSecretStoreTest { + + record SecretVersion(String version, SecretVersionState state, String value) {} + + static final Map> secrets = new HashMap<>(); + static final List clients = new ArrayList<>(); + + @BeforeEach + void reset() { + secrets.clear(); + clients.clear(); + } + + AsmSecretStore createStore() { + return new AsmSecretStore(MockSecretsManagerClient::new); + } + + @Test + void it_creates_one_credentials_and_client_per_vault_and_closes_them() { + var vault1 = VaultName.of("vault1"); + var vault2 = VaultName.of("vault2"); + + var secret1 = new SecretVersion("1", SecretVersionState.CURRENT, "secret1"); + var secret2 = new SecretVersion("2", SecretVersionState.CURRENT, "secret2"); + + var key1 = new Key(vault1, SecretName.of("secret1")); + var key2 = new Key(vault2, SecretName.of("secret2")); + + secrets.put(key1, List.of(secret1)); + secrets.put(key2, List.of(secret2)); + + try (var store = createStore()){ + store.getSecret(key1); + store.getSecret(key2); + + assertEquals(2, clients.size()); + } + assertTrue(clients.stream().allMatch(c -> c.isClosed)); + } + + @Test + void it_returns_secret_with_given_version() { + var vault = VaultName.of("vault1"); + var key = new Key(vault, SecretName.of("secret1")); + + var expected1 = new SecretVersion("1", SecretVersionState.CURRENT, "v1"); + var expected2 = new SecretVersion("2", SecretVersionState.PENDING, "v2"); + secrets.put(key, List.of(expected1, expected2)); + + try (var store = createStore()) { + var retrieved1 = store.getSecret(key, SecretVersionId.of("1")); + var retrieved2 = store.getSecret(key, SecretVersionId.of("2")); + + assertSame(expected1, retrieved1); + assertSame(expected2, retrieved2); + } + } + + @Test + void it_returns_current_version_if_no_version_is_given() { + var vault = VaultName.of("vault1"); + var key = new Key(vault, SecretName.of("secret1")); + + var current = new SecretVersion("1", SecretVersionState.CURRENT, "current"); + var pending = new SecretVersion("2", SecretVersionState.PENDING, "pending"); + secrets.put(key, List.of(current, pending)); + + try (var store = createStore()) { + var retrieved = store.getSecret(key); + assertSame(current, retrieved); + } + } + + @Test + void it_throws_exception_if_secret_not_found() { + var vault = VaultName.of("vault1"); + var key = new Key(vault, SecretName.of("secret1")); + try (var store = createStore()) { + var e = assertThrows(IllegalArgumentException.class, () -> store.getSecret(key)); + assertEquals("Failed to retrieve current version of secret with key vault1/secret1", e.getMessage()); + + e = assertThrows(IllegalArgumentException.class, () -> store.getSecret(key, SecretVersionId.of("1"))); + assertEquals("Failed to retrieve secret with key vault1/secret1, version: 1", e.getMessage()); + } + } + + @Test + void it_throws_exception_if_version_not_found() { + var vault = VaultName.of("vault1"); + var key = new Key(vault, SecretName.of("secret1")); + + var v1 = new SecretVersion("1", SecretVersionState.CURRENT, "v1"); + secrets.put(key, List.of(v1)); + + try (var store = createStore()) { + var e = assertThrows(IllegalArgumentException.class, () -> store.getSecret(key, SecretVersionId.of("2"))); + assertEquals("Failed to retrieve secret with key vault1/secret1, version: 2", e.getMessage()); + } + + } + + @Test + void it_lists_secret_versions_in_sorted_order() { + var vault = VaultName.of("vault1"); + var key = new Key(vault, SecretName.of("secret1")); + + var prev = new SecretVersion("1", SecretVersionState.PREVIOUS, "v1"); + var curr = new SecretVersion("2", SecretVersionState.CURRENT, "v2"); + var pend = new SecretVersion("3", SecretVersionState.PENDING, "v3"); + + secrets.put(key, List.of(prev, pend, curr)); + try (var store = createStore()) { + var versions = store.listSecretVersions(key); + assertEquals(3, versions.size()); + assertSame(pend, versions.get(0)); + assertSame(curr, versions.get(1)); + assertSame(prev, versions.get(2)); + } + } + + @Test + void it_returns_empty_list_of_versions_for_unknown_secret() { + var vault = VaultName.of("vault1"); + var key = new Key(vault, SecretName.of("secret1")); + try (var store = createStore()) { + var versions = store.listSecretVersions(key); + assertEquals(0, versions.size()); + } + } + + private void assertSame(SecretVersion version, Secret secret) { + assertEquals(version.value(), secret.secretAsString()); + assertEquals(version.version(), secret.version().value()); + } + + private static Key toKey(String awsId) { + var parts = awsId.split("/"); + return new Key(VaultName.of(parts[0]), SecretName.of(parts[1])); + } + + private static class MockSecretsManagerClient implements SecretsManagerClient { + final VaultName vault; + boolean isClosed = false; + + MockSecretsManagerClient(VaultName vault) { + this.vault = vault; + clients.add(this); + } + + @Override + public GetSecretValueResponse getSecretValue(GetSecretValueRequest request) { + String id = request.secretId(); + String reqVersion = request.versionId(); + + var versions = secrets.get(toKey(id)); + if (versions == null) { + throw ResourceNotFoundException.builder().message("Secret not found").build(); + } + var secret = findSecret(versions, reqVersion); + return GetSecretValueResponse.builder() + .name(request.secretId()) + .secretString(secret.value()) + .versionId(secret.version) + .versionStages(List.of(toAwsStage(secret.state))) + .build(); + } + + SecretVersion findSecret(List versions, String reqVersion) { + return versions.stream() + .filter(reqVersion == null ? + v -> v.state() == SecretVersionState.CURRENT + : v -> v.version().equals(reqVersion)) + .findFirst() + .orElseThrow(() -> ResourceNotFoundException.builder().message("Version not found: " + reqVersion).build()); + } + + @Override + public ListSecretVersionIdsResponse listSecretVersionIds(ListSecretVersionIdsRequest request) throws InvalidNextTokenException, ResourceNotFoundException, InternalServiceErrorException, InvalidParameterException, AwsServiceException, SdkClientException, SecretsManagerException { + return ListSecretVersionIdsResponse.builder() + .name(request.secretId()) + .versions(secrets.getOrDefault(toKey(request.secretId()), List.of()).stream() + .map(version -> SecretVersionsListEntry.builder() + .versionId(version.version()) + .versionStages(List.of(toAwsStage(version.state()))) + .build()) + .toList()) + .build(); + } + + private String toAwsStage(SecretVersionState state) { + return switch(state) { + case CURRENT -> "AWSCURRENT"; + case PENDING -> "AWSPENDING"; + case PREVIOUS -> "AWSPREVIOUS"; + default -> throw new IllegalArgumentException("Unknown state: " + state); + }; + } + + @Override + public String serviceName() { + return MockSecretsManagerClient.class.getSimpleName(); + } + + @Override + public void close() { + isClosed = true; + } + } + +} From 8d9fa157eb4396c1e1e44fc23b12953811bea7a1 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 23 Sep 2024 14:46:51 +0200 Subject: [PATCH 6/8] Don't seal abstract class with implementation in another reop --- .../src/main/java/ai/vespa/secret/aws/AsmSecretStoreBase.java | 4 +--- .../src/main/resources/{ => configdefinitions}/asm-secret.def | 0 2 files changed, 1 insertion(+), 3 deletions(-) rename jdisc-cloud-aws/src/main/resources/{ => configdefinitions}/asm-secret.def (100%) diff --git a/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStoreBase.java b/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStoreBase.java index 72ddc8c8918b..6c0ebc734eb2 100644 --- a/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStoreBase.java +++ b/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/AsmSecretStoreBase.java @@ -23,9 +23,7 @@ * * @author gjoranv */ -public sealed abstract class AsmSecretStoreBase extends AbstractComponent - implements AutoCloseable - permits AsmSecretStore, AsmSecretStoreWriter { +public abstract class AsmSecretStoreBase extends AbstractComponent implements AutoCloseable { public static final String AWSCURRENT = "AWSCURRENT"; diff --git a/jdisc-cloud-aws/src/main/resources/asm-secret.def b/jdisc-cloud-aws/src/main/resources/configdefinitions/asm-secret.def similarity index 100% rename from jdisc-cloud-aws/src/main/resources/asm-secret.def rename to jdisc-cloud-aws/src/main/resources/configdefinitions/asm-secret.def From 97a4f549dc8d1f169ebc76a60c24ba6dbe071a55 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 23 Sep 2024 17:28:18 +0200 Subject: [PATCH 7/8] Move SecretsImpl to jdisc-cloud-aws --- .../model/container/xml/CloudSecrets.java | 4 +- .../java/ai/vespa/secret/aws/SecretsImpl.java | 53 ++++++++ .../ai/vespa/secret/aws/SecretsImplTest.java | 120 ++++++++++++++++++ 3 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/SecretsImpl.java create mode 100644 jdisc-cloud-aws/src/test/java/ai/vespa/secret/aws/SecretsImplTest.java diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudSecrets.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudSecrets.java index 354bb024dad5..50f499766795 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudSecrets.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudSecrets.java @@ -14,8 +14,8 @@ */ public class CloudSecrets extends SimpleComponent implements SecretsConfig.Producer { - private static final String CLASS = "ai.vespa.secret.cloud.SecretsImpl"; - private static final String BUNDLE = "cloud-common"; + private static final String CLASS = "ai.vespa.secret.aws.SecretsImpl"; + private static final String BUNDLE = "jdisc-cloud-aws"; private final List secrets = new ArrayList<>(); diff --git a/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/SecretsImpl.java b/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/SecretsImpl.java new file mode 100644 index 000000000000..1cd02caefd86 --- /dev/null +++ b/jdisc-cloud-aws/src/main/java/ai/vespa/secret/aws/SecretsImpl.java @@ -0,0 +1,53 @@ +package ai.vespa.secret.aws; + +import ai.vespa.secret.Secret; +import ai.vespa.secret.Secrets; +import ai.vespa.secret.config.SecretsConfig; +import ai.vespa.secret.internal.TypedSecretStore; +import ai.vespa.secret.model.Key; +import ai.vespa.secret.model.SecretName; +import ai.vespa.secret.model.VaultName; + +import javax.inject.Inject; + +/** + * Implementation of the {@link Secrets} interface for Vespa cloud. + * + * @author lesters + */ +public class SecretsImpl implements Secrets { + + private final SecretsConfig secretsConfig; + private final TypedSecretStore secretStore; + + @Inject + public SecretsImpl(SecretsConfig config, AsmSecretStore asmSecretStore) { + this.secretStore = asmSecretStore; + this.secretsConfig = config; + } + + // For testing + SecretsImpl(SecretsConfig secretsConfig, TypedSecretStore secretStore) { + this.secretsConfig = secretsConfig; + this.secretStore = secretStore; + } + + @Override + public Secret get(String key) { + SecretsConfig.Secret secretConfig = secretsConfig.secret(key); + if (secretConfig == null) { + throw new IllegalArgumentException("Secret with key '" + key + "' not found in secrets config"); + } + + VaultName vaultName = VaultName.of(secretConfig.vault()); + SecretName secretName = SecretName.of(secretConfig.name()); + + var secret = secretStore.getSecret(new Key(vaultName, secretName)); + if (secret == null) { + throw new IllegalArgumentException("Secret with key '" + key + "' not found in secret store"); + } + + return secret::secretAsString; + } + +} diff --git a/jdisc-cloud-aws/src/test/java/ai/vespa/secret/aws/SecretsImplTest.java b/jdisc-cloud-aws/src/test/java/ai/vespa/secret/aws/SecretsImplTest.java new file mode 100644 index 000000000000..225d4d86fcd7 --- /dev/null +++ b/jdisc-cloud-aws/src/test/java/ai/vespa/secret/aws/SecretsImplTest.java @@ -0,0 +1,120 @@ +package ai.vespa.secret.aws; + +import ai.vespa.secret.config.SecretsConfig; +import ai.vespa.secret.internal.TypedSecretStore; +import ai.vespa.secret.model.Key; +import ai.vespa.secret.model.Secret; +import ai.vespa.secret.model.SecretVersionId; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +public class SecretsImplTest { + + private record SecretConfig(String key, String name, String vault) {} + + private final static Map secretsInVault = Map.of( + "my-api-key", "0123456789", + "another-api-key", "9876543210" + ); + + private final static List secretsConfig = List.of( + new SecretConfig("myApiKey", "my-api-key", "prod"), + new SecretConfig("anotherApiKey", "another-api-key", "prod"), + new SecretConfig("mySecret", "my-secret", "prod") // not in vault + ); + + private final static SecretsImpl secrets = createSecrets(); + + @Test + public void testSecretsCanBeRetrieved() { + assertEquals("0123456789", secrets.get("myApiKey").current()); + assertEquals("9876543210", secrets.get("anotherApiKey").current()); + } + + @Test + public void testThrowOnUnknownSecrets() { + try { + secrets.get("unknown"); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Secret with key 'unknown' not found in secrets config", e.getMessage()); + } + } + + @Test + public void testSecretInConfigButNotInVault() { + try { + secrets.get("mySecret"); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Secret with key 'mySecret' not found in secret store", e.getMessage()); + } + } + + private static SecretsImpl createSecrets() { + var config = createSecretsConfig(); + var secretStore = createSecretStore(); + return new SecretsImpl(config, secretStore); + } + + private static SecretsConfig createSecretsConfig() { + SecretsConfig.Builder builder = new SecretsConfig.Builder(); + secretsConfig.forEach(secret -> + builder.secret(secret.key(), new SecretsConfig.Secret.Builder().name(secret.name()).vault(secret.vault())) + ); + return builder.build(); + } + + private static TypedSecretStore createSecretStore() { + var secretStore = new MockSecretStore(); + secretsInVault.forEach((k, v) -> { + var key = new Key("prod", k); + var secretValue = v.getBytes(); + var version = new SecretVersionId("1"); + secretStore.putSecret(key, new Secret(key, secretValue, version)); + }); + return secretStore; + } + + private static class MockSecretStore implements TypedSecretStore { + + private Map secrets = new HashMap<>(); + + public void putSecret(Key key, Secret secret) { + secrets.put(key, secret); + } + + @Override + public Secret getSecret(Key key, SecretVersionId version) { + return secrets.get(key); + } + + @Override + public Secret getSecret(Key key) { + return getSecret(key, null); + } + + @Override + public Type type() { + return Type.PUBLIC; + } + + @Override + public String getSecret(String key) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public String getSecret(String key, int version) { + throw new UnsupportedOperationException("Not implemented"); + } + } + + +} From d31a17112eee64cf2afb01caf93e3a24a21b914a Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 23 Sep 2024 17:36:50 +0200 Subject: [PATCH 8/8] Install jdisc-cloud-aws on controller/configserver --- jdisc-cloud-aws/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdisc-cloud-aws/CMakeLists.txt b/jdisc-cloud-aws/CMakeLists.txt index 7d7a79eb7bd6..b760315caa16 100644 --- a/jdisc-cloud-aws/CMakeLists.txt +++ b/jdisc-cloud-aws/CMakeLists.txt @@ -1,3 +1,3 @@ # Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -install_jar(jdisc-cloud-aws-jar-with-dependencies.jar) +install_configserver_component(jdisc-cloud-aws) install_config_definitions()