Reference secrets by name, do not inject

Warehouses/object-stores require secrets (access-keys, tokens, etc). These secrets are currently injected into the object-store specific configuration holders, when the effective bucket configuration is calculated. The configuration of warehouses/object-stores should be distinct from the definition of secrets. This requires to not have the secrets in a bucket configuration at any time, but instead _reference_ secrets by name. These names are then resolved to secrets. This change replaces the `Secret` types in all bucket configuration types with a `String` as the _reference_ to the secret. Also implicitly refactors most of `:nessie-catalog-secrets-api` module.
projectnessie · Aug 27, 2024 · f50537d · f50537d
1 parent a5419b5
commit f50537d
Show file tree

Hide file tree

Showing 84 changed files with 1,808 additions and 1,287 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -823,18 +823,33 @@ jobs:
         run: |
           ct list-changed --target-branch ${{ github.event.repository.default_branch }}
 
+      - name: Run 'helm template' validation
+        run: |
+          cd helm/nessie
+          for f in values.yaml ci/*.yaml; do
+            echo "::group::helm template $f"
+            helm template --debug --namespace nessie-ns --values $f .
+            echo "::endgroup::"
+          done
+
       - name: Run chart-testing (lint)
         run: ct lint --debug --charts ./helm/nessie
 
       - name: Show pods
         run: kubectl get pods -A
 
+      - name: Install secrets
+        run: |
+          kubectl create namespace nessie-ns
+          kubectl apply --namespace nessie-ns $(find helm/nessie/ci/secrets -name "*.yaml" -printf '-f %p ')
+
       - name: Run chart-testing (install)
         run: |
           echo "Using image: ${DOCKER_IMAGE}"
           echo "        tag: ${DOCKER_VERSION}"
-      
+
           ct install \
+            --namespace nessie-ns \
             --helm-extra-set-args "--set=image.repository=${DOCKER_IMAGE} --set=image.tag=${DOCKER_VERSION}" \
             --debug --charts ./helm/nessie
 

diff --git a/bom/build.gradle.kts b/bom/build.gradle.kts
@@ -57,6 +57,7 @@ dependencies {
     api(project(":nessie-quarkus-ext-deployment"))
     api(project(":nessie-quarkus-ext"))
     api(project(":nessie-quarkus-rest"))
+    api(project(":nessie-quarkus-secrets"))
     api(project(":nessie-server-admin-tool"))
     api(project(":nessie-quarkus"))
     api(project(":nessie-quarkus-tests"))

diff --git a/...files/impl/src/jmh/java/org/projectnessie/catalog/files/adls/AdlsClientResourceBench.java b/...files/impl/src/jmh/java/org/projectnessie/catalog/files/adls/AdlsClientResourceBench.java
@@ -20,12 +20,12 @@
 import static org.projectnessie.catalog.files.BenchUtils.mockServer;
 import static org.projectnessie.catalog.secrets.BasicCredentials.basicCredentials;
 import static org.projectnessie.catalog.secrets.KeySecret.keySecret;
+import static org.projectnessie.catalog.secrets.UnsafePlainTextSecretsProvider.unsafePlainTextSecretsProvider;
 
 import com.azure.core.http.HttpClient;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Map;
-import java.util.stream.Collectors;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
 import org.openjdk.jmh.annotations.Fork;
@@ -64,24 +64,23 @@ public void init() {
       AdlsConfig adlsConfig = AdlsConfig.builder().build();
       HttpClient httpClient = AdlsClients.buildSharedHttpClient(adlsConfig);
 
+      SecretsProvider secretsProvider =
+          unsafePlainTextSecretsProvider(
+              Map.of(
+                  "the-account", basicCredentials("foo", "foo").asMap(),
+                  "the-key", keySecret("foo").asMap()));
+
       AdlsProgrammaticOptions adlsOptions =
           ImmutableAdlsProgrammaticOptions.builder()
               .defaultOptions(
                   ImmutableAdlsNamedFileSystemOptions.builder()
-                      .account(basicCredentials("foo", "foo"))
-                      .sasToken(keySecret("foo"))
+                      .account("the-account")
+                      .sasToken("the-key")
                       .endpoint(server.getAdlsGen2BaseUri().toString())
                       .build())
               .build();
 
-      clientSupplier =
-          new AdlsClientSupplier(
-              httpClient,
-              adlsOptions,
-              new SecretsProvider(
-                  (names) ->
-                      names.stream()
-                          .collect(Collectors.toMap(k -> k, k -> Map.of("secret", "secret")))));
+      clientSupplier = new AdlsClientSupplier(httpClient, adlsOptions, secretsProvider);
     }
 
     @TearDown

diff --git a/...g/files/impl/src/jmh/java/org/projectnessie/catalog/files/gcs/GcsClientResourceBench.java b/...g/files/impl/src/jmh/java/org/projectnessie/catalog/files/gcs/GcsClientResourceBench.java
@@ -19,12 +19,13 @@
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.projectnessie.catalog.files.BenchUtils.mockServer;
 import static org.projectnessie.catalog.files.gcs.GcsLocation.gcsLocation;
+import static org.projectnessie.catalog.secrets.TokenSecret.tokenSecret;
+import static org.projectnessie.catalog.secrets.UnsafePlainTextSecretsProvider.unsafePlainTextSecretsProvider;
 
 import com.google.auth.http.HttpTransportFactory;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Map;
-import java.util.stream.Collectors;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
 import org.openjdk.jmh.annotations.Fork;
@@ -39,7 +40,6 @@
 import org.openjdk.jmh.annotations.Warmup;
 import org.openjdk.jmh.infra.Blackhole;
 import org.projectnessie.catalog.secrets.SecretsProvider;
-import org.projectnessie.catalog.secrets.TokenSecret;
 import org.projectnessie.objectstoragemock.ObjectStorageMock;
 import org.projectnessie.storage.uri.StorageUri;
 
@@ -63,23 +63,19 @@ public void init() {
 
       HttpTransportFactory httpTransportFactory = GcsClients.buildSharedHttpTransportFactory();
 
+      SecretsProvider secretsProvider =
+          unsafePlainTextSecretsProvider(Map.of("the-token", tokenSecret("foo", null).asMap()));
+
       GcsProgrammaticOptions gcsOptions =
           ImmutableGcsProgrammaticOptions.builder()
               .defaultOptions(
                   ImmutableGcsNamedBucketOptions.builder()
-                      .oauth2Token(TokenSecret.tokenSecret("foo", null))
+                      .oauth2Token("the-token")
                       .host(server.getGcsBaseUri())
                       .build())
               .build();
 
-      storageSupplier =
-          new GcsStorageSupplier(
-              httpTransportFactory,
-              gcsOptions,
-              new SecretsProvider(
-                  (names) ->
-                      names.stream()
-                          .collect(Collectors.toMap(k -> k, k -> Map.of("secret", "secret")))));
+      storageSupplier = new GcsStorageSupplier(httpTransportFactory, gcsOptions, secretsProvider);
     }
 
     @TearDown

diff --git a/...log/files/impl/src/jmh/java/org/projectnessie/catalog/files/s3/S3ClientResourceBench.java b/...log/files/impl/src/jmh/java/org/projectnessie/catalog/files/s3/S3ClientResourceBench.java
@@ -19,11 +19,11 @@
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.projectnessie.catalog.files.BenchUtils.mockServer;
 import static org.projectnessie.catalog.secrets.BasicCredentials.basicCredentials;
+import static org.projectnessie.catalog.secrets.UnsafePlainTextSecretsProvider.unsafePlainTextSecretsProvider;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Map;
-import java.util.stream.Collectors;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
 import org.openjdk.jmh.annotations.Fork;
@@ -61,14 +61,18 @@ public static class BenchmarkParam {
     public void init() {
       server = mockServer(mock -> {});
 
+      SecretsProvider secretsProvider =
+          unsafePlainTextSecretsProvider(
+              Map.of("the-access-key", basicCredentials("foo", "bar").asMap()));
+
       S3Config s3config = S3Config.builder().build();
-      httpClient = S3Clients.apacheHttpClient(s3config, new SecretsProvider(names -> Map.of()));
+      httpClient = S3Clients.apacheHttpClient(s3config, secretsProvider);
 
       S3ProgrammaticOptions s3options =
           ImmutableS3ProgrammaticOptions.builder()
               .defaultOptions(
                   ImmutableS3NamedBucketOptions.builder()
-                      .accessKey(basicCredentials("foo", "bar"))
+                      .accessKey("the-access-key")
                       .region("eu-central-1")
                       .endpoint(server.getS3BaseUri())
                       .pathStyleAccess(true)
@@ -77,15 +81,7 @@ public void init() {
 
       S3Sessions sessions = new S3Sessions("foo", null);
 
-      clientSupplier =
-          new S3ClientSupplier(
-              httpClient,
-              s3options,
-              new SecretsProvider(
-                  (names) ->
-                      names.stream()
-                          .collect(Collectors.toMap(k -> k, k -> Map.of("secret", "secret")))),
-              sessions);
+      clientSupplier = new S3ClientSupplier(httpClient, s3options, sessions, secretsProvider);
     }
 
     @TearDown

diff --git a/...les/impl/src/jmh/java/org/projectnessie/catalog/files/s3/S3SessionCacheResourceBench.java b/...les/impl/src/jmh/java/org/projectnessie/catalog/files/s3/S3SessionCacheResourceBench.java
@@ -20,8 +20,10 @@
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.projectnessie.catalog.files.BenchUtils.mockServer;
 import static org.projectnessie.catalog.secrets.BasicCredentials.basicCredentials;
+import static org.projectnessie.catalog.secrets.UnsafePlainTextSecretsProvider.unsafePlainTextSecretsProvider;
 
 import java.net.URI;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ThreadLocalRandom;
@@ -72,21 +74,29 @@ public static class BenchmarkParam {
     public void init() {
       server = mockServer(mock -> {});
 
+      Map<String, Map<String, String>> secretsMap = new HashMap<>();
+      for (int i = 0; i < numBucketOptions; i++) {
+        secretsMap.put("access-key-" + i, basicCredentials("foo" + i, "bar" + 1).asMap());
+      }
+      secretsMap.put("the-access-key", basicCredentials("foo", "bar").asMap());
+      SecretsProvider secretsProvider = unsafePlainTextSecretsProvider(secretsMap);
+
       S3Config s3config = S3Config.builder().build();
-      httpClient = S3Clients.apacheHttpClient(s3config, new SecretsProvider(names -> Map.of()));
+      httpClient = S3Clients.apacheHttpClient(s3config, secretsProvider);
 
       S3Options s3options =
           ImmutableS3ProgrammaticOptions.builder()
               .defaultOptions(
                   ImmutableS3NamedBucketOptions.builder()
-                      .accessKey(basicCredentials("foo", "bar"))
+                      .accessKey("the-access-key")
                       .region("eu-central-1")
                       .pathStyleAccess(true)
                       .build())
               .build();
 
       StsClientsPool stsClientsPool = new StsClientsPool(s3options, httpClient, null);
-      stsCredentialsManager = new StsCredentialsManager(s3options, stsClientsPool, null);
+      stsCredentialsManager =
+          new StsCredentialsManager(s3options, stsClientsPool, secretsProvider, null);
 
       List<String> regions =
           Region.regions().stream()
@@ -100,7 +110,7 @@ public void init() {
               .mapToObj(
                   i ->
                       ImmutableS3NamedBucketOptions.builder()
-                          .accessKey(basicCredentials("foo" + i, "bar" + 1))
+                          .accessKey("access-key-" + 1)
                           .region(regions.get(i % regions.size()))
                           .stsEndpoint(stsEndpoint)
                           .clientIam(

diff --git a/...log/files/impl/src/main/java/org/projectnessie/catalog/files/adls/AdlsClientSupplier.java b/...log/files/impl/src/main/java/org/projectnessie/catalog/files/adls/AdlsClientSupplier.java
@@ -51,6 +51,8 @@
 import org.projectnessie.catalog.files.adls.AdlsFileSystemOptions.AzureAuthType;
 import org.projectnessie.catalog.files.api.StorageLocations;
 import org.projectnessie.catalog.secrets.BasicCredentials;
+import org.projectnessie.catalog.secrets.KeySecret;
+import org.projectnessie.catalog.secrets.SecretType;
 import org.projectnessie.catalog.secrets.SecretsProvider;
 import org.projectnessie.storage.uri.StorageUri;
 import org.slf4j.Logger;
@@ -94,7 +96,7 @@ DataLakeFileSystemClient fileSystemClient(AdlsLocation location) {
     Configuration clientConfig = buildClientConfiguration();
 
     AdlsFileSystemOptions fileSystemOptions =
-        adlsOptions.effectiveOptionsForFileSystem(location.container(), secretsProvider);
+        adlsOptions.effectiveOptionsForFileSystem(location.container());
 
     String endpoint = endpointForLocation(location, fileSystemOptions);
 
@@ -229,6 +231,12 @@ private boolean applyCredentials(
         BasicCredentials account =
             fileSystemOptions
                 .account()
+                .map(
+                    secretName ->
+                        secretsProvider.getSecret(
+                            secretName, SecretType.BASIC, BasicCredentials.class))
+                .filter(Optional::isPresent)
+                .map(Optional::get)
                 .orElseThrow(() -> new IllegalStateException("storage shared key missing"));
 
         sharedKeyCredentialConsumer.accept(
@@ -238,6 +246,11 @@ private boolean applyCredentials(
         sasTokenConsumer.accept(
             fileSystemOptions
                 .sasToken()
+                .map(
+                    secretName ->
+                        secretsProvider.getSecret(secretName, SecretType.KEY, KeySecret.class))
+                .filter(Optional::isPresent)
+                .map(Optional::get)
                 .orElseThrow(() -> new IllegalStateException("SAS token missing"))
                 .key());
         return true;

diff --git a/.../files/impl/src/main/java/org/projectnessie/catalog/files/adls/AdlsFileSystemOptions.java b/.../files/impl/src/main/java/org/projectnessie/catalog/files/adls/AdlsFileSystemOptions.java
@@ -18,8 +18,6 @@
 import java.time.Duration;
 import java.time.temporal.ChronoUnit;
 import java.util.Optional;
-import org.projectnessie.catalog.secrets.BasicCredentials;
-import org.projectnessie.catalog.secrets.KeySecret;
 
 public interface AdlsFileSystemOptions {
 
@@ -30,14 +28,14 @@ public interface AdlsFileSystemOptions {
   Optional<AzureAuthType> authType();
 
   /**
-   * Fully-qualified account name, e.g. {@code "myaccount.dfs.core.windows.net"} and account key,
-   * configured using the {@code name} and {@code secret} fields. If not specified, it will be
-   * queried via the configured credentials provider.
+   * Name of the basic-credentials secret containing the fully-qualified account name, e.g. {@code
+   * "myaccount.dfs.core.windows.net"} and account key, configured using the {@code name} and {@code
+   * secret} fields. If not specified, it will be queried via the configured credentials provider.
    */
-  Optional<BasicCredentials> account();
+  Optional<String> account();
 
-  /** SAS token to access the ADLS file system. */
-  Optional<KeySecret> sasToken();
+  /** Name of the key-secret containing the SAS token to access the ADLS file system. */
+  Optional<String> sasToken();
 
   /**
    * Enable short-lived user-delegation SAS tokens per file-system.

diff --git a/catalog/files/impl/src/main/java/org/projectnessie/catalog/files/adls/AdlsObjectIO.java b/catalog/files/impl/src/main/java/org/projectnessie/catalog/files/adls/AdlsObjectIO.java
@@ -187,8 +187,7 @@ void icebergConfigOverrides(
     String storageAccount = storageAccounts.iterator().next();
 
     AdlsOptions adlsOptions = clientSupplier.adlsOptions();
-    AdlsFileSystemOptions fileSystemOptions =
-        adlsOptions.effectiveOptionsForFileSystem(fileSystem, clientSupplier.secretsProvider());
+    AdlsFileSystemOptions fileSystemOptions = adlsOptions.effectiveOptionsForFileSystem(fileSystem);
     fileSystemOptions
         .endpoint()
         .ifPresent(e -> config.accept(ADLS_CONNECTION_STRING_PREFIX + storageAccount, e));