Refactor k8s_secrets_storage to eliminate CodeClimate errors

This change includes a bunch of refactoring to eliminate the following
CodeClimate issues:
    - Function NewProviderForType has 7 arguments (exceeds 4 allowed). Consider refactoring.
      Found in pkg/secrets/provide_conjur_secrets.go
    - Function NewProvider has 5 arguments (exceeds 4 allowed). Consider refactoring.
      Found in pkg/secrets/k8s_secrets_storage/provide_conjur_secrets.go
    - Method k8sProvider.Provide has 6 return statements (exceeds 4 allowed).
      Found in pkg/secrets/k8s_secrets_storage/provide_conjur_secrets.go

Changes include:

- Eliminate the following arguments from secrets.NewProviderForType:
  - k8s.RetrieveK8sSecret
  - k8s.UpdateK8sSecret
  Now these dependencies are injected in a non-exported version of this
  function.
- Similar to previous change, eliminated the same arguments in
  NewProvider in k8s_secrets_storage/provide_conjur_secrets.go.
- Converted some functions to receiver methods to eliminate a bunch of
  arguments for those functions.
- Consolidated some functions in k8sProvider.Provide to eliminate some
  return statements.
- Reorganized tests provide_conjur_secrets_test.go to match refactoring
  changes in the functional code.
- Because of the many changes in provide_conjur_secrets_test.go
  (previous bullet), converted Convey-based tests to table-driven
  Golang tests.
- Fixed the mock AddSecret function. The current implementation only
  allows one Conjur secret per Kubernetes secret when adding a
  Kubernetes Secret to the mock database. This function should allow
  for each Kubernetes Secret to include mappings for multiple
  Conjur secrets.

cmd/secrets-provider/main.go

@@ -13,7 +13,6 @@ import (
 	"github.com/cyberark/secrets-provider-for-k8s/pkg/secrets"
 	"github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/annotations"
 	"github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/clients/conjur"
-	"github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/clients/k8s"
 	secretsConfigProvider "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/config"
 )
 
@@ -62,15 +61,16 @@ func main() {
 		printErrorAndExit(err.Error())
 	}
 
+	providerConfig := secrets.ProviderConfig{
+		StoreType:          secretsConfig.StoreType,
+		PodNamespace:       secretsConfig.PodNamespace,
+		RequiredK8sSecrets: secretsConfig.RequiredK8sSecrets,
+		SecretFileBasePath: secretsBasePath,
+		AnnotationsMap:     annotationsMap,
+	}
 	provideSecrets, errs := secrets.NewProviderForType(
-		k8s.RetrieveK8sSecret,
-		k8s.UpdateK8sSecret,
 		secretRetriever.Retrieve,
-		secretsConfig.StoreType,
-		secretsConfig.PodNamespace,
-		secretsConfig.RequiredK8sSecrets,
-		secretsBasePath,
-		annotationsMap,
+		providerConfig,
 	)
 	logErrorsAndConditionalExit(errs, nil, messages.CSPFK053E)
 

pkg/secrets/clients/conjur/mocks/conjur_secrets_retriever.go

@@ -11,14 +11,14 @@ import (
 	validating that those secrets with 'execute' permissions can be fetched.
 */
 
-type ConjurMockClient struct {
+type ConjurClient struct {
 	CanExecute bool
 	// TODO: CanExecute is really just used to assert on the presence of errors
 	// 	and should probably just be an optional error.
 	Database map[string]string
 }
 
-func (c ConjurMockClient) RetrieveSecrets(secretIds []string) (map[string][]byte, error) {
+func (c *ConjurClient) RetrieveSecrets(secretIds []string) (map[string][]byte, error) {
 	res := make(map[string][]byte)
 
 	if !c.CanExecute {
@@ -38,20 +38,20 @@ func (c ConjurMockClient) RetrieveSecrets(secretIds []string) (map[string][]byte
 	return res, nil
 }
 
-func NewConjurMockClient() ConjurMockClient {
+func NewConjurClient() *ConjurClient {
 	database := map[string]string{
 		"conjur_variable1":             "conjur_secret1",
 		"conjur_variable2":             "conjur_secret2",
 		"conjur_variable_empty_secret": "",
 	}
 
-	return ConjurMockClient{
+	return &ConjurClient{
 		CanExecute: true,
 		Database:   database,
 	}
 }
 
-func (c ConjurMockClient) AddSecrets(
+func (c *ConjurClient) AddSecrets(
 	secrets map[string]string,
 ) {
 	for id, secret := range secrets {

pkg/secrets/k8s_secrets_storage/mocks/k8s_secrets_client.go

@@ -7,50 +7,77 @@ import (
 	v1 "k8s.io/api/core/v1"
 )
 
-type KubeSecretsMockClient struct {
-	// Mocks a K8s database. Maps k8s secret names to mock K8s secrets.
-	Database map[string]map[string][]byte
+// K8sSecrets represents a collection of Kubernetes Secrets to be populated
+// into the mock Kubernetes client's database. The logical hierarchy
+// represented by this structure is:
+// - Each Kubernetes Secret contains a 'Data' field.
+// - Each 'Data' field contains one or entries that are key/value pairs.
+// - The value in each 'Data' field entry can be a nested set of
+//   key/value pairs. In particular, for the entry with the key
+//   'conjur-info', the value is expected to be a mapping of application
+//   secret names to the corresponding Conjur variable ID (or policy path)
+//   that should be used to retrieve the secret value.
+type K8sSecrets map[string]k8sSecretData
+type k8sSecretData map[string]k8sSecretDataValues
+type k8sSecretDataValues map[string]string
+
+// KubeSecretsClient implements a mock Kubernetes client for testing
+// Kubernetes Secrets access by the Secrets Provider. This client provides:
+// - A Kubernetes Secret retrieve function
+// - A Kubernetes Secret update function
+// Kubernetes Secrets are populated for this mock client via the
+// AddSecret method. Retrieval and update errors can be simulated
+// for testing by setting the 'CanRetrieve' and 'CanUpdate' flags
+// (respectively) to false.
+type KubeSecretsClient struct {
+	// Mocks a K8s database. Maps k8s secret names to K8s secrets.
+	database map[string]map[string][]byte
 	// TODO: CanRetrieve and CanUpdate are really just used to assert on the presence of errors
 	// 	and should probably just be an optional error.
 	CanRetrieve bool
 	CanUpdate   bool
 }
 
-func NewKubeSecretsMockClient() KubeSecretsMockClient {
-	client := KubeSecretsMockClient{
-		Database:    map[string]map[string][]byte{},
+// NewKubeSecretsClient creates an instance of a KubeSecretsClient
+func NewKubeSecretsClient() *KubeSecretsClient {
+	client := KubeSecretsClient{
+		database:    map[string]map[string][]byte{},
 		CanRetrieve: true,
 		CanUpdate:   true,
 	}
 
-	return client
+	return &client
 }
 
-func (c KubeSecretsMockClient) AddSecret(
+// AddSecret adds a Kubernetes Secret to the mock Kubernetes Secrets client's
+// database.
+func (c *KubeSecretsClient) AddSecret(
 	secretName string,
-	key string,
-	keyConjurPath string,
+	secretData k8sSecretData,
 ) {
-	conjurMap := map[string]string{
-		key: keyConjurPath,
-	}
-	conjurMapBytes, err := yaml.Marshal(conjurMap)
-	if err != nil {
-		panic(err)
+	// Convert string values to YAML format
+	yamlizedSecretData := map[string][]byte{}
+	for key, value := range secretData {
+		yamlValue, err := yaml.Marshal(value)
+		if err != nil {
+			panic(err)
+		}
+		yamlizedSecretData[key] = yamlValue
 	}
 
-	c.Database[secretName] = map[string][]byte{
-		"conjur-map": conjurMapBytes,
-	}
+	c.database[secretName] = yamlizedSecretData
 }
 
-func (c KubeSecretsMockClient) RetrieveSecret(_ string, secretName string) (*v1.Secret, error) {
+// RetrieveSecret retrieves a Kubernetes Secret from the mock Kubernetes
+// Secrets client's database.
+func (c *KubeSecretsClient) RetrieveSecret(_ string, secretName string) (*v1.Secret, error) {
+
 	if !c.CanRetrieve {
 		return nil, errors.New("custom error")
 	}
 
 	// Check if the secret exists in the mock K8s DB
-	secretData, ok := c.Database[secretName]
+	secretData, ok := c.database[secretName]
 	if !ok {
 		return nil, errors.New("custom error")
 	}
@@ -60,15 +87,28 @@ func (c KubeSecretsMockClient) RetrieveSecret(_ string, secretName string) (*v1.
 	}, nil
 }
 
-func (c KubeSecretsMockClient) UpdateSecret(_ string, secretName string, originalK8sSecret *v1.Secret, stringDataEntriesMap map[string][]byte) error {
+// UpdateSecret updates a Kubernetes Secret in the mock Kubernetes
+// Secrets client's database.
+func (c *KubeSecretsClient) UpdateSecret(
+	_ string, secretName string,
+	originalK8sSecret *v1.Secret,
+	stringDataEntriesMap map[string][]byte) error {
+
 	if !c.CanUpdate {
 		return errors.New("custom error")
 	}
 
-	secretToUpdate := c.Database[secretName]
+	secretToUpdate := c.database[secretName]
 	for key, value := range stringDataEntriesMap {
 		secretToUpdate[key] = value
 	}
 
 	return nil
 }
+
+// InspectSecret provides a way for unit tests to view the 'Data' field
+// content of a Kubernetes Secret by reading this content directly from
+// the mock client's database.
+func (c *KubeSecretsClient) InspectSecret(secretName string) map[string][]byte {
+	return c.database[secretName]
+}

pkg/secrets/k8s_secrets_storage/mocks/logger.go

@@ -0,0 +1,82 @@
+package mocks
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+)
+
+// Logger is used to implement logging functions for testing the
+// Kubernetes Secrets storage provider.
+type Logger struct {
+	errors   []string
+	warnings []string
+	infos    []string
+	debugs   []string
+}
+
+// NewLogger returns a shiny, new Logger
+func NewLogger() *Logger {
+	return &Logger{}
+}
+
+// RecordedError logs that an error has occurred and returns a new error
+// with the given error message.
+func (l *Logger) RecordedError(msg string, args ...interface{}) error {
+	errStr := fmt.Sprintf(msg, args...)
+	l.errors = append(l.errors, errStr)
+	return errors.New(errStr)
+}
+
+// Error logs an error.
+func (l *Logger) Error(msg string, args ...interface{}) {
+	l.errors = append(l.errors, fmt.Sprintf(msg, args...))
+}
+
+// Warn logs a warning.
+func (l *Logger) Warn(msg string, args ...interface{}) {
+	l.warnings = append(l.warnings, fmt.Sprintf(msg, args...))
+}
+
+// Info logs an info message.
+func (l *Logger) Info(msg string, args ...interface{}) {
+	l.infos = append(l.infos, fmt.Sprintf(msg, args...))
+}
+
+// Debug logs a debug message.
+func (l *Logger) Debug(msg string, args ...interface{}) {
+	l.debugs = append(l.debugs, fmt.Sprintf(msg, args...))
+}
+
+func (l *Logger) messageWasLogged(msg string, loggedMsgs []string) bool {
+	for _, loggedMsg := range loggedMsgs {
+		if strings.Contains(loggedMsg, msg) {
+			return true
+		}
+	}
+	return false
+}
+
+// ErrorWasLogged determines if an error string appears in any
+// errors that have been logged.
+func (l *Logger) ErrorWasLogged(errStr string) bool {
+	return l.messageWasLogged(errStr, l.errors)
+}
+
+// WarningWasLogged determines if a warning string appears in any
+// warning messages that have been logged.
+func (l *Logger) WarningWasLogged(warning string) bool {
+	return l.messageWasLogged(warning, l.warnings)
+}
+
+// InfoWasLogged determines if a warning string appears in any
+// info messages that have been logged.
+func (l *Logger) InfoWasLogged(info string) bool {
+	return l.messageWasLogged(info, l.infos)
+}
+
+// DebugWasLogged determines if a debug string appears in any
+// debug messages that have been logged.
+func (l *Logger) DebugWasLogged(debug string) bool {
+	return l.messageWasLogged(debug, l.debugs)
+}