-
Notifications
You must be signed in to change notification settings - Fork 122
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
Hashicorp feature tmkms #840
base: main
Are you sure you want to change the base?
Hashicorp feature tmkms #840
Conversation
Huh, there is no fix for: https://rustsec.org/advisories/RUSTSEC-2023-0071 yet (but it is optional dependency anyway) EDIT: I need to fix the CI tests (integration test requires running vault) |
@tony-iqlusion it's ready for review 🙏 |
@mkaczanowski may I ask if you considered vault disconnection issues? Will it reconnect? I noticed that even if I have multiple vault instances, when I restart one of them, |
I was unable to reproduce the connection issues |
@tony-iqlusion any ETA on merging this? |
I should have some time to review it soon. Please be patient. |
Note: I would still like to get this into the v0.14 release but my time on TMKMS has been taken up by vote extension signing support. I hope to be able to review it soon when other TMKMS-related work is done. |
@mkaczanowski I am currently testing this PR, and it seems that CA certificate does not work:
Works fine if I set |
7deb16d
to
d2bb758
Compare
@helder-moreira you can try now. I did rework bunch of things (as u can see in the commit history). I'll be testing it live soon, but code wise I think it is pretty much done |
7ba45ac
to
67d61d5
Compare
TL;DR
This is a rebased version of #613 with some changes:
VAULT_CACERT
andVAULT_SKIP_VERFIY
are now configurableTest plan
I've tested this live and also via unittests and integration test: