fix test failures and review comments

helidon-io · Jun 26, 2023 · 3b508e0 · 3b508e0
1 parent 4f46f6a
commit 3b508e0
Show file tree

Hide file tree

Showing 9 changed files with 172 additions and 177 deletions.
diff --git a/...rations/vault/hcp/src/main/java/io/helidon/examples/integrations/vault/hcp/VaultMain.java b/...rations/vault/hcp/src/main/java/io/helidon/examples/integrations/vault/hcp/VaultMain.java
@@ -18,7 +18,6 @@
 
 import java.time.Duration;
 
-import io.helidon.common.socket.SocketOptions;
 import io.helidon.config.Config;
 import io.helidon.integrations.vault.Vault;
 import io.helidon.integrations.vault.secrets.cubbyhole.CubbyholeSecrets;
@@ -57,11 +56,8 @@ public static void main(String[] args) {
         // 1. Token based authentication
         Vault tokenVault = Vault.builder()
                 .config(config.get("vault.token"))
-                .updateWebClient(client -> client.channelOptions(
-                        SocketOptions.builder()
-                                .connectTimeout(Duration.ofSeconds(5))
-                                .readTimeout(Duration.ofSeconds(5))
-                                .build()))
+                .updateWebClient(it -> it.connectTimeout(Duration.ofSeconds(5))
+                        .readTimeout(Duration.ofSeconds(5)))
                 .build();
 
         // 2. App role based authentication - must be created after we obtain the role id an token

diff --git a/integrations/common/rest/src/main/java/io/helidon/integrations/common/rest/RestApi.java b/integrations/common/rest/src/main/java/io/helidon/integrations/common/rest/RestApi.java
@@ -262,8 +262,9 @@ <R, T extends ApiOptionalResponse<R>> T invokeOptional(Method method,
      * @param <T> type of the built {@link io.helidon.integrations.common.rest.RestApi}
      */
     abstract class Builder<B extends Builder<B, T>, T extends RestApi> implements io.helidon.common.Builder<B, T> {
-        private final Http1ClientBuilder webClientBuilder = Http1Client.builder().followRedirect(true).keepAlive(true);
-
+        private final Http1ClientBuilder webClientBuilder = Http1Client.builder()
+                .followRedirect(true)
+                .keepAlive(true);
         private FtHandler ftHandler = FaultTolerance.builder().build();
         private JsonBuilderFactory jsonBuilderFactory;
         private JsonReaderFactory jsonReaderFactory;

diff --git a/...ons/vault/auths/k8s/src/main/java/io/helidon/integrations/vault/auths/k8s/K8sRestApi.java b/...ons/vault/auths/k8s/src/main/java/io/helidon/integrations/vault/auths/k8s/K8sRestApi.java
@@ -27,7 +27,7 @@
 
 class K8sRestApi extends VaultRestApi {
 
-    private static final Http.HeaderName VAULT_TOKEN_HEADER_NAME =  Http.Header.create("X-Vault-Token");
+    private static final Http.HeaderName VAULT_TOKEN_HEADER_NAME = Http.Header.create("X-Vault-Token");
     private final AtomicReference<VaultTokenBase> currentToken = new AtomicReference<>();
 
     private final K8sAuth auth;
@@ -56,10 +56,7 @@ protected Http1ClientRequest updateRequestBuilderCommon(Http1ClientRequest reque
 
         if (k8sToken != null) {
             if (!k8sToken.renewable() || k8sToken.created().plus(k8sToken.leaseDuration()).isAfter(Instant.now())) {
-                requestBuilder.headers(headers -> {
-                    headers.add(VAULT_TOKEN_HEADER_NAME, k8sToken.token());
-                    return headers;
-                });
+                requestBuilder.header(VAULT_TOKEN_HEADER_NAME, k8sToken.token());
                 return requestBuilder;
             }
         }
@@ -68,10 +65,7 @@ protected Http1ClientRequest updateRequestBuilderCommon(Http1ClientRequest reque
         Login.Response response = auth.login(Login.Request.create(roleName, jwtToken));
         VaultTokenBase token = response.token();
         currentToken.set(token);
-        requestBuilder.headers(headers -> {
-            headers.add(VAULT_TOKEN_HEADER_NAME, token.token());
-            return headers;
-        });
+        requestBuilder.header(VAULT_TOKEN_HEADER_NAME, token.token());
         return requestBuilder;
     }
 

diff --git a/...s/vault/auths/k8s/src/main/java/io/helidon/integrations/vault/auths/k8s/K8sVaultAuth.java b/...s/vault/auths/k8s/src/main/java/io/helidon/integrations/vault/auths/k8s/K8sVaultAuth.java
@@ -84,17 +84,18 @@ public Optional<RestApi> authenticate(Config config, Vault.Builder vaultBuilder)
 
         String jwtToken;
         if (this.serviceAccountToken == null) {
-            Optional<String> maybeToken = config.get("auth.k8s.service-account-token").asString().or(() -> {
-                Path tokenPath = Paths.get(tokenLocation);
-                if (!Files.exists(tokenPath)) {
-                    return Optional.empty();
-                }
-                try {
-                    return Optional.of(Files.readString(tokenPath));
-                } catch (IOException e) {
-                    throw new VaultApiException("Failed to read token from " + tokenPath.toAbsolutePath(), e);
-                }
-            });
+            Optional<String> maybeToken = config.get("auth.k8s.service-account-token").asString()
+                    .or(() -> {
+                        Path tokenPath = Paths.get(tokenLocation);
+                        if (!Files.exists(tokenPath)) {
+                            return Optional.empty();
+                        }
+                        try {
+                            return Optional.of(Files.readString(tokenPath));
+                        } catch (IOException e) {
+                            throw new VaultApiException("Failed to read token from " + tokenPath.toAbsolutePath(), e);
+                        }
+                    });
 
             if (maybeToken.isEmpty()) {
                 return Optional.empty();
@@ -105,36 +106,36 @@ public Optional<RestApi> authenticate(Config config, Vault.Builder vaultBuilder)
         }
 
         String roleName = Optional.ofNullable(this.tokenRole)
-                                  .or(() -> config.get("auth.k8s.token-role")
-                                                  .asString()
-                                                  .asOptional())
-                                  .orElseThrow(() -> new VaultApiException(
-                                          "Token role must be defined when using Kubernetes vault authentication."));
+                .or(() -> config.get("auth.k8s.token-role")
+                        .asString()
+                        .asOptional())
+                .orElseThrow(() -> new VaultApiException(
+                        "Token role must be defined when using Kubernetes vault authentication."));
 
         // this may be changed in the future, when running with a sidecar (there should be a way to get the address from evn)
         String address = vaultBuilder.address()
-                                     .orElseThrow(() -> new VaultApiException(
-                                             "Address is required when using k8s authentication"));
+                .orElseThrow(() -> new VaultApiException(
+                        "Address is required when using k8s authentication"));
 
         // explicitly use default
         Vault.Builder loginVaultBuilder = Vault.builder()
-                                               .address(address)
-                                               .disableVaultAuthDiscovery()
-                                               .faultTolerance(vaultBuilder.ftHandler())
-                                               .updateWebClient(it -> vaultBuilder.webClientUpdater().accept(it))
-                                               .addVaultAuth(NoVaultAuth.create());
+                .address(address)
+                .disableVaultAuthDiscovery()
+                .faultTolerance(vaultBuilder.ftHandler())
+                .updateWebClient(it -> vaultBuilder.webClientUpdater().accept(it))
+                .addVaultAuth(NoVaultAuth.create());
 
         vaultBuilder.baseNamespace().ifPresent(loginVaultBuilder::baseNamespace);
 
         Vault loginVault = loginVaultBuilder.build();
         String methodPath = Optional.ofNullable(this.methodPath)
-                                    .orElseGet(() -> config.get("auth.k8s.path")
-                                                           .asString()
-                                                           .orElse(K8sAuth.AUTH_METHOD.defaultPath()));
+                .orElseGet(() -> config.get("auth.k8s.path")
+                        .asString()
+                        .orElse(K8sAuth.AUTH_METHOD.defaultPath()));
 
         LOGGER.log(Level.INFO,
-                "Authenticated Vault {0}/{1} using k8s, role \"{2}\"",
-                address, methodPath, roleName);
+                   "Authenticated Vault {0}/{1} using k8s, role \"{2}\"",
+                   address, methodPath, roleName);
 
         return Optional.of(K8sRestApi.k8sBuilder()
                                    .webClientBuilder(webclient -> {

diff --git a/...e/src/main/java/io/helidon/integrations/vault/secrets/cubbyhole/CubbyholeSecretsImpl.java b/...e/src/main/java/io/helidon/integrations/vault/secrets/cubbyhole/CubbyholeSecretsImpl.java
@@ -16,8 +16,11 @@
 
 package io.helidon.integrations.vault.secrets.cubbyhole;
 
+import java.util.Optional;
+
 import io.helidon.integrations.common.rest.RestApi;
 import io.helidon.integrations.vault.ListSecrets;
+import io.helidon.integrations.vault.Secret;
 import io.helidon.integrations.vault.Vault;
 import io.helidon.integrations.vault.VaultApiException;
 import io.helidon.integrations.vault.VaultOptionalResponse;
@@ -59,26 +62,28 @@ public VaultOptionalResponse<ListSecrets.Response> list(ListSecrets.Request requ
     public CreateCubbyhole.Response create(CreateCubbyhole.Request request) throws VaultRestException {
         String path = request.path();
 
-        return get(path)
-                .map(secret -> {
-                    String apiPath = mount + "/" + path;
-                    return restApi.post(apiPath, request, CreateCubbyhole.Response.builder());
-                }).orElseThrow(() -> new VaultApiException(
-                        "Cannot create a secret that already exists on path: \"%s\", please use update",
-                        path));
+        Optional<Secret> secret = get(path);
+        if (secret.isPresent()) {
+            throw new VaultApiException(
+                    "Cannot create a secret that already exists on path: \"%s\", please use update",
+                    path);
+        }
+        String apiPath = mount + "/" + path;
+        return restApi.post(apiPath, request, CreateCubbyhole.Response.builder());
     }
 
     @Override
     public UpdateCubbyhole.Response update(UpdateCubbyhole.Request request) {
         String path = request.path();
 
-        return get(path)
-                .map(secret -> {
-                    String apiPath = mount + "/" + path;
-                    return restApi.put(apiPath, request, UpdateCubbyhole.Response.builder());
-                }).orElseThrow(() -> new VaultApiException(
-                        "Cannot update a secret that does not exist on path: \"%s\", please use create",
-                        path));
+        Optional<Secret> secret = get(path);
+        if (secret.isEmpty()) {
+            throw new VaultApiException(
+                    "Cannot update a secret that does not exist on path: \"%s\", please use create",
+                    path);
+        }
+        String apiPath = mount + "/" + path;
+        return restApi.put(apiPath, request, UpdateCubbyhole.Response.builder());
     }
 
     @Override

diff --git a/...ecrets/kv1/src/main/java/io/helidon/integrations/vault/secrets/kv1/Kv1EngineProvider.java b/...ecrets/kv1/src/main/java/io/helidon/integrations/vault/secrets/kv1/Kv1EngineProvider.java
@@ -36,15 +36,9 @@ public class Kv1EngineProvider implements SecretsEngineProvider<Kv1Secrets>,
         List<InjectionType<?>> injectables = new LinkedList<>();
 
         injectables.add(InjectionType.create(Kv1Secrets.class,
-                (vault, config, instanceConfig) -> instanceConfig.vaultPath()
-                                                                 .map(it -> vault.secrets(Kv1Secrets.ENGINE, it))
-                                                                 .orElseGet(() -> vault.secrets(Kv1Secrets.ENGINE))));
-
-        injectables.add(InjectionType.create(Kv1Secrets.class,
-                (vault, config, instanceConfig) ->
-                        instanceConfig.vaultPath()
-                                      .map(it -> vault.secrets(Kv1Secrets.ENGINE, it))
-                                      .orElseGet(() -> vault.secrets(Kv1Secrets.ENGINE))));
+                                             (vault, config, instanceConfig) -> instanceConfig.vaultPath()
+                                                     .map(it -> vault.secrets(Kv1Secrets.ENGINE, it))
+                                                     .orElseGet(() -> vault.secrets(Kv1Secrets.ENGINE))));
 
         INJECTABLES = List.copyOf(injectables);
     }

diff --git a/...t/secrets/kv1/src/main/java/io/helidon/integrations/vault/secrets/kv1/Kv1SecretsImpl.java b/...t/secrets/kv1/src/main/java/io/helidon/integrations/vault/secrets/kv1/Kv1SecretsImpl.java
@@ -16,8 +16,11 @@
 
 package io.helidon.integrations.vault.secrets.kv1;
 
+import java.util.Optional;
+
 import io.helidon.integrations.common.rest.RestApi;
 import io.helidon.integrations.vault.ListSecrets;
+import io.helidon.integrations.vault.Secret;
 import io.helidon.integrations.vault.Vault;
 import io.helidon.integrations.vault.VaultApiException;
 import io.helidon.integrations.vault.VaultOptionalResponse;
@@ -38,10 +41,10 @@ public VaultOptionalResponse<ListSecrets.Response> list(ListSecrets.Request requ
         String apiPath = mount + "/" + request.path().orElse("");
 
         return restApi.invokeOptional(Vault.LIST,
-                apiPath,
-                request,
-                VaultOptionalResponse.<ListSecrets.Response, JsonObject>vaultResponseBuilder()
-                                     .entityProcessor(ListSecrets.Response::create));
+                                      apiPath,
+                                      request,
+                                      VaultOptionalResponse.<ListSecrets.Response, JsonObject>vaultResponseBuilder()
+                                              .entityProcessor(ListSecrets.Response::create));
     }
 
     @Override
@@ -50,34 +53,36 @@ public VaultOptionalResponse<GetKv1.Response> get(GetKv1.Request request) {
         String apiPath = mount + "/" + path;
 
         return restApi.get(apiPath, request, VaultOptionalResponse.<GetKv1.Response, JsonObject>vaultResponseBuilder()
-                                                                  .entityProcessor(it -> GetKv1.Response.create(path, it)));
+                .entityProcessor(it -> GetKv1.Response.create(path, it)));
     }
 
     @Override
     public CreateKv1.Response create(CreateKv1.Request request) {
         String path = request.path();
 
-        return get(path)
-                .map(secret -> {
-                    String apiPath = mount + "/" + path;
+        Optional<Secret> secret = get(path);
+        if (secret.isPresent()) {
+            throw new VaultApiException("Cannot create a secret that already exists on path: \""
+                                                + path
+                                                + "\", please use update");
+        }
+        String apiPath = mount + "/" + path;
 
-                    return restApi.post(apiPath, request, CreateKv1.Response.builder());
-                }).orElseThrow(() -> new VaultApiException(
-                        "Cannot create a secret that already exists on path: \"%s\", please use update",
-                        path));
+        return restApi.post(apiPath, request, CreateKv1.Response.builder());
     }
 
     @Override
     public UpdateKv1.Response update(UpdateKv1.Request request) {
         String path = request.path();
 
-        return get(path)
-                .map(secret -> {
-                    String apiPath = mount + "/" + path;
-                    return restApi.put(apiPath, request, UpdateKv1.Response.builder());
-                }).orElseThrow(() -> new VaultApiException(
-                        "Cannot update a secret that does not exist on path: \"%s\", please use create",
-                        path));
+        Optional<Secret> secret = get(path);
+        if (secret.isEmpty()) {
+            throw new VaultApiException(
+                    "Cannot update a secret that does not exist on path: \"%s\", please use create",
+                    path);
+        }
+        String apiPath = mount + "/" + path;
+        return restApi.put(apiPath, request, UpdateKv1.Response.builder());
     }
 
     @Override

diff --git a/...t/secrets/kv2/src/main/java/io/helidon/integrations/vault/secrets/kv2/Kv2SecretsImpl.java b/...t/secrets/kv2/src/main/java/io/helidon/integrations/vault/secrets/kv2/Kv2SecretsImpl.java
@@ -16,6 +16,8 @@
 
 package io.helidon.integrations.vault.secrets.kv2;
 
+import java.util.Optional;
+
 import io.helidon.common.http.Http;
 import io.helidon.integrations.common.rest.RestApi;
 import io.helidon.integrations.vault.ListSecrets;
@@ -40,10 +42,10 @@ public VaultOptionalResponse<ListSecrets.Response> list(ListSecrets.Request requ
 
         return restApi
                 .invokeOptional(Vault.LIST,
-                        apiPath,
-                        request,
-                        VaultOptionalResponse.<ListSecrets.Response, JsonObject>vaultResponseBuilder()
-                                             .entityProcessor(ListSecrets.Response::create));
+                                apiPath,
+                                request,
+                                VaultOptionalResponse.<ListSecrets.Response, JsonObject>vaultResponseBuilder()
+                                        .entityProcessor(ListSecrets.Response::create));
     }
 
     @Override
@@ -52,24 +54,25 @@ public VaultOptionalResponse<GetKv2.Response> get(GetKv2.Request request) {
         String apiPath = mount + "/data/" + path;
 
         return restApi.get(apiPath, request, VaultOptionalResponse.<GetKv2.Response, JsonObject>vaultResponseBuilder()
-                                                                  .entityProcessor(json -> GetKv2.Response.create(path, json)));
+                .entityProcessor(json -> GetKv2.Response.create(path, json)));
     }
 
     @Override
     public UpdateKv2.Response update(UpdateKv2.Request request) {
         String path = request.path();
 
-        return get(path)
-                .map(secret -> {
-                    String apiPath = mount + "/data/" + path;
-
-                    return restApi.invokeWithResponse(Http.Method.POST,
-                            apiPath,
-                            request,
-                            UpdateKv2.Response.builder());
-                }).orElseThrow(() -> new VaultApiException(
-                        "Cannot update a secret that does not exist on path: \"%s\", please use create",
-                        path));
+        Optional<Kv2Secret> secret = get(path);
+        if (secret.isEmpty()) {
+            throw new VaultApiException(
+                    "Cannot update a secret that does not exist on path: \"%s\", please use create",
+                    path);
+        }
+        String apiPath = mount + "/data/" + path;
+
+        return restApi.invokeWithResponse(Http.Method.POST,
+                                          apiPath,
+                                          request,
+                                          UpdateKv2.Response.builder());
     }
 
     @Override