Merge pull request #420 from FGA-GCES/mateus/code_smells

Fix some code smells

e2e/framework/addon/vault.go

@@ -65,6 +65,8 @@ type Vault struct {
 	AppRolePath   string
 }
 
+const privatePemType = "RSA PRIVATE KEY"
+
 func NewVault(namespace string) *Vault {
 	repo := "hashicorp-" + namespace
 	return &Vault{
@@ -298,7 +300,7 @@ func genVaultCertificates(namespace string) ([]byte, []byte, []byte, []byte, []b
 		return nil, nil, nil, nil, nil, nil, fmt.Errorf("unable to generate vault server cert")
 	}
 	serverKeyPem := pem.EncodeToMemory(&pem.Block{
-		Type:  "RSA PRIVATE KEY",
+		Type:  privatePemType,
 		Bytes: x509.MarshalPKCS1PrivateKey(serverKey)},
 	)
 	// gen client ca + certs
@@ -311,7 +313,7 @@ func genVaultCertificates(namespace string) ([]byte, []byte, []byte, []byte, []b
 		return nil, nil, nil, nil, nil, nil, fmt.Errorf("unable to generate vault server cert")
 	}
 	clientKeyPem := pem.EncodeToMemory(&pem.Block{
-		Type:  "RSA PRIVATE KEY",
+		Type:  privatePemType,
 		Bytes: x509.MarshalPKCS1PrivateKey(clientKey)},
 	)
 	return serverRootPem, serverPem, serverKeyPem, clientRootPem, clientPem, clientKeyPem, err
@@ -323,7 +325,7 @@ func genVaultJWTKeys() ([]byte, []byte, string, error) {
 		return nil, nil, "", err
 	}
 	privPem := pem.EncodeToMemory(&pem.Block{
-		Type:  "RSA PRIVATE KEY",
+		Type:  privatePemType,
 		Bytes: x509.MarshalPKCS1PrivateKey(key),
 	})
 	pk, err := x509.MarshalPKIXPublicKey(&key.PublicKey)

e2e/suite/aws/provider.go

@@ -42,6 +42,8 @@ type SMProvider struct {
 	framework *framework.Framework
 }
 
+const secretName = "provider-secret"
+
 func newSMProvider(f *framework.Framework, url string) *SMProvider {
 	sess, err := session.NewSessionWithOptions(session.Options{
 		Config: aws.Config{
@@ -82,7 +84,7 @@ func (s *SMProvider) BeforeEach() {
 	By("creating a AWS SM credentials secret")
 	awsCreds := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "provider-secret",
+			Name:      secretName,
 			Namespace: s.framework.Namespace.Name,
 		},
 		StringData: map[string]string{
@@ -107,11 +109,11 @@ func (s *SMProvider) BeforeEach() {
 					Auth: esv1alpha1.AWSAuth{
 						SecretRef: &esv1alpha1.AWSAuthSecretRef{
 							AccessKeyID: esmeta.SecretKeySelector{
-								Name: "provider-secret",
+								Name: secretName,
 								Key:  "kid",
 							},
 							SecretAccessKey: esmeta.SecretKeySelector{
-								Name: "provider-secret",
+								Name: secretName,
 								Key:  "sak",
 							},
 						},

pkg/provider/aws/auth/auth_test.go

@@ -37,7 +37,17 @@ import (
 	fakesess "github.com/external-secrets/external-secrets/pkg/provider/aws/auth/fake"
 )
 
+const (
+	myServiceAcc = "my-service-account"
+	otherNs      = "other-ns"
+)
+
 func TestNewSession(t *testing.T) {
+	const (
+		esNamespace    = "es-namespace"
+		platformTeamNs = "platform-team-ns"
+	)
+
 	rows := []TestSessionRow{
 		{
 			name:      "nil store",
@@ -261,7 +271,7 @@ func TestNewSession(t *testing.T) {
 		},
 		{
 			name:      "ClusterStore should use credentials from a specific namespace",
-			namespace: "es-namespace",
+			namespace: esNamespace,
 			store: &esv1alpha1.ClusterSecretStore{
 				TypeMeta: metav1.TypeMeta{
 					APIVersion: esv1alpha1.ClusterSecretStoreKindAPIVersion,
@@ -274,12 +284,12 @@ func TestNewSession(t *testing.T) {
 								SecretRef: &esv1alpha1.AWSAuthSecretRef{
 									AccessKeyID: esmeta.SecretKeySelector{
 										Name:      "onesecret",
-										Namespace: aws.String("platform-team-ns"),
+										Namespace: aws.String(platformTeamNs),
 										Key:       "one",
 									},
 									SecretAccessKey: esmeta.SecretKeySelector{
 										Name:      "onesecret",
-										Namespace: aws.String("platform-team-ns"),
+										Namespace: aws.String(platformTeamNs),
 										Key:       "two",
 									},
 								},
@@ -292,7 +302,7 @@ func TestNewSession(t *testing.T) {
 				{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "onesecret",
-						Namespace: "platform-team-ns",
+						Namespace: platformTeamNs,
 					},
 					Data: map[string][]byte{
 						"one": []byte("1111"),
@@ -306,7 +316,7 @@ func TestNewSession(t *testing.T) {
 		},
 		{
 			name:      "namespace is mandatory when using ClusterStore with SecretKeySelector",
-			namespace: "es-namespace",
+			namespace: esNamespace,
 			store: &esv1alpha1.ClusterSecretStore{
 				TypeMeta: metav1.TypeMeta{
 					APIVersion: esv1alpha1.ClusterSecretStoreKindAPIVersion,
@@ -335,19 +345,19 @@ func TestNewSession(t *testing.T) {
 		},
 		{
 			name:      "jwt auth via cluster secret store",
-			namespace: "es-namespace",
+			namespace: esNamespace,
 			sa: &v1.ServiceAccount{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "my-service-account",
-					Namespace: "other-ns",
+					Name:      myServiceAcc,
+					Namespace: otherNs,
 					Annotations: map[string]string{
 						roleARNAnnotation: "my-sa-role",
 					},
 				},
 			},
 			jwtProvider: func(name, namespace, roleArn, region string) (credentials.Provider, error) {
-				assert.Equal(t, "my-service-account", name)
-				assert.Equal(t, "other-ns", namespace)
+				assert.Equal(t, myServiceAcc, name)
+				assert.Equal(t, otherNs, namespace)
 				assert.Equal(t, "my-sa-role", roleArn)
 				return fakesess.CredentialsProvider{
 					RetrieveFunc: func() (credentials.Value, error) {
@@ -372,8 +382,8 @@ func TestNewSession(t *testing.T) {
 							Auth: esv1alpha1.AWSAuth{
 								JWTAuth: &esv1alpha1.AWSJWTAuth{
 									ServiceAccountRef: &esmeta.ServiceAccountSelector{
-										Name:      "my-service-account",
-										Namespace: aws.String("other-ns"),
+										Name:      myServiceAcc,
+										Namespace: aws.String(otherNs),
 									},
 								},
 							},
@@ -424,8 +434,8 @@ func testRow(t *testing.T, row TestSessionRow) {
 	}
 	err := kc.Create(context.Background(), &authv1.TokenRequest{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "my-service-account",
-			Namespace: "other-ns",
+			Name:      myServiceAcc,
+			Namespace: otherNs,
 		},
 	})
 	assert.Nil(t, err)

pkg/provider/aws/secretsmanager/secretsmanager_test.go

@@ -42,6 +42,8 @@ type secretsManagerTestCase struct {
 	expectedCounter *int
 }
 
+const unexpectedErrorString = "[%d] unexpected error: %s, expected: '%s'"
+
 func makeValidSecretsManagerTestCase() *secretsManagerTestCase {
 	smtc := secretsManagerTestCase{
 		fakeClient:     fakesm.NewClient(),
@@ -174,7 +176,7 @@ func TestSecretsManagerGetSecret(t *testing.T) {
 		}
 		out, err := sm.GetSecret(context.Background(), *v.remoteRef)
 		if !ErrorContains(err, v.expectError) {
-			t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
+			t.Errorf(unexpectedErrorString, k, err.Error(), v.expectError)
 		}
 		if err == nil && string(out) != v.expectedSecret {
 			t.Errorf("[%d] unexpected secret: expected %s, got %s", k, v.expectedSecret, string(out))
@@ -223,7 +225,7 @@ func TestCaching(t *testing.T) {
 		sm.client = v.fakeClient
 		out, err := sm.GetSecret(context.Background(), *v.remoteRef)
 		if !ErrorContains(err, v.expectError) {
-			t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
+			t.Errorf(unexpectedErrorString, k, err.Error(), v.expectError)
 		}
 		if err == nil && string(out) != v.expectedSecret {
 			t.Errorf("[%d] unexpected secret: expected %s, got %s", k, v.expectedSecret, string(out))
@@ -269,7 +271,7 @@ func TestGetSecretMap(t *testing.T) {
 		}
 		out, err := sm.GetSecretMap(context.Background(), *v.remoteRef)
 		if !ErrorContains(err, v.expectError) {
-			t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
+			t.Errorf(unexpectedErrorString, k, err.Error(), v.expectError)
 		}
 		if err == nil && !cmp.Equal(out, v.expectedData) {
 			t.Errorf("[%d] unexpected secret data: expected %#v, got %#v", k, v.expectedData, out)

pkg/provider/schema/schema_test.go

@@ -26,6 +26,8 @@ import (
 
 type PP struct{}
 
+const shouldBeRegistered = "provider should be registered"
+
 // New constructs a SecretsManager Provider.
 func (p *PP) NewClient(ctx context.Context, store esv1alpha1.GenericStore, kube client.Client, namespace string) (provider.SecretsClient, error) {
 	return p, nil
@@ -113,7 +115,7 @@ func runTest(t *testing.T, name string, provider *esv1alpha1.SecretStoreProvider
 	}
 	Register(testProvider, secretStore.Spec.Provider)
 	p1, ok := GetProviderByName(name)
-	assert.True(t, ok, "provider should be registered")
+	assert.True(t, ok, shouldBeRegistered)
 	assert.Equal(t, testProvider, p1)
 	p2, err := GetProvider(secretStore)
 	assert.Nil(t, err)
@@ -139,7 +141,7 @@ func TestForceRegister(t *testing.T) {
 		},
 	})
 	p1, ok := GetProviderByName("aws")
-	assert.True(t, ok, "provider should be registered")
+	assert.True(t, ok, shouldBeRegistered)
 	assert.Equal(t, testProvider, p1)
 	p2, err := GetProvider(secretStore)
 	assert.Nil(t, err)
@@ -162,7 +164,7 @@ func TestRegisterGCP(t *testing.T) {
 
 	ForceRegister(testProvider, secretStore.Spec.Provider)
 	p1, ok := GetProviderByName("gcpsm")
-	assert.True(t, ok, "provider should be registered")
+	assert.True(t, ok, shouldBeRegistered)
 	assert.Equal(t, testProvider, p1)
 
 	p2, err := GetProvider(secretStore)