Merge pull request #104 from 1Password/secret-annotations

Stop copying annotations from OnePasswordItem and Deployment to Secret

CHANGELOG.md

@@ -9,6 +9,7 @@
 ## Fixes
 
 - 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
 

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