Skip to content

Commit

Permalink
Mount flag syntax to mitigate confusion from KV-v2 path discrepancies (…
Browse files Browse the repository at this point in the history 
…#14807)

* Add explanation to help text and flag usage text

* KV get with new mount flag

* Clearer naming

* KV Put, Patch, Metadata Get + corresponding tests

* KV Delete, Destroy, Rollback, Undelete, MetadataDelete, MetadataPatch, MetadataPut

* Update KV-v2 docs to use mount flag syntax

* Add changelog

* Run make fmt

* Clarify deprecation message in help string

* Address style comments
  • Loading branch information
@digivava
digivava authored Apr 6, 2022
1 parent 9c6d25a commit 74248e1
Show file tree
Hide file tree
Showing 16 changed files with 827 additions and 257 deletions.
3 changes: 3 additions & 0 deletions changelog/14807.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
cli: Alternative flag-based syntax for KV to mitigate confusion from automatically appended /data
```
14 changes: 10 additions & 4 deletions command/kv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,25 @@ Usage: vault kv <subcommand> [options] [args]
Create or update the key named "foo" in the "secret" mount with the value
"bar=baz":
$ vault kv put secret/foo bar=baz
$ vault kv put -mount=secret foo bar=baz
Read this value back:
$ vault kv get secret/foo
$ vault kv get -mount=secret foo
Get metadata for the key:
$ vault kv metadata get secret/foo
$ vault kv metadata get -mount=secret foo
Get a specific version of the key:
$ vault kv get -version=1 secret/foo
$ vault kv get -mount=secret -version=1 foo
The deprecated path-like syntax can also be used, but this should be avoided
for KV v2, as the fact that it is not actually the full API path to
the secret (secret/data/foo) can cause confusion:
$ vault kv get secret/foo
Please see the individual subcommand help for detailed usage information.
`
Expand Down
81 changes: 61 additions & 20 deletions command/kv_delete.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package command

import (
"fmt"
"path"
"strings"

"github.com/hashicorp/vault/api"
Expand All @@ -18,6 +19,7 @@ type KVDeleteCommand struct {
*BaseCommand

flagVersions []string
flagMount string
}

func (c *KVDeleteCommand) Synopsis() string {
Expand All @@ -34,11 +36,17 @@ Usage: vault kv delete [options] PATH
To delete the latest version of the key "foo":
$ vault kv delete -mount=secret foo
The deprecated path-like syntax can also be used, but this should be avoided
for KV v2, as the fact that it is not actually the full API path to
the secret (secret/data/foo) can cause confusion:
$ vault kv delete secret/foo
To delete version 3 of key foo:
$ vault kv delete -versions=3 secret/foo
$ vault kv delete -mount=secret -versions=3 foo
To delete all versions and metadata, see the "vault kv metadata" subcommand.
Expand All @@ -61,6 +69,17 @@ func (c *KVDeleteCommand) Flags() *FlagSets {
Usage: `Specifies the version numbers to delete.`,
})

f.StringVar(&StringVar{
Name: "mount",
Target: &c.flagMount,
Default: "", // no default, because the handling of the next arg is determined by whether this flag has a value
Usage: `Specifies the path where the KV backend is mounted. If specified,
the next argument will be interpreted as the secret path. If this flag is
not specified, the next argument will be interpreted as the combined mount
path and secret path, with /data/ automatically appended between KV
v2 secrets.`,
})

return set
}

Expand Down Expand Up @@ -96,22 +115,54 @@ func (c *KVDeleteCommand) Run(args []string) int {
return 2
}

path := sanitizePath(args[0])
mountPath, v2, err := isKVv2(path, client)
if err != nil {
c.UI.Error(err.Error())
return 2
// If true, we're working with "-mount=secret foo" syntax.
// If false, we're using "secret/foo" syntax.
mountFlagSyntax := (c.flagMount != "")

var (
mountPath string
partialPath string
v2 bool
)

// Parse the paths and grab the KV version
if mountFlagSyntax {
// In this case, this arg is the secret path (e.g. "foo").
partialPath = sanitizePath(args[0])
mountPath = sanitizePath(c.flagMount)
_, v2, err = isKVv2(mountPath, client)
if err != nil {
c.UI.Error(err.Error())
return 2
}
} else {
// In this case, this arg is a path-like combination of mountPath/secretPath.
// (e.g. "secret/foo")
partialPath = sanitizePath(args[0])
mountPath, v2, err = isKVv2(partialPath, client)
if err != nil {
c.UI.Error(err.Error())
return 2
}
}

var secret *api.Secret
var fullPath string
if v2 {
secret, err = c.deleteV2(path, mountPath, client)
secret, err = c.deleteV2(partialPath, mountPath, client)
fullPath = addPrefixToKVPath(partialPath, mountPath, "data")
} else {
secret, err = client.Logical().Delete(path)
// v1
if mountFlagSyntax {
fullPath = path.Join(mountPath, partialPath)
} else {
fullPath = partialPath
}
secret, err = client.Logical().Delete(fullPath)
}

if err != nil {
c.UI.Error(fmt.Sprintf("Error deleting %s: %s", path, err))
c.UI.Error(fmt.Sprintf("Error deleting %s: %s", fullPath, err))
if secret != nil {
OutputSecret(c.UI, secret)
}
Expand All @@ -121,7 +172,7 @@ func (c *KVDeleteCommand) Run(args []string) int {
if secret == nil {
// Don't output anything unless using the "table" format
if Format(c.UI) == "table" {
c.UI.Info(fmt.Sprintf("Success! Data deleted (if it existed) at: %s", path))
c.UI.Info(fmt.Sprintf("Success! Data deleted (if it existed) at: %s", fullPath))
}
return 0
}
Expand All @@ -139,22 +190,12 @@ func (c *KVDeleteCommand) deleteV2(path, mountPath string, client *api.Client) (
switch {
case len(c.flagVersions) > 0:
path = addPrefixToKVPath(path, mountPath, "delete")
if err != nil {
return nil, err
}

data := map[string]interface{}{
"versions": kvParseVersionsFlags(c.flagVersions),
}

secret, err = client.Logical().Write(path, data)
default:

path = addPrefixToKVPath(path, mountPath, "data")
if err != nil {
return nil, err
}

secret, err = client.Logical().Delete(path)
}

Expand Down
62 changes: 53 additions & 9 deletions command/kv_destroy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type KVDestroyCommand struct {
*BaseCommand

flagVersions []string
flagMount string
}

func (c *KVDestroyCommand) Synopsis() string {
Expand All @@ -32,6 +33,12 @@ Usage: vault kv destroy [options] KEY
To destroy version 3 of key foo:
$ vault kv destroy -mount=secret -versions=3 foo
The deprecated path-like syntax can also be used, but this should be avoided
for KV v2, as the fact that it is not actually the full API path to
the secret (secret/data/foo) can cause confusion:
$ vault kv destroy -versions=3 secret/foo
Additional flags and more advanced use cases are detailed below.
Expand All @@ -53,6 +60,17 @@ func (c *KVDestroyCommand) Flags() *FlagSets {
Usage: `Specifies the version numbers to destroy.`,
})

f.StringVar(&StringVar{
Name: "mount",
Target: &c.flagMount,
Default: "", // no default, because the handling of the next arg is determined by whether this flag has a value
Usage: `Specifies the path where the KV backend is mounted. If specified,
the next argument will be interpreted as the secret path. If this flag is
not specified, the next argument will be interpreted as the combined mount
path and secret path, with /data/ automatically appended between KV
v2 secrets.`,
})

return set
}

Expand Down Expand Up @@ -86,25 +104,51 @@ func (c *KVDestroyCommand) Run(args []string) int {
c.UI.Error("No versions provided, use the \"-versions\" flag to specify the version to destroy.")
return 1
}

var err error
path := sanitizePath(args[0])

client, err := c.Client()
if err != nil {
c.UI.Error(err.Error())
return 2
}

mountPath, v2, err := isKVv2(path, client)
if err != nil {
c.UI.Error(err.Error())
return 2
// If true, we're working with "-mount=secret foo" syntax.
// If false, we're using "secret/foo" syntax.
mountFlagSyntax := (c.flagMount != "")

var (
mountPath string
partialPath string
v2 bool
)

// Parse the paths and grab the KV version
if mountFlagSyntax {
// In this case, this arg is the secret path (e.g. "foo").
partialPath = sanitizePath(args[0])
mountPath = sanitizePath(c.flagMount)
_, v2, err = isKVv2(mountPath, client)
if err != nil {
c.UI.Error(err.Error())
return 2
}
} else {
// In this case, this arg is a path-like combination of mountPath/secretPath.
// (e.g. "secret/foo")
partialPath = sanitizePath(args[0])
mountPath, v2, err = isKVv2(partialPath, client)
if err != nil {
c.UI.Error(err.Error())
return 2
}
}

if !v2 {
c.UI.Error("Destroy not supported on KV Version 1")
return 1
}
path = addPrefixToKVPath(path, mountPath, "destroy")
destroyPath := addPrefixToKVPath(partialPath, mountPath, "destroy")
if err != nil {
c.UI.Error(err.Error())
return 2
Expand All @@ -114,9 +158,9 @@ func (c *KVDestroyCommand) Run(args []string) int {
"versions": kvParseVersionsFlags(c.flagVersions),
}

secret, err := client.Logical().Write(path, data)
secret, err := client.Logical().Write(destroyPath, data)
if err != nil {
c.UI.Error(fmt.Sprintf("Error writing data to %s: %s", path, err))
c.UI.Error(fmt.Sprintf("Error writing data to %s: %s", destroyPath, err))
if secret != nil {
OutputSecret(c.UI, secret)
}
Expand All @@ -125,7 +169,7 @@ func (c *KVDestroyCommand) Run(args []string) int {
if secret == nil {
// Don't output anything unless using the "table" format
if Format(c.UI) == "table" {
c.UI.Info(fmt.Sprintf("Success! Data written to: %s", path))
c.UI.Info(fmt.Sprintf("Success! Data written to: %s", destroyPath))
}
return 0
}
Expand Down
Loading

0 comments on commit 74248e1

Please sign in to comment.