Merge remote-tracking branch 'origin/main' into item-status

pkg/kubernetessecrets/kubernetes_secrets_builder.go

@@ -27,7 +27,6 @@ import (
 const OnepasswordPrefix = "operator.1password.io"
 const NameAnnotation = OnepasswordPrefix + "/item-name"
 const VersionAnnotation = OnepasswordPrefix + "/item-version"
-const restartAnnotation = OnepasswordPrefix + "/last-restarted"
 const ItemPathAnnotation = OnepasswordPrefix + "/item-path"
 const RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto-restart"
 
@@ -62,13 +61,22 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa
 		return err
 	}
 
-	currentAnnotations := currentSecret.Annotations
-	currentLabels := currentSecret.Labels
+	// Check if the secret types are being changed on the update.
+	// Avoid Opaque and "" are treated as different on check.
+	wantSecretType := secretType
+	if wantSecretType == "" {
+		wantSecretType = string(corev1.SecretTypeOpaque)
+	}
 	currentSecretType := string(currentSecret.Type)
-	if !reflect.DeepEqual(currentSecretType, secretType) {
+	if currentSecretType == "" {
+		currentSecretType = string(corev1.SecretTypeOpaque)
+	}
+	if currentSecretType != wantSecretType {
 		return ErrCannotUpdateSecretType
 	}
 
+	currentAnnotations := currentSecret.Annotations
+	currentLabels := currentSecret.Labels
 	if !reflect.DeepEqual(currentAnnotations, secretAnnotations) || !reflect.DeepEqual(currentLabels, labels) {
 		log.Info(fmt.Sprintf("Updating Secret %v at namespace '%v'", secret.Name, secret.Namespace))
 		currentSecret.ObjectMeta.Annotations = secretAnnotations

pkg/onepassword/secret_update_handler.go

@@ -91,9 +91,10 @@ func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecret
 
 func (h *SecretUpdateHandler) restartDeployment(deployment *appsv1.Deployment) {
 	log.Info(fmt.Sprintf("Deployment %q at namespace %q references an updated secret. Restarting", deployment.GetName(), deployment.Namespace))
-	deployment.Spec.Template.Annotations = map[string]string{
-		RestartAnnotation: time.Now().String(),
+	if deployment.Spec.Template.Annotations == nil {
+		deployment.Spec.Template.Annotations = map[string]string{}
 	}
+	deployment.Spec.Template.Annotations[RestartAnnotation] = time.Now().String()
 	err := h.client.Update(context.Background(), deployment)
 	if err != nil {
 		log.Error(err, "Problem restarting deployment")

pkg/onepassword/secret_update_handler_test.go

@@ -122,6 +122,9 @@ var tests = []testUpdateSecretTask{
 			},
 			Spec: appsv1.DeploymentSpec{
 				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{"external-annotation": "some-value"},
+					},
 					Spec: corev1.PodSpec{
 						Containers: []corev1.Container{
 							{
@@ -235,6 +238,9 @@ var tests = []testUpdateSecretTask{
 			},
 			Spec: appsv1.DeploymentSpec{
 				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{"external-annotation": "some-value"},
+					},
 					Spec: corev1.PodSpec{
 						Volumes: []corev1.Volume{
 							{
@@ -342,6 +348,9 @@ var tests = []testUpdateSecretTask{
 			},
 			Spec: appsv1.DeploymentSpec{
 				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{"external-annotation": "some-value"},
+					},
 					Spec: corev1.PodSpec{
 						Containers: []corev1.Container{
 							{
@@ -411,6 +420,9 @@ var tests = []testUpdateSecretTask{
 			},
 			Spec: appsv1.DeploymentSpec{
 				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{"external-annotation": "some-value"},
+					},
 					Spec: corev1.PodSpec{
 						Containers: []corev1.Container{
 							{
@@ -482,6 +494,9 @@ var tests = []testUpdateSecretTask{
 			},
 			Spec: appsv1.DeploymentSpec{
 				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{"external-annotation": "some-value"},
+					},
 					Spec: corev1.PodSpec{
 						Containers: []corev1.Container{
 							{
@@ -553,6 +568,9 @@ var tests = []testUpdateSecretTask{
 			},
 			Spec: appsv1.DeploymentSpec{
 				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{"external-annotation": "some-value"},
+					},
 					Spec: corev1.PodSpec{
 						Containers: []corev1.Container{
 							{
@@ -630,6 +648,9 @@ var tests = []testUpdateSecretTask{
 			},
 			Spec: appsv1.DeploymentSpec{
 				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{"external-annotation": "some-value"},
+					},
 					Spec: corev1.PodSpec{
 						Containers: []corev1.Container{
 							{
@@ -703,6 +724,9 @@ var tests = []testUpdateSecretTask{
 			},
 			Spec: appsv1.DeploymentSpec{
 				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{"external-annotation": "some-value"},
+					},
 					Spec: corev1.PodSpec{
 						Containers: []corev1.Container{
 							{
@@ -829,6 +853,16 @@ func TestUpdateSecretHandler(t *testing.T) {
 			} else {
 				assert.False(t, testData.expectedRestart, "Deployment was restarted but should not have been.")
 			}
+
+			oldPodTemplateAnnotations := testData.existingDeployment.Spec.Template.ObjectMeta.Annotations
+			newPodTemplateAnnotations := deployment.Spec.Template.Annotations
+			for name, expected := range oldPodTemplateAnnotations {
+				actual, ok := newPodTemplateAnnotations[name]
+				if assert.Truef(t, ok, "Annotation %s was present in original pod template but was dropped after update", name) {
+					assert.Equalf(t, expected, actual, "Annotation value for %s original pod template has changed", name)
+					continue
+				}
+			}
 		})
 	}
 }