quarkiverse / quarkus-vault

Quarkus HashiCorp Vault extension
Apache License 2.0
19 stars 26 forks source link

Update version of vault image to latest stable official image 1.15.0 #178

Closed bmscomp closed 1 year ago

bmscomp commented 1 year ago

The main purpose of this pull request is to use the latest stable version of vault image on hashicorp/vault instead the already used one vault, as mentioned on https://hub.docker.com/_/vault/ that Upcoming in Vault 1.14, we will stop publishing official Dockerhub images and publish only our Verified Publisher images. Users of Docker images should pull from hashicorp/vault instead of vault. Verified Publisher images can be found at https://hub.docker.com/r/hashicorp/vaulton this pull request, an update of a recent version ofvault` can keep fresh the library and make it fit to the upcoming new changes and prevent the drastic changes when a lot of breaking changes happens on certain versions

kdubb commented 11 months ago

@bmscomp Where is -dev-tls being set for this container?

kdubb commented 11 months ago

Let me elaborate. You changed the protocol from http to https but this requires -dev-tls to be passed to the started container but that isn't being passed. Also, -dev-tls requires a minimum of Vault 1.12, meaning for people pinned to older Vault versions this won't work.

I've checked testcontainers which still defaults to Vault 1.1.3 and uses a default http address

I've also tried running a local 1.15 container using docker run --cap-add=IPC_LOCK hashicorp/vault which defaults to using an http address.

So I'm left a bit confused at how this is actually working. DevServicesVaultProcessor wasn't updated to pass -dev-tls and yet, at the very least, this PR passed CI checks.

kdubb commented 11 months ago

@vsevel @bmscomp I think we let this slip by. After a bit of a refresher I realized I don't think we ever actually use "dev services" to start vault for the integration tests (or any tests). We use a custom configuration to boot Vault with TLS support but that has nothing to do with this change.

I don't think dev services works at all at the moment. @bmscomp Have you tried actually tried using this in a project with dev services active?

How I discovered this is we are finally upgrade to Q3.5.2 and basically none of our Vault dev services start because the containers are started using http and the address was changed to https

bmscomp commented 11 months ago

@kdubb I am working on that right mow, we need to enhance tests regarding dev services

vsevel commented 11 months ago

How I discovered this is we are finally upgrade to Q3.5.2 and basically none of our Vault dev services start because the containers are started using http and the address was changed to https

we were not cautious enough on this. thanks @kdubb for raising a flag.

bmscomp commented 11 months ago

@kdubb @vsevel I think it's a good idea to add an integration or system test to make sure such important feature have no regression, rather then let this for a manual test, I'll try next days to work on something like this

kdubb commented 11 months ago

@bmscomp It should be very easy. Any integration test having a configuration that does not provide a vault url/credentials should automatically start devservices.