From b16960057a082e89e4113c9a5fc8bf3ad707d202 Mon Sep 17 00:00:00 2001
From: Marton Soos <marton.soos@agilebits.com>
Date: Fri, 18 Feb 2022 10:47:14 +0100
Subject: [PATCH] Update tests and add new test

---
 .../deployment/deployment_controller_test.go  |  6 +--
 .../onepassworditem/onepassworditem_test.go   | 51 +++++++++++++++++--
 .../kubernetes_secrets_builder.go             |  7 ++-
 3 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go
index ee6ac97..bd98231 100644
--- a/pkg/controller/deployment/deployment_controller_test.go
+++ b/pkg/controller/deployment/deployment_controller_test.go
@@ -329,7 +329,7 @@ var tests = []testReconcileItem{
 					op.VersionAnnotation: "456",
 				},
 			},
-			Type: corev1.SecretTypeOpaque,
+			Type: corev1.SecretType(""),
 			Data: expectedSecretData,
 		},
 		expectedError: nil,
@@ -341,7 +341,7 @@ var tests = []testReconcileItem{
 					op.VersionAnnotation: fmt.Sprint(version),
 				},
 			},
-			Type: corev1.SecretTypeOpaque,
+			Type: corev1.SecretType(""),
 			Data: expectedSecretData,
 		},
 		opItem: map[string]string{
@@ -375,7 +375,7 @@ var tests = []testReconcileItem{
 					op.VersionAnnotation: fmt.Sprint(version),
 				},
 			},
-			Type: corev1.SecretTypeOpaque,
+			Type: corev1.SecretType(""),
 			Data: expectedSecretData,
 		},
 		opItem: map[string]string{
diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go
index f6bf8e2..9add483 100644
--- a/pkg/controller/onepassworditem/onepassworditem_test.go
+++ b/pkg/controller/onepassworditem/onepassworditem_test.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/1Password/onepassword-operator/pkg/kubernetessecrets"
 	"github.com/1Password/onepassword-operator/pkg/mocks"
 	op "github.com/1Password/onepassword-operator/pkg/onepassword"
 
@@ -185,7 +186,6 @@ var tests = []testReconcileItem{
 				},
 				Labels: map[string]string{},
 			},
-			Type: corev1.SecretTypeOpaque,
 			Data: expectedSecretData,
 		},
 		opItem: map[string]string{
@@ -224,7 +224,7 @@ var tests = []testReconcileItem{
 				},
 				Labels: map[string]string{},
 			},
-			Type: corev1.SecretTypeOpaque,
+			Type: corev1.SecretTypeBasicAuth,
 			Data: expectedSecretData,
 		},
 		expectedError: nil,
@@ -280,6 +280,50 @@ var tests = []testReconcileItem{
 			passKey: password,
 		},
 	},
+	{
+		testName: "Error if secret type is changed",
+		customResource: &onepasswordv1.OnePasswordItem{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       onePasswordItemKind,
+				APIVersion: onePasswordItemAPIVersion,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      name,
+				Namespace: namespace,
+			},
+			Spec: onepasswordv1.OnePasswordItemSpec{
+				ItemPath: itemPath,
+			},
+			Type: "custom",
+		},
+		existingSecret: &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      name,
+				Namespace: namespace,
+				Annotations: map[string]string{
+					op.VersionAnnotation: fmt.Sprint(version),
+				},
+			},
+			Type: corev1.SecretTypeOpaque,
+			Data: expectedSecretData,
+		},
+		expectedError: kubernetessecrets.ErrCannotUpdateSecretType,
+		expectedResultSecret: &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      name,
+				Namespace: namespace,
+				Annotations: map[string]string{
+					op.VersionAnnotation: fmt.Sprint(version),
+				},
+			},
+			Type: corev1.SecretTypeOpaque,
+			Data: expectedSecretData,
+		},
+		opItem: map[string]string{
+			userKey: username,
+			passKey: password,
+		},
+	},
 	{
 		testName: "Secret from 1Password item with invalid K8s labels",
 		customResource: &onepasswordv1.OnePasswordItem{
@@ -305,7 +349,6 @@ var tests = []testReconcileItem{
 					op.VersionAnnotation: fmt.Sprint(version),
 				},
 			},
-			Type: corev1.SecretTypeOpaque,
 			Data: expectedSecretData,
 		},
 		opItem: map[string]string{
@@ -338,7 +381,6 @@ var tests = []testReconcileItem{
 					op.VersionAnnotation: fmt.Sprint(version),
 				},
 			},
-			Type: corev1.SecretTypeOpaque,
 			Data: map[string][]byte{
 				"password":       []byte(password),
 				"username":       []byte(username),
@@ -380,7 +422,6 @@ var tests = []testReconcileItem{
 					op.VersionAnnotation: fmt.Sprint(version),
 				},
 			},
-			Type: corev1.SecretTypeOpaque,
 			Data: map[string][]byte{
 				"password":          []byte(password),
 				"username":          []byte(username),
diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go
index 5845118..d3fa890 100644
--- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go
+++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go
@@ -9,6 +9,8 @@ import (
 
 	"reflect"
 
+	errs "errors"
+
 	"github.com/1Password/connect-sdk-go/onepassword"
 	"github.com/1Password/onepassword-operator/pkg/utils"
 	corev1 "k8s.io/api/core/v1"
@@ -16,7 +18,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	kubeValidate "k8s.io/apimachinery/pkg/util/validation"
-	errs "errors"
 
 	kubernetesClient "sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -29,6 +30,8 @@ const restartAnnotation = OnepasswordPrefix + "/last-restarted"
 const ItemPathAnnotation = OnepasswordPrefix + "/item-path"
 const RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto-restart"
 
+var ErrCannotUpdateSecretType = errs.New("Cannot change secret type. Secret type is immutable")
+
 var log = logf.Log
 
 func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart string, labels map[string]string, secretType string, secretAnnotations map[string]string) error {
@@ -68,7 +71,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa
 	currentLabels := currentSecret.Labels
 	currentSecretType := string(currentSecret.Type)
 	if !reflect.DeepEqual(currentSecretType, secretType) {
-		return errs.New("Cannot change secret type. Secret type is immutable")
+		return ErrCannotUpdateSecretType
 	}
 
 	if !reflect.DeepEqual(currentAnnotations, secretAnnotations) || !reflect.DeepEqual(currentLabels, labels) {