Skip to content

Commit

Permalink
fix writing secrets to vault KV(v1 and v2)
Browse files Browse the repository at this point in the history
Writing to a Vault KV v1 and v2 were both broken.

V1 had a nil pointer dereference panic in Fetch where it tried to access
variables on the replied Secret which is always nil with V1.

V2 didn't get the sent in data structure correct as it didn't have the
additional map[string]interface{"data":...} wrapping around the values
that is required by V2.

Fixes #1252
  • Loading branch information
eikenb committed Aug 13, 2019
1 parent f183073 commit 4a84b72
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 16 deletions.
13 changes: 5 additions & 8 deletions dependency/vault_read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,21 +232,17 @@ func TestVaultReadQuery_Fetch_KVv2(t *testing.T) {

// Write an initial value to the secret path
err := vault.CreateSecret("data/foo/bar", map[string]interface{}{
"data": map[string]interface{}{
"ttl": "100ms", // explicitly make this a short duration for testing
"zip": "zap",
},
"ttl": "100ms", // explicitly make this a short duration for testing
"zip": "zap",
})
if err != nil {
t.Fatal(err)
}

// Write a new value to increment the version
err = vault.CreateSecret("data/foo/bar", map[string]interface{}{
"data": map[string]interface{}{
"ttl": "100ms", // explicitly make this a short duration for testing
"zip": "zop",
},
"ttl": "100ms", // explicitly make this a short duration for testing
"zip": "zop",
})
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -437,6 +433,7 @@ func TestVaultReadQuery_Fetch_PKI_Anonymous(t *testing.T) {
})
_, err = anonClient.vault.client.Auth().Token().LookupSelf()
if err == nil || !strings.Contains(err.Error(), "missing client token") {
// check environment for VAULT_TOKEN
t.Fatalf("expected a missing client token error but found: %v", err)
}

Expand Down
26 changes: 18 additions & 8 deletions dependency/vault_write.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,14 @@ func (d *VaultWriteQuery) Fetch(clients *ClientSet, opts *QueryOptions) (interfa
return nil, nil, errors.Wrap(err, d.String())
}

// Print any warnings
printVaultWarnings(d, vaultSecret.Warnings)

// Create the cloned secret which will be exposed to the template.
d.vaultSecret = vaultSecret
d.secret = transformSecret(vaultSecret)
// vaultSecret == nil when writing to KVv1 engines
if vaultSecret != nil {
// Print any warnings
printVaultWarnings(d, vaultSecret.Warnings)
// Create the cloned secret which will be exposed to the template.
d.vaultSecret = vaultSecret
d.secret = transformSecret(vaultSecret)
}

return respWithMetadata(d.secret)
}
Expand Down Expand Up @@ -168,12 +170,20 @@ func (d *VaultWriteQuery) writeSecret(clients *ClientSet, opts *QueryOptions) (*
RawQuery: opts.String(),
})

vaultSecret, err := clients.Vault().Logical().Write(d.path, d.data)
data := d.data

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

vaultSecret, err := clients.Vault().Logical().Write(d.path, data)
if err != nil {
return nil, errors.Wrap(err, d.String())
}
// always nil when KVv1 engine, when isv2==false
if vaultSecret == nil {
if _, isv2, _ := isKVv2(clients.Vault(), d.path); isv2 {
if isv2 {
return nil, fmt.Errorf("no secret exists at %s", d.path)
}
}
Expand Down

0 comments on commit 4a84b72

Please sign in to comment.