Add PATCH support to Vault CLI #17650
Conversation
|
Ah, looks like I've gotta add a real PKI engine here. Hmmm... Something for tomorrow! |
| type PatchCommand struct { | ||
| *BaseCommand | ||
|
|
||
| flagForce bool |
There was a problem hiding this comment.
I'm not sure we would need a force flag for a patch given that they should already be expecting partial data. In the scenario where the data is empty (i.e. no KV pairs were provided) then no change should occur.
There was a problem hiding this comment.
Wouldn't we still want a force/non-force flag, so without the -force option, no PATCH call would be issued? It seems weird to issue a PATCH call with no data in it by default, unless that behavior was specifically requested... Kinda why I felt better leaving the flag than removing it.
My 2c. though :-)
There was a problem hiding this comment.
I don't feel strongly about it. That's why I approved :). It just seemed potentially less dangerous to allow an empty PATCH is all.
Ah, missed the failing tests :). I'll leave my approval though as I imagine the tests fixes won't result in much of a change. |
cipherboy
force-pushed
the
cipherboy-add-patch-support-to-cli
branch
from
5bd9f3c to
9c9137f
Compare
|
\o/ tests now pass locally and I've spun up a PKI instance to test against. |
This is based off the existing write command, using the JSONMergePatch(...) API client method rather than Write(...), allowing us to update specific fields. Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
cipherboy
force-pushed
the
cipherboy-add-patch-support-to-cli
branch
from
9c9137f to
59b8bb2
Compare
|
Thanks all! Hopefully the CI will pass now that we've picked up that new CI config... |
This adds
patchas a top-level command in thevaultCLI, allowing access to these endpoints from our default CLI. Under the covers, we simply delegate info to aclient.Logical().JSONMergePatch(...)call instead ofWrite(...), making this fairly simple to implement. Unlikevault kv patchthough, we don't dump all of the data in an innerdata: ...struct however, as non-KVv2 PATCH APIs don't require that structure.