From 66a840fb5ecd932cda42d9f4903fa22442098bfc Mon Sep 17 00:00:00 2001 From: Kyle Morton Date: Wed, 31 Aug 2022 08:36:43 -0500 Subject: [PATCH 1/7] feat: Added get keys functionality Signed-off-by: Kyle Morton --- internal/pkg/vault/secrets.go | 62 ++++++++++ internal/pkg/vault/secrets_test.go | 192 ++++++++++++++++++++++++++++- 2 files changed, 253 insertions(+), 1 deletion(-) diff --git a/internal/pkg/vault/secrets.go b/internal/pkg/vault/secrets.go index a8ea55c5..b13f163c 100644 --- a/internal/pkg/vault/secrets.go +++ b/internal/pkg/vault/secrets.go @@ -452,3 +452,65 @@ func (c *Client) store(subPath string, secrets map[string]string) error { return nil } + +// GetKeys retrieves the keys at the provided sub-path. +func (c *Client) GetKeys(subPath string) ([]string, error) { + data, err := c.getAllPaths(subPath) + if err != nil { + return nil, err + } + return data, nil +} + +// getAllKeys obtains all the keys that reside at the provided sub-path. +func (c *Client) getAllPaths(subPath string) ([]string, error) { + url, err := c.Config.BuildSecretsPathURL(subPath) + if err != nil { + return nil, err + } + + c.lc.Debug(fmt.Sprintf("Using Secrets URL of `%s`", url)) + + req, err := http.NewRequest("LIST", url, nil) + if err != nil { + return nil, err + } + + req.Header.Set(c.Config.Authentication.AuthType, c.Config.Authentication.AuthToken) + + if c.Config.Namespace != "" { + req.Header.Set(NamespaceHeader, c.Config.Namespace) + } + + resp, err := c.HttpCaller.Do(req) + + if err != nil { + return nil, err + } + defer func() { + _ = resp.Body.Close() + }() + + if resp.StatusCode == 404 { + return nil, pkg.NewErrPathNotFound(fmt.Sprintf("Received a '%d' response from the secret store", resp.StatusCode)) + } + + if resp.StatusCode < 200 || resp.StatusCode > 299 { + return nil, pkg.NewErrSecretStore(fmt.Sprintf("Received a '%d' response from the secret store", resp.StatusCode)) + } + + var result map[string]map[string][]interface{} + err = json.NewDecoder(resp.Body).Decode(&result) + if err != nil { + return nil, err + } + + data := result["data"]["keys"] + // Cast the keys to strings + var secretKeys []string + for _, v := range data { + secretKeys = append(secretKeys, v.(string)) + } + + return secretKeys, nil +} diff --git a/internal/pkg/vault/secrets_test.go b/internal/pkg/vault/secrets_test.go index b55a8a0d..79373acc 100644 --- a/internal/pkg/vault/secrets_test.go +++ b/internal/pkg/vault/secrets_test.go @@ -789,6 +789,14 @@ func getTestSecretsData() map[string]map[string]string { } } +func getTestSecretsKeysData() map[string]map[string][]string { + return map[string]map[string][]string{ + "data": { + "keys": {"one", "uno", "two", "dos", "three", "tres"}, + }, + } +} + type ErrorMockCaller struct { StatusCode int ReturnError bool @@ -812,6 +820,7 @@ func (emc *ErrorMockCaller) Do(_ *http.Request) (*http.Response, error) { type InMemoryMockCaller struct { Data map[string]map[string]string + DataList map[string]map[string][]string Result map[string]string DoCallCount int nErrorsReturned int @@ -846,7 +855,18 @@ func (caller *InMemoryMockCaller) Do(req *http.Request) (*http.Response, error) Body: ioutil.NopCloser(bytes.NewBufferString(string(r))), StatusCode: 200, }, nil - + case "LIST": + if req.URL.Path != testPath { + return &http.Response{ + Body: ioutil.NopCloser(bytes.NewBufferString("")), + StatusCode: 404, + }, nil + } + r, _ := json.Marshal(caller.DataList) + return &http.Response{ + Body: ioutil.NopCloser(bytes.NewBufferString(string(r))), + StatusCode: 200, + }, nil case http.MethodPost: if req.URL.Path != testPath { return &http.Response{ @@ -865,3 +885,173 @@ func (caller *InMemoryMockCaller) Do(req *http.Request) (*http.Response, error) return nil, errors.New("unsupported HTTP method") } } + +func TestHttpSecretStoreManager_GetKeys(t *testing.T) { + TestConnError := pkg.NewErrSecretStore("testing conn error") + TestConnErrorPathNotFound := pkg.NewErrPathNotFound("testing path error") + testData := getTestSecretsKeysData() + tests := []struct { + name string + path string + expectedValues []string + expectError bool + expectedErrorType error + expectedDoCallNum int + caller pkg.Caller + }{ + { + name: "Get Key", + path: testPath, + expectedValues: []string{"one", "uno"}, + expectError: false, + expectedErrorType: nil, + expectedDoCallNum: 1, + caller: &InMemoryMockCaller{ + DataList: testData, + }, + }, + { + name: "Get Two Keys", + path: testPath, + expectedValues: []string{"one", "uno", "two", "dos"}, + expectError: false, + expectedErrorType: nil, + expectedDoCallNum: 1, + caller: &InMemoryMockCaller{ + DataList: testData, + }, + }, + { + name: "Get all keys", + path: testPath, + expectedValues: []string{"one", "uno", "two", "dos", "three", "tres"}, + expectError: false, + expectedErrorType: nil, + expectedDoCallNum: 1, + caller: &InMemoryMockCaller{ + DataList: testData, + }, + }, + { + name: "Get non-existent Key", + path: "/error", + expectedValues: nil, + expectError: true, + expectedErrorType: pkg.NewErrPathNotFound("Does not exist"), + expectedDoCallNum: 1, + caller: &ErrorMockCaller{ + ReturnError: false, + StatusCode: 404, + ErrorType: pkg.NewErrPathNotFound("Does not exist"), + }, + }, + { + name: "Get all non-existent Keys", + path: testPath, + expectedValues: nil, + expectError: true, + expectedErrorType: pkg.NewErrPathNotFound("Does not exist"), + expectedDoCallNum: 1, + caller: &ErrorMockCaller{ + ReturnError: false, + StatusCode: 404, + ErrorType: pkg.NewErrPathNotFound("Does not exist"), + }, + }, + { + name: "Handle HTTP no path error", + path: testPath, + expectedValues: nil, + expectError: true, + expectedErrorType: TestConnErrorPathNotFound, + expectedDoCallNum: 1, + caller: &ErrorMockCaller{ + ReturnError: false, + StatusCode: 404, + ErrorType: pkg.NewErrPathNotFound("Not found"), + }, + }, + { + name: "Handle non-200 HTTP response", + path: testPath, + expectedValues: nil, + expectError: true, + expectedErrorType: TestConnError, + expectedDoCallNum: 1, + caller: &ErrorMockCaller{ + ReturnError: false, + StatusCode: 400, + ErrorType: pkg.NewErrSecretStore("Error"), + }, + }, + { + name: "Get Key with unknown path", + path: "/nonexistentpath", + expectedValues: nil, + expectError: true, + expectedErrorType: TestConnErrorPathNotFound, + expectedDoCallNum: 1, + caller: &InMemoryMockCaller{ + DataList: testData, + }, + }, + { + name: "URL Error", + path: "bad path for URL", + expectedValues: nil, + expectError: true, + expectedErrorType: errors.New(""), + caller: &InMemoryMockCaller{ + DataList: testData, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cfgHTTP := types.SecretConfig{ + Host: "localhost", + Port: 8080, + Protocol: "http", + Namespace: testNamespace, + } + ssm := Client{ + Config: cfgHTTP, + HttpCaller: test.caller, + lc: logger.NewMockClient(), + } + + actual, err := ssm.GetKeys(test.path) + if test.expectError { + require.Error(t, err) + + eet := reflect.TypeOf(test.expectedErrorType) + aet := reflect.TypeOf(err) + if !aet.AssignableTo(eet) { + t.Errorf("Expected error of type %v, but got an error of type %v", eet, aet) + } + + return + } + + var mockType string + var callCount int + switch v := test.caller.(type) { + case *ErrorMockCaller: + mockType = "ErrorMockCaller" + callCount = v.DoCallCount + case *InMemoryMockCaller: + mockType = "InMemoryMockCaller" + callCount = v.DoCallCount + } + + require.Equalf(t, test.expectedDoCallNum, callCount, + "Expected %d %s.Do calls, got %d", mockType, test.expectedDoCallNum, callCount) + + for k, expected := range test.expectedValues { + if actual[k] != expected { + assert.Equalf(t, expected, actual[k], "Expected value '%s', but got '%s'", expected, actual[k]) + } + } + }) + } +} From 34a77adc35b152ce44a60dd967f88dc7080e668d Mon Sep 17 00:00:00 2001 From: Kyle Morton Date: Wed, 31 Aug 2022 11:49:59 -0500 Subject: [PATCH 2/7] feat: Updated get keys functionality Signed-off-by: Kyle Morton --- internal/pkg/vault/secrets.go | 13 +++++++++++-- internal/pkg/vault/secrets_test.go | 17 +++++++++-------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/internal/pkg/vault/secrets.go b/internal/pkg/vault/secrets.go index b13f163c..0ee89f8c 100644 --- a/internal/pkg/vault/secrets.go +++ b/internal/pkg/vault/secrets.go @@ -453,13 +453,22 @@ func (c *Client) store(subPath string, secrets map[string]string) error { return nil } -// GetKeys retrieves the keys at the provided sub-path. +// GetKeys retrieves the keys at the provided sub-path. Secret Store returns an array of keys for a given path when +// retrieving a list of keys, versus a k/v map when retrieving secrets. func (c *Client) GetKeys(subPath string) ([]string, error) { data, err := c.getAllPaths(subPath) if err != nil { return nil, err } - return data, nil + + var validKeys []string + for _, key := range data { + if len(key) == strings.Index(key, "/")+1 || strings.Index(key, "/") < 1 { + validKeys = append(validKeys, key) + } + } + + return validKeys, nil } // getAllKeys obtains all the keys that reside at the provided sub-path. diff --git a/internal/pkg/vault/secrets_test.go b/internal/pkg/vault/secrets_test.go index 79373acc..25cfb675 100644 --- a/internal/pkg/vault/secrets_test.go +++ b/internal/pkg/vault/secrets_test.go @@ -45,7 +45,7 @@ const ( // define as constants to avoid using global variables as global variables are evil to the whole package level scope: // Global variables can cause side effects which are difficult to keep track of. A code in one function may // change the variables state while another unrelated chunk of code may be affected by it. - testPath = "/data" + testPath = "/secret1" testNamespace = "database" ) @@ -789,10 +789,10 @@ func getTestSecretsData() map[string]map[string]string { } } -func getTestSecretsKeysData() map[string]map[string][]string { +func listTestSecretsKeysData() map[string]map[string][]string { return map[string]map[string][]string{ "data": { - "keys": {"one", "uno", "two", "dos", "three", "tres"}, + "keys": {"secret1", "secret2", "secret3/", "secret4/secret1"}, }, } } @@ -889,7 +889,7 @@ func (caller *InMemoryMockCaller) Do(req *http.Request) (*http.Response, error) func TestHttpSecretStoreManager_GetKeys(t *testing.T) { TestConnError := pkg.NewErrSecretStore("testing conn error") TestConnErrorPathNotFound := pkg.NewErrPathNotFound("testing path error") - testData := getTestSecretsKeysData() + testData := listTestSecretsKeysData() tests := []struct { name string path string @@ -902,7 +902,7 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { { name: "Get Key", path: testPath, - expectedValues: []string{"one", "uno"}, + expectedValues: []string{"secret1"}, expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, @@ -913,7 +913,7 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { { name: "Get Two Keys", path: testPath, - expectedValues: []string{"one", "uno", "two", "dos"}, + expectedValues: []string{"secret1", "secret2"}, expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, @@ -924,7 +924,7 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { { name: "Get all keys", path: testPath, - expectedValues: []string{"one", "uno", "two", "dos", "three", "tres"}, + expectedValues: []string{"secret1", "secret2", "secret3/"}, expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, @@ -1046,7 +1046,8 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { require.Equalf(t, test.expectedDoCallNum, callCount, "Expected %d %s.Do calls, got %d", mockType, test.expectedDoCallNum, callCount) - + fmt.Println("a ", actual) + fmt.Println("ae ", test.expectedValues) for k, expected := range test.expectedValues { if actual[k] != expected { assert.Equalf(t, expected, actual[k], "Expected value '%s', but got '%s'", expected, actual[k]) From 5708ff6032bd09458f310e9b4d022f3cbf4534f8 Mon Sep 17 00:00:00 2001 From: Kyle Morton Date: Wed, 31 Aug 2022 14:43:07 -0500 Subject: [PATCH 3/7] feat: Updated unit test functionality Signed-off-by: Kyle Morton --- internal/pkg/vault/secrets_test.go | 67 +++++++++++++++++------------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/internal/pkg/vault/secrets_test.go b/internal/pkg/vault/secrets_test.go index 25cfb675..afdbfd75 100644 --- a/internal/pkg/vault/secrets_test.go +++ b/internal/pkg/vault/secrets_test.go @@ -46,6 +46,8 @@ const ( // Global variables can cause side effects which are difficult to keep track of. A code in one function may // change the variables state while another unrelated chunk of code may be affected by it. testPath = "/secret1" + testPath2 = "/secret2" + testPath3 = "/secret3" testNamespace = "database" ) @@ -789,10 +791,21 @@ func getTestSecretsData() map[string]map[string]string { } } -func listTestSecretsKeysData() map[string]map[string][]string { - return map[string]map[string][]string{ - "data": { - "keys": {"secret1", "secret2", "secret3/", "secret4/secret1"}, +func listTestSecretsKeysData() map[string]map[string]map[string][]string { + + return map[string]map[string]map[string][]string{ + testPath: { + "data": { + "keys": {"one", "two", "three/", "four/one"}, + }, + }, testPath2: { + "data": { + "keys": {}, + }, + }, testPath3: { + "data": { + "keys": {"four/one"}, + }, }, } } @@ -856,7 +869,7 @@ func (caller *InMemoryMockCaller) Do(req *http.Request) (*http.Response, error) StatusCode: 200, }, nil case "LIST": - if req.URL.Path != testPath { + if req.URL.Path != testPath && req.URL.Path != testPath2 && req.URL.Path != testPath3 && req.URL.Path != "" { return &http.Response{ Body: ioutil.NopCloser(bytes.NewBufferString("")), StatusCode: 404, @@ -902,39 +915,39 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { { name: "Get Key", path: testPath, - expectedValues: []string{"secret1"}, + expectedValues: []string{"one", "two", "three/"}, expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ - DataList: testData, + DataList: testData[testPath], }, }, { - name: "Get Two Keys", - path: testPath, - expectedValues: []string{"secret1", "secret2"}, + name: "No keys error", + path: testPath2, + expectedValues: []string{}, expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ - DataList: testData, + DataList: testData[testPath2], }, }, { - name: "Get all keys", - path: testPath, - expectedValues: []string{"secret1", "secret2", "secret3/"}, + name: "Subpath", + path: testPath3, + expectedValues: nil, expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ - DataList: testData, + DataList: testData[testPath3], }, }, { name: "Get non-existent Key", - path: "/error", + path: "/one", expectedValues: nil, expectError: true, expectedErrorType: pkg.NewErrPathNotFound("Does not exist"), @@ -946,16 +959,14 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { }, }, { - name: "Get all non-existent Keys", - path: testPath, - expectedValues: nil, - expectError: true, - expectedErrorType: pkg.NewErrPathNotFound("Does not exist"), + name: "Get all Keys", + path: "", + expectedValues: []string{"one", "two", "three/"}, + expectError: false, + expectedErrorType: nil, expectedDoCallNum: 1, - caller: &ErrorMockCaller{ - ReturnError: false, - StatusCode: 404, - ErrorType: pkg.NewErrPathNotFound("Does not exist"), + caller: &InMemoryMockCaller{ + DataList: testData[testPath], }, }, { @@ -992,7 +1003,7 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { expectedErrorType: TestConnErrorPathNotFound, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ - DataList: testData, + DataList: testData[testPath2], }, }, { @@ -1002,7 +1013,7 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { expectError: true, expectedErrorType: errors.New(""), caller: &InMemoryMockCaller{ - DataList: testData, + DataList: testData[testPath2], }, }, } @@ -1046,8 +1057,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { require.Equalf(t, test.expectedDoCallNum, callCount, "Expected %d %s.Do calls, got %d", mockType, test.expectedDoCallNum, callCount) - fmt.Println("a ", actual) - fmt.Println("ae ", test.expectedValues) for k, expected := range test.expectedValues { if actual[k] != expected { assert.Equalf(t, expected, actual[k], "Expected value '%s', but got '%s'", expected, actual[k]) From 52c5176faeb6eebee081d3b1770cc721b4822059 Mon Sep 17 00:00:00 2001 From: Kyle Morton Date: Wed, 31 Aug 2022 17:28:52 -0500 Subject: [PATCH 4/7] feat: Updated unit test Signed-off-by: Kyle Morton --- internal/pkg/vault/secrets.go | 11 ++----- internal/pkg/vault/secrets_test.go | 48 +++++++++++++----------------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/internal/pkg/vault/secrets.go b/internal/pkg/vault/secrets.go index 0ee89f8c..326e8586 100644 --- a/internal/pkg/vault/secrets.go +++ b/internal/pkg/vault/secrets.go @@ -461,14 +461,7 @@ func (c *Client) GetKeys(subPath string) ([]string, error) { return nil, err } - var validKeys []string - for _, key := range data { - if len(key) == strings.Index(key, "/")+1 || strings.Index(key, "/") < 1 { - validKeys = append(validKeys, key) - } - } - - return validKeys, nil + return data, nil } // getAllKeys obtains all the keys that reside at the provided sub-path. @@ -478,7 +471,7 @@ func (c *Client) getAllPaths(subPath string) ([]string, error) { return nil, err } - c.lc.Debug(fmt.Sprintf("Using Secrets URL of `%s`", url)) + c.lc.Debugf("Using Secrets URL of `%s`", url) req, err := http.NewRequest("LIST", url, nil) if err != nil { diff --git a/internal/pkg/vault/secrets_test.go b/internal/pkg/vault/secrets_test.go index afdbfd75..f0bc7065 100644 --- a/internal/pkg/vault/secrets_test.go +++ b/internal/pkg/vault/secrets_test.go @@ -48,6 +48,7 @@ const ( testPath = "/secret1" testPath2 = "/secret2" testPath3 = "/secret3" + testPath4 = "" testNamespace = "database" ) @@ -390,7 +391,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path string keys []string expectedValues map[string]string - expectError bool expectedErrorType error expectedDoCallNum int caller pkg.Caller @@ -400,7 +400,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: testPath, keys: []string{"one"}, expectedValues: map[string]string{"one": "uno"}, - expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -412,7 +411,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: testPath, keys: []string{"one", "two"}, expectedValues: map[string]string{"one": "uno", "two": "dos"}, - expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -424,7 +422,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: testPath, keys: nil, expectedValues: map[string]string{"one": "uno", "two": "dos", "three": "tres"}, - expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -436,7 +433,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: testPath, keys: []string{"Does not exist"}, expectedValues: nil, - expectError: true, expectedErrorType: pkg.NewErrSecretsNotFound([]string{"Does not exist"}), expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -448,7 +444,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: testPath, keys: []string{"Does not exist", "Also does not exist"}, expectedValues: nil, - expectError: true, expectedErrorType: pkg.NewErrSecretsNotFound([]string{"Does not exist", "Also does not exist"}), expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -460,7 +455,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: testPath, keys: []string{"one", "Does not exist", "Also does not exist"}, expectedValues: nil, - expectError: true, expectedErrorType: pkg.NewErrSecretsNotFound([]string{"Does not exist", "Also does not exist"}), expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -472,7 +466,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: testPath, keys: []string{"Does not exist"}, expectedValues: nil, - expectError: true, expectedErrorType: TestConnErrorPathNotFound, expectedDoCallNum: 1, caller: &ErrorMockCaller{ @@ -486,7 +479,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: testPath, keys: []string{"Does not exist"}, expectedValues: nil, - expectError: true, expectedErrorType: TestConnError, expectedDoCallNum: 1, caller: &ErrorMockCaller{ @@ -500,7 +492,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: "/nonexistentpath", keys: []string{"one"}, expectedValues: nil, - expectError: true, expectedErrorType: TestConnErrorPathNotFound, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -512,7 +503,6 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { path: "bad path for URL", keys: []string{"one"}, expectedValues: nil, - expectError: true, expectedErrorType: errors.New(""), caller: &InMemoryMockCaller{ Data: testData, @@ -534,7 +524,7 @@ func TestHttpSecretStoreManager_GetValue(t *testing.T) { } actual, err := ssm.GetSecrets(test.path, test.keys...) - if test.expectError { + if test.expectedErrorType != nil { require.Error(t, err) eet := reflect.TypeOf(test.expectedErrorType) @@ -792,11 +782,11 @@ func getTestSecretsData() map[string]map[string]string { } func listTestSecretsKeysData() map[string]map[string]map[string][]string { - + // The "testPath" result set defined below is also used in test cases for "GetKeys()". return map[string]map[string]map[string][]string{ testPath: { "data": { - "keys": {"one", "two", "three/", "four/one"}, + "keys": {"one", "two", "three/", "four/"}, }, }, testPath2: { "data": { @@ -804,7 +794,12 @@ func listTestSecretsKeysData() map[string]map[string]map[string][]string { }, }, testPath3: { "data": { - "keys": {"four/one"}, + "keys": {"four/"}, + }, + }, + testPath4: { + "data": { + "keys": {"four/"}, }, }, } @@ -869,7 +864,8 @@ func (caller *InMemoryMockCaller) Do(req *http.Request) (*http.Response, error) StatusCode: 200, }, nil case "LIST": - if req.URL.Path != testPath && req.URL.Path != testPath2 && req.URL.Path != testPath3 && req.URL.Path != "" { + acceptedPaths := listTestSecretsKeysData() + if _, ok := acceptedPaths[req.URL.Path]; !ok { return &http.Response{ Body: ioutil.NopCloser(bytes.NewBufferString("")), StatusCode: 404, @@ -960,13 +956,13 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { }, { name: "Get all Keys", - path: "", - expectedValues: []string{"one", "two", "three/"}, + path: testPath4, + expectedValues: []string{"four/"}, expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ - DataList: testData[testPath], + DataList: testData[testPath4], }, }, { @@ -977,9 +973,8 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { expectedErrorType: TestConnErrorPathNotFound, expectedDoCallNum: 1, caller: &ErrorMockCaller{ - ReturnError: false, - StatusCode: 404, - ErrorType: pkg.NewErrPathNotFound("Not found"), + StatusCode: 404, + ErrorType: pkg.NewErrPathNotFound("Not found"), }, }, { @@ -990,9 +985,8 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { expectedErrorType: TestConnError, expectedDoCallNum: 1, caller: &ErrorMockCaller{ - ReturnError: false, - StatusCode: 400, - ErrorType: pkg.NewErrSecretStore("Error"), + StatusCode: 400, + ErrorType: pkg.NewErrSecretStore("Error"), }, }, { @@ -1025,13 +1019,13 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { Protocol: "http", Namespace: testNamespace, } - ssm := Client{ + client := Client{ Config: cfgHTTP, HttpCaller: test.caller, lc: logger.NewMockClient(), } - actual, err := ssm.GetKeys(test.path) + actual, err := client.GetKeys(test.path) if test.expectError { require.Error(t, err) From e5c1b3811163c2a8f4468e576cbb468415046950 Mon Sep 17 00:00:00 2001 From: Kyle Morton Date: Thu, 1 Sep 2022 08:53:32 -0500 Subject: [PATCH 5/7] feat:Added interface Signed-off-by: Kyle Morton --- secrets/interfaces.go | 4 ++++ secrets/mocks/SecretClient.go | 40 ++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/secrets/interfaces.go b/secrets/interfaces.go index ed64548b..c4a8dca3 100644 --- a/secrets/interfaces.go +++ b/secrets/interfaces.go @@ -48,6 +48,10 @@ type SecretClient interface { // SetAuthToken sets the internal Auth Token with the new value specified. SetAuthToken(ctx context.Context, token string) error + + // GetKeys retrieves the keys at the provided sub-path. Secret Store returns an array of keys for a given path when + // retrieving a list of keys, versus a k/v map when retrieving secrets. + GetKeys(subPath string) ([]string, error) } // SecretStoreClient provides a contract for managing a Secret Store from a secret store provider. diff --git a/secrets/mocks/SecretClient.go b/secrets/mocks/SecretClient.go index ff709cff..47f818c1 100644 --- a/secrets/mocks/SecretClient.go +++ b/secrets/mocks/SecretClient.go @@ -1,4 +1,4 @@ -// Code generated by mockery v0.0.0-dev. DO NOT EDIT. +// Code generated by mockery v2.14.0. DO NOT EDIT. package mocks @@ -34,6 +34,29 @@ func (_m *SecretClient) GenerateConsulToken(serviceKey string) (string, error) { return r0, r1 } +// GetKeys provides a mock function with given fields: subPath +func (_m *SecretClient) GetKeys(subPath string) ([]string, error) { + ret := _m.Called(subPath) + + var r0 []string + if rf, ok := ret.Get(0).(func(string) []string); ok { + r0 = rf(subPath) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(subPath) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetSecrets provides a mock function with given fields: subPath, keys func (_m *SecretClient) GetSecrets(subPath string, keys ...string) (map[string]string, error) { _va := make([]interface{}, len(keys)) @@ -91,3 +114,18 @@ func (_m *SecretClient) StoreSecrets(subPath string, _a1 map[string]string) erro return r0 } + +type mockConstructorTestingTNewSecretClient interface { + mock.TestingT + Cleanup(func()) +} + +// NewSecretClient creates a new instance of SecretClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewSecretClient(t mockConstructorTestingTNewSecretClient) *SecretClient { + mock := &SecretClient{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} From 6927774243332f5fedaa77d1512dd983e8e2f634 Mon Sep 17 00:00:00 2001 From: Kyle Morton Date: Thu, 1 Sep 2022 10:54:18 -0500 Subject: [PATCH 6/7] feat: Fixed unit test Signed-off-by: Kyle Morton --- pkg/listener/poll_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/listener/poll_test.go b/pkg/listener/poll_test.go index 411f615a..9e726f2e 100644 --- a/pkg/listener/poll_test.go +++ b/pkg/listener/poll_test.go @@ -98,6 +98,10 @@ func (mssm MockSecretClient) StoreSecrets(path string, secrets map[string]string return nil } +func (mssm MockSecretClient) GetKeys(subPath string) ([]string, error) { + return nil, nil +} + func (mssm MockSecretClient) GetTokenDetails() (*types.TokenMetadata, error) { return nil, nil } From b8c65c93f47b5e367d712c857259e8e673e47cd6 Mon Sep 17 00:00:00 2001 From: Kyle Morton Date: Thu, 1 Sep 2022 13:43:47 -0500 Subject: [PATCH 7/7] feat: Updated unit test Signed-off-by: Kyle Morton --- internal/pkg/vault/secrets_test.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/internal/pkg/vault/secrets_test.go b/internal/pkg/vault/secrets_test.go index f0bc7065..711928d9 100644 --- a/internal/pkg/vault/secrets_test.go +++ b/internal/pkg/vault/secrets_test.go @@ -903,7 +903,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name string path string expectedValues []string - expectError bool expectedErrorType error expectedDoCallNum int caller pkg.Caller @@ -912,7 +911,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name: "Get Key", path: testPath, expectedValues: []string{"one", "two", "three/"}, - expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -923,7 +921,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name: "No keys error", path: testPath2, expectedValues: []string{}, - expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -934,7 +931,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name: "Subpath", path: testPath3, expectedValues: nil, - expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -945,7 +941,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name: "Get non-existent Key", path: "/one", expectedValues: nil, - expectError: true, expectedErrorType: pkg.NewErrPathNotFound("Does not exist"), expectedDoCallNum: 1, caller: &ErrorMockCaller{ @@ -958,7 +953,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name: "Get all Keys", path: testPath4, expectedValues: []string{"four/"}, - expectError: false, expectedErrorType: nil, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -969,7 +963,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name: "Handle HTTP no path error", path: testPath, expectedValues: nil, - expectError: true, expectedErrorType: TestConnErrorPathNotFound, expectedDoCallNum: 1, caller: &ErrorMockCaller{ @@ -981,7 +974,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name: "Handle non-200 HTTP response", path: testPath, expectedValues: nil, - expectError: true, expectedErrorType: TestConnError, expectedDoCallNum: 1, caller: &ErrorMockCaller{ @@ -993,7 +985,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name: "Get Key with unknown path", path: "/nonexistentpath", expectedValues: nil, - expectError: true, expectedErrorType: TestConnErrorPathNotFound, expectedDoCallNum: 1, caller: &InMemoryMockCaller{ @@ -1004,7 +995,6 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { name: "URL Error", path: "bad path for URL", expectedValues: nil, - expectError: true, expectedErrorType: errors.New(""), caller: &InMemoryMockCaller{ DataList: testData[testPath2], @@ -1026,7 +1016,7 @@ func TestHttpSecretStoreManager_GetKeys(t *testing.T) { } actual, err := client.GetKeys(test.path) - if test.expectError { + if test.expectedErrorType != nil { require.Error(t, err) eet := reflect.TypeOf(test.expectedErrorType)