Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Vault Enterprise Namespace #15353

Merged
merged 1 commit into from Mar 9, 2021
Merged

Add support for Vault Enterprise Namespace #15353

merged 1 commit into from Mar 9, 2021

Conversation

vsevel
Copy link
Contributor

@vsevel vsevel commented Feb 26, 2021

Fixes #15349

@quarkus-bot
Copy link

quarkus-bot bot commented Feb 26, 2021

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

@vsevel
Copy link
Contributor Author

vsevel commented Feb 26, 2021

@sfauvart this should add the necessary bits. can you take a look and give it a try? I don't have an enterprise vault to test myself. just get my branch and compile it locally with ./mvnw -Dquickly. I add the header except for the root apis. let me know how it goes.

@sfauvart
Copy link

Hello @vsevel !
First of all, thank you for your fast reply !
For me, it's OK, it work very well. that's what I wanted 💯 👍

If you want, you can give a try with a local instance of vault-enterprise (in a 'demo' mode for 30 min).
I create a gist with script that init secret in a namespace :
https://gist.github.com/sfauvart/d48772adedb5c8656183923fb9ec5cfd

It can be used in a quarkus project with the quarkus-smallrye-openapi extension and result simply to change the title of swagger-ui with 'Quarkus API from Vault'.

@sberyozkin
Copy link
Member

sberyozkin commented Mar 5, 2021

@sfauvart Hi, are you OK for Vincent to have ahead with this PR as you have confirmed above it works for you ? thanks

@vsevel
Copy link
Contributor Author

vsevel commented Mar 7, 2021

hi @sberyozkin just to let you know I have been struggling to write proper testing. I will continue these coming days.

@vsevel vsevel changed the title [WIP] Add support for Vault Enterprise Namespace Add support for Vault Enterprise Namespace Mar 7, 2021
@vsevel
Copy link
Contributor Author

vsevel commented Mar 8, 2021

I have been struggling with testing.
I was not able to start the hashicorp/vault-enterprise image using testcontainers (my guess is that the startup status is expressed differently, but I could not get a clear understanding of what was going on).
So I decided to use wiremock to simulate a vault server, and check that the appropriate header was received on a standard readSecret operation.
One of the issue I got was I could not do the verify in the test method, because I could not find a way to get a reference onto the wiremock server, because the WiremockVault lived in a different class loader than the test itself.
So I moved the verify in the stop method. but it does seem to be called.
Eventually I solved that by adding a withHeader filter on the stubbing part. But that is a hacky workaround that is very limiting.
But the bigger issue is that I could not get both the VaultTestLifecycleManager and the WiremockVault to live together in the same test module. I tried to limit the scope of the WiremockVault to just VaultEnterpriseITCase with @QuarkusTestResource(value = WiremockVault.class, restrictToAnnotatedClass = true), but that did not work out.
I removed the WIP status on the issue to demonstrate the problem, and I will reach out for help.

@vsevel
Copy link
Contributor Author

vsevel commented Mar 9, 2021

hi @sberyozkin I have found a solution for the tests. one of the changes involved moving the vault url to the config file, rather than returning it from the lifecycle manager.
I have squashed and rebased.
if you could take a look and tell me if you think it is ready that would be great. thanks.

@sberyozkin
Copy link
Member

Hi Vincent @vsevel, sorry for a delay, I did not quite understand the earlier problem :-), but the test now looks clean and simple, thanks for making it work, so it all LGTM and I'm happy to merge

@sberyozkin sberyozkin merged commit 75dc8df into quarkusio:master Mar 9, 2021
@quarkus-bot quarkus-bot bot added this to the 1.13 - master milestone Mar 9, 2021
@vsevel vsevel deleted the vault_enterprise branch March 9, 2021 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Vault Enterprise Namespaces
3 participants