Merge pull request #105 from 1Password/release/v1.4.1

Prepare Release - v1.4.1

.VERSION

@@ -1 +1 @@
-1.4.0
+1.4.1

CHANGELOG.md

@@ -1,18 +1,25 @@
-[//]: # "START/LATEST"
+[//]: # (START/LATEST)
-
 # Latest
 
 ## Features
+  * A user-friendly description of a new feature. {issue-number}
 
-- A user-friendly description of a new feature. {issue-number}
+## Fixes
+ * A user-friendly description of a fix. {issue-number}
+
+## Security
+ * A user-friendly description of a security fix. {issue-number}
+
+---
+
+[//]: # "START/v1.4.1"
+
+# v1.4.1
 
 ## Fixes
 
-- A user-friendly description of a fix. {issue-number}
+- OwnerReferences on secrets are now persisted after an item is updated. {#101}
-
+- Annotations from a Deployment or OnePasswordItem are no longer applied to Secrets that are created for it. {#102}
-## Security
-
-- A user-friendly description of a security fix. {issue-number}
 
 ---
 

pkg/controller/deployment/deployment_controller.go

@@ -218,5 +218,5 @@ func (r *ReconcileDeployment) HandleApplyingDeployment(deployment *appsv1.Deploy
 		UID:        deployment.GetUID(),
 	}
 
-	return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, namespace, item, annotations[op.RestartDeploymentsAnnotation], secretLabels, secretType, annotations, ownerRef)
+	return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, namespace, item, annotations[op.RestartDeploymentsAnnotation], secretLabels, secretType, ownerRef)
 }

pkg/controller/deployment/deployment_controller_test.go

@@ -281,7 +281,6 @@ var tests = []testReconcileItem{
 				Annotations: map[string]string{
 					op.VersionAnnotation:  fmt.Sprint(version),
 					op.ItemPathAnnotation: itemPath,
-					op.NameAnnotation:     name,
 				},
 			},
 			Data: expectedSecretData,
@@ -294,7 +293,6 @@ var tests = []testReconcileItem{
 				Annotations: map[string]string{
 					op.VersionAnnotation:  fmt.Sprint(version),
 					op.ItemPathAnnotation: itemPath,
-					op.NameAnnotation:     name,
 				},
 				Labels: map[string]string(nil),
 			},
@@ -385,7 +383,7 @@ var tests = []testReconcileItem{
 	},
 }
 
-func TestReconcileDepoyment(t *testing.T) {
+func TestReconcileDeployment(t *testing.T) {
 	for _, testData := range tests {
 		t.Run(testData.testName, func(t *testing.T) {
 

pkg/controller/onepassworditem/onepassworditem_controller.go

@@ -147,9 +147,8 @@ func (r *ReconcileOnePasswordItem) removeOnePasswordFinalizerFromOnePasswordItem
 func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1.OnePasswordItem, request reconcile.Request) error {
 	secretName := resource.GetName()
 	labels := resource.Labels
-	annotations := resource.Annotations
 	secretType := resource.Type
-	autoRestart := annotations[op.RestartDeploymentsAnnotation]
+	autoRestart := resource.Annotations[op.RestartDeploymentsAnnotation]
 
 	item, err := onepassword.GetOnePasswordItemByPath(r.opConnectClient, resource.Spec.ItemPath)
 	if err != nil {
@@ -168,5 +167,5 @@ func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1
 		UID:        resource.GetUID(),
 	}
 
-	return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, resource.Namespace, item, autoRestart, labels, secretType, annotations, ownerRef)
+	return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, resource.Namespace, item, autoRestart, labels, secretType, ownerRef)
 }

pkg/kubernetessecrets/kubernetes_secrets_builder.go

@@ -35,18 +35,13 @@ var ErrCannotUpdateSecretType = errs.New("Cannot change secret type. Secret type
 
 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, ownerRef *metav1.OwnerReference) error {
+func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart string, labels map[string]string, secretType string, ownerRef *metav1.OwnerReference) error {
-
 	itemVersion := fmt.Sprint(item.Version)
-
+	secretAnnotations := map[string]string{
-	// If secretAnnotations is nil we create an empty map so we can later assign values for the OP Annotations in the map
+		VersionAnnotation:  itemVersion,
-	if secretAnnotations == nil {
+		ItemPathAnnotation: fmt.Sprintf("vaults/%v/items/%v", item.Vault.ID, item.ID),
-		secretAnnotations = map[string]string{}
 	}
 
-	secretAnnotations[VersionAnnotation] = itemVersion
-	secretAnnotations[ItemPathAnnotation] = fmt.Sprintf("vaults/%v/items/%v", item.Vault.ID, item.ID)
-
 	if autoRestart != "" {
 		_, err := utils.StringToBool(autoRestart)
 		if err != nil {

pkg/kubernetessecrets/kubernetes_secrets_builder_test.go

@@ -33,12 +33,9 @@ func TestCreateKubernetesSecretFromOnePasswordItem(t *testing.T) {
 
 	kubeClient := fake.NewFakeClient()
 	secretLabels := map[string]string{}
-	secretAnnotations := map[string]string{
-		"testAnnotation": "exists",
-	}
 	secretType := ""
 
-	err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, secretAnnotations, nil)
+	err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, nil)
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
 	}
@@ -50,10 +47,6 @@ func TestCreateKubernetesSecretFromOnePasswordItem(t *testing.T) {
 	}
 	compareFields(item.Fields, createdSecret.Data, t)
 	compareAnnotationsToItem(createdSecret.Annotations, item, t)
-
-	if createdSecret.Annotations["testAnnotation"] != "exists" {
-		t.Errorf("Expected testAnnotation to be merged with existing annotations, but wasn't.")
-	}
 }
 
 func TestKubernetesSecretFromOnePasswordItemOwnerReferences(t *testing.T) {
@@ -68,9 +61,6 @@ func TestKubernetesSecretFromOnePasswordItemOwnerReferences(t *testing.T) {
 
 	kubeClient := fake.NewFakeClient()
 	secretLabels := map[string]string{}
-	secretAnnotations := map[string]string{
-		"testAnnotation": "exists",
-	}
 	secretType := ""
 
 	ownerRef := &metav1.OwnerReference{
@@ -79,7 +69,7 @@ func TestKubernetesSecretFromOnePasswordItemOwnerReferences(t *testing.T) {
 		Name:       "test-deployment",
 		UID:        types.UID("test-uid"),
 	}
-	err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, secretAnnotations, ownerRef)
+	err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, ownerRef)
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
 	}
@@ -116,10 +106,9 @@ func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) {
 
 	kubeClient := fake.NewFakeClient()
 	secretLabels := map[string]string{}
-	secretAnnotations := map[string]string{}
 	secretType := ""
 
-	err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, secretAnnotations, nil)
+	err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, nil)
 
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
@@ -131,7 +120,7 @@ func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) {
 	newItem.Version = 456
 	newItem.Vault.ID = "hfnjvi6aymbsnfc2xeeoheizda"
 	newItem.ID = "h46bb3jddvay7nxopfhvlwg35q"
-	err = CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &newItem, restartDeploymentAnnotation, secretLabels, secretType, secretAnnotations, nil)
+	err = CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &newItem, restartDeploymentAnnotation, secretLabels, secretType, nil)
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
 	}
@@ -232,12 +221,9 @@ func TestCreateKubernetesTLSSecretFromOnePasswordItem(t *testing.T) {
 
 	kubeClient := fake.NewFakeClient()
 	secretLabels := map[string]string{}
-	secretAnnotations := map[string]string{
-		"testAnnotation": "exists",
-	}
 	secretType := "kubernetes.io/tls"
 
-	err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, secretAnnotations, nil)
+	err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, nil)
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
 	}

pkg/onepassword/secret_update_handler.go

@@ -134,15 +134,21 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]*
 				log.Info(fmt.Sprintf("Secret '%v' has been updated in 1Password but is set to be ignored. Updates to an ignored secret will not trigger an update to a kubernetes secret or a rolling restart.", secret.GetName()))
 				secret.Annotations[VersionAnnotation] = itemVersion
 				secret.Annotations[ItemPathAnnotation] = itemPathString
-				h.client.Update(context.Background(), &secret)
+				if err := h.client.Update(context.Background(), &secret); err != nil {
+					log.Error(err, "failed to update secret %s annotations to version %d: %s", secret.Name, itemVersion, err)
+					continue
+				}
 				continue
 			}
 			log.Info(fmt.Sprintf("Updating kubernetes secret '%v'", secret.GetName()))
 			secret.Annotations[VersionAnnotation] = itemVersion
 			secret.Annotations[ItemPathAnnotation] = itemPathString
-			updatedSecret := kubeSecrets.BuildKubernetesSecretFromOnePasswordItem(secret.Name, secret.Namespace, secret.Annotations, secret.Labels, string(secret.Type), *item, nil)
+			secret.Data = kubeSecrets.BuildKubernetesSecretData(item.Fields, item.Files)
-			log.Info(fmt.Sprintf("New secret path: %v and version: %v", updatedSecret.Annotations[ItemPathAnnotation], updatedSecret.Annotations[VersionAnnotation]))
+			log.Info(fmt.Sprintf("New secret path: %v and version: %v", secret.Annotations[ItemPathAnnotation], secret.Annotations[VersionAnnotation]))
-			h.client.Update(context.Background(), updatedSecret)
+			if err := h.client.Update(context.Background(), &secret); err != nil {
+				log.Error(err, "failed to update secret %s to version %d: %s", secret.Name, itemVersion, err)
+				continue
+			}
 			if updatedSecrets[secret.Namespace] == nil {
 				updatedSecrets[secret.Namespace] = make(map[string]*corev1.Secret)
 			}