Skip to content

Commit 97410de

Browse files
committed
Fix for undelete for mount paths
* Pulled in a fix from kv_delete.go that adds partialPath to the mountPath to support paths that are multiple levels deep. * Added undelete tests to validate that KV secrets can be successfully undeleted when mounted at a multi-level mount path. * Added changelog txt file
1 parent 1584475 commit 97410de

File tree

3 files changed

+169
-8
lines changed

3 files changed

+169
-8
lines changed

changelog/19811.txt

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
kv_undelete: Undelete now properly handles KV-V2 mount paths that are more than one layer deep.
3+
```

command/kv_test.go

+149
Original file line numberDiff line numberDiff line change
@@ -1522,6 +1522,155 @@ func TestPadEqualSigns(t *testing.T) {
15221522
})
15231523
}
15241524
}
1525+
func testKVUndeleteCommand(tb testing.TB) (*cli.MockUi, *KVUndeleteCommand) {
1526+
tb.Helper()
1527+
1528+
ui := cli.NewMockUi()
1529+
return ui, &KVUndeleteCommand{
1530+
BaseCommand: &BaseCommand{
1531+
UI: ui,
1532+
},
1533+
}
1534+
}
1535+
1536+
func TestKVUndeleteCommand(t *testing.T) {
1537+
t.Parallel()
1538+
1539+
cases := []struct {
1540+
name string
1541+
args []string
1542+
outStrings []string
1543+
code int
1544+
}{
1545+
{
1546+
"not_enough_args",
1547+
[]string{},
1548+
[]string{"Not enough arguments"},
1549+
1,
1550+
},
1551+
{
1552+
"too_many_args",
1553+
[]string{"foo", "bar"},
1554+
[]string{"Too many arguments"},
1555+
1,
1556+
},
1557+
{
1558+
"no_versions",
1559+
[]string{"-mount", "kv", "/read/foo"},
1560+
[]string{"No versions provided"},
1561+
1,
1562+
},
1563+
{
1564+
"v2_mount_flag_syntax",
1565+
[]string{"-versions", "1", "-mount", "kv", "read/foo"},
1566+
[]string{"Success! Data written to: kv/undelete/read/foo"},
1567+
0,
1568+
},
1569+
{
1570+
"v2_mount_flag_syntax_complex_1",
1571+
[]string{"-versions", "1", "-mount", "secrets/testapp", "test"},
1572+
[]string{"Success! Data written to: secrets/testapp/undelete/test"},
1573+
0,
1574+
},
1575+
{
1576+
"v2_mount_flag_syntax_complex_2",
1577+
[]string{"-versions", "1", "-mount", "secrets/x/testapp", "test"},
1578+
[]string{"Success! Data written to: secrets/x/testapp/undelete/test"},
1579+
0,
1580+
},
1581+
}
1582+
1583+
t.Run("validations", func(t *testing.T) {
1584+
t.Parallel()
1585+
1586+
for _, tc := range cases {
1587+
tc := tc
1588+
1589+
t.Run(tc.name, func(t *testing.T) {
1590+
t.Parallel()
1591+
1592+
client, closer := testVaultServer(t)
1593+
defer closer()
1594+
if err := client.Sys().Mount("kv/", &api.MountInput{
1595+
Type: "kv-v2",
1596+
}); err != nil {
1597+
t.Fatal(err)
1598+
}
1599+
1600+
if err := client.Sys().Mount("secrets/testapp", &api.MountInput{
1601+
Type: "kv-v2",
1602+
}); err != nil {
1603+
t.Fatal(err)
1604+
}
1605+
1606+
// Additional layer of mount path
1607+
if err := client.Sys().Mount("secrets/x/testapp", &api.MountInput{
1608+
Type: "kv-v2",
1609+
}); err != nil {
1610+
t.Fatal(err)
1611+
}
1612+
1613+
// Give time for the upgrade code to run/finish
1614+
time.Sleep(time.Second)
1615+
1616+
if _, err := client.Logical().Write("kv/data/read/foo", map[string]interface{}{
1617+
"data": map[string]interface{}{
1618+
"foo": "bar",
1619+
},
1620+
}); err != nil {
1621+
t.Fatal(err)
1622+
}
1623+
1624+
// Delete the entry so we can undelete it
1625+
if _, err := client.Logical().Delete("kv/data/read/foo"); err != nil {
1626+
t.Fatal(err)
1627+
}
1628+
1629+
if _, err := client.Logical().Write("secrets/testapp/data/test", map[string]interface{}{
1630+
"data": map[string]interface{}{
1631+
"complex": "yes",
1632+
},
1633+
}); err != nil {
1634+
t.Fatal(err)
1635+
}
1636+
1637+
if _, err := client.Logical().Write("secrets/x/testapp/data/test", map[string]interface{}{
1638+
"data": map[string]interface{}{
1639+
"complex": "yes",
1640+
},
1641+
}); err != nil {
1642+
t.Fatal(err)
1643+
}
1644+
1645+
// Delete the entry so we can undelete it
1646+
if _, err := client.Logical().Delete("secrets/x/testapp/data/test"); err != nil {
1647+
t.Fatal(err)
1648+
}
1649+
1650+
// Delete the entry so we can undelete it
1651+
if _, err := client.Logical().Delete("secrets/testapp/data/test"); err != nil {
1652+
t.Fatal(err)
1653+
}
1654+
1655+
ui, cmd := testKVUndeleteCommand(t)
1656+
cmd.client = client
1657+
1658+
code := cmd.Run(tc.args)
1659+
if code != tc.code {
1660+
t.Errorf("expected %d to be %d", code, tc.code)
1661+
}
1662+
1663+
combined := ui.OutputWriter.String() + ui.ErrorWriter.String()
1664+
1665+
for _, str := range tc.outStrings {
1666+
if !strings.Contains(combined, str) {
1667+
t.Errorf("expected %q to contain %q", combined, str)
1668+
}
1669+
}
1670+
})
1671+
}
1672+
})
1673+
}
15251674

15261675
func createTokenForPolicy(t *testing.T, client *api.Client, policy string) (*api.SecretAuth, error) {
15271676
t.Helper()

command/kv_undelete.go

+17-8
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package command
55

66
import (
77
"fmt"
8+
"path"
89
"strings"
910

1011
"github.com/mitchellh/cli"
@@ -35,12 +36,12 @@ Usage: vault kv undelete [options] KEY
3536
This restores the data, allowing it to be returned on get requests.
3637
3738
To undelete version 3 of key "foo":
38-
39+
3940
$ vault kv undelete -mount=secret -versions=3 foo
4041
41-
The deprecated path-like syntax can also be used, but this should be avoided,
42-
as the fact that it is not actually the full API path to
43-
the secret (secret/data/foo) can cause confusion:
42+
The deprecated path-like syntax can also be used, but this should be avoided,
43+
as the fact that it is not actually the full API path to
44+
the secret (secret/data/foo) can cause confusion:
4445
4546
$ vault kv undelete -versions=3 secret/foo
4647
@@ -67,10 +68,10 @@ func (c *KVUndeleteCommand) Flags() *FlagSets {
6768
Name: "mount",
6869
Target: &c.flagMount,
6970
Default: "", // no default, because the handling of the next arg is determined by whether this flag has a value
70-
Usage: `Specifies the path where the KV backend is mounted. If specified,
71-
the next argument will be interpreted as the secret path. If this flag is
72-
not specified, the next argument will be interpreted as the combined mount
73-
path and secret path, with /data/ automatically appended between KV
71+
Usage: `Specifies the path where the KV backend is mounted. If specified,
72+
the next argument will be interpreted as the secret path. If this flag is
73+
not specified, the next argument will be interpreted as the combined mount
74+
path and secret path, with /data/ automatically appended between KV
7475
v2 secrets.`,
7576
})
7677

@@ -134,6 +135,14 @@ func (c *KVUndeleteCommand) Run(args []string) int {
134135
c.UI.Error(err.Error())
135136
return 2
136137
}
138+
if v2 {
139+
// Without this join, mountPaths that are deeper
140+
// than the root path E.G. secrets/myapp will get
141+
// pruned down to myapp/undelete/<secret> which
142+
// is incorrect.
143+
// This technique was lifted from kv_delete.go.
144+
partialPath = path.Join(mountPath, partialPath)
145+
}
137146
} else {
138147
// In this case, this arg is a path-like combination of mountPath/secretPath.
139148
// (e.g. "secret/foo")

0 commit comments

Comments
 (0)