ansible-collections / community.hashi_vault

Ansible collection for managing and working with HashiCorp Vault.
https://docs.ansible.com/ansible/devel/collections/community/hashi_vault/index.html
GNU General Public License v3.0
80 stars 59 forks source link

Add KV v2 delete functionality. #300

Closed idwagner closed 1 year ago

idwagner commented 1 year ago
SUMMARY

I have a use case to be able to delete (versions) of kv2 data in Vault. I may be able to contribute to this feature.

Since the KV v2 API is a bit more complex than some of the other secret engines, I'm not sure if it would make sense to have vault_kv2_{verb} modules, or just generic vault_{rest_api_verb} modules (i.e. futureproofing for other features like list, metadata, destroy and undelete).

ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION
- name: Delete version 5 of a secret from kv2 with a different mount via the remote host
  community.hashi_vault.vault_kv2_delete_version:
    url: https://vault:8201
    path: path/to/secret
    versions: [5]
briantist commented 1 year ago

Hello & welcome @idwagner !

Delete functionality is definitely something I'd like to see. I expect that we would have both a generic vault_delete and more specific like vault_kv2_delete (with the latter taking a versions option like you specified).

This is similar to how we have vault_read and vault_kv2_get currently.

There is no outstanding work on this that I'm aware of right now, so if you'd like to contribute it, I definitely encourage you to do so. You do not have to contribute both versions; since you're more interested in the KV2 use case, that might make the most sense.

I encourage you to read our Contributor guide thoroughly. I do maintain strict standards for testing, but I can help out as well.

The more that's already done in the PR, the easier is it for me to get it over the line with small improvements.

Please also feel free to post a WIP PR in a draft state and push commits as you make them. This gives me an opportunity to make suggestions early while they are still easy to change, rather than having to do a long review with many interdependent changes.

Thank you!

idwagner commented 1 year ago

Thanks @briantist I'll start looking through the guide and the code. One thing that came up as I was looking through the api was that delete current version and delete specific versions are two separate methods in the Vault api and hvac. Would you like to follow a one to one relationship for the ansible modules, or have something like one vault_kv2_deleteversion which has an optional list[int] versions param which will default to default current version when not supplied?

briantist commented 1 year ago

That's a good question.. my first inclination to have a single vault_kv2_delete module that takes the option list of versions, and chooses which hvac method to call based on that. It's pretty easy to implement. It does slightly complicate testing. We also don't know necessarily know what changes will come in the future that could make us wish we split them up; though I'd hope those would come with a new major version of the kv store.

Do you have a preference?

idwagner commented 1 year ago

I'm a bit on the fence about it, but leaning towards a single module. For usability it seems nice to have one module to support both latest and specific secret versions. It looks like delete is the only function in kv v2 that has two separate api's like this.

The return from both api's are the same (no data, just response code), and Vault doesn't provide any feedback in the response on whether the secret path exists, or a version you asked to delete actually exists or not.

idwagner commented 1 year ago

I've created a WIP PR #304 for this.

Of note, In the module response, I've just returned the hvac response code (seems always 204) from the request, as Vault doesn't tell you if your delete actually changed anything. I'm not sure if there is any other data that would be valuable to return.