From b16e4ec566ddc70f6c393ca2cf5fecbfe94c2bc1 Mon Sep 17 00:00:00 2001 From: Julian van den Berkmortel Date: Wed, 10 Nov 2021 21:29:27 +0100 Subject: [PATCH 1/2] add test to demonstrate internal annotations getting lost (#4111) --- api/krusty/multiplepatch_test.go | 61 ++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/api/krusty/multiplepatch_test.go b/api/krusty/multiplepatch_test.go index 5fb5fed3f5..0d0e40e1c6 100644 --- a/api/krusty/multiplepatch_test.go +++ b/api/krusty/multiplepatch_test.go @@ -1646,3 +1646,64 @@ spec: type: NodePort `) } + +// test for #4111, currently demonstrates incorrect behaviour +func TestPatchPreservesInternalAnnotations(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteK(".", ` +nameSuffix: -abc +resources: + - fluentd.yaml +patchesJson6902: + - path: patch.yaml + target: + name: fluentd-sa + kind: ServiceAccount + version: v1 +`) + th.WriteF("fluentd.yaml", ` +apiVersion: v1 +kind: DaemonSet +metadata: + name: fluentd +spec: + template: + spec: + containers: + - image: fluentd:latest + name: fluentd + serviceAccountName: fluentd-sa +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: fluentd-sa +`) + th.WriteF("patch.yaml", ` +- op: add + path: /metadata/annotations + value: + note: this is a test annotation +`) + m := th.Run(".", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: DaemonSet +metadata: + name: fluentd-abc +spec: + template: + spec: + containers: + - image: fluentd:latest + name: fluentd + serviceAccountName: fluentd-sa +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + annotations: + note: this is a test annotation + name: fluentd-sa-abc +`) +} \ No newline at end of file From b6cb6c8ae9435f1645a35b0f8ba694605fb99bc7 Mon Sep 17 00:00:00 2001 From: Julian van den Berkmortel Date: Wed, 10 Nov 2021 21:41:11 +0100 Subject: [PATCH 2/2] fix build annotations getting lost after applying JSON 6902 patch --- api/builtins/PatchJson6902Transformer.go | 12 ++++++++++++ api/builtins/PatchTransformer.go | 8 ++++++++ api/krusty/multiplepatch_test.go | 6 +++--- .../PatchJson6902Transformer.go | 12 ++++++++++++ plugin/builtin/patchjson6902transformer/go.mod | 1 + plugin/builtin/patchtransformer/PatchTransformer.go | 8 ++++++++ plugin/builtin/patchtransformer/go.mod | 1 + 7 files changed, 45 insertions(+), 3 deletions(-) diff --git a/api/builtins/PatchJson6902Transformer.go b/api/builtins/PatchJson6902Transformer.go index dfe4db43bd..cb02c42417 100644 --- a/api/builtins/PatchJson6902Transformer.go +++ b/api/builtins/PatchJson6902Transformer.go @@ -12,6 +12,7 @@ import ( "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/yaml" ) @@ -78,12 +79,23 @@ func (p *PatchJson6902TransformerPlugin) Transform(m resmap.ResMap) error { return err } for _, res := range resources { + internalAnnotations := kioutil.GetInternalAnnotations(&res.RNode) + err = res.ApplyFilter(patchjson6902.Filter{ Patch: p.JsonOp, }) if err != nil { return err } + + annotations := res.GetAnnotations() + for key, value := range internalAnnotations { + annotations[key] = value + } + err = res.SetAnnotations(annotations) + if err != nil { + return err + } } return nil } diff --git a/api/builtins/PatchTransformer.go b/api/builtins/PatchTransformer.go index 07038af1ec..dc51748c80 100644 --- a/api/builtins/PatchTransformer.go +++ b/api/builtins/PatchTransformer.go @@ -12,6 +12,7 @@ import ( "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/yaml" ) @@ -111,12 +112,19 @@ func (p *PatchTransformerPlugin) transformJson6902(m resmap.ResMap, patch jsonpa } for _, res := range resources { res.StorePreviousId() + internalAnnotations := kioutil.GetInternalAnnotations(&res.RNode) err = res.ApplyFilter(patchjson6902.Filter{ Patch: p.Patch, }) if err != nil { return err } + + annotations := res.GetAnnotations() + for key, value := range internalAnnotations { + annotations[key] = value + } + err = res.SetAnnotations(annotations) } return nil } diff --git a/api/krusty/multiplepatch_test.go b/api/krusty/multiplepatch_test.go index 0d0e40e1c6..9935fba535 100644 --- a/api/krusty/multiplepatch_test.go +++ b/api/krusty/multiplepatch_test.go @@ -1647,7 +1647,7 @@ spec: `) } -// test for #4111, currently demonstrates incorrect behaviour +// test for #4111 func TestPatchPreservesInternalAnnotations(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK(".", ` @@ -1697,7 +1697,7 @@ spec: containers: - image: fluentd:latest name: fluentd - serviceAccountName: fluentd-sa + serviceAccountName: fluentd-sa-abc --- apiVersion: v1 kind: ServiceAccount @@ -1706,4 +1706,4 @@ metadata: note: this is a test annotation name: fluentd-sa-abc `) -} \ No newline at end of file +} diff --git a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go index 3e4f619c98..19f1c78d25 100644 --- a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go +++ b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/yaml" ) @@ -82,12 +83,23 @@ func (p *plugin) Transform(m resmap.ResMap) error { return err } for _, res := range resources { + internalAnnotations := kioutil.GetInternalAnnotations(&res.RNode) + err = res.ApplyFilter(patchjson6902.Filter{ Patch: p.JsonOp, }) if err != nil { return err } + + annotations := res.GetAnnotations() + for key, value := range internalAnnotations { + annotations[key] = value + } + err = res.SetAnnotations(annotations) + if err != nil { + return err + } } return nil } diff --git a/plugin/builtin/patchjson6902transformer/go.mod b/plugin/builtin/patchjson6902transformer/go.mod index 8d096afd83..9ca52c2d90 100644 --- a/plugin/builtin/patchjson6902transformer/go.mod +++ b/plugin/builtin/patchjson6902transformer/go.mod @@ -6,6 +6,7 @@ require ( github.com/evanphx/json-patch v4.11.0+incompatible github.com/pkg/errors v0.9.1 sigs.k8s.io/kustomize/api v0.8.9 + sigs.k8s.io/kustomize/kyaml v0.12.0 sigs.k8s.io/yaml v1.2.0 ) diff --git a/plugin/builtin/patchtransformer/PatchTransformer.go b/plugin/builtin/patchtransformer/PatchTransformer.go index 54fe9dedb7..2d02cf28f6 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer.go +++ b/plugin/builtin/patchtransformer/PatchTransformer.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/yaml" ) @@ -115,12 +116,19 @@ func (p *plugin) transformJson6902(m resmap.ResMap, patch jsonpatch.Patch) error } for _, res := range resources { res.StorePreviousId() + internalAnnotations := kioutil.GetInternalAnnotations(&res.RNode) err = res.ApplyFilter(patchjson6902.Filter{ Patch: p.Patch, }) if err != nil { return err } + + annotations := res.GetAnnotations() + for key, value := range internalAnnotations { + annotations[key] = value + } + err = res.SetAnnotations(annotations) } return nil } diff --git a/plugin/builtin/patchtransformer/go.mod b/plugin/builtin/patchtransformer/go.mod index 59744066fd..3a9b85e017 100644 --- a/plugin/builtin/patchtransformer/go.mod +++ b/plugin/builtin/patchtransformer/go.mod @@ -5,6 +5,7 @@ go 1.16 require ( github.com/evanphx/json-patch v4.11.0+incompatible sigs.k8s.io/kustomize/api v0.8.9 + sigs.k8s.io/kustomize/kyaml v0.12.0 sigs.k8s.io/yaml v1.2.0 )