Skip to content

Commit

Permalink
fix: 'secrets' function does not work as expected with Vault kv-v2 se…
Browse files Browse the repository at this point in the history
…crets engine
  • Loading branch information
yilmazo authored and eikenb committed May 21, 2021
1 parent d243934 commit 86fe134
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 10 deletions.
33 changes: 31 additions & 2 deletions dependency/vault_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"log"
"net/url"
"path"
"sort"
"strings"
"time"
Expand Down Expand Up @@ -59,13 +60,22 @@ func (d *VaultListQuery) Fetch(clients *ClientSet, opts *QueryOptions) (interfac
}
}

secretsPath := d.path

// Checking secret engine version. If it's v2, we should shim /metadata/
// to secret path if necessary.
mountPath, isV2, _ := isKVv2(clients.Vault(), secretsPath)
if isV2 {
secretsPath = shimKvV2ListPath(secretsPath, mountPath)
}

// If we got this far, we either didn't have a secret to renew, the secret was
// not renewable, or the renewal failed, so attempt a fresh list.
log.Printf("[TRACE] %s: LIST %s", d, &url.URL{
Path: "/v1/" + d.path,
Path: "/v1/" + secretsPath,
RawQuery: opts.String(),
})
secret, err := clients.Vault().Logical().List(d.path)
secret, err := clients.Vault().Logical().List(secretsPath)
if err != nil {
return nil, nil, errors.Wrap(err, d.String())
}
Expand Down Expand Up @@ -124,3 +134,22 @@ func (d *VaultListQuery) String() string {
func (d *VaultListQuery) Type() Type {
return TypeVault
}

// shimKvV2ListPath aligns the supported legacy path to KV v2 specs by inserting
// /metadata/ into the path for listing secrets. Paths with /metadata/ are not modified.
func shimKvV2ListPath(rawPath, mountPath string) string {
mountPath = strings.TrimSuffix(mountPath, "/")

if strings.HasPrefix(rawPath, path.Join(mountPath, "metadata")) {
// It doesn't need modifying.
return rawPath
}

switch {
case rawPath == mountPath:
return path.Join(mountPath, "metadata")
default:
rawPath = strings.TrimPrefix(rawPath, mountPath)
return path.Join(mountPath, "metadata", rawPath)
}
}
34 changes: 30 additions & 4 deletions dependency/vault_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ func TestVaultListQuery_Fetch(t *testing.T) {
clients, vault := testVaultServer(t, "listfetch", "1")
secretsPath := vault.secretsPath

clientsKvV2, vaultKvV2 := testVaultServer(t, "listfetchV2", "2")
secretsPathV2 := vaultKvV2.secretsPath

err := vault.CreateSecret("foo/bar", map[string]interface{}{
"ttl": "100ms", // explicitly make this a short duration for testing
"zip": "zap",
Expand All @@ -79,20 +82,43 @@ func TestVaultListQuery_Fetch(t *testing.T) {
t.Fatal(err)
}

err = vaultKvV2.CreateSecret("foo/bar", map[string]interface{}{
"ttl": "100ms", // explicitly make this a short duration for testing
"zip": "zap",
})
if err != nil {
t.Fatal(err)
}

cases := []struct {
name string
i string
exp []string
name string
i string
exp []string
clients *ClientSet
}{
{
"exists",
secretsPath,
[]string{"foo/"},
clients,
},
{
"no_exist",
"not/a/real/path/like/ever",
nil,
clients,
},
{
"exists kv-v2",
secretsPathV2,
[]string{"foo/"},
clientsKvV2,
},
{
"no_exist kv-v2",
"not/a/real/path/like/ever",
nil,
clientsKvV2,
},
}

Expand All @@ -103,7 +129,7 @@ func TestVaultListQuery_Fetch(t *testing.T) {
t.Fatal(err)
}

act, _, err := d.Fetch(clients, nil)
act, _, err := d.Fetch(tc.clients, nil)
if err != nil {
t.Fatal(err)
}
Expand Down
7 changes: 4 additions & 3 deletions dependency/vault_write.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,15 @@ func (d *VaultWriteQuery) writeSecret(clients *ClientSet, opts *QueryOptions) (*
RawQuery: opts.String(),
})

path := d.path
data := d.data

_, isv2, _ := isKVv2(clients.Vault(), d.path)
mountPath, isv2, _ := isKVv2(clients.Vault(), path)
if isv2 {
path = shimKVv2Path(path, mountPath)
data = map[string]interface{}{"data": d.data}
}

vaultSecret, err := clients.Vault().Logical().Write(d.path, data)
vaultSecret, err := clients.Vault().Logical().Write(path, data)
if err != nil {
return nil, errors.Wrap(err, d.String())
}
Expand Down
32 changes: 32 additions & 0 deletions dependency/vault_write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,38 @@ func TestVaultWriteSecretKV_Fetch(t *testing.T) {
}
assert.Equal(t, exp, act.Data["data"])
})

// VaultWriteQuery should work properly on kv-v2 secrets engines if /data/
// is not present on secret path
t.Run("write_secret_v2_without_data_in_path", func(t *testing.T) {
clients, vault := testVaultServer(t, "write_secret_v2_without_data_in_path", "2")
secretsPath := vault.secretsPath

path := secretsPath + "/foo"
exp := map[string]interface{}{
"bar": "zed",
}

wq, err := NewVaultWriteQuery(path, exp)
if err != nil {
t.Fatal(err)
}

_, _, err = wq.Fetch(clients, &QueryOptions{})
if err != nil {
fmt.Println(err)
}

rq, err := NewVaultReadQuery(path)
if err != nil {
t.Fatal(err)
}
act, err := rq.readSecret(clients, nil)
if err != nil {
t.Fatal(err)
}
assert.Equal(t, exp, act.Data["data"])
})
}

func TestVaultWriteQuery_Fetch(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion docs/templating-language.md
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,9 @@ To iterate and list over every secret in the generic secret backend in Vault:
{{ with secret (printf "secret/%s" .) }}{{ range $k, $v := .Data }}
{{ $k }}: {{ $v }}
{{ end }}{{ end }}{{ end }}
```
```

`.Data` should be replaced with `.Data.data` for KV-V2 secrets engines.

You should probably never do this.

Expand Down

0 comments on commit 86fe134

Please sign in to comment.