Merge pull request #119 from 1Password/release/v1.5.0

Release v1.5.0

.VERSION

@@ -1 +1 @@
-1.4.1
+1.5.0

CHANGELOG.md

@@ -12,6 +12,18 @@
 
 ---
 
+[//]: # (START/v1.5.0)
+# v1.5.0
+
+## Features
+ * `OnePasswordItem` now contains a `status` which contains the status of creating the kubernetes secret for a OnePasswordItem. {#52}
+
+## Fixes
+ * The operator no longer logs an error about changing the secret type if the secret type is not actually being changed.
+ * Annotations on a deployment are no longer removed when the operator triggers a restart. {#112}
+
+---
+
 [//]: # "START/v1.4.1"
 
 # v1.4.1

deploy/crds/onepassword.com_onepassworditems_crd.yaml

@@ -12,8 +12,6 @@ spec:
   scope: Namespaced
   versions:
   - name: v1
-    served: true
-    storage: true
     schema:
       openAPIV3Schema:
         description: OnePasswordItem is the Schema for the onepassworditems API
@@ -38,8 +36,41 @@ spec:
             type: object
           status:
             description: OnePasswordItemStatus defines the observed state of OnePasswordItem
+            properties:
+              conditions:
+                description: 'Important: Run "operator-sdk generate k8s" to regenerate
+                  code after modifying this file Add custom validation using kubebuilder
+                  tags: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html'
+                items:
+                  properties:
+                    lastTransitionTime:
+                      description: Last time the condition transit from one status
+                        to another.
+                      format: date-time
+                      type: string
+                    message:
+                      description: Human-readable message indicating details about
+                        last transition.
+                      type: string
+                    status:
+                      description: Status of the condition, one of True, False, Unknown.
+                      type: string
+                    type:
+                      description: Type of job condition, Completed.
+                      type: string
+                  required:
+                  - status
+                  - type
+                  type: object
+                type: array
+            required:
+            - conditions
             type: object
           type:
             description: 'Kubernetes secret type. More info: https://kubernetes.io/docs/concepts/configuration/secret/#secret-types'
             type: string
         type: object
+    served: true
+    storage: true
+    subresources:
+      status: {}

pkg/apis/onepassword/v1/onepasswordsecret_types.go

@@ -11,11 +11,31 @@ type OnePasswordItemSpec struct {
 	ItemPath string `json:"itemPath,omitempty"`
 }
 
+type OnePasswordItemConditionType string
+
+const (
+	// OnePasswordItemReady means the Kubernetes secret is ready for use.
+	OnePasswordItemReady OnePasswordItemConditionType = "Ready"
+)
+
+type OnePasswordItemCondition struct {
+	// Type of job condition, Completed.
+	Type OnePasswordItemConditionType `json:"type"`
+	// Status of the condition, one of True, False, Unknown.
+	Status metav1.ConditionStatus `json:"status"`
+	// Last time the condition transit from one status to another.
+	// +optional
+	LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
+	// Human-readable message indicating details about last transition.
+	// +optional
+	Message string `json:"message,omitempty"`
+}
+
 // OnePasswordItemStatus defines the observed state of OnePasswordItem
 type OnePasswordItemStatus struct {
-	// INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
 	// Important: Run "operator-sdk generate k8s" to regenerate code after modifying this file
 	// Add custom validation using kubebuilder tags: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html
+	Conditions []OnePasswordItemCondition `json:"conditions"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -26,7 +46,9 @@ type OnePasswordItemStatus struct {
 type OnePasswordItem struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
-	Type              string `json:"type,omitempty"`
+
+	// Kubernetes secret type. More info: https://kubernetes.io/docs/concepts/configuration/secret/#secret-types
+	Type string `json:"type,omitempty"`
 
 	Spec   OnePasswordItemSpec   `json:"spec,omitempty"`
 	Status OnePasswordItemStatus `json:"status,omitempty"`

pkg/apis/onepassword/v1/zz_generated.deepcopy.go

@@ -1,3 +1,4 @@
+//go:build !ignore_autogenerated
 // +build !ignore_autogenerated
 
 // Code generated by operator-sdk. DO NOT EDIT.
@@ -14,7 +15,7 @@ func (in *OnePasswordItem) DeepCopyInto(out *OnePasswordItem) {
 	out.TypeMeta = in.TypeMeta
 	in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
 	out.Spec = in.Spec
-	out.Status = in.Status
+	in.Status.DeepCopyInto(&out.Status)
 	return
 }
 
@@ -36,6 +37,23 @@ func (in *OnePasswordItem) DeepCopyObject() runtime.Object {
 	return nil
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *OnePasswordItemCondition) DeepCopyInto(out *OnePasswordItemCondition) {
+	*out = *in
+	in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime)
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OnePasswordItemCondition.
+func (in *OnePasswordItemCondition) DeepCopy() *OnePasswordItemCondition {
+	if in == nil {
+		return nil
+	}
+	out := new(OnePasswordItemCondition)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *OnePasswordItemList) DeepCopyInto(out *OnePasswordItemList) {
 	*out = *in
@@ -88,6 +106,13 @@ func (in *OnePasswordItemSpec) DeepCopy() *OnePasswordItemSpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *OnePasswordItemStatus) DeepCopyInto(out *OnePasswordItemStatus) {
 	*out = *in
+	if in.Conditions != nil {
+		in, out := &in.Conditions, &out.Conditions
+		*out = make([]OnePasswordItemCondition, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
 	return
 }
 

pkg/controller/onepassworditem/onepassworditem_controller.go

@@ -96,10 +96,11 @@ func (r *ReconcileOnePasswordItem) Reconcile(request reconcile.Request) (reconci
 		}
 
 		// Handles creation or updating secrets for deployment if needed
-		if err := r.HandleOnePasswordItem(onepassworditem, request); err != nil {
-			return reconcile.Result{}, err
+		err := r.HandleOnePasswordItem(onepassworditem, request)
+		if updateStatusErr := r.updateStatus(onepassworditem, err); updateStatusErr != nil {
+			return reconcile.Result{}, fmt.Errorf("cannot update status: %s", updateStatusErr)
 		}
-		return reconcile.Result{}, nil
+		return reconcile.Result{}, err
 	}
 	// If one password finalizer exists then we must cleanup associated secrets
 	if utils.ContainsString(onepassworditem.ObjectMeta.Finalizers, finalizer) {
@@ -169,3 +170,34 @@ func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1
 
 	return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, resource.Namespace, item, autoRestart, labels, secretType, ownerRef)
 }
+
+func (r *ReconcileOnePasswordItem) updateStatus(resource *onepasswordv1.OnePasswordItem, err error) error {
+	existingCondition := findCondition(resource.Status.Conditions, onepasswordv1.OnePasswordItemReady)
+	updatedCondition := existingCondition
+	if err != nil {
+		updatedCondition.Message = err.Error()
+		updatedCondition.Status = metav1.ConditionFalse
+	} else {
+		updatedCondition.Message = ""
+		updatedCondition.Status = metav1.ConditionTrue
+	}
+
+	if existingCondition.Status != updatedCondition.Status {
+		updatedCondition.LastTransitionTime = metav1.Now()
+	}
+
+	resource.Status.Conditions = []onepasswordv1.OnePasswordItemCondition{updatedCondition}
+	return r.kubeClient.Status().Update(context.Background(), resource)
+}
+
+func findCondition(conditions []onepasswordv1.OnePasswordItemCondition, t onepasswordv1.OnePasswordItemConditionType) onepasswordv1.OnePasswordItemCondition {
+	for _, c := range conditions {
+		if c.Type == t {
+			return c
+		}
+	}
+	return onepasswordv1.OnePasswordItemCondition{
+		Type:   t,
+		Status: metav1.ConditionUnknown,
+	}
+}

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"
 
@@ -45,8 +44,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa
 	if autoRestart != "" {
 		_, err := utils.StringToBool(autoRestart)
 		if err != nil {
-			log.Error(err, "Error parsing %v annotation on Secret %v. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, secretName)
-			return err
+			return fmt.Errorf("Error parsing %v annotation on Secret %v. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, secretName)
 		}
 		secretAnnotations[RestartDeploymentsAnnotation] = autoRestart
 	}
@@ -63,19 +61,31 @@ 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
 		currentSecret.ObjectMeta.Labels = labels
 		currentSecret.Data = secret.Data
-		return kubeClient.Update(context.Background(), currentSecret)
+		if err := kubeClient.Update(context.Background(), currentSecret); err != nil {
+			return fmt.Errorf("Kubernetes secret update failed: %w", err)
+		}
+		return nil
 	}
 
 	log.Info(fmt.Sprintf("Secret with name %v and version %v already exists", secret.Name, secret.Annotations[VersionAnnotation]))

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
+				}
+			}
 		})
 	}
 }