Merge pull request #421 from FGA-GCES/refactor/cognitiveComplexity

refactor bad cognitive complexity code smeels

pkg/provider/ibm/provider.go

@@ -108,82 +108,102 @@ func (ibm *providerIBM) GetSecret(ctx context.Context, ref esv1alpha1.ExternalSe
 
 	switch secretType {
 	case sm.GetSecretOptionsSecretTypeArbitraryConst:
-		response, _, err := ibm.IBMClient.GetSecret(
-			&sm.GetSecretOptions{
-				SecretType: core.StringPtr(sm.GetSecretOptionsSecretTypeArbitraryConst),
-				ID:         &secretName,
-			})
-		if err != nil {
-			return nil, err
-		}
 
-		secret := response.Resources[0].(*sm.SecretResource)
-		secretData := secret.SecretData.(map[string]interface{})
-		arbitrarySecretPayload := secretData["payload"].(string)
-		return []byte(arbitrarySecretPayload), nil
+		return getArbitrarySecret(ibm, &secretName)
 
 	case sm.CreateSecretOptionsSecretTypeUsernamePasswordConst:
+
 		if ref.Property == "" {
 			return nil, fmt.Errorf("remoteRef.property required for secret type username_password")
 		}
-		response, _, err := ibm.IBMClient.GetSecret(
-			&sm.GetSecretOptions{
-				SecretType: core.StringPtr(sm.CreateSecretOptionsSecretTypeUsernamePasswordConst),
-				ID:         &secretName,
-			})
-		if err != nil {
-			return nil, err
-		}
-
-		secret := response.Resources[0].(*sm.SecretResource)
-		secretData := secret.SecretData.(map[string]interface{})
-
-		if val, ok := secretData[ref.Property]; ok {
-			return []byte(val.(string)), nil
-		}
-		return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
+		return getUsernamePasswordSecret(ibm, &secretName, ref)
 
 	case sm.CreateSecretOptionsSecretTypeIamCredentialsConst:
-		response, _, err := ibm.IBMClient.GetSecret(
-			&sm.GetSecretOptions{
-				SecretType: core.StringPtr(sm.CreateSecretOptionsSecretTypeIamCredentialsConst),
-				ID:         &secretName,
-			})
-		if err != nil {
-			return nil, err
-		}
 
-		secret := response.Resources[0].(*sm.SecretResource)
-		secretData := *secret.APIKey
-
-		return []byte(secretData), nil
+		return getIamCredentialsSecret(ibm, &secretName)
 
 	case sm.CreateSecretOptionsSecretTypeImportedCertConst:
+
 		if ref.Property == "" {
 			return nil, fmt.Errorf("remoteRef.property required for secret type imported_cert")
 		}
-		response, _, err := ibm.IBMClient.GetSecret(
-			&sm.GetSecretOptions{
-				SecretType: core.StringPtr(sm.CreateSecretOptionsSecretTypeImportedCertConst),
-				ID:         &secretName,
-			})
-		if err != nil {
-			return nil, err
-		}
-
-		secret := response.Resources[0].(*sm.SecretResource)
-		secretData := secret.SecretData.(map[string]interface{})
-
-		if val, ok := secretData[ref.Property]; ok {
-			return []byte(val.(string)), nil
-		}
-		return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
 
+		return getImportCertSecret(ibm, &secretName, ref)
 	default:
 		return nil, fmt.Errorf("unknown secret type %s", secretType)
 	}
 }
 
+func getArbitrarySecret(ibm *providerIBM, secretName *string) ([]byte, error) {
+	response, _, err := ibm.IBMClient.GetSecret(
+		&sm.GetSecretOptions{
+			SecretType: core.StringPtr(sm.GetSecretOptionsSecretTypeArbitraryConst),
+			ID:         secretName,
+		})
+	if err != nil {
+		return nil, err
+	}
+
+	secret := response.Resources[0].(*sm.SecretResource)
+	secretData := secret.SecretData.(map[string]interface{})
+	arbitrarySecretPayload := secretData["payload"].(string)
+	return []byte(arbitrarySecretPayload), nil
+}
+
+func getImportCertSecret(ibm *providerIBM, secretName *string, ref esv1alpha1.ExternalSecretDataRemoteRef) ([]byte, error) {
+	response, _, err := ibm.IBMClient.GetSecret(
+		&sm.GetSecretOptions{
+			SecretType: core.StringPtr(sm.CreateSecretOptionsSecretTypeImportedCertConst),
+			ID:         secretName,
+		})
+	if err != nil {
+		return nil, err
+	}
+
+	secret := response.Resources[0].(*sm.SecretResource)
+	secretData := secret.SecretData.(map[string]interface{})
+
+	if val, ok := secretData[ref.Property]; ok {
+		return []byte(val.(string)), nil
+	}
+	return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
+}
+
+func getIamCredentialsSecret(ibm *providerIBM, secretName *string) ([]byte, error) {
+	response, _, err := ibm.IBMClient.GetSecret(
+		&sm.GetSecretOptions{
+			SecretType: core.StringPtr(sm.CreateSecretOptionsSecretTypeIamCredentialsConst),
+			ID:         secretName,
+		})
+	if err != nil {
+		return nil, err
+	}
+
+	secret := response.Resources[0].(*sm.SecretResource)
+	secretData := *secret.APIKey
+
+	return []byte(secretData), nil
+}
+
+func getUsernamePasswordSecret(ibm *providerIBM, secretName *string, ref esv1alpha1.ExternalSecretDataRemoteRef) ([]byte, error) {
+	response, _, err := ibm.IBMClient.GetSecret(
+		&sm.GetSecretOptions{
+			SecretType: core.StringPtr(sm.CreateSecretOptionsSecretTypeUsernamePasswordConst),
+			ID:         secretName,
+		})
+	if err != nil {
+		return nil, err
+	}
+
+	secret := response.Resources[0].(*sm.SecretResource)
+	secretData := secret.SecretData.(map[string]interface{})
+
+	if val, ok := secretData[ref.Property]; ok {
+		return []byte(val.(string)), nil
+	}
+	return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
+}
+
 func (ibm *providerIBM) GetSecretMap(ctx context.Context, ref esv1alpha1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
 	if utils.IsNil(ibm.IBMClient) {
 		return nil, fmt.Errorf(errUninitalizedIBMProvider)
@@ -213,16 +233,13 @@ func (ibm *providerIBM) GetSecretMap(ctx context.Context, ref esv1alpha1.Externa
 		secretData := secret.SecretData.(map[string]interface{})
 		arbitrarySecretPayload := secretData["payload"].(string)
 
-		kv := make(map[string]string)
+		kv := make(map[string]interface{})
 		err = json.Unmarshal([]byte(arbitrarySecretPayload), &kv)
 		if err != nil {
 			return nil, fmt.Errorf(errJSONSecretUnmarshal, err)
 		}
 
-		secretMap := make(map[string][]byte)
-		for k, v := range kv {
-			secretMap[k] = []byte(v)
-		}
+		secretMap := byteArrayMap(kv)
 
 		return secretMap, nil
 
@@ -239,10 +256,7 @@ func (ibm *providerIBM) GetSecretMap(ctx context.Context, ref esv1alpha1.Externa
 		secret := response.Resources[0].(*sm.SecretResource)
 		secretData := secret.SecretData.(map[string]interface{})
 
-		secretMap := make(map[string][]byte)
-		for k, v := range secretData {
-			secretMap[k] = []byte(v.(string))
-		}
+		secretMap := byteArrayMap(secretData)
 
 		return secretMap, nil
 
@@ -277,10 +291,7 @@ func (ibm *providerIBM) GetSecretMap(ctx context.Context, ref esv1alpha1.Externa
 		secret := response.Resources[0].(*sm.SecretResource)
 		secretData := secret.SecretData.(map[string]interface{})
 
-		secretMap := make(map[string][]byte)
-		for k, v := range secretData {
-			secretMap[k] = []byte(v.(string))
-		}
+		secretMap := byteArrayMap(secretData)
 
 		return secretMap, nil
 
@@ -289,6 +300,14 @@ func (ibm *providerIBM) GetSecretMap(ctx context.Context, ref esv1alpha1.Externa
 	}
 }
 
+func byteArrayMap(secretData map[string]interface{}) map[string][]byte {
+	secretMap := make(map[string][]byte)
+	for k, v := range secretData {
+		secretMap[k] = []byte(v.(string))
+	}
+	return secretMap
+}
+
 func (ibm *providerIBM) Close(ctx context.Context) error {
 	return nil
 }

pkg/provider/vault/vault.go

@@ -316,67 +316,115 @@ func getCertFromConfigMap(v *client) ([]byte, error) {
 }
 
 func (v *client) setAuth(ctx context.Context, client Client, cfg *vault.Config) error {
+	tokenExists, err := setSecretKeyToken(ctx, v, client)
+	if tokenExists {
+		return err
+	}
+
+	tokenExists, err = setAppRoleToken(ctx, v, client)
+	if tokenExists {
+		return err
+	}
+
+	tokenExists, err = setKubernetesAuthToken(ctx, v, client)
+	if tokenExists {
+		return err
+	}
+
+	tokenExists, err = setLdapAuthToken(ctx, v, client)
+	if tokenExists {
+		return err
+	}
+
+	tokenExists, err = setJwtAuthToken(ctx, v, client)
+	if tokenExists {
+		return err
+	}
+
+	tokenExists, err = setCertAuthToken(ctx, v, client, cfg)
+	if tokenExists {
+		return err
+	}
+
+	return errors.New(errAuthFormat)
+}
+
+func setAppRoleToken(ctx context.Context, v *client, client Client) (bool, error) {
 	tokenRef := v.store.Auth.TokenSecretRef
 	if tokenRef != nil {
 		token, err := v.secretKeyRef(ctx, tokenRef)
 		if err != nil {
-			return err
+			return true, err
 		}
 		client.SetToken(token)
-		return nil
+		return true, nil
 	}
+	return false, nil
+}
 
+func setSecretKeyToken(ctx context.Context, v *client, client Client) (bool, error) {
 	appRole := v.store.Auth.AppRole
 	if appRole != nil {
 		token, err := v.requestTokenWithAppRoleRef(ctx, client, appRole)
 		if err != nil {
-			return err
+			return true, err
 		}
 		client.SetToken(token)
-		return nil
+		return true, nil
 	}
+	return false, nil
+}
 
+func setKubernetesAuthToken(ctx context.Context, v *client, client Client) (bool, error) {
 	kubernetesAuth := v.store.Auth.Kubernetes
 	if kubernetesAuth != nil {
 		token, err := v.requestTokenWithKubernetesAuth(ctx, client, kubernetesAuth)
 		if err != nil {
-			return err
+			return true, err
 		}
 		client.SetToken(token)
-		return nil
+		return true, nil
 	}
+	return false, nil
+}
 
+func setLdapAuthToken(ctx context.Context, v *client, client Client) (bool, error) {
 	ldapAuth := v.store.Auth.Ldap
 	if ldapAuth != nil {
 		token, err := v.requestTokenWithLdapAuth(ctx, client, ldapAuth)
 		if err != nil {
-			return err
+			return true, err
 		}
 		client.SetToken(token)
-		return nil
+		return true, nil
 	}
+	return false, nil
+}
 
+func setJwtAuthToken(ctx context.Context, v *client, client Client) (bool, error) {
 	jwtAuth := v.store.Auth.Jwt
 	if jwtAuth != nil {
 		token, err := v.requestTokenWithJwtAuth(ctx, client, jwtAuth)
 		if err != nil {
-			return err
+			return true, err
 		}
 		client.SetToken(token)
-		return nil
+		return true, nil
 	}
+	return false, nil
+}
 
+func setCertAuthToken(ctx context.Context, v *client, client Client, cfg *vault.Config) (bool, error) {
 	certAuth := v.store.Auth.Cert
 	if certAuth != nil {
 		token, err := v.requestTokenWithCertAuth(ctx, client, certAuth, cfg)
 		if err != nil {
-			return err
+			return true, err
 		}
 		client.SetToken(token)
-		return nil
+		return true, nil
 	}
-
-	return errors.New(errAuthFormat)
+	return false, nil
 }
 
 func (v *client) secretKeyRefForServiceAccount(ctx context.Context, serviceAccountRef *esmeta.ServiceAccountSelector) (string, error) {
@@ -493,43 +541,16 @@ func kubeParameters(role, jwt string) map[string]string {
 }
 
 func (v *client) requestTokenWithKubernetesAuth(ctx context.Context, client Client, kubernetesAuth *esv1alpha1.VaultKubernetesAuth) (string, error) {
-	jwtString := ""
-	if kubernetesAuth.ServiceAccountRef != nil {
-		jwt, err := v.secretKeyRefForServiceAccount(ctx, kubernetesAuth.ServiceAccountRef)
-		if err != nil {
-			return "", err
-		}
-		jwtString = jwt
-	} else if kubernetesAuth.SecretRef != nil {
-		tokenRef := kubernetesAuth.SecretRef
-		if tokenRef.Key == "" {
-			tokenRef = kubernetesAuth.SecretRef.DeepCopy()
-			tokenRef.Key = "token"
-		}
-		jwt, err := v.secretKeyRef(ctx, tokenRef)
-		if err != nil {
-			return "", err
-		}
-		jwtString = jwt
-	} else {
-		// Kubernetes authentication is specified, but without a referenced
-		// Kubernetes secret. We check if the file path for in-cluster service account
-		// exists and attempt to use the token for Vault Kubernetes auth.
-		if _, err := os.Stat(serviceAccTokenPath); err != nil {
-			return "", fmt.Errorf(errServiceAccount, err)
-		}
-		jwtByte, err := ioutil.ReadFile(serviceAccTokenPath)
-		if err != nil {
-			return "", fmt.Errorf(errServiceAccount, err)
-		}
-		jwtString = string(jwtByte)
+	jwtString, err := getJwtString(ctx, v, kubernetesAuth)
+	if err != nil {
+		return "", err
 	}
 
 	parameters := kubeParameters(kubernetesAuth.Role, jwtString)
 	url := strings.Join([]string{"/v1", "auth", kubernetesAuth.Path, "login"}, "/")
 	request := client.NewRequest("POST", url)
 
-	err := request.SetJSONBody(parameters)
+	err = request.SetJSONBody(parameters)
 	if err != nil {
 		return "", fmt.Errorf(errVaultReqParams, err)
 	}
@@ -554,6 +575,39 @@ func (v *client) requestTokenWithKubernetesAuth(ctx context.Context, client Clie
 	return token, nil
 }
 
+func getJwtString(ctx context.Context, v *client, kubernetesAuth *esv1alpha1.VaultKubernetesAuth) (string, error) {
+	if kubernetesAuth.ServiceAccountRef != nil {
+		jwt, err := v.secretKeyRefForServiceAccount(ctx, kubernetesAuth.ServiceAccountRef)
+		if err != nil {
+			return "", err
+		}
+		return jwt, nil
+	} else if kubernetesAuth.SecretRef != nil {
+		tokenRef := kubernetesAuth.SecretRef
+		if tokenRef.Key == "" {
+			tokenRef = kubernetesAuth.SecretRef.DeepCopy()
+			tokenRef.Key = "token"
+		}
+		jwt, err := v.secretKeyRef(ctx, tokenRef)
+		if err != nil {
+			return "", err
+		}
+		return jwt, nil
+	} else {
+		// Kubernetes authentication is specified, but without a referenced
+		// Kubernetes secret. We check if the file path for in-cluster service account
+		// exists and attempt to use the token for Vault Kubernetes auth.
+		if _, err := os.Stat(serviceAccTokenPath); err != nil {
+			return "", fmt.Errorf(errServiceAccount, err)
+		}
+		jwtByte, err := ioutil.ReadFile(serviceAccTokenPath)
+		if err != nil {
+			return "", fmt.Errorf(errServiceAccount, err)
+		}
+		return string(jwtByte), nil
+	}
+}
+
 func (v *client) requestTokenWithLdapAuth(ctx context.Context, client Client, ldapAuth *esv1alpha1.VaultLdapAuth) (string, error) {
 	username := strings.TrimSpace(ldapAuth.Username)
 

pkg/provider/vault/vault_test.go

@@ -144,6 +144,23 @@ func newVaultTokenIDResponse(token string) *vault.Response {
 	})
 }
 
+type args struct {
+	newClientFunc func(c *vault.Config) (Client, error)
+	store         esv1alpha1.GenericStore
+	kube          kclient.Client
+	ns            string
+}
+
+type want struct {
+	err error
+}
+
+type testCase struct {
+	reason string
+	args   args
+	want   want
+}
+
 func clientWithLoginMock(c *vault.Config) (Client, error) {
 	return &fake.VaultClient{
 		MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
@@ -182,22 +199,7 @@ MIICsTCCAZkCFEJJ4daz5sxkFlzq9n1djLEuG7bmMA0GCSqGSIb3DQEBCwUAMBMxETAPBgNVBAMMCHZh
 `)
 	secretData := []byte(secretDataString)
 
-	type args struct {
-		newClientFunc func(c *vault.Config) (Client, error)
-		store         esv1alpha1.GenericStore
-		kube          kclient.Client
-		ns            string
-	}
-
-	type want struct {
-		err error
-	}
-
-	cases := map[string]struct {
-		reason string
-		args   args
-		want   want
-	}{
+	cases := map[string]testCase{
 		"InvalidVaultStore": {
 			reason: "Should return error if given an invalid vault store.",
 			args: args{
@@ -472,20 +474,24 @@ MIICsTCCAZkCFEJJ4daz5sxkFlzq9n1djLEuG7bmMA0GCSqGSIb3DQEBCwUAMBMxETAPBgNVBAMMCHZh
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			conn := &connector{
-				newVaultClient: tc.args.newClientFunc,
-			}
-			if tc.args.newClientFunc == nil {
-				conn.newVaultClient = newVaultClient
-			}
-			_, err := conn.NewClient(context.Background(), tc.args.store, tc.args.kube, tc.args.ns)
-			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
-				t.Errorf("\n%s\nvault.New(...): -want error, +got error:\n%s", tc.reason, diff)
-			}
+			vaultTest(t, name, tc)
 		})
 	}
 }
 
+func vaultTest(t *testing.T, name string, tc testCase) {
+	conn := &connector{
+		newVaultClient: tc.args.newClientFunc,
+	}
+	if tc.args.newClientFunc == nil {
+		conn.newVaultClient = newVaultClient
+	}
+	_, err := conn.NewClient(context.Background(), tc.args.store, tc.args.kube, tc.args.ns)
+	if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
+		t.Errorf("\n%s\nvault.New(...): -want error, +got error:\n%s", tc.reason, diff)
+	}
+}
+
 func TestGetSecretMap(t *testing.T) {
 	errBoom := errors.New("boom")