-
Notifications
You must be signed in to change notification settings - Fork 59
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 content for KV get operations #246
add content for KV get operations #246
Conversation
Docs Build 📝Thank you for contribution!✨ This PR has been merged and the docs are now incorporated into |
Codecov Report
@@ Coverage Diff @@
## main #246 +/- ##
==========================================
+ Coverage 97.49% 97.94% +0.44%
==========================================
Files 56 65 +9
Lines 2518 3060 +542
Branches 241 268 +27
==========================================
+ Hits 2455 2997 +542
Misses 49 49
Partials 14 14
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
I only glanced over it (and ignored the tests :) ), but it looks good to me!
thanks for taking a look @felixfontein ! |
custom_metadata": null | ||
deletion_time": "" | ||
destroyed": false | ||
version": 2 |
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.
Super late, but are these missing opening quotes?
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.
I think it's more that they have closing quotes that shouldn't be there :-)
This is actually valid YAML (just checked), you get a dictionary where the keys end with "
. Which is likely not what the API returns.
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.
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.
SUMMARY
Adding modules and lookups for KV get.
I started with a single
vault_kv_get
that would take akv_version
option to decide which KV version we're dealing with, but ultimately decided on separate modules/plugins for kv1 and kv2: both the inputs (options) and outputs (return data) are different, so trying to mix them doesn't make a lot of sense.I have mulled over the possibility of a generic
vault_kv_get
in addition that could A) take akv_version
, B) read the mount point info from Vault to determine which KV version it is (extra round trip + requires permissions), or C) both (likekv_version
could be1
,2
, or'auto'
). I am still not so sure this "generic" one is a good idea. I won't be implementing it in this PR, but if there's demand we'll have a public discussion about it.ISSUE TYPE
COMPONENT NAME
vault_kv1_get (lookup & module)
vault_kv2_get (lookup & module)
ADDITIONAL INFORMATION
[Feedback Wanted] upcoming kv modules, path, and mount_point #229
Resolves hashi_vault should support secret v2 path (without adding /data/) #9
Fixes several modules are missing from the action group #251