refactor: Rework to be proper abstraction of a SecretStore and add SecretStoreClient from edgex-go

internal/pkg/vault/management.go

@@ -36,14 +36,12 @@ func (c *Client) HealthCheck() (int, error) {
 		ResponseObject:       nil,
 	})
 
-	// Heath check returns 5xx codes when unhealthy;
-	// return error object only if we don't get numeric code back
-	if code == 0 {
-		return 0, err
+	// If code is 0 there was a more serious error that prevented request for executing
+	if code != 0 {
+		c.lc.Infof("Vault health check HTTP status: StatusCode: %d", code)
 	}
 
-	c.lc.Infof("vault health check HTTP status: StatusCode: %d", code)
-	return code, nil
+	return code, err
 }
 
 func (c *Client) Init(secretThreshold int, secretShares int) (types.InitResponse, error) {
@@ -139,7 +137,7 @@ func (c *Client) EnableKVSecretEngine(token string, mountPoint string, kvVersion
 		Path:                 urlPath,
 		JSONObject:           parameters,
 		BodyReader:           nil,
-		OperationDescription: "update mounts",
+		OperationDescription: "update mounts for KV",
 		ExpectedStatusCode:   http.StatusNoContent,
 		ResponseObject:       nil,
 	})
@@ -180,14 +178,18 @@ func (c *Client) CheckSecretEngineInstalled(token string, mountPoint string, eng
 		Path:                 MountsAPI,
 		JSONObject:           nil,
 		BodyReader:           nil,
-		OperationDescription: "query mounts for Consul",
+		OperationDescription: "query mounts for" + engine,
 		ExpectedStatusCode:   http.StatusOK,
 		ResponseObject:       &response,
 	})
 
+	if err != nil {
+		return false, err
+	}
+
 	if mountData := response.Data[mountPoint]; mountData.Type == engine {
 		return true, nil
 	}
 
-	return false, err
+	return false, nil
 }

internal/pkg/vault/managment_test.go → internal/pkg/vault/management_test.go

@@ -64,7 +64,7 @@ func TestHealthCheckUninitialized(t *testing.T) {
 	client := createClient(t, ts.URL, mockLogger)
 
 	code, err := client.HealthCheck()
-	require.NoError(t, err)
+	require.Error(t, err)
 	assert.Equal(t, http.StatusNotImplemented, code)
 }
 
@@ -79,7 +79,7 @@ func TestHealthCheckSealed(t *testing.T) {
 	client := createClient(t, ts.URL, mockLogger)
 
 	code, err := client.HealthCheck()
-	require.NoError(t, err)
+	require.Error(t, err)
 	assert.Equal(t, http.StatusServiceUnavailable, code)
 }
 
@@ -210,6 +210,21 @@ func TestCheckSecretEngineInstalled(t *testing.T) {
 					"seal_wrap": false,
 					"type": "kv"
 				},
+				"consul/": {
+					"accessor": "consul_cb2f6638",
+					"config": {
+					  "default_lease_ttl": 0,
+					  "force_no_cache": false,
+					  "max_lease_ttl": 0
+					},
+					"description": "consul secret storage",
+					"external_entropy_access": false,
+					"local": false,
+					"options": {},
+					"seal_wrap": false,
+					"type": "consul",
+					"uuid": "512886f9-61e1-d662-dd1b-d583f20e1875"
+				},
 				"sys/": {
 					"accessor": "system_5e0c411d",
 					"config": {
@@ -232,12 +247,25 @@ func TestCheckSecretEngineInstalled(t *testing.T) {
 
 	client := createClient(t, ts.URL, mockLogger)
 
-	// Act
-	installed, err := client.CheckSecretEngineInstalled(expectedToken, "secret/", "kv")
+	tests := []struct {
+		name       string
+		mountPath  string
+		engineType string
+	}{
+		{"kv v1 secret storage installed", "secret/", KeyValue},
+		{"consul secret storage installed", "consul/", Consul},
+	}
 
-	// Assert
-	require.NoError(t, err)
-	assert.True(t, installed)
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			// Act
+			installed, err := client.CheckSecretEngineInstalled("fake-token", test.mountPath, test.engineType)
+
+			// Assert
+			require.NoError(t, err)
+			require.True(t, installed)
+		})
+	}
 }
 
 func TestCheckSecretEngineNotInstalled(t *testing.T) {
@@ -314,12 +342,25 @@ func TestCheckSecretEngineNotInstalled(t *testing.T) {
 
 	client := createClient(t, ts.URL, mockLogger)
 
-	// Act
-	installed, err := client.CheckSecretEngineInstalled(expectedToken, "secret/", "kv")
+	tests := []struct {
+		name       string
+		mountPath  string
+		engineType string
+	}{
+		{"kv v1 secret storage not installed", "secret/", KeyValue},
+		{"consul secret storage not installed", "consul/", Consul},
+	}
 
-	// Assert
-	require.NoError(t, err)
-	assert.False(t, installed)
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			// Act
+			installed, err := client.CheckSecretEngineInstalled("fake-token", test.mountPath, test.engineType)
+
+			// Assert
+			require.NoError(t, err)
+			require.False(t, installed)
+		})
+	}
 }
 
 func TestEnableKVSecretEngine(t *testing.T) {

internal/pkg/vault/models.go

@@ -101,7 +101,7 @@ type SecretsEngineOptions struct {
 
 // SecretsEngineConfig is config for /v1/sys/mounts
 type SecretsEngineConfig struct {
-	DefaultLeaseTTLDuration string
+	DefaultLeaseTTLDuration string `json:"default_lease_ttl"`
 }
 
 // EnableSecretsEngineRequest is the POST request to /v1/sys/mounts

internal/pkg/vault/request.go

@@ -57,7 +57,10 @@ func (c *Client) doRequest(params RequestArgs) (int, error) {
 		params.BodyReader = bytes.NewReader(body)
 	}
 
-	targetUrl := c.Config.BuildSecretsPathURL(params.Path)
+	targetUrl, err := c.Config.BuildSecretsPathURL(params.Path)
+	if err != nil {
+		return 0, err
+	}
 
 	req, err := http.NewRequest(params.Method, targetUrl, params.BodyReader)
 	if err != nil {

internal/pkg/vault/roottoken.go

@@ -124,12 +124,16 @@ func (c *Client) rootTokenSubmitKey(key string, nonce string) (bool, string, err
 		ResponseObject:       response,
 	})
 
+	if err != nil {
+		return false, "", err
+	}
+
 	var encodedToken string
 	if response.Complete {
 		encodedToken = response.EncodedToken
 	}
 
-	return response.Complete, encodedToken, err
+	return response.Complete, encodedToken, nil
 }
 
 func (c *Client) rootTokenDecodeToken(encodedToken string, otp string) (string, error) {

internal/pkg/vault/secrets.go

@@ -170,7 +170,10 @@ func (c *Client) StoreSecrets(subPath string, secrets map[string]string) error {
 
 func (c *Client) getTokenDetails() (*types.TokenMetadata, error) {
 	// call Vault's token self lookup API
-	url := c.Config.BuildURL(lookupSelfVaultAPI)
+	url, err := c.Config.BuildURL(lookupSelfVaultAPI)
+	if err != nil {
+		return nil, err
+	}
 
 	req, err := http.NewRequest(http.MethodGet, url, nil)
 	if err != nil {
@@ -312,8 +315,10 @@ func (c *Client) doTokenRefreshPeriodically(renewInterval time.Duration,
 
 func (c *Client) renewToken() error {
 	// call Vault's renew self API
-	url := c.Config.BuildURL(renewSelfVaultAPI)
-
+	url, err := c.Config.BuildURL(renewSelfVaultAPI)
+	if err != nil {
+		return err
+	}
 	req, err := http.NewRequest(http.MethodPost, url, nil)
 	if err != nil {
 		return err
@@ -342,7 +347,11 @@ func (c *Client) renewToken() error {
 
 // getAllKeys obtains all the keys that reside at the provided sub-path.
 func (c *Client) getAllKeys(subPath string) (map[string]string, error) {
-	url := c.Config.BuildSecretsPathURL(subPath)
+	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(http.MethodGet, url, nil)
@@ -402,7 +411,10 @@ func (c *Client) store(subPath string, secrets map[string]string) error {
 		return nil
 	}
 
-	url := c.Config.BuildSecretsPathURL(subPath)
+	url, err := c.Config.BuildSecretsPathURL(subPath)
+	if err != nil {
+		return err
+	}
 
 	c.lc.Debug(fmt.Sprintf("Using Secrets URL of `%s`", url))