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

improve kv CLI to remove data or custom metadata using kv patch #18067

Merged
merged 3 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/18067.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
cli/kv: improve kv CLI to remove data or custom metadata using kv patch
```
39 changes: 30 additions & 9 deletions command/kv_metadata_patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ var (
type KVMetadataPatchCommand struct {
*BaseCommand

flagMaxVersions int
flagCASRequired BoolPtr
flagDeleteVersionAfter time.Duration
flagCustomMetadata map[string]string
flagMount string
testStdin io.Reader // for tests
flagMaxVersions int
flagCASRequired BoolPtr
flagDeleteVersionAfter time.Duration
flagCustomMetadata map[string]string
flagRemoveCustomMetadata []string
flagMount string
testStdin io.Reader // for tests
}

func (c *KVMetadataPatchCommand) Synopsis() string {
Expand Down Expand Up @@ -65,6 +66,10 @@ Usage: vault kv metadata patch [options] KEY

$ vault kv metadata patch -mount=secret -custom-metadata=foo=abc -custom-metadata=bar=123 foo

To remove custom meta data from the corresponding path in the key-value store, kv metadata patch can be used.

$ vault kv metadata patch -mount=secret -remove-custom-metadata=bar foo

Additional flags and more advanced use cases are detailed below.

` + c.Flags().Help()
Expand Down Expand Up @@ -111,6 +116,13 @@ func (c *KVMetadataPatchCommand) Flags() *FlagSets {
This can be specified multiple times to add multiple pieces of metadata.`,
})

f.StringSliceVar(&StringSliceVar{
Name: "remove-custom-metadata",
Target: &c.flagRemoveCustomMetadata,
Default: []string{},
Usage: "Key to remove from custom metadata. To specify multiple values, specify this flag multiple times.",
})

f.StringVar(&StringVar{
Name: "mount",
Target: &c.flagMount,
Expand Down Expand Up @@ -198,7 +210,7 @@ func (c *KVMetadataPatchCommand) Run(args []string) int {

fullPath := addPrefixToKVPath(partialPath, mountPath, "metadata")

data := map[string]interface{}{}
data := make(map[string]interface{}, 0)

if c.flagMaxVersions >= 0 {
data["max_versions"] = c.flagMaxVersions
Expand All @@ -212,10 +224,19 @@ func (c *KVMetadataPatchCommand) Run(args []string) int {
data["delete_version_after"] = c.flagDeleteVersionAfter.String()
}

if len(c.flagCustomMetadata) > 0 {
data["custom_metadata"] = c.flagCustomMetadata
customMetadata := make(map[string]interface{})

for key, value := range c.flagCustomMetadata {
customMetadata[key] = value
}

for _, key := range c.flagRemoveCustomMetadata {
// A null in a JSON merge patch payload will remove the associated key
customMetadata[key] = nil
}

data["custom_metadata"] = customMetadata

secret, err := client.Logical().JSONMergePatch(context.Background(), fullPath, data)
if err != nil {
c.UI.Error(fmt.Sprintf("Error writing data to %s: %s", fullPath, err))
Expand Down
23 changes: 23 additions & 0 deletions command/kv_metadata_patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,29 @@ func TestKvMetadataPatchCommand_Flags(t *testing.T) {
},
},
},
{
"remove-custom_metadata",
[]string{"-custom-metadata=baz=ghi", "-remove-custom-metadata=foo"},
"Success!",
0,
map[string]interface{}{
"custom_metadata": map[string]interface{}{
"bar": "def",
"baz": "ghi",
},
},
},
{
"remove-custom_metadata-multiple",
[]string{"-custom-metadata=baz=ghi", "-remove-custom-metadata=foo", "-remove-custom-metadata=bar"},
"Success!",
0,
map[string]interface{}{
"custom_metadata": map[string]interface{}{
"baz": "ghi",
},
},
},
{
"delete_version_after_success",
[]string{"-delete-version-after=5s"},
Expand Down
32 changes: 27 additions & 5 deletions command/kv_patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ var (
type KVPatchCommand struct {
*BaseCommand

flagCAS int
flagMethod string
flagMount string
testStdin io.Reader // for tests
flagCAS int
flagMethod string
flagMount string
testStdin io.Reader // for tests
flagRemoveData []string
}

func (c *KVPatchCommand) Synopsis() string {
Expand Down Expand Up @@ -76,6 +77,10 @@ Usage: vault kv patch [options] KEY [DATA]

$ vault kv patch -mount=secret -method=rw foo bar=baz

To remove data from the corresponding path in the key-value store, kv patch can be used.

$ vault kv patch -mount=secret -remove-data=bar foo

Additional flags and more advanced use cases are detailed below.

` + c.Flags().Help()
Expand Down Expand Up @@ -117,6 +122,13 @@ func (c *KVPatchCommand) Flags() *FlagSets {
v2 secrets.`,
})

f.StringSliceVar(&StringSliceVar{
Name: "remove-data",
Target: &c.flagRemoveData,
Default: []string{},
Usage: "Key to remove from data. To specify multiple values, specify this flag multiple times.",
})

return set
}

Expand Down Expand Up @@ -147,7 +159,7 @@ func (c *KVPatchCommand) Run(args []string) int {
case len(args) < 1:
c.UI.Error(fmt.Sprintf("Not enough arguments (expected >1, got %d)", len(args)))
return 1
case len(args) == 1:
case len(c.flagRemoveData) == 0 && len(args) == 1:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, great change to this check! I can definitely imagine scenarios where users only want to remove data.

c.UI.Error("Must supply data")
return 1
}
Expand Down Expand Up @@ -211,6 +223,16 @@ func (c *KVPatchCommand) Run(args []string) int {
return 2
}

// collecting data to be removed
if newData == nil {
newData = make(map[string]interface{})
}

for _, key := range c.flagRemoveData {
// A null in a JSON merge patch payload will remove the associated key
newData[key] = nil
Copy link
Contributor

@ccapurso ccapurso Nov 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Not sure if we wanted to include the same comment that you had above in the kv metadata patch implementation mentioning why the nil is important. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it would be good context! Thanks for the comment.

}

// Check the method and behave accordingly
var secret *api.Secret
var code int
Expand Down
6 changes: 6 additions & 0 deletions vault/external_tests/kv/kv_patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ func TestKV_Patch_RootToken(t *testing.T) {
data := map[string]interface{}{
"data": map[string]interface{}{
"bar": "baz",
"foo": "qux",
},
}

Expand All @@ -263,6 +264,7 @@ func TestKV_Patch_RootToken(t *testing.T) {
data := map[string]interface{}{
"data": map[string]interface{}{
"bar": "quux",
"foo": nil,
},
}
return client.Logical().JSONMergePatch(context.Background(), "kv/data/foo", data)
Expand All @@ -288,4 +290,8 @@ func TestKV_Patch_RootToken(t *testing.T) {
if bar != "quux" {
t.Fatalf("expected bar to be quux but it was %q", bar)
}

if _, ok := secret.Data["data"].(map[string]interface{})["foo"]; ok {
t.Fatalf("expected data not to include foo")
}
}