From a142d706893063b96c693d7f733241de3deb654e Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 11 Aug 2022 19:15:25 -0400 Subject: [PATCH 1/8] Add a sentinel error for missing kv secrets --- api/kv.go | 6 ++++++ api/kv_v1.go | 7 +++++-- api/kv_v2.go | 32 ++++++++++++++++++++++---------- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/api/kv.go b/api/kv.go index 16437582e703c..37699df266f9f 100644 --- a/api/kv.go +++ b/api/kv.go @@ -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. // diff --git a/api/kv_v1.go b/api/kv_v1.go index d269070bc38b0..dfa181d14abda 100644 --- a/api/kv_v1.go +++ b/api/kv_v1.go @@ -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{ @@ -36,10 +36,13 @@ func (kv *KVv1) Get(ctx context.Context, secretPath string) (*KVSecret, error) { func (kv *KVv1) Put(ctx context.Context, secretPath string, data map[string]interface{}) error { pathToWriteTo := fmt.Sprintf("%s/%s", kv.mountPath, secretPath) - _, err := kv.c.Logical().WriteWithContext(ctx, pathToWriteTo, data) + secret, err := kv.c.Logical().WriteWithContext(ctx, pathToWriteTo, data) if err != nil { return fmt.Errorf("error writing secret to %s: %w", pathToWriteTo, err) } + if secret == nil { + return fmt.Errorf("%w: after writing to %s", ErrSecretNotFound, pathToWriteTo) + } return nil } diff --git a/api/kv_v2.go b/api/kv_v2.go index f0f59abfe57ac..ba4445933110a 100644 --- a/api/kv_v2.go +++ b/api/kv_v2.go @@ -115,7 +115,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 +149,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", err, version, pathToRead) } kvSecret, err := extractDataAndVersionMetadata(secret) @@ -175,7 +175,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 +202,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 +244,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) @@ -291,10 +291,13 @@ func (kv *KVv2) PutMetadata(ctx context.Context, secretPath string, metadata KVM metadataMap[casRequiredKey] = metadata.CASRequired metadataMap[customMetadataKey] = metadata.CustomMetadata - _, err := kv.c.Logical().WriteWithContext(ctx, pathToWriteTo, metadataMap) + secret, err := kv.c.Logical().WriteWithContext(ctx, pathToWriteTo, metadataMap) if err != nil { return fmt.Errorf("error writing secret metadata to %s: %w", pathToWriteTo, err) } + if secret == nil { + return fmt.Errorf("%w: after writing to %s", ErrSecretNotFound, pathToWriteTo) + } return nil } @@ -357,10 +360,13 @@ func (kv *KVv2) PatchMetadata(ctx context.Context, secretPath string, metadata K return fmt.Errorf("unable to create map for JSON merge patch request: %w", err) } - _, err = kv.c.Logical().JSONMergePatch(ctx, pathToWriteTo, md) + secret, err := kv.c.Logical().JSONMergePatch(ctx, pathToWriteTo, md) if err != nil { return fmt.Errorf("error patching metadata at %s: %w", pathToWriteTo, err) } + if secret == nil { + return fmt.Errorf("%w: after patching metadata at %s", ErrSecretNotFound, pathToWriteTo) + } return nil } @@ -427,10 +433,13 @@ func (kv *KVv2) Undelete(ctx context.Context, secretPath string, versions []int) "versions": versions, } - _, err := kv.c.Logical().WriteWithContext(ctx, pathToUndelete, data) + secret, err := kv.c.Logical().WriteWithContext(ctx, pathToUndelete, data) if err != nil { return fmt.Errorf("error undeleting secret metadata at %s: %w", pathToUndelete, err) } + if secret == nil { + return fmt.Errorf("%w: after undeleting secret at %s", ErrSecretNotFound, pathToUndelete) + } return nil } @@ -478,7 +487,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) @@ -698,7 +707,10 @@ func mergePatch(ctx context.Context, client *Client, mountPath string, secretPat 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) + } + if secret == nil { + return nil, fmt.Errorf("%w: performing merge patch at %s", ErrSecretNotFound, pathToMergePatch) } metadata, err := extractVersionMetadata(secret) From bbcca1462db1bab076a5467dab451b7a7a3ea1f0 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 11 Aug 2022 19:22:01 -0400 Subject: [PATCH 2/8] one more --- api/kv_v2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/kv_v2.go b/api/kv_v2.go index ba4445933110a..41c8f7f2cb406 100644 --- a/api/kv_v2.go +++ b/api/kv_v2.go @@ -149,7 +149,7 @@ func (kv *KVv2) GetVersion(ctx context.Context, secretPath string, version int) return nil, err } if secret == nil { - return nil, fmt.Errorf("%w: for version %d at %s", err, version, pathToRead) + return nil, fmt.Errorf("%w: for version %d at %s", ErrSecretNotFound, version, pathToRead) } kvSecret, err := extractDataAndVersionMetadata(secret) From ec47c575f84461f1e866c48a8b868210b5ad30cb Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 11 Aug 2022 19:51:05 -0400 Subject: [PATCH 3/8] Add changelog --- changelog/16699.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/16699.txt diff --git a/changelog/16699.txt b/changelog/16699.txt new file mode 100644 index 0000000000000..55e479fca4fdb --- /dev/null +++ b/changelog/16699.txt @@ -0,0 +1,3 @@ +```release-note:bug +api: Add a sentinel error for missing KV secrets +``` From 829f42d7c26f730408fd79956497d60301e6fc83 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Fri, 12 Aug 2022 14:03:17 -0400 Subject: [PATCH 4/8] Remove redundant secret == nil checks --- api/kv_v1.go | 5 +---- api/kv_v2.go | 60 +++++++++++++++++++++++++++------------------------- 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/api/kv_v1.go b/api/kv_v1.go index dfa181d14abda..22ba992384b79 100644 --- a/api/kv_v1.go +++ b/api/kv_v1.go @@ -36,13 +36,10 @@ func (kv *KVv1) Get(ctx context.Context, secretPath string) (*KVSecret, error) { func (kv *KVv1) Put(ctx context.Context, secretPath string, data map[string]interface{}) error { pathToWriteTo := fmt.Sprintf("%s/%s", kv.mountPath, secretPath) - secret, err := kv.c.Logical().WriteWithContext(ctx, pathToWriteTo, data) + _, err := kv.c.Logical().WriteWithContext(ctx, pathToWriteTo, data) if err != nil { return fmt.Errorf("error writing secret to %s: %w", pathToWriteTo, err) } - if secret == nil { - return fmt.Errorf("%w: after writing to %s", ErrSecretNotFound, pathToWriteTo) - } return nil } diff --git a/api/kv_v2.go b/api/kv_v2.go index 41c8f7f2cb406..c67e0de0a40ed 100644 --- a/api/kv_v2.go +++ b/api/kv_v2.go @@ -2,7 +2,9 @@ package api import ( "context" + "errors" "fmt" + "net/http" "sort" "strconv" "time" @@ -291,13 +293,10 @@ func (kv *KVv2) PutMetadata(ctx context.Context, secretPath string, metadata KVM metadataMap[casRequiredKey] = metadata.CASRequired metadataMap[customMetadataKey] = metadata.CustomMetadata - secret, err := kv.c.Logical().WriteWithContext(ctx, pathToWriteTo, metadataMap) + _, err := kv.c.Logical().WriteWithContext(ctx, pathToWriteTo, metadataMap) if err != nil { return fmt.Errorf("error writing secret metadata to %s: %w", pathToWriteTo, err) } - if secret == nil { - return fmt.Errorf("%w: after writing to %s", ErrSecretNotFound, pathToWriteTo) - } return nil } @@ -328,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) @@ -360,13 +359,10 @@ func (kv *KVv2) PatchMetadata(ctx context.Context, secretPath string, metadata K return fmt.Errorf("unable to create map for JSON merge patch request: %w", err) } - secret, err := kv.c.Logical().JSONMergePatch(ctx, pathToWriteTo, md) + _, err = kv.c.Logical().JSONMergePatch(ctx, pathToWriteTo, md) if err != nil { return fmt.Errorf("error patching metadata at %s: %w", pathToWriteTo, err) } - if secret == nil { - return fmt.Errorf("%w: after patching metadata at %s", ErrSecretNotFound, pathToWriteTo) - } return nil } @@ -433,13 +429,10 @@ func (kv *KVv2) Undelete(ctx context.Context, secretPath string, versions []int) "versions": versions, } - secret, err := kv.c.Logical().WriteWithContext(ctx, pathToUndelete, data) + _, err := kv.c.Logical().WriteWithContext(ctx, pathToUndelete, data) if err != nil { return fmt.Errorf("error undeleting secret metadata at %s: %w", pathToUndelete, err) } - if secret == nil { - return fmt.Errorf("%w: after undeleting secret at %s", ErrSecretNotFound, pathToUndelete) - } return nil } @@ -696,21 +689,30 @@ 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) - } - - 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) + 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) + } } return nil, fmt.Errorf("error performing merge patch to %s: %w", pathToMergePatch, err) } if secret == nil { - return nil, fmt.Errorf("%w: performing merge patch at %s", ErrSecretNotFound, pathToMergePatch) + return nil, fmt.Errorf("%w: performing merge patch to %s", ErrSecretNotFound, pathToMergePatch) } metadata, err := extractVersionMetadata(secret) @@ -742,7 +744,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 From 36960c38df8289faa6a03d5d207ceb5607c0bbf5 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Fri, 12 Aug 2022 14:03:38 -0400 Subject: [PATCH 5/8] Add checks for ErrSecretNotFound --- vault/external_tests/api/kv_helpers_test.go | 85 ++++++++++++++++----- 1 file changed, 65 insertions(+), 20 deletions(-) diff --git a/vault/external_tests/api/kv_helpers_test.go b/vault/external_tests/api/kv_helpers_test.go index 83f83d42e3017..9552cdc5d5edc 100644 --- a/vault/external_tests/api/kv_helpers_test.go +++ b/vault/external_tests/api/kv_helpers_test.go @@ -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,17 @@ 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) { + _, err = client.KVv2(v1MountPath).Get(context.Background(), "does/not/exist") + if err == nil { + t.Fatalf("KVv2.Get is expected to return an error for a missing secret") + } + 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) + } + }) + + t.Run("kv v2: multiple versions", func(t *testing.T) { teardownTest, _ := setupKVv2Test(t) defer teardownTest(t) @@ -149,7 +175,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 +219,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 +249,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 +273,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 +310,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 +380,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 +447,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 +474,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 +502,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 +574,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) From 3a86d579eb5a1ac410684c050b597d2f96d24d5b Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Fri, 12 Aug 2022 14:12:31 -0400 Subject: [PATCH 6/8] one more test --- vault/external_tests/api/kv_helpers_test.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/vault/external_tests/api/kv_helpers_test.go b/vault/external_tests/api/kv_helpers_test.go index 9552cdc5d5edc..f005a35d97ad2 100644 --- a/vault/external_tests/api/kv_helpers_test.go +++ b/vault/external_tests/api/kv_helpers_test.go @@ -142,13 +142,23 @@ func TestKVHelpers(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 err == nil { - t.Fatalf("KVv2.Get is expected to return an error for a missing secret") - } 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) { From 8dab3e3b847657b77cdf69570c7aadd1088a4b10 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Fri, 12 Aug 2022 15:52:20 -0400 Subject: [PATCH 7/8] bug -> improvement --- changelog/16699.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/16699.txt b/changelog/16699.txt index 55e479fca4fdb..4a96e868a191f 100644 --- a/changelog/16699.txt +++ b/changelog/16699.txt @@ -1,3 +1,3 @@ -```release-note:bug +```release-note:improvement api: Add a sentinel error for missing KV secrets ``` From 7923ea9fe31bc24b4edb3d504452b788f9f8d4c7 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Fri, 12 Aug 2022 15:52:53 -0400 Subject: [PATCH 8/8] remove one extra secret == nil check --- api/kv_v2.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/kv_v2.go b/api/kv_v2.go index c67e0de0a40ed..7a98cfeefd234 100644 --- a/api/kv_v2.go +++ b/api/kv_v2.go @@ -690,6 +690,7 @@ func mergePatch(ctx context.Context, client *Client, mountPath string, secretPat secret, err := client.Logical().JSONMergePatch(ctx, pathToMergePatch, wrappedData) if err != nil { var re *ResponseError + if errors.As(err, &re) { switch re.StatusCode { // 403 @@ -711,9 +712,6 @@ func mergePatch(ctx context.Context, client *Client, mountPath string, secretPat return nil, fmt.Errorf("error performing merge patch to %s: %w", pathToMergePatch, err) } - if secret == nil { - return nil, fmt.Errorf("%w: performing merge patch to %s", ErrSecretNotFound, pathToMergePatch) - } metadata, err := extractVersionMetadata(secret) if err != nil {