-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add -mount
flag to kv list command
#19378
Changes from all commits
1dfaeb5
9081063
94e04fb
6376f0b
27219af
83d4064
9d586e4
05b7c8e
ec2137f
e4e9612
eb80b01
be8164a
b66c73e
77d7ee5
3b07e13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:bug | ||
cli/kv: add -mount flag to kv list | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package command | |
|
||
import ( | ||
"fmt" | ||
"path" | ||
"strings" | ||
|
||
"github.com/mitchellh/cli" | ||
|
@@ -15,6 +16,7 @@ var ( | |
|
||
type KVListCommand struct { | ||
*BaseCommand | ||
flagMount string | ||
} | ||
|
||
func (c *KVListCommand) Synopsis() string { | ||
|
@@ -40,7 +42,23 @@ Usage: vault kv list [options] PATH | |
} | ||
|
||
func (c *KVListCommand) Flags() *FlagSets { | ||
return c.flagSet(FlagSetHTTP | FlagSetOutputFormat) | ||
set := c.flagSet(FlagSetHTTP | FlagSetOutputFormat) | ||
|
||
// Common Options | ||
f := set.NewFlagSet("Common Options") | ||
|
||
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 | ||
} | ||
|
||
func (c *KVListCommand) AutocompleteArgs() complete.Predictor { | ||
|
@@ -62,8 +80,11 @@ func (c *KVListCommand) Run(args []string) int { | |
args = f.Args() | ||
switch { | ||
case len(args) < 1: | ||
c.UI.Error(fmt.Sprintf("Not enough arguments (expected 1, got %d)", len(args))) | ||
return 1 | ||
if c.flagMount == "" { | ||
c.UI.Error(fmt.Sprintf("Not enough arguments (expected 1, got %d)", len(args))) | ||
return 1 | ||
} | ||
args = []string{""} | ||
case len(args) > 1: | ||
c.UI.Error(fmt.Sprintf("Too many arguments (expected 1, got %d)", len(args))) | ||
return 1 | ||
|
@@ -75,25 +96,56 @@ func (c *KVListCommand) Run(args []string) int { | |
return 2 | ||
} | ||
|
||
// Sanitize path | ||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic is the same as in kv put |
||
// 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, v2, err = isKVv2(sanitizePath(c.flagMount), client) | ||
if err != nil { | ||
c.UI.Error(err.Error()) | ||
return 2 | ||
} | ||
|
||
if v2 { | ||
path = addPrefixToKVPath(path, mountPath, "metadata") | ||
if v2 { | ||
partialPath = path.Join(mountPath, partialPath) | ||
} | ||
} 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 | ||
} | ||
} | ||
|
||
secret, err := client.Logical().List(path) | ||
// Add /metadata to v2 paths only | ||
var fullPath string | ||
if v2 { | ||
fullPath = addPrefixToKVPath(partialPath, mountPath, "metadata") | ||
} else { | ||
// v1 | ||
if mountFlagSyntax { | ||
fullPath = path.Join(mountPath, partialPath) | ||
} else { | ||
Comment on lines
+138
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a bit hard to follow these if blocks, perhaps they can be rearranged a bit for clarity? In particular we are doing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just copied the logic from kv put each command in |
||
fullPath = partialPath | ||
} | ||
} | ||
|
||
secret, err := client.Logical().List(fullPath) | ||
if err != nil { | ||
c.UI.Error(fmt.Sprintf("Error listing %s: %s", path, err)) | ||
c.UI.Error(fmt.Sprintf("Error listing %s: %s", fullPath, err)) | ||
return 2 | ||
} | ||
|
||
|
@@ -111,12 +163,12 @@ func (c *KVListCommand) Run(args []string) int { | |
} | ||
|
||
if secret == nil || secret.Data == nil { | ||
c.UI.Error(fmt.Sprintf("No value found at %s", path)) | ||
c.UI.Error(fmt.Sprintf("No value found at %s", fullPath)) | ||
return 2 | ||
} | ||
|
||
if !ok { | ||
c.UI.Error(fmt.Sprintf("No entries found at %s", path)) | ||
c.UI.Error(fmt.Sprintf("No entries found at %s", fullPath)) | ||
return 2 | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -590,6 +590,131 @@ func TestKVGetCommand(t *testing.T) { | |
}) | ||
} | ||
|
||
func testKVListCommand(tb testing.TB) (*cli.MockUi, *KVListCommand) { | ||
tb.Helper() | ||
ui := cli.NewMockUi() | ||
cmd := &KVListCommand{ | ||
BaseCommand: &BaseCommand{ | ||
UI: ui, | ||
}, | ||
} | ||
|
||
return ui, cmd | ||
} | ||
|
||
// TestKVListCommand runs tests for `vault kv list` | ||
func TestKVListCommand(t *testing.T) { | ||
testCases := []struct { | ||
name string | ||
args []string | ||
outStrings []string | ||
code int | ||
}{ | ||
{ | ||
dhuckins marked this conversation as resolved.
Show resolved
Hide resolved
|
||
name: "default", | ||
args: []string{"kv/my-prefix"}, | ||
outStrings: []string{"secret-0", "secret-1", "secret-2"}, | ||
code: 0, | ||
}, | ||
{ | ||
name: "not_enough_args", | ||
args: []string{}, | ||
outStrings: []string{"Not enough arguments"}, | ||
code: 1, | ||
}, | ||
{ | ||
name: "v2_default_with_mount", | ||
args: []string{"-mount", "kv", "my-prefix"}, | ||
outStrings: []string{"secret-0", "secret-1", "secret-2"}, | ||
code: 0, | ||
}, | ||
{ | ||
name: "v1_default_with_mount", | ||
args: []string{"kv/my-prefix"}, | ||
outStrings: []string{"secret-0", "secret-1", "secret-2"}, | ||
code: 0, | ||
}, | ||
{ | ||
name: "v2_not_found", | ||
args: []string{"kv/nope/not/once/never"}, | ||
outStrings: []string{"No value found at kv/metadata/nope/not/once/never"}, | ||
code: 2, | ||
}, | ||
{ | ||
name: "v1_mount_only", | ||
args: []string{"kv"}, | ||
outStrings: []string{"my-prefix"}, | ||
code: 0, | ||
}, | ||
{ | ||
name: "v2_mount_only", | ||
args: []string{"-mount", "kv"}, | ||
outStrings: []string{"my-prefix"}, | ||
code: 0, | ||
}, | ||
{ | ||
// this is behavior that should be tested | ||
// `kv` here is an explicit mount | ||
// `my-prefix` is not | ||
// the current kv code will ignore `my-prefix` | ||
name: "ignore_multi_part_mounts", | ||
args: []string{"-mount", "kv/my-prefix"}, | ||
outStrings: []string{"my-prefix"}, | ||
code: 0, | ||
}, | ||
Comment on lines
+655
to
+664
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @digivava when you have a moment, can you give this PR another pass please? |
||
} | ||
|
||
t.Run("validations", func(t *testing.T) { | ||
t.Parallel() | ||
|
||
for _, testCase := range testCases { | ||
testCase := testCase | ||
|
||
t.Run(testCase.name, func(t *testing.T) { | ||
t.Parallel() | ||
|
||
// test setup | ||
client, closer := testVaultServer(t) | ||
defer closer() | ||
|
||
// enable kv-v2 backend | ||
if err := client.Sys().Mount("kv/", &api.MountInput{ | ||
Type: "kv-v2", | ||
}); err != nil { | ||
t.Fatal(err) | ||
} | ||
time.Sleep(time.Second) | ||
|
||
ctx := context.Background() | ||
for i := 0; i < 3; i++ { | ||
path := fmt.Sprintf("my-prefix/secret-%d", i) | ||
_, err := client.KVv2("kv/").Put(ctx, path, map[string]interface{}{ | ||
"foo": "bar", | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
} | ||
|
||
ui, cmd := testKVListCommand(t) | ||
cmd.client = client | ||
|
||
code := cmd.Run(testCase.args) | ||
if code != testCase.code { | ||
t.Errorf("expected %d to be %d", code, testCase.code) | ||
} | ||
|
||
combined := ui.OutputWriter.String() + ui.ErrorWriter.String() | ||
for _, str := range testCase.outStrings { | ||
if !strings.Contains(combined, str) { | ||
t.Errorf("expected %q to contain %q", combined, str) | ||
} | ||
} | ||
}) | ||
} | ||
}) | ||
} | ||
|
||
func testKVMetadataGetCommand(tb testing.TB) (*cli.MockUi, *KVMetadataGetCommand) { | ||
tb.Helper() | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is copied from
kv get
vault/command/kv_get.go
Lines 72 to 76 in e4e9612
so going to leave for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps adjusting both descriptions for clarity would help.