refactor tests

controllers/deployment_controller_test.go

@@ -32,7 +32,7 @@ var _ = Describe("Deployment controller", func() {
 	var deploymentResource *appsv1.Deployment
 	createdSecret := &v1.Secret{}
 
-	MakeDeployment := func() {
+	makeDeployment := func() {
 		ctx = context.Background()
 
 		deploymentKey = types.NamespacedName{
@@ -41,7 +41,7 @@ var _ = Describe("Deployment controller", func() {
 		}
 
 		secretKey = types.NamespacedName{
-			Name:      ItemName,
+			Name:      item1.Name,
 			Namespace: namespace,
 		}
 
@@ -55,8 +55,8 @@ var _ = Describe("Deployment controller", func() {
 				Name:      deploymentKey.Name,
 				Namespace: deploymentKey.Namespace,
 				Annotations: map[string]string{
-					op.ItemPathAnnotation: itemPath,
-					op.NameAnnotation:     ItemName,
+					op.ItemPathAnnotation: item1.Path,
+					op.NameAnnotation:     item1.Name,
 				},
 			},
 			Spec: appsv1.DeploymentSpec{
@@ -90,10 +90,10 @@ var _ = Describe("Deployment controller", func() {
 			}
 			return true
 		}, timeout, interval).Should(BeTrue())
-		Expect(createdSecret.Data).Should(Equal(expectedSecretData))
+		Expect(createdSecret.Data).Should(Equal(item1.SecretData))
 	}
 
-	var CleanK8sResources = func() {
+	cleanK8sResources := func() {
 		// failed test runs that don't clean up leave resources behind.
 		err := k8sClient.DeleteAllOf(context.Background(), &onepasswordv1.OnePasswordItem{}, client.InNamespace(namespace))
 		Expect(err).ToNot(HaveOccurred())
@@ -105,14 +105,14 @@ var _ = Describe("Deployment controller", func() {
 		Expect(err3).ToNot(HaveOccurred())
 	}
 
-	var MockGetItemFunc = func() {
+	mockGetItemFunc := func() {
 		mocks.DoGetItemFunc = func(uuid string, vaultUUID string) (*onepassword.Item, error) {
 			item := onepassword.Item{}
 			item.Fields = []*onepassword.ItemField{}
-			for k, v := range itemData {
+			for k, v := range item1.Data {
 				item.Fields = append(item.Fields, &onepassword.ItemField{Label: k, Value: v})
 			}
-			item.Version = version
+			item.Version = item1.Version
 			item.Vault.ID = vaultUUID
 			item.ID = uuid
 			return &item, nil
@@ -120,14 +120,14 @@ var _ = Describe("Deployment controller", func() {
 	}
 
 	BeforeEach(func() {
-		CleanK8sResources()
-		MockGetItemFunc()
+		cleanK8sResources()
+		mockGetItemFunc()
 		time.Sleep(time.Second) // TODO: can we achieve that with ginkgo?
-		MakeDeployment()
+		makeDeployment()
 	})
 
 	Context("Deployment with secrets from 1Password", func() {
-		It("Should Handle a deployment correctly", func() {
+		It("Should delete secret if deployment is deleted", func() {
 			By("Deleting the pod")
 			Eventually(func() error {
 				f := &appsv1.Deployment{}
@@ -154,10 +154,10 @@ var _ = Describe("Deployment controller", func() {
 			mocks.DoGetItemFunc = func(uuid string, vaultUUID string) (*onepassword.Item, error) {
 				item := onepassword.Item{}
 				item.Fields = []*onepassword.ItemField{}
-				for k, v := range itemData2 {
+				for k, v := range item2.Data {
 					item.Fields = append(item.Fields, &onepassword.ItemField{Label: k, Value: v})
 				}
-				item.Version = version2
+				item.Version = item2.Version
 				item.Vault.ID = vaultUUID
 				item.ID = uuid
 				return &item, nil
@@ -172,8 +172,8 @@ var _ = Describe("Deployment controller", func() {
 						Name:      deploymentKey.Name,
 						Namespace: deploymentKey.Namespace,
 						Annotations: map[string]string{
-							op.ItemPathAnnotation: itemPath2,
-							op.NameAnnotation:     ItemName,
+							op.ItemPathAnnotation: item2.Path,
+							op.NameAnnotation:     item1.Name,
 						},
 					},
 					Spec: appsv1.DeploymentSpec{
@@ -214,13 +214,10 @@ var _ = Describe("Deployment controller", func() {
 				}
 				return true
 			}, timeout, interval).Should(BeTrue())
-			Expect(updatedSecret.Data).Should(Equal(expectedSecretData2))
+			Expect(updatedSecret.Data).Should(Equal(item2.SecretData))
 		})
 
-		It("Should not update if Annotations have not changed", func() {
-			deployment1 := &appsv1.Deployment{}
-			k8sClient.Get(ctx, deploymentKey, deployment1)
-
+		It("Should not update secret if Annotations have not changed", func() {
 			By("Updating secret without changing annotations")
 			Eventually(func() error {
 				updatedDeployment := &appsv1.Deployment{
@@ -232,8 +229,8 @@ var _ = Describe("Deployment controller", func() {
 						Name:      deploymentKey.Name,
 						Namespace: deploymentKey.Namespace,
 						Annotations: map[string]string{
-							op.ItemPathAnnotation: itemPath,
-							op.NameAnnotation:     ItemName,
+							op.ItemPathAnnotation: item1.Path,
+							op.NameAnnotation:     item1.Name,
 						},
 					},
 					Spec: appsv1.DeploymentSpec{
@@ -263,9 +260,6 @@ var _ = Describe("Deployment controller", func() {
 				return nil
 			}, timeout, interval).Should(Succeed())
 
-			deployment2 := &appsv1.Deployment{}
-			k8sClient.Get(ctx, deploymentKey, deployment2)
-
 			// TODO: can we achieve the same without sleep?
 			time.Sleep(time.Millisecond * 10)
 			By("Reading updated K8s secret")
@@ -277,7 +271,7 @@ var _ = Describe("Deployment controller", func() {
 				}
 				return true
 			}, timeout, interval).Should(BeTrue())
-			Expect(updatedSecret.Data).Should(Equal(expectedSecretData))
+			Expect(updatedSecret.Data).Should(Equal(item1.SecretData))
 		})
 
 		It("Should not delete secret created via deployment if it's used in another container", func() {

controllers/onepassworditem_controller_test.go

@@ -24,7 +24,7 @@ const (
 	iceCream  = "freezing blue 20%"
 )
 
-var _ = Describe("OnePasswordItem controller", func() {
+var _ = Describe("TestItem controller", func() {
 	BeforeEach(func() {
 		// failed test runs that don't clean up leave resources behind.
 		err := k8sClient.DeleteAllOf(context.Background(), &onepasswordv1.OnePasswordItem{}, client.InNamespace(namespace))
@@ -35,10 +35,10 @@ var _ = Describe("OnePasswordItem controller", func() {
 		mocks.DoGetItemFunc = func(uuid string, vaultUUID string) (*onepassword.Item, error) {
 			item := onepassword.Item{}
 			item.Fields = []*onepassword.ItemField{}
-			for k, v := range itemData {
+			for k, v := range item1.Data {
 				item.Fields = append(item.Fields, &onepassword.ItemField{Label: k, Value: v})
 			}
-			item.Version = version
+			item.Version = item1.Version
 			item.Vault.ID = vaultUUID
 			item.ID = uuid
 			return &item, nil
@@ -49,11 +49,11 @@ var _ = Describe("OnePasswordItem controller", func() {
 		It("Should handle 1Password Item and secret correctly", func() {
 			ctx := context.Background()
 			spec := onepasswordv1.OnePasswordItemSpec{
-				ItemPath: itemPath,
+				ItemPath: item1.Path,
 			}
 
 			key := types.NamespacedName{
-				Name:      ItemName,
+				Name:      item1.Name,
 				Namespace: namespace,
 			}
 
@@ -65,7 +65,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 				Spec: spec,
 			}
 
-			By("Creating a new OnePasswordItem successfully")
+			By("Creating a new TestItem successfully")
 			Expect(k8sClient.Create(ctx, toCreate)).Should(Succeed())
 
 			created := &onepasswordv1.OnePasswordItem{}
@@ -86,7 +86,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 				}
 				return true
 			}, timeout, interval).Should(BeTrue())
-			Expect(createdSecret.Data).Should(Equal(expectedSecretData))
+			Expect(createdSecret.Data).Should(Equal(item1.SecretData))
 
 			By("Updating existing secret successfully")
 			newData := map[string]string{
@@ -105,7 +105,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 				for k, v := range newData {
 					item.Fields = append(item.Fields, &onepassword.ItemField{Label: k, Value: v})
 				}
-				item.Version = version + 1
+				item.Version = item1.Version + 1
 				item.Vault.ID = vaultUUID
 				item.ID = uuid
 				return &item, nil
@@ -123,7 +123,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 			}, timeout, interval).Should(BeTrue())
 			Expect(updatedSecret.Data).Should(Equal(newDataByte))
 
-			By("Deleting the OnePasswordItem successfully")
+			By("Deleting the TestItem successfully")
 			Eventually(func() error {
 				f := &onepasswordv1.OnePasswordItem{}
 				err := k8sClient.Get(ctx, key, f)
@@ -147,7 +147,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 		It("Should handle 1Password Item with fields and sections that have invalid K8s labels correctly", func() {
 			ctx := context.Background()
 			spec := onepasswordv1.OnePasswordItemSpec{
-				ItemPath: itemPath,
+				ItemPath: item1.Path,
 			}
 
 			key := types.NamespacedName{
@@ -185,13 +185,13 @@ var _ = Describe("OnePasswordItem controller", func() {
 				for k, v := range testData {
 					item.Fields = append(item.Fields, &onepassword.ItemField{Label: k, Value: v})
 				}
-				item.Version = version + 1
+				item.Version = item1.Version + 1
 				item.Vault.ID = vaultUUID
 				item.ID = uuid
 				return &item, nil
 			}
 
-			By("Creating a new OnePasswordItem successfully")
+			By("Creating a new TestItem successfully")
 			Expect(k8sClient.Create(ctx, toCreate)).Should(Succeed())
 
 			created := &onepasswordv1.OnePasswordItem{}
@@ -214,7 +214,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 			}, timeout, interval).Should(BeTrue())
 			Expect(createdSecret.Data).Should(Equal(expectedData))
 
-			By("Deleting the OnePasswordItem successfully")
+			By("Deleting the TestItem successfully")
 			Eventually(func() error {
 				f := &onepasswordv1.OnePasswordItem{}
 				err := k8sClient.Get(ctx, key, f)
@@ -235,14 +235,14 @@ var _ = Describe("OnePasswordItem controller", func() {
 			}, timeout, interval).ShouldNot(Succeed())
 		})
 
-		It("Should not update K8s secret if OnePasswordItem Version or VaultPath has not changed", func() {
+		It("Should not update K8s secret if TestItem Version or VaultPath has not changed", func() {
 			ctx := context.Background()
 			spec := onepasswordv1.OnePasswordItemSpec{
-				ItemPath: itemPath,
+				ItemPath: item1.Path,
 			}
 
 			key := types.NamespacedName{
-				Name:      ItemName,
+				Name:      item1.Name,
 				Namespace: namespace,
 			}
 
@@ -254,7 +254,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 				Spec: spec,
 			}
 
-			By("Creating a new OnePasswordItem successfully")
+			By("Creating a new TestItem successfully")
 			Expect(k8sClient.Create(ctx, toCreate)).Should(Succeed())
 
 			item := &onepasswordv1.OnePasswordItem{}
@@ -269,9 +269,9 @@ var _ = Describe("OnePasswordItem controller", func() {
 				err := k8sClient.Get(ctx, key, createdSecret)
 				return err == nil
 			}, timeout, interval).Should(BeTrue())
-			Expect(createdSecret.Data).Should(Equal(expectedSecretData))
+			Expect(createdSecret.Data).Should(Equal(item1.SecretData))
 
-			By("Updating OnePasswordItem type")
+			By("Updating TestItem type")
 			Eventually(func() bool {
 				err1 := k8sClient.Get(ctx, key, item)
 				if err1 != nil {
@@ -288,13 +288,13 @@ var _ = Describe("OnePasswordItem controller", func() {
 				err := k8sClient.Get(ctx, key, secret)
 				return err == nil
 			}, timeout, interval).Should(BeTrue())
-			Expect(secret.Data).Should(Equal(expectedSecretData))
+			Expect(secret.Data).Should(Equal(item1.SecretData))
 		})
 
-		It("Should update type of existing K8s Secret using OnePasswordItem", func() {
+		It("Should update type of existing K8s Secret using TestItem", func() {
 			ctx := context.Background()
 			spec := onepasswordv1.OnePasswordItemSpec{
-				ItemPath: itemPath,
+				ItemPath: item1.Path,
 			}
 
 			key := types.NamespacedName{
@@ -311,7 +311,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 				Type: string(v1.SecretTypeBasicAuth),
 			}
 
-			By("Creating a new OnePasswordItem successfully")
+			By("Creating a new TestItem successfully")
 			Expect(k8sClient.Create(ctx, toCreate)).Should(Succeed())
 
 			By("Reading K8s secret")
@@ -323,11 +323,11 @@ var _ = Describe("OnePasswordItem controller", func() {
 			Expect(secret.Type).Should(Equal(v1.SecretTypeBasicAuth))
 		})
 
-		It("Should create custom K8s Secret type using OnePasswordItem", func() {
+		It("Should create custom K8s Secret type using TestItem", func() {
 			const customType = "CustomType"
 			ctx := context.Background()
 			spec := onepasswordv1.OnePasswordItemSpec{
-				ItemPath: itemPath,
+				ItemPath: item1.Path,
 			}
 
 			key := types.NamespacedName{
@@ -344,7 +344,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 				Type: customType,
 			}
 
-			By("Creating a new OnePasswordItem successfully")
+			By("Creating a new TestItem successfully")
 			Expect(k8sClient.Create(ctx, toCreate)).Should(Succeed())
 
 			By("Reading K8s secret")
@@ -358,11 +358,13 @@ var _ = Describe("OnePasswordItem controller", func() {
 			}, timeout, interval).Should(BeTrue())
 			Expect(secret.Type).Should(Equal(v1.SecretType(customType)))
 		})
+	})
 
+	Context("Failing part", func() {
 		It("Should throw an error if K8s Secret type is changed", func() {
 			ctx := context.Background()
 			spec := onepasswordv1.OnePasswordItemSpec{
-				ItemPath: itemPath,
+				ItemPath: item1.Path,
 			}
 
 			key := types.NamespacedName{
@@ -378,7 +380,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 				Spec: spec,
 			}
 
-			By("Creating a new OnePasswordItem successfully")
+			By("Creating a new TestItem successfully")
 			Expect(k8sClient.Create(ctx, toCreate)).Should(Succeed())
 
 			By("Reading K8s secret")
@@ -401,14 +403,12 @@ var _ = Describe("OnePasswordItem controller", func() {
 				return true
 			}, timeout, interval).Should(BeFalse())
 		})
-	})
 
-	Context("Failing part", func() {
-		When("OnePasswordItem name contains `_`", func() {
+		When("TestItem name contains `_`", func() {
 			It("An error occurred", func() {
 				ctx := context.Background()
 				spec := onepasswordv1.OnePasswordItemSpec{
-					ItemPath: itemPath,
+					ItemPath: item1.Path,
 				}
 
 				key := types.NamespacedName{
@@ -424,17 +424,17 @@ var _ = Describe("OnePasswordItem controller", func() {
 					Spec: spec,
 				}
 
-				By("Creating a new OnePasswordItem")
+				By("Creating a new TestItem")
 				Expect(k8sClient.Create(ctx, toCreate)).To(HaveOccurred())
 
 			})
 		})
 
-		When("OnePasswordItem name contains capital letters", func() {
+		When("TestItem name contains capital letters", func() {
 			It("An error occurred", func() {
 				ctx := context.Background()
 				spec := onepasswordv1.OnePasswordItemSpec{
-					ItemPath: itemPath,
+					ItemPath: item1.Path,
 				}
 
 				key := types.NamespacedName{
@@ -450,7 +450,7 @@ var _ = Describe("OnePasswordItem controller", func() {
 					Spec: spec,
 				}
 
-				By("Creating a new OnePasswordItem")
+				By("Creating a new TestItem")
 				Expect(k8sClient.Create(ctx, toCreate)).To(HaveOccurred())
 			})
 		})

controllers/suite_test.go

@@ -26,7 +26,6 @@ package controllers
 
 import (
 	"context"
-	"fmt"
 	"path/filepath"
 	"regexp"
 	"testing"
@@ -52,35 +51,12 @@ import (
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
-var (
-	cfg       *rest.Config
-	k8sClient client.Client
-	testEnv   *envtest.Environment
-	ctx       context.Context
-	cancel    context.CancelFunc
-
-	itemData = map[string]string{
-		"username": username,
-		"password": password,
-	}
-	itemData2 = map[string]string{
-		"username": username2,
-		"password": password2,
-	}
-)
-
 const (
-	vaultId  = "hfnjvi6aymbsnfc2xeeoheizda"
-	itemId   = "nwrhuano7bcwddcviubpp4mhfq"
 	username = "test-user"
 	password = "QmHumKc$mUeEem7caHtbaBaJ"
-	version  = 123
 
-	vaultId2  = "hfnjvi6aymbsnfc2xeeoheizd2"
-	itemId2   = "nwrhuano7bcwddcviubpp4mhf2"
 	username2 = "test-user2"
 	password2 = "4zotzqDqXKasLFT2jzTs"
-	version2  = 456
 
 	annotationRegExpString = "^operator.1password.io\\/[a-zA-Z\\.]+"
 )
@@ -88,8 +64,6 @@ const (
 // Define utility constants for object names and testing timeouts/durations and intervals.
 const (
 	namespace = "default"
-	ItemName  = "test-item"
-	ItemName2 = "test-item2"
 
 	timeout  = time.Second * 10
 	duration = time.Second * 10
@@ -97,22 +71,51 @@ const (
 )
 
 var (
+	cfg                       *rest.Config
+	k8sClient                 client.Client
+	testEnv                   *envtest.Environment
+	ctx                       context.Context
+	cancel                    context.CancelFunc
 	onePasswordItemReconciler *OnePasswordItemReconciler
 	deploymentReconciler      *DeploymentReconciler
 
-	itemPath           = fmt.Sprintf("vaults/%v/items/%v", vaultId, itemId)
-	expectedSecretData = map[string][]byte{
-		"password": []byte(password),
-		"username": []byte(username),
+	item1 = &TestItem{
+		Name:    "test-item",
+		Version: 123,
+		Path:    "vaults/hfnjvi6aymbsnfc2xeeoheizda/items/nwrhuano7bcwddcviubpp4mhfq",
+		Data: map[string]string{
+			"username": username,
+			"password": password,
+		},
+		SecretData: map[string][]byte{
+			"password": []byte(password),
+			"username": []byte(username),
+		},
 	}
 
-	itemPath2           = fmt.Sprintf("vaults/%v/items/%v", vaultId2, itemId2)
-	expectedSecretData2 = map[string][]byte{
-		"password": []byte(password2),
-		"username": []byte(username2),
+	item2 = &TestItem{
+		Name:    "test-item2",
+		Path:    "vaults/hfnjvi6aymbsnfc2xeeoheizd2/items/nwrhuano7bcwddcviubpp4mhf2",
+		Version: 456,
+		Data: map[string]string{
+			"username": username2,
+			"password": password2,
+		},
+		SecretData: map[string][]byte{
+			"password": []byte(password2),
+			"username": []byte(username2),
+		},
 	}
 )
 
+type TestItem struct {
+	Name       string
+	Version    int
+	Path       string
+	Data       map[string]string
+	SecretData map[string][]byte
+}
+
 func TestAPIs(t *testing.T) {
 	RegisterFailHandler(Fail)