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

Missing Patch function in api/logical.go #12330

Closed
shlok007 opened this issue Aug 15, 2021 · 3 comments · Fixed by #12687
Closed

Missing Patch function in api/logical.go #12330

shlok007 opened this issue Aug 15, 2021 · 3 comments · Fixed by #12687
Labels
core/secret devex Developer Experience enhancement internal Mark for internal discussion/implementation secret/kv

Comments

@shlok007
Copy link

Is your feature request related to a problem? Please describe.
I was trying to append secrets to a file path using kv version 2 and that worked as expected using the patch command from the CLI. However, while using go api, I can see there is no function available for the same.

Describe the solution you'd like
By implementing a Patch in logical.go, I should be able to do the following to send the patch requests:

func PatchSecret(client *VaultClient, path string, secrets map[string]interface{}) bool {
	_, err := client.client.Logical().Patch(patch, secrets)
}

I am novice to vault so if for any reason this doesn't make sense, please do help me out in comments below!

Describe alternatives you've considered
Alternative is to fetch all the secrets, append new ones and send a put request. However this will require some control as this might fail while running multiple independent processes of the server which is storing secrets to vault.

@pmmukh
Copy link
Contributor

pmmukh commented Aug 16, 2021

Hi @shlok007 ! Thank you for submitting this issue! This totally makes sense, and you are correct, there does not at this moment exist a PATCH api for the kv-v2 secrets engine, hence the absence of a Patch function in the client, the cli itself does the same thing as the alternative you've written out, which is to do a read followed by a write. I'm going to be tagging this as a future feature enhancement, till then the alternative approach you are considering sounds like the right way to go!

@shlok007
Copy link
Author

@pmmukh If the cli does the same thing, there could be an issue with concurrent patch request being sent to vault. For now, in any case, we can create a patch function leveraging the existing read and write methods to read the secrets, merge and write the new one, just like the cli itself. Happy to help, will you be willing to accept a contribution for the same?

@pmmukh
Copy link
Contributor

pmmukh commented Aug 17, 2021

@shlok007 you are correct, if issuing concurrent cli commands for the patch call, it could be subject to the same race condition issues. Also, that approach sounds like the right call, for the time being!
In regards to contributing, firstly, thank you for offering to contribute to this feature! We're currently considering this or a similar feature to implement internally, so for the time being will not be accepting community contributions for this change. However, we will be tracking this issue and will update it once there is a resolution!

@pmmukh pmmukh added the internal Mark for internal discussion/implementation label Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core/secret devex Developer Experience enhancement internal Mark for internal discussion/implementation secret/kv
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants