-
Notifications
You must be signed in to change notification settings - Fork 46
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
support for vault approle authentication #74
Conversation
Signed-off-by: alexshe <alexshe@wix.com>
667204f
to
dba08e8
Compare
Codecov Report
@@ Coverage Diff @@
## master #74 +/- ##
=========================================
+ Coverage 82.86% 82.97% +0.1%
=========================================
Files 23 23
Lines 1418 1427 +9
=========================================
+ Hits 1175 1184 +9
Misses 243 243
Continue to review full report at Codecov.
|
Thanks @AlexShemeshWix It looks generally good but I'll have to try it to fully apprehend how it works. Also, It'll have to be part of an RC2 of the chaostoolkit-lib I believe. Be patient as this might take a few days since it's holiday season here. Thanks. |
Yes. Take your time. Ive implemented the dirty hack of actually retriving token and injecting it to env var before i run chaos test. So its no hurry. |
I see. I will try but you may be right, it may not work really well and needs fixing! |
I'm finally going to have some time soon to look at it! Thanks for your patience @AlexShemeshWix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'm going to fix the couple of minor comments that I would make normally:
- add CHANGELOG entry
- extend README entry to indicate what we support
But since I've left this PR sit there for too long, I'll do it this time :)
url = configuration.get("vault_addr") | ||
client = hvac.Client(url=url) | ||
|
||
if "vault_token" in configuration.keys(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be written
if "vault_token" in configuration:
|
||
if "vault_token" in configuration.keys(): | ||
client.token = configuration.get("vault_token") | ||
elif "vault_role_id" in configuration.keys() and \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be written
if "vault_role_id" in configuration and "vault_role_secret" in configuration:
Its in the best practices of Hashicorp Vault to authenticate with time limited tokens received from approle auth endpoint.