luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Add a sentinel error for missing KV secrets (#16699)

This commit is contained in:
Anton Averchenkov 2022-08-12 19:29:42 -04:00 committed by GitHub
parent 67f2f4ea2d
commit 6d45a421ff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 120 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
api/kv.go
View File

@ -1,5 +1,11 @@
package api
import "errors"
// ErrSecretNotFound is returned by KVv1 and KVv2 wrappers to indicate that the
// secret is missing at the given location.
var ErrSecretNotFound = errors.New("secret not found")
// A KVSecret is a key-value secret returned by Vault's KV secrets engine,
// and is the most basic type of secret stored in Vault.
//

2
api/kv_v1.go
View File

@ -19,7 +19,7 @@ func (kv *KVv1) Get(ctx context.Context, secretPath string) (*KVSecret, error) {
return nil, fmt.Errorf("error encountered while reading secret at %s: %w", pathToRead, err)
}
if secret == nil {
return nil, fmt.Errorf("no secret found at %s", pathToRead)
return nil, fmt.Errorf("%w: at %s", ErrSecretNotFound, pathToRead)
}
return &KVSecret{

58
api/kv_v2.go
View File

@ -2,7 +2,9 @@ package api
import (
"context"
"errors"
"fmt"
"net/http"
"sort"
"strconv"
"time"
@ -115,7 +117,7 @@ func (kv *KVv2) Get(ctx context.Context, secretPath string) (*KVSecret, error) {
return nil, fmt.Errorf("error encountered while reading secret at %s: %w", pathToRead, err)
}
if secret == nil {
return nil, fmt.Errorf("no secret found at %s", pathToRead)
return nil, fmt.Errorf("%w: at %s", ErrSecretNotFound, pathToRead)
}
kvSecret, err := extractDataAndVersionMetadata(secret)
@ -149,7 +151,7 @@ func (kv *KVv2) GetVersion(ctx context.Context, secretPath string, version int)
return nil, err
}
if secret == nil {
return nil, fmt.Errorf("no secret with version %d found at %s", version, pathToRead)
return nil, fmt.Errorf("%w: for version %d at %s", ErrSecretNotFound, version, pathToRead)
}
kvSecret, err := extractDataAndVersionMetadata(secret)
@ -175,7 +177,7 @@ func (kv *KVv2) GetVersionsAsList(ctx context.Context, secretPath string) ([]KVV
return nil, err
}
if secret == nil || secret.Data == nil {
return nil, fmt.Errorf("no secret metadata found at %s", pathToRead)
return nil, fmt.Errorf("%w: no metadata at %s", ErrSecretNotFound, pathToRead)
}
md, err := extractFullMetadata(secret)
@ -202,7 +204,7 @@ func (kv *KVv2) GetMetadata(ctx context.Context, secretPath string) (*KVMetadata
return nil, err
}
if secret == nil || secret.Data == nil {
return nil, fmt.Errorf("no secret metadata found at %s", pathToRead)
return nil, fmt.Errorf("%w: no metadata at %s", ErrSecretNotFound, pathToRead)
}
md, err := extractFullMetadata(secret)
@ -244,7 +246,7 @@ func (kv *KVv2) Put(ctx context.Context, secretPath string, data map[string]inte
return nil, fmt.Errorf("error writing secret to %s: %w", pathToWriteTo, err)
}
if secret == nil {
return nil, fmt.Errorf("no secret was written to %s", pathToWriteTo)
return nil, fmt.Errorf("%w: after writing to %s", ErrSecretNotFound, pathToWriteTo)
}
metadata, err := extractVersionMetadata(secret)
@ -325,19 +327,19 @@ func (kv *KVv2) Patch(ctx context.Context, secretPath string, newData map[string
// Determine which kind of patch to use,
// the newer HTTP Patch style or the older read-then-write style
var kvs *KVSecret
var perr error
var err error
switch patchMethod {
case "rw":
kvs, perr = readThenWrite(ctx, kv.c, kv.mountPath, secretPath, newData)
kvs, err = readThenWrite(ctx, kv.c, kv.mountPath, secretPath, newData)
case "patch":
kvs, perr = mergePatch(ctx, kv.c, kv.mountPath, secretPath, newData, opts...)
kvs, err = mergePatch(ctx, kv.c, kv.mountPath, secretPath, newData, opts...)
case "":
kvs, perr = mergePatch(ctx, kv.c, kv.mountPath, secretPath, newData, opts...)
kvs, err = mergePatch(ctx, kv.c, kv.mountPath, secretPath, newData, opts...)
default:
return nil, fmt.Errorf("unsupported patch method provided; value for patch method should be string \"rw\" or \"patch\"")
}
if perr != nil {
return nil, fmt.Errorf("unable to perform patch: %w", perr)
if err != nil {
return nil, fmt.Errorf("unable to perform patch: %w", err)
}
if kvs == nil {
return nil, fmt.Errorf("no secret was written to %s", secretPath)
@ -478,7 +480,7 @@ func (kv *KVv2) Rollback(ctx context.Context, secretPath string, toVersion int)
// Now run it again and read the version we want to roll back to
rollbackVersion, err := kv.GetVersion(ctx, secretPath, toVersion)
if err != nil {
return nil, fmt.Errorf("unable to get previous version %d of secret: %s", toVersion, err)
return nil, fmt.Errorf("unable to get previous version %d of secret: %w", toVersion, err)
}
err = validateRollbackVersion(rollbackVersion)
@ -687,18 +689,28 @@ func mergePatch(ctx context.Context, client *Client, mountPath string, secretPat
secret, err := client.Logical().JSONMergePatch(ctx, pathToMergePatch, wrappedData)
if err != nil {
// If it's a 405, that probably means the server is running a pre-1.9
// Vault version that doesn't support the HTTP PATCH method.
// Fall back to the old way of doing it.
if re, ok := err.(*ResponseError); ok && re.StatusCode == 405 {
return readThenWrite(ctx, client, mountPath, secretPath, newData)
var re *ResponseError
if errors.As(err, &re) {
switch re.StatusCode {
// 403
case http.StatusForbidden:
return nil, fmt.Errorf("received 403 from Vault server; please ensure that token's policy has \"patch\" capability: %w", err)
// 404
case http.StatusNotFound:
return nil, fmt.Errorf("%w: performing merge patch to %s", ErrSecretNotFound, pathToMergePatch)
// 405
case http.StatusMethodNotAllowed:
// If it's a 405, that probably means the server is running a pre-1.9
// Vault version that doesn't support the HTTP PATCH method.
// Fall back to the old way of doing it.
return readThenWrite(ctx, client, mountPath, secretPath, newData)
}
}
if re, ok := err.(*ResponseError); ok && re.StatusCode == 403 {
return nil, fmt.Errorf("received 403 from Vault server; please ensure that token's policy has \"patch\" capability: %w", err)
}
return nil, fmt.Errorf("error performing merge patch to %s: %s", pathToMergePatch, err)
return nil, fmt.Errorf("error performing merge patch to %s: %w", pathToMergePatch, err)
}
metadata, err := extractVersionMetadata(secret)
@ -730,7 +742,7 @@ func readThenWrite(ctx context.Context, client *Client, mountPath string, secret
// Make sure the secret already exists
if existingVersion == nil || existingVersion.Data == nil {
return nil, fmt.Errorf("no existing secret was found at %s when doing read-then-write patch operation: %w", secretPath, err)
return nil, fmt.Errorf("%w: at %s as part of read-then-write patch operation", ErrSecretNotFound, secretPath)
}
// Verify existing secret has metadata

3
changelog/16699.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
api: Add a sentinel error for missing KV secrets
```

95
vault/external_tests/api/kv_helpers_test.go
View File

@ -2,6 +2,7 @@ package api
import (
"context"
"errors"
"reflect"
"testing"
"time"
@ -80,7 +81,7 @@ func TestKVHelpers(t *testing.T) {
}
//// v1 ////
t.Run("kv v1 helpers", func(t *testing.T) {
t.Run("kv v1: put, get, and delete data", func(t *testing.T) {
if err := client.KVv1(v1MountPath).Put(context.Background(), secretPath, secretData); err != nil {
t.Fatal(err)
}
@ -97,10 +98,25 @@ func TestKVHelpers(t *testing.T) {
if err := client.KVv1(v1MountPath).Delete(context.Background(), secretPath); err != nil {
t.Fatal(err)
}
_, err = client.KVv1(v1MountPath).Get(context.Background(), secretPath)
if !errors.Is(err, api.ErrSecretNotFound) {
t.Fatalf("KVv1.Get is expected to return an api.ErrSecretNotFound wrapped error after secret had been deleted; got %v", err)
}
})
t.Run("kv v1: get secret that does not exist", func(t *testing.T) {
_, err = client.KVv1(v1MountPath).Get(context.Background(), "does/not/exist")
if err == nil {
t.Fatalf("KVv1.Get is expected to return an error for a missing secret")
}
if !errors.Is(err, api.ErrSecretNotFound) {
t.Fatalf("KVv1.Get is expected to return an api.ErrSecretNotFound wrapped error for a missing secret; got %v", err)
}
})
//// v2 ////
t.Run("get data and full metadata", func(t *testing.T) {
t.Run("kv v2: get data and full metadata", func(t *testing.T) {
teardownTest, originalSecret := setupKVv2Test(t)
defer teardownTest(t)
@ -125,7 +141,27 @@ func TestKVHelpers(t *testing.T) {
}
})
t.Run("multiple versions", func(t *testing.T) {
t.Run("kv v2: get secret that does not exist", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)
_, err = client.KVv2(v1MountPath).Get(context.Background(), "does/not/exist")
if !errors.Is(err, api.ErrSecretNotFound) {
t.Fatalf("KVv2.Get is expected to return an api.ErrSecretNotFound wrapped error for a missing secret; got %v", err)
}
_, err = client.KVv2(v1MountPath).GetMetadata(context.Background(), "does/not/exist")
if !errors.Is(err, api.ErrSecretNotFound) {
t.Fatalf("KVv2.GetMetadata is expected to return an api.ErrSecretNotFound wrapped error for a missing secret; got %v", err)
}
_, err = client.KVv2(v1MountPath).GetVersion(context.Background(), secretPath, 99)
if !errors.Is(err, api.ErrSecretNotFound) {
t.Fatalf("KVv2.GetVersion is expected to return an api.ErrSecretNotFound wrapped error for a missing secret version; got %v", err)
}
})
t.Run("kv v2: multiple versions", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)
@ -149,7 +185,7 @@ func TestKVHelpers(t *testing.T) {
}
})
t.Run("delete and undelete", func(t *testing.T) {
t.Run("kv v2: delete and undelete", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)
@ -193,9 +229,18 @@ func TestKVHelpers(t *testing.T) {
if err != nil {
t.Fatal(err)
}
s1AfterUndelete, err := client.KVv2(v2MountPath).GetVersion(context.Background(), secretPath, 1)
if err != nil {
t.Fatal(err)
}
if s1AfterUndelete.Data == nil {
t.Fatalf("data is empty for the first version of the secret despite this version being undeleted")
}
})
t.Run("destroy", func(t *testing.T) {
t.Run("kv v2: destroy", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)
@ -214,7 +259,7 @@ func TestKVHelpers(t *testing.T) {
}
})
t.Run("use named functional options and generic WithOption", func(t *testing.T) {
t.Run("kv v2: use named functional options and generic WithOption", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)
@ -238,7 +283,7 @@ func TestKVHelpers(t *testing.T) {
}
})
t.Run("patch", func(t *testing.T) {
t.Run("kv v2: patch", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)
@ -275,14 +320,6 @@ func TestKVHelpers(t *testing.T) {
t.Fatalf("incorrect version %d, expected 4", patchRW.VersionMetadata.Version)
}
// patch something that doesn't exist
_, err = client.KVv2(v2MountPath).Patch(context.Background(), "nonexistent-secret", map[string]interface{}{
"no": "nope",
})
if err == nil {
t.Fatal("expected error from trying to patch something that doesn't exist")
}
secretAfterPatches, err := client.KVv2(v2MountPath).Get(context.Background(), secretPath)
if err != nil {
t.Fatal(err)
@ -353,7 +390,25 @@ func TestKVHelpers(t *testing.T) {
}
})
t.Run("roll back to an old version", func(t *testing.T) {
t.Run("kv v2: patch a secret that does not exist", func(t *testing.T) {
for _, method := range [][]api.KVOption{
{},
{api.WithMergeMethod(api.KVMergeMethodPatch)},
{api.WithMergeMethod(api.KVMergeMethodReadWrite)},
} {
_, err = client.KVv2(v2MountPath).Patch(
context.Background(),
"does/not/exist",
map[string]interface{}{"no": "nope"},
method...,
)
if !errors.Is(err, api.ErrSecretNotFound) {
t.Fatalf("expected an api.ErrSecretNotFound wrapped error from trying to patch something that doesn't exist for %v method; got: %v", method, err)
}
}
})
t.Run("kv v2: roll back to an old version", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)
@ -402,7 +457,7 @@ func TestKVHelpers(t *testing.T) {
}
})
t.Run("delete all versions of a secret", func(t *testing.T) {
t.Run("kv v2: delete all versions of a secret", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)
@ -429,7 +484,7 @@ func TestKVHelpers(t *testing.T) {
}
})
t.Run("create a secret with metadata but no data", func(t *testing.T) {
t.Run("kv v2: create a secret with metadata but no data", func(t *testing.T) {
// put and patch metadata
////
noDataSecretPath := "empty"
@ -457,7 +512,7 @@ func TestKVHelpers(t *testing.T) {
}
})
t.Run("put and patch metadata", func(t *testing.T) {
t.Run("kv v2: put and patch metadata", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)
@ -529,7 +584,7 @@ func TestKVHelpers(t *testing.T) {
}
})
t.Run("patch with explicit zero values", func(t *testing.T) {
t.Run("kv v2: patch with explicit zero values", func(t *testing.T) {
teardownTest, _ := setupKVv2Test(t)
defer teardownTest(t)