Merge pull request #4703 from KnVerey/name-ref-annotations

Fix NameReference transformer handling of self-references in annotations

api/krusty/namedspacedserviceaccounts_test.go

@@ -70,8 +70,5 @@ resources:
 `)
 
 	err := th.RunWithErr(".", th.MakeDefaultOptions())
-	assert.EqualError(t, err,
-		"updating name reference in 'subjects' field of 'ClusterRoleBinding.v1.rbac.authorization.k8s.io/crb-a.[noNs]': "+
-			"considering field 'subjects' of object ClusterRoleBinding.v1.rbac.authorization.k8s.io/crb-a.[noNs]:  "+
-			"found multiple possible referrals: ServiceAccount.v1.[noGrp]/sa.a, ServiceAccount.v1.[noGrp]/sa.b")
+	assert.Contains(t, err.Error(), "found multiple possible referrals: ServiceAccount.v1.[noGrp]/sa.a, ServiceAccount.v1.[noGrp]/sa.b")
 }

api/krusty/namereference_test.go

@@ -589,3 +589,85 @@ metadata:
   namespace: kube-system
 `)
 }
+
+func TestIssue4682_NameReferencesToSelfInAnnotations(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK(".", `
+namespace: newNs
+resources:
+  - resources.yaml
+
+nameSuffix: -updated
+
+configurations:
+  - kustomize-nameref.yaml 
+`)
+	th.WriteF("kustomize-nameref.yaml", `
+nameReference:
+  - kind: Namespace
+    fieldSpecs:
+      - path: data/theNamespace
+        kind: ConfigMap
+        version: v1
+      - path: metadata/annotations/theNamespace
+        kind: ConfigMap
+        version: v1      
+      - path: metadata/annotations/theNamespace
+        kind: Namespace
+        version: v1
+  - kind: ConfigMap
+    fieldSpecs:
+      - path: data/theConfigMap
+        kind: ConfigMap
+        version: v1
+      - path: metadata/annotations/theConfigMap
+        kind: ConfigMap
+        version: v1   
+      - path: metadata/annotations/theConfigMap
+        kind: Namespace
+        version: v1
+`)
+	th.WriteF("resources.yaml", `
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  annotations:
+    theConfigMap: cm
+    theNamespace: oldNs
+  name: cm
+  namespace: oldNs
+data:
+  theConfigMap: cm
+  theNamespace: oldNs
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  annotations:
+    theConfigMap: cm
+    theNamespace: oldNs
+  name: oldNs
+`)
+	m := th.Run(".", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: v1
+data:
+  theConfigMap: cm-updated
+  theNamespace: newNs
+kind: ConfigMap
+metadata:
+  annotations:
+    theConfigMap: cm-updated
+    theNamespace: newNs
+  name: cm-updated
+  namespace: newNs
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  annotations:
+    theConfigMap: cm-updated
+    theNamespace: newNs
+  name: newNs
+`)
+}

api/resource/resource.go

@@ -255,13 +255,9 @@ func (r *Resource) appendCsvAnnotation(name, value string) {
 	if value == "" {
 		return
 	}
-	annotations := r.GetAnnotations()
-	if existing, ok := annotations[name]; ok {
-		annotations[name] = existing + "," + value
-	} else {
-		annotations[name] = value
-	}
-	if err := r.SetAnnotations(annotations); err != nil {
+	currentValue := r.getCsvAnnotation(name)
+	newValue := strings.Join(append(currentValue, value), ",")
+	if err := r.RNode.PipeE(kyaml.SetAnnotation(name, newValue)); err != nil {
 		panic(err)
 	}
 }

api/resource/resource_test.go

@@ -1402,31 +1402,31 @@ spec:
 `))
 	assert.NoError(t, err)
 	r.AppendRefBy(resid.FromString("knd1.ver1.gr1/name1.ns1"))
-	assert.Equal(t, r.RNode.MustString(), `apiVersion: v1
+	assert.Equal(t, `apiVersion: v1
 kind: Deployment
 metadata:
   name: clown
   annotations:
-    internal.config.kubernetes.io/refBy: knd1.ver1.gr1/name1.ns1
+    internal.config.kubernetes.io/refBy: 'knd1.ver1.gr1/name1.ns1'
 spec:
   numReplicas: 1
-`)
+`, r.RNode.MustString())
 	assert.Equal(t, r.GetRefBy(), []resid.ResId{resid.FromString("knd1.ver1.gr1/name1.ns1")})
 
 	r.AppendRefBy(resid.FromString("knd2.ver2.gr2/name2.ns2"))
-	assert.Equal(t, r.RNode.MustString(), `apiVersion: v1
+	assert.Equal(t, `apiVersion: v1
 kind: Deployment
 metadata:
   name: clown
   annotations:
-    internal.config.kubernetes.io/refBy: knd1.ver1.gr1/name1.ns1,knd2.ver2.gr2/name2.ns2
+    internal.config.kubernetes.io/refBy: 'knd1.ver1.gr1/name1.ns1,knd2.ver2.gr2/name2.ns2'
 spec:
   numReplicas: 1
-`)
-	assert.Equal(t, r.GetRefBy(), []resid.ResId{
+`, r.RNode.MustString())
+	assert.Equal(t, []resid.ResId{
 		resid.FromString("knd1.ver1.gr1/name1.ns1"),
 		resid.FromString("knd2.ver2.gr2/name2.ns2"),
-	})
+	}, r.GetRefBy())
 }
 
 func TestOrigin(t *testing.T) {