Merge pull request #425 from FGA-GCES/fix/code-smells

adjusting code smells

e2e/suite/common/common.go

@@ -22,6 +22,14 @@ import (
 	"github.com/external-secrets/external-secrets/e2e/framework"
 )
 
+const (
+	// Constants.
+	dockerConfigExampleName    = "docker-config-example"
+	dockerConfigJSONKey        = ".dockerconfigjson"
+	mysecretToStringTemplating = "{{ .mysecret | toString }}"
+	sshPrivateKey              = "ssh-privatekey"
+)
+
 // This case creates multiple secrets with simple key/value pairs and syncs them using multiple .Spec.Data blocks.
 // Not supported by: vault.
 func SimpleDataSync(f *framework.Framework) (string, func(*framework.TestCase)) {
@@ -289,7 +297,7 @@ func NestedJSONWithGJSON(f *framework.Framework) (string, func(*framework.TestCa
 // not supported by: vault.
 func DockerJSONConfig(f *framework.Framework) (string, func(*framework.TestCase)) {
 	return "[common] should sync docker configurated json secrets with template simple", func(tc *framework.TestCase) {
-		cloudSecretName := fmt.Sprintf("%s-%s", f.Namespace.Name, "docker-config-example")
+		cloudSecretName := fmt.Sprintf("%s-%s", f.Namespace.Name, dockerConfigExampleName)
 		dockerconfig := `{"auths":{"https://index.docker.io/v1/": {"auth": "c3R...zE2"}}}`
 		cloudSecretValue := fmt.Sprintf(`{"dockerconfig": %s}`, dockerconfig)
 		tc.Secrets = map[string]string{
@@ -299,7 +307,7 @@ func DockerJSONConfig(f *framework.Framework) (string, func(*framework.TestCase)
 		tc.ExpectedSecret = &v1.Secret{
 			Type: v1.SecretTypeOpaque,
 			Data: map[string][]byte{
-				".dockerconfigjson": []byte(dockerconfig),
+				dockerConfigJSONKey: []byte(dockerconfig),
 			},
 		}
 
@@ -315,7 +323,7 @@ func DockerJSONConfig(f *framework.Framework) (string, func(*framework.TestCase)
 
 		tc.ExternalSecret.Spec.Target.Template = &esv1alpha1.ExternalSecretTemplate{
 			Data: map[string]string{
-				".dockerconfigjson": "{{ .mysecret | toString }}",
+				dockerConfigJSONKey: mysecretToStringTemplating,
 			},
 		}
 	}
@@ -326,7 +334,7 @@ func DockerJSONConfig(f *framework.Framework) (string, func(*framework.TestCase)
 // Need to have a key holding dockerconfig to be supported by vault.
 func DataPropertyDockerconfigJSON(f *framework.Framework) (string, func(*framework.TestCase)) {
 	return "[common] should sync docker configurated json secrets with template", func(tc *framework.TestCase) {
-		cloudSecretName := fmt.Sprintf("%s-%s", f.Namespace.Name, "docker-config-example")
+		cloudSecretName := fmt.Sprintf("%s-%s", f.Namespace.Name, dockerConfigExampleName)
 		dockerconfigString := `"{\"auths\":{\"https://index.docker.io/v1/\": {\"auth\": \"c3R...zE2\"}}}"`
 		dockerconfig := `{"auths":{"https://index.docker.io/v1/": {"auth": "c3R...zE2"}}}`
 		cloudSecretValue := fmt.Sprintf(`{"dockerconfig": %s}`, dockerconfigString)
@@ -337,7 +345,7 @@ func DataPropertyDockerconfigJSON(f *framework.Framework) (string, func(*framewo
 		tc.ExpectedSecret = &v1.Secret{
 			Type: v1.SecretTypeDockerConfigJson,
 			Data: map[string][]byte{
-				".dockerconfigjson": []byte(dockerconfig),
+				dockerConfigJSONKey: []byte(dockerconfig),
 			},
 		}
 
@@ -354,7 +362,7 @@ func DataPropertyDockerconfigJSON(f *framework.Framework) (string, func(*framewo
 		tc.ExternalSecret.Spec.Target.Template = &esv1alpha1.ExternalSecretTemplate{
 			Type: v1.SecretTypeDockerConfigJson,
 			Data: map[string]string{
-				".dockerconfigjson": "{{ .mysecret | toString }}",
+				dockerConfigJSONKey: mysecretToStringTemplating,
 			},
 		}
 	}
@@ -411,7 +419,7 @@ func SSHKeySync(f *framework.Framework) (string, func(*framework.TestCase)) {
 		tc.ExpectedSecret = &v1.Secret{
 			Type: v1.SecretTypeSSHAuth,
 			Data: map[string][]byte{
-				"ssh-privatekey": []byte(sshSecretValue),
+				sshPrivateKey: []byte(sshSecretValue),
 			},
 		}
 
@@ -427,7 +435,7 @@ func SSHKeySync(f *framework.Framework) (string, func(*framework.TestCase)) {
 		tc.ExternalSecret.Spec.Target.Template = &esv1alpha1.ExternalSecretTemplate{
 			Type: v1.SecretTypeSSHAuth,
 			Data: map[string]string{
-				"ssh-privatekey": "{{ .mysecret | toString }}",
+				sshPrivateKey: mysecretToStringTemplating,
 			},
 		}
 	}
@@ -436,7 +444,7 @@ func SSHKeySync(f *framework.Framework) (string, func(*framework.TestCase)) {
 // This case adds an ssh private key secret and syncs it.
 func SSHKeySyncDataProperty(f *framework.Framework) (string, func(*framework.TestCase)) {
 	return "[common] should sync ssh key with provider.", func(tc *framework.TestCase) {
-		cloudSecretName := fmt.Sprintf("%s-%s", f.Namespace.Name, "docker-config-example")
+		cloudSecretName := fmt.Sprintf("%s-%s", f.Namespace.Name, dockerConfigExampleName)
 		SSHKey := `-----BEGIN OPENSSH PRIVATE KEY-----
 		b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABlwAAAAdzc2gtcn
 		NhAAAAAwEAAQAAAYEAsARoZUqo6L5dd0WRjZ2QPq/kKlbjtUY1njzJ01UtdC1u1eSJFUnV
@@ -483,7 +491,7 @@ func SSHKeySyncDataProperty(f *framework.Framework) (string, func(*framework.Tes
 		tc.ExpectedSecret = &v1.Secret{
 			Type: v1.SecretTypeSSHAuth,
 			Data: map[string][]byte{
-				"ssh-privatekey": []byte(SSHKey),
+				sshPrivateKey: []byte(SSHKey),
 			},
 		}
 
@@ -500,7 +508,7 @@ func SSHKeySyncDataProperty(f *framework.Framework) (string, func(*framework.Tes
 		tc.ExternalSecret.Spec.Target.Template = &esv1alpha1.ExternalSecretTemplate{
 			Type: v1.SecretTypeSSHAuth,
 			Data: map[string]string{
-				"ssh-privatekey": "{{ .mysecret | toString }}",
+				sshPrivateKey: mysecretToStringTemplating,
 			},
 		}
 	}

pkg/provider/aws/auth/auth_test.go

@@ -38,17 +38,13 @@ import (
 )
 
 const (
-	myServiceAcc = "my-service-account"
-	myRole       = "my-sa-role"
-	otherNs      = "other-ns"
+	esNamespaceKey      = "es-namespace"
+	platformTeamNsKey   = "platform-team-ns"
+	myServiceAccountKey = "my-service-account"
+	otherNsName         = "other-ns"
 )
 
 func TestNewSession(t *testing.T) {
-	const (
-		esNamespace    = "es-namespace"
-		platformTeamNs = "platform-team-ns"
-	)
-
 	rows := []TestSessionRow{
 		{
 			name:      "nil store",
@@ -272,7 +268,7 @@ func TestNewSession(t *testing.T) {
 		},
 		{
 			name:      "ClusterStore should use credentials from a specific namespace",
-			namespace: esNamespace,
+			namespace: esNamespaceKey,
 			store: &esv1alpha1.ClusterSecretStore{
 				TypeMeta: metav1.TypeMeta{
 					APIVersion: esv1alpha1.ClusterSecretStoreKindAPIVersion,
@@ -285,12 +281,12 @@ func TestNewSession(t *testing.T) {
 								SecretRef: &esv1alpha1.AWSAuthSecretRef{
 									AccessKeyID: esmeta.SecretKeySelector{
 										Name:      "onesecret",
-										Namespace: aws.String(platformTeamNs),
+										Namespace: aws.String(platformTeamNsKey),
 										Key:       "one",
 									},
 									SecretAccessKey: esmeta.SecretKeySelector{
 										Name:      "onesecret",
-										Namespace: aws.String(platformTeamNs),
+										Namespace: aws.String(platformTeamNsKey),
 										Key:       "two",
 									},
 								},
@@ -303,7 +299,7 @@ func TestNewSession(t *testing.T) {
 				{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "onesecret",
-						Namespace: platformTeamNs,
+						Namespace: platformTeamNsKey,
 					},
 					Data: map[string][]byte{
 						"one": []byte("1111"),
@@ -317,7 +313,7 @@ func TestNewSession(t *testing.T) {
 		},
 		{
 			name:      "namespace is mandatory when using ClusterStore with SecretKeySelector",
-			namespace: esNamespace,
+			namespace: esNamespaceKey,
 			store: &esv1alpha1.ClusterSecretStore{
 				TypeMeta: metav1.TypeMeta{
 					APIVersion: esv1alpha1.ClusterSecretStoreKindAPIVersion,
@@ -346,20 +342,20 @@ func TestNewSession(t *testing.T) {
 		},
 		{
 			name:      "jwt auth via cluster secret store",
-			namespace: esNamespace,
+			namespace: esNamespaceKey,
 			sa: &v1.ServiceAccount{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      myServiceAcc,
-					Namespace: otherNs,
+					Name:      myServiceAccountKey,
+					Namespace: otherNsName,
 					Annotations: map[string]string{
-						roleARNAnnotation: myRole,
+						roleARNAnnotation: "my-sa-role",
 					},
 				},
 			},
 			jwtProvider: func(name, namespace, roleArn, region string) (credentials.Provider, error) {
-				assert.Equal(t, myServiceAcc, name)
-				assert.Equal(t, otherNs, namespace)
-				assert.Equal(t, myRole, roleArn)
+				assert.Equal(t, myServiceAccountKey, name)
+				assert.Equal(t, otherNsName, namespace)
+				assert.Equal(t, "my-sa-role", roleArn)
 				return fakesess.CredentialsProvider{
 					RetrieveFunc: func() (credentials.Value, error) {
 						return credentials.Value{
@@ -383,8 +379,8 @@ func TestNewSession(t *testing.T) {
 							Auth: esv1alpha1.AWSAuth{
 								JWTAuth: &esv1alpha1.AWSJWTAuth{
 									ServiceAccountRef: &esmeta.ServiceAccountSelector{
-										Name:      myServiceAcc,
-										Namespace: aws.String(otherNs),
+										Name:      myServiceAccountKey,
+										Namespace: aws.String(otherNsName),
 									},
 								},
 							},
@@ -396,46 +392,6 @@ func TestNewSession(t *testing.T) {
 			expectedKeyID:     "3333",
 			expectedSecretKey: "4444",
 		},
-		{
-			name: "should not accept ServiceAccountRefs with nil Namespace",
-			sa: &v1.ServiceAccount{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      myServiceAcc,
-					Namespace: otherNs,
-					Annotations: map[string]string{
-						roleARNAnnotation: myRole,
-					},
-				},
-			},
-			jwtProvider: func(name, namespace, roleArn, region string) (credentials.Provider, error) {
-				return fakesess.CredentialsProvider{
-					RetrieveFunc: func() (credentials.Value, error) {
-						return credentials.Value{}, nil
-					},
-					IsExpiredFunc: func() bool { return false },
-				}, nil
-			},
-			store: &esv1alpha1.ClusterSecretStore{
-				TypeMeta: metav1.TypeMeta{
-					APIVersion: esv1alpha1.ClusterSecretStoreKindAPIVersion,
-					Kind:       esv1alpha1.ClusterSecretStoreKind,
-				},
-				Spec: esv1alpha1.SecretStoreSpec{
-					Provider: &esv1alpha1.SecretStoreProvider{
-						AWS: &esv1alpha1.AWSProvider{
-							Auth: esv1alpha1.AWSAuth{
-								JWTAuth: &esv1alpha1.AWSJWTAuth{
-									ServiceAccountRef: &esmeta.ServiceAccountSelector{
-										Name: myServiceAcc,
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			expectErr: "serviceAccountRef has no Namespace field (mandatory for ClusterSecretStore specs)",
-		},
 	}
 	for i := range rows {
 		row := rows[i]
@@ -475,8 +431,8 @@ func testRow(t *testing.T, row TestSessionRow) {
 	}
 	err := kc.Create(context.Background(), &authv1.TokenRequest{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      myServiceAcc,
-			Namespace: otherNs,
+			Name:      myServiceAccountKey,
+			Namespace: otherNsName,
 		},
 	})
 	assert.Nil(t, err)