quarkiverse / quarkus-vault

Quarkus HashiCorp Vault extension
Apache License 2.0
18 stars 22 forks source link

Creating a transit secret engine key in 4.0.0-alpha.2 throws a VaultClientException with status 204 #249

Closed vsevel closed 4 months ago

vsevel commented 4 months ago

204 should be a success in that situation.

seems to be related to:

  public VaultRequest<VaultSecretsTransitCreateKeyResult> createKey(String mountPath, String name,
      VaultSecretsTransitCreateKeyParams params) {
    return VaultRequest.post(getTraceOpName("Create Key"))
        .path(mountPath, "keys", name)
        .body(params)
        .expectOkStatus()
        .build(VaultLeasedResultExtractor.of(VaultSecretsTransitCreateKeyResult.class));
  }

the old code was:

    public Uni<Void> createTransitKey(VaultClient vaultClient, String token, String keyName, VaultTransitCreateKeyBody body) {
        return vaultClient.post(opName("Create Key"), "transit/keys/" + keyName, token, body, 204);
    }
kdubb commented 4 months ago

What version of Vault? This is tested (hundreds of times) in VaultSecretsTransitTest.

kdubb commented 4 months ago

The old code had an escape hatch for when Vault decided to produce warnings for a normally 204 method. When it did this it would return a 200 instead of 204 and include only the warnings in the response. The old code would basically allow any 204 to be promoted to a 200 and ignore anything but warnings. I think that is probably how the old code worked.

Transit's "Create Key" certainly does seem to normally return a 200 and the info for the created key.

vsevel commented 4 months ago

I can see that now. https://developer.hashicorp.com/vault/api-docs#http-status-codes I believe we get a 204 when the key already exists. so we do need to restore the old behavior, which seems correct according to documentation. correct?

kdubb commented 4 months ago

That was a bit misleading... the old behavior doesn't exist but warning extraction happens well before interpreting the status code. So the promotion from 204 -> 200 is no longer necessary.

I believe the old "Create Key" was relying on this promotion from 204 -> 200 and ignoring the result. From the client test suite, it seems clear that "Create Key" returns a 200 and information about the newly created key.

kdubb commented 4 months ago

Maybe add the stack trace for your error and we can pin down what is happening.

vsevel commented 4 months ago

did not happen with the previous vault extension version. I think we need to do something about it.

VaultClientException{operationName='VAULT [SECRETS (transit)] Create Key', requestPath='https://.../v1/transit/keys/...', status=204}
    at io.quarkus.vault.client.http.VaultHttpClient.throwVaultException(VaultHttpClient.java:80)
    at io.quarkus.vault.client.http.VaultHttpClient.lambda$buildResponse$0(VaultHttpClient.java:33)
    at java.base@21.0.2/java.util.concurrent.CompletableFuture.uniApplyNow(CompletableFuture.java:684)
    at java.base@21.0.2/java.util.concurrent.CompletableFuture.uniApplyStage(CompletableFuture.java:662)
    at java.base@21.0.2/java.util.concurrent.CompletableFuture.thenApply(CompletableFuture.java:2200)
    at java.base@21.0.2/java.util.concurrent.CompletableFuture$MinimalStage.thenApply(CompletableFuture.java:2948)
    at io.quarkus.vault.client.http.VaultHttpClient.buildResponse(VaultHttpClient.java:29)
    at io.quarkus.vault.client.http.vertx.VertxVaultHttpClient.buildResponse(VertxVaultHttpClient.java:70)
    at io.quarkus.vault.client.http.vertx.VertxVaultHttpClient.lambda$execute$0(VertxVaultHttpClient.java:33)
    at java.base@21.0.2/java.util.concurrent.CompletableFuture$UniCompose.tryFire(CompletableFuture.java:1150)
    at java.base@21.0.2/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
    at java.base@21.0.2/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2179)
    at io.vertx.core.Future.lambda$toCompletionStage$3(Future.java:581)
    at io.vertx.core.impl.future.FutureImpl$4.onSuccess(FutureImpl.java:176)
    at io.vertx.core.impl.future.FutureBase.emitSuccess(FutureBase.java:66)
    at io.vertx.core.impl.future.FutureImpl.tryComplete(FutureImpl.java:246)
    at io.vertx.core.impl.future.PromiseImpl.onSuccess(PromiseImpl.java:49)
    at io.vertx.core.impl.future.PromiseImpl.handle(PromiseImpl.java:41)
    at io.vertx.core.impl.future.PromiseImpl.handle(PromiseImpl.java:23)
    at io.vertx.ext.web.client.impl.HttpContext.handleDispatchResponse(HttpContext.java:397)
    at io.vertx.ext.web.client.impl.HttpContext.execute(HttpContext.java:384)
    at io.vertx.ext.web.client.impl.HttpContext.next(HttpContext.java:362)
    at io.vertx.ext.web.client.impl.HttpContext.fire(HttpContext.java:329)
    at io.vertx.ext.web.client.impl.HttpContext.dispatchResponse(HttpContext.java:291)
    at io.vertx.ext.web.client.impl.HttpContext.lambda$null$7(HttpContext.java:507)
    at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:276)
    at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:258)
    at io.vertx.core.impl.ContextInternal.lambda$runOnContext$0(ContextInternal.java:56)
    at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:173)
    at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:166)
    at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:566)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.base@21.0.2/java.lang.Thread.runWith(Thread.java:1596)
    at java.base@21.0.2/java.lang.Thread.run(Thread.java:1583)
    at org.graalvm.nativeimage.builder/com.oracle.svm.core.thread.PlatformThreads.threadStartRoutine(PlatformThreads.java:833)
    at org.graalvm.nativeimage.builder/com.oracle.svm.core.posix.thread.PosixPlatformThreads.pthreadStartRoutine(PosixPlatformThreads.java:211)
kdubb commented 4 months ago

So which Vault version did it work and what version is it failing in?

vsevel commented 4 months ago

Vault did not change