From 4ebf9eca102b7d6702413103439abbc8d48da831 Mon Sep 17 00:00:00 2001 From: John Eikenberry Date: Thu, 15 Aug 2019 21:33:10 -0700 Subject: [PATCH 1/2] test for issue1198, err reading deleted secret --- dependency/dependency_test.go | 10 ++++++++++ dependency/vault_read_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/dependency/dependency_test.go b/dependency/dependency_test.go index a80aa6121..9d8365d42 100644 --- a/dependency/dependency_test.go +++ b/dependency/dependency_test.go @@ -149,6 +149,16 @@ func (v *vaultServer) CreateSecret(path string, data map[string]interface{}, return err } +// deleteSecret lets us delete keys as needed for tests +func (v *vaultServer) deleteSecret(path string) error { + path = v.secretsPath + "/" + path + _, err := testClients.Vault().Logical().Delete(path) + if err != nil { + fmt.Println(err) + } + return err +} + func TestCanShare(t *testing.T) { t.Parallel() diff --git a/dependency/vault_read_test.go b/dependency/vault_read_test.go index 7f423c3e3..f18e48d75 100644 --- a/dependency/vault_read_test.go +++ b/dependency/vault_read_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/hashicorp/vault/api" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -325,6 +326,34 @@ func TestVaultReadQuery_Fetch_KVv2(t *testing.T) { }) } + t.Run("read_deleted", func(t *testing.T) { + // only needed for KVv2 as KVv1 doesn't have metadata + path := "data/foo/zed" + // create and delete a secret + err = vault.CreateSecret(path, map[string]interface{}{ + "zip": "zop", + }) + if err != nil { + t.Fatal(err) + } + err = vault.deleteSecret(path) + if err != nil { + t.Fatal(err) + } + // now look for entry with metadata but no data (deleted secret) + path = vault.secretsPath + "/" + path + d, err := NewVaultReadQuery(path) + if err != nil { + t.Fatal(err) + } + _, _, err = d.Fetch(clients, nil) + if err == nil { + t.Fatal("Nil received when error expected") + } + exp_err := fmt.Errorf("no secret exists at %s", path) + assert.Equal(t, exp_err, errors.Cause(err)) + }) + t.Run("stops", func(t *testing.T) { d, err := NewVaultReadQuery(secretsPath + "/foo/bar") if err != nil { From f52ffb820a8080f073a9f4570f5702497786240f Mon Sep 17 00:00:00 2001 From: John Eikenberry Date: Thu, 15 Aug 2019 21:33:35 -0700 Subject: [PATCH 2/2] fix error on reading deleted secret When reading a deleted secret from vault, vault would return meta-data and no data. The code wasn't setup to handle that and would error out in the template with an "nil pointer evaluating interface {}" error. This checks for that and returns the normal "no secret exists" error that triggers the standard retrying behavior instead of exiting. Fixes #1198 --- dependency/dependency_test.go | 3 +-- dependency/vault_read.go | 17 ++++++++++++++--- dependency/vault_read_test.go | 7 +++++-- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/dependency/dependency_test.go b/dependency/dependency_test.go index 9d8365d42..ba1aceb19 100644 --- a/dependency/dependency_test.go +++ b/dependency/dependency_test.go @@ -151,8 +151,7 @@ func (v *vaultServer) CreateSecret(path string, data map[string]interface{}, // deleteSecret lets us delete keys as needed for tests func (v *vaultServer) deleteSecret(path string) error { - path = v.secretsPath + "/" + path - _, err := testClients.Vault().Logical().Delete(path) + _, err := testClients.Vault().Logical().Delete(v.secretsPath + "/" + path) if err != nil { fmt.Println(err) } diff --git a/dependency/vault_read.go b/dependency/vault_read.go index eae6fc916..5479f90af 100644 --- a/dependency/vault_read.go +++ b/dependency/vault_read.go @@ -147,7 +147,8 @@ func (d *VaultReadQuery) readSecret(clients *ClientSet, opts *QueryOptions) (*ap if d.isKVv2 == nil { mountPath, isKVv2, err := isKVv2(vaultClient, d.rawPath) if err != nil { - log.Printf("[WARN] %s: failed to check if %s is KVv2, assume not: %s", d, d.rawPath, err) + log.Printf("[WARN] %s: failed to check if %s is KVv2, "+ + "assume not: %s", d, d.rawPath, err) isKVv2 = false d.secretPath = d.rawPath } else if isKVv2 { @@ -163,12 +164,22 @@ func (d *VaultReadQuery) readSecret(clients *ClientSet, opts *QueryOptions) (*ap Path: "/v1/" + d.secretPath, RawQuery: queryString, }) - vaultSecret, err := vaultClient.Logical().ReadWithData(d.secretPath, d.queryValues) + vaultSecret, err := vaultClient.Logical().ReadWithData(d.secretPath, + d.queryValues) + if err != nil { return nil, errors.Wrap(err, d.String()) } - if vaultSecret == nil { + if vaultSecret == nil || deletedKVv2(vaultSecret) { return nil, fmt.Errorf("no secret exists at %s", d.secretPath) } return vaultSecret, nil } + +func deletedKVv2(s *api.Secret) bool { + switch md := s.Data["metadata"].(type) { + case map[string]interface{}: + return md["deletion_time"] != "" + } + return false +} diff --git a/dependency/vault_read_test.go b/dependency/vault_read_test.go index f18e48d75..597ea6db4 100644 --- a/dependency/vault_read_test.go +++ b/dependency/vault_read_test.go @@ -350,8 +350,11 @@ func TestVaultReadQuery_Fetch_KVv2(t *testing.T) { if err == nil { t.Fatal("Nil received when error expected") } - exp_err := fmt.Errorf("no secret exists at %s", path) - assert.Equal(t, exp_err, errors.Cause(err)) + exp_err := fmt.Sprintf("no secret exists at %s", path) + if errors.Cause(err).Error() != exp_err { + t.Fatalf("Unexpected error received.\nexpected '%s'\ngot: '%s'", + exp_err, errors.Cause(err)) + } }) t.Run("stops", func(t *testing.T) {