diff --git a/kyaml/comments/comments.go b/kyaml/comments/comments.go index 95b81dda4..127391525 100644 --- a/kyaml/comments/comments.go +++ b/kyaml/comments/comments.go @@ -37,6 +37,18 @@ func (c *copier) VisitScalar(s walk.Sources, _ *openapi.ResourceSchema) (*yaml.R func (c *copier) VisitList(s walk.Sources, _ *openapi.ResourceSchema, _ walk.ListKind) ( *yaml.RNode, error) { copy(s.Dest(), s.Origin()) + destItems := s.Dest().Content() + originItems := s.Origin().Content() + + for i := 0; i < len(destItems) && i < len(originItems); i++ { + dest := destItems[i] + origin := originItems[i] + + if dest.Value == origin.Value { + copy(yaml.NewRNode(dest), yaml.NewRNode(origin)) + } + } + return s.Dest(), nil } @@ -45,13 +57,13 @@ func copy(from, to *yaml.RNode) { if from == nil || to == nil { return } - if from.Document().LineComment != "" { + if to.Document().LineComment == "" { to.Document().LineComment = from.Document().LineComment } - if from.Document().HeadComment != "" { + if to.Document().HeadComment == "" { to.Document().HeadComment = from.Document().HeadComment } - if from.Document().FootComment != "" { + if to.Document().FootComment == "" { to.Document().FootComment = from.Document().FootComment } } diff --git a/kyaml/comments/comments_test.go b/kyaml/comments/comments_test.go index 8a706bafe..35674b2bc 100644 --- a/kyaml/comments/comments_test.go +++ b/kyaml/comments/comments_test.go @@ -4,6 +4,7 @@ package comments import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -11,7 +12,15 @@ import ( ) func TestCopyComments(t *testing.T) { - from, err := yaml.Parse(`# A + testCases := []struct { + name string + from string + to string + expected string + }{ + { + name: "copy_comments", + from: `# A # # B @@ -22,31 +31,13 @@ spec: # comment 1 # comment 2 replicas: 3 # comment 3 # comment 4 -`) - if !assert.NoError(t, err) { - t.FailNow() - } - - to, err := yaml.Parse(`apiVersion: apps/v1 +`, + to: `apiVersion: apps/v1 kind: Deployment spec: replicas: 4 -`) - if !assert.NoError(t, err) { - t.FailNow() - } - - err = CopyComments(from, to) - if !assert.NoError(t, err) { - t.FailNow() - } - - actual, err := to.String() - if !assert.NoError(t, err) { - t.FailNow() - } - - expected := `# A +`, + expected: `# A # # B @@ -57,9 +48,274 @@ spec: # comment 1 # comment 2 replicas: 4 # comment 3 # comment 4 -` +`, + }, - if !assert.Equal(t, expected, actual) { - t.FailNow() + { + name: "associative_list", + from: ` +apiVersion: apps/v1 +kind: Deployment +spec: + template: + spec: + containers: + - name: foo + image: bar # comment 1 +`, + to: ` +apiVersion: apps/v1 +kind: Deployment +spec: + template: + spec: + containers: + - name: foo + image: bar +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +spec: + template: + spec: + containers: + - name: foo + image: bar # comment 1 +`, + }, + + { + name: "keep_comments", + from: `# A +# +# B + +# C +apiVersion: apps/v1 +kind: Deployment +spec: # comment 1 + # comment 2 + replicas: 3 # comment 3 + # comment 4 +`, + to: `apiVersion: apps/v1 +kind: Deployment +spec: + replicas: 4 # comment 5 +`, + expected: `# A +# +# B + +# C +apiVersion: apps/v1 +kind: Deployment +spec: # comment 1 + # comment 2 + replicas: 4 # comment 5 + # comment 4 +`, + }, + + { + name: "copy_item_comments", + from: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a # comment +`, + to: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a # comment +`, + }, + + { + name: "copy_item_comments_2", + from: ` +apiVersion: apps/v1 +kind: Deployment +items: +# comment +- a +`, + to: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +items: +# comment +- a +`, + }, + + { + name: "copy_item_comments_middle", + from: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a +- b # comment +- c +`, + to: ` +apiVersion: apps/v1 +kind: Deployment +items: +- d +- b +- e +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +items: +- d +- b # comment +- e +`, + }, + + { + name: "copy_item_comments_moved", + from: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a +- b # comment +- c +`, + to: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a +- c +- b +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a +- c +- b +`, + }, + + { + name: "copy_item_comments_no_match", + from: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a # comment +`, + to: ` +apiVersion: apps/v1 +kind: Deployment +items: +- b +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +items: +- b +`, + }, + + { + name: "copy_item_comments_add", + from: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a # comment +`, + to: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a +- b +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a # comment +- b +`, + }, + + { + name: "copy_item_comments_remove", + from: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a # comment +- b +`, + to: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +items: +- a # comment +`, + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + from, err := yaml.Parse(tc.from) + if !assert.NoError(t, err) { + t.FailNow() + } + + to, err := yaml.Parse(tc.to) + if !assert.NoError(t, err) { + t.FailNow() + } + + err = CopyComments(from, to) + if !assert.NoError(t, err) { + t.FailNow() + } + + actual, err := to.String() + if !assert.NoError(t, err) { + t.FailNow() + } + + if !assert.Equal(t, strings.TrimSpace(tc.expected), strings.TrimSpace(actual)) { + t.FailNow() + } + }) } } diff --git a/kyaml/fn/runtime/runtimeutil/runtimeutil.go b/kyaml/fn/runtime/runtimeutil/runtimeutil.go index d2321bf01..29e2ea0ad 100644 --- a/kyaml/fn/runtime/runtimeutil/runtimeutil.go +++ b/kyaml/fn/runtime/runtimeutil/runtimeutil.go @@ -5,11 +5,13 @@ package runtimeutil import ( "bytes" + "fmt" "io" "io/ioutil" "path" "strings" + "sigs.k8s.io/kustomize/kyaml/comments" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" @@ -44,6 +46,8 @@ type FunctionFilter struct { // exit saves the error returned from Run exit error + + ids map[string]*yaml.RNode } // GetExit returns the error from Run @@ -138,6 +142,11 @@ func (c *FunctionFilter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { return nil, err } + // set ids on each input so it is possible to copy comments from inputs back to outputs + if err := c.setIds(input); err != nil { + return nil, err + } + // write the input err = kio.ByteWriter{ WrappingAPIVersion: kio.ResourceListAPIVersion, @@ -160,6 +169,11 @@ func (c *FunctionFilter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { return nil, err } + // copy the comments from the inputs to the outputs + if err := c.setComments(output); err != nil { + return nil, err + } + if err := c.doResults(r); err != nil { return nil, err } @@ -178,6 +192,50 @@ func (c *FunctionFilter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { return append(output, saved...), nil } +const idAnnotation = "config.k8s.io/id" + +func (c *FunctionFilter) setIds(nodes []*yaml.RNode) error { + // set the id on each node to map inputs to outputs + var id int + c.ids = map[string]*yaml.RNode{} + for i := range nodes { + id++ + idStr := fmt.Sprintf("%v", id) + err := nodes[i].PipeE(yaml.SetAnnotation(idAnnotation, idStr)) + if err != nil { + return errors.Wrap(err) + } + c.ids[idStr] = nodes[i] + } + return nil +} + +func (c *FunctionFilter) setComments(nodes []*yaml.RNode) error { + for i := range nodes { + node := nodes[i] + anID, err := node.Pipe(yaml.GetAnnotation(idAnnotation)) + if err != nil { + return errors.Wrap(err) + } + if anID == nil { + continue + } + + var in *yaml.RNode + var found bool + if in, found = c.ids[anID.YNode().Value]; !found { + continue + } + if err := comments.CopyComments(in, node); err != nil { + return errors.Wrap(err) + } + if err := node.PipeE(yaml.ClearAnnotation(idAnnotation)); err != nil { + return errors.Wrap(err) + } + } + return nil +} + func (c *FunctionFilter) doResults(r *kio.ByteReader) error { // Write the results to a file if configured to do so if c.ResultsFile != "" && r.Results != nil { diff --git a/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go b/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go index 883264d89..cc474b4d7 100644 --- a/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go +++ b/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go @@ -486,6 +486,7 @@ items: name: service-foo annotations: config.kubernetes.io/path: 'foo/bar/s.yaml' + config.k8s.io/id: '1' functionConfig: apiVersion: example.com/v1 kind: Example @@ -504,6 +505,7 @@ items: annotations: config.kubernetes.io/path: 'foo/bar/s.yaml' new: annotation + config.k8s.io/id: '1' functionConfig: apiVersion: example.com/v1 kind: Example @@ -573,6 +575,7 @@ items: name: service-foo annotations: config.kubernetes.io/path: 'foo/bar/s.yaml' + config.k8s.io/id: '1' functionConfig: apiVersion: example.com/v1 kind: Example @@ -591,6 +594,7 @@ items: annotations: config.kubernetes.io/path: 'foo/bar/s.yaml' new: annotation + config.k8s.io/id: '1' functionConfig: apiVersion: example.com/v1 kind: Example @@ -657,12 +661,14 @@ items: name: deployment-foo annotations: config.kubernetes.io/path: 'baz/bar/d.yaml' + config.k8s.io/id: '1' - apiVersion: v1 kind: Service metadata: name: service-foo annotations: config.kubernetes.io/path: 'foo/bar/s.yaml' + config.k8s.io/id: '2' functionConfig: apiVersion: example.com/v1 kind: Example @@ -680,6 +686,7 @@ items: name: deployment-foo annotations: config.kubernetes.io/path: 'baz/bar/d.yaml' + config.k8s.io/id: '1' - apiVersion: v1 kind: Service metadata: @@ -687,6 +694,7 @@ items: annotations: config.kubernetes.io/path: 'foo/bar/s.yaml' new: annotation + config.k8s.io/id: '2' functionConfig: apiVersion: example.com/v1 kind: Example @@ -823,6 +831,7 @@ items: name: service-foo annotations: config.kubernetes.io/path: 'foo/bar/s.yaml' + config.k8s.io/id: '1' functionConfig: apiVersion: example.com/v1 kind: Example @@ -840,6 +849,7 @@ items: name: service-foo annotations: config.kubernetes.io/path: 'foo/bar/s.yaml' + config.k8s.io/id: '1' new: annotation functionConfig: apiVersion: example.com/v1 @@ -893,6 +903,107 @@ metadata: name: deployment-foo annotations: config.kubernetes.io/path: 'baz/bar/d.yaml' +`, + }, + }, + + { + name: "copy_comments", + run: testRun{ + expectedInput: `apiVersion: config.kubernetes.io/v1alpha1 +kind: ResourceList +items: +- apiVersion: apps/v1 + kind: Deployment + metadata: + name: deployment-foo + annotations: + config.kubernetes.io/path: 'foo/b.yaml' + config.k8s.io/id: '1' +- apiVersion: v1 + kind: Service + metadata: + name: service-foo # name comment + annotations: + config.kubernetes.io/path: 'foo/a.yaml' + config.k8s.io/id: '2' +functionConfig: + apiVersion: example.com/v1 + kind: Example + metadata: + name: foo + annotations: + config.kubernetes.io/path: 'foo/f.yaml' +`, + // delete the comment + output: `apiVersion: config.kubernetes.io/v1alpha1 +kind: ResourceList +items: +- apiVersion: apps/v1 + kind: Deployment + metadata: + name: deployment-foo + annotations: + config.kubernetes.io/path: 'foo/b.yaml' + config.k8s.io/id: '1' +- apiVersion: v1 + kind: Service + metadata: + name: service-foo + annotations: + config.kubernetes.io/path: 'foo/a.yaml' + config.k8s.io/id: '2' + new: annotation +functionConfig: + apiVersion: example.com/v1 + kind: Example + metadata: + name: foo + annotations: + config.kubernetes.io/path: 'foo/f.yaml' +`, + }, + functionConfig: ` +apiVersion: example.com/v1 +kind: Example +metadata: + name: foo + annotations: + config.kubernetes.io/path: 'foo/f.yaml' +`, + input: []string{ + ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: deployment-foo + annotations: + config.kubernetes.io/path: 'foo/b.yaml' +`, + ` +apiVersion: v1 +kind: Service +metadata: + name: service-foo # name comment + annotations: + config.kubernetes.io/path: 'foo/a.yaml' +`}, + expectedOutput: []string{ + ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: deployment-foo + annotations: + config.kubernetes.io/path: 'foo/b.yaml' +`, ` +apiVersion: v1 +kind: Service +metadata: + name: service-foo # name comment + annotations: + config.kubernetes.io/path: 'foo/a.yaml' + new: annotation `, }, }, diff --git a/kyaml/fn/runtime/starlark/starlark.go b/kyaml/fn/runtime/starlark/starlark.go index 1d2bd37e2..de251007a 100644 --- a/kyaml/fn/runtime/starlark/starlark.go +++ b/kyaml/fn/runtime/starlark/starlark.go @@ -12,7 +12,6 @@ import ( "github.com/qri-io/starlib/util" "go.starlark.net/starlark" - "sigs.k8s.io/kustomize/kyaml/comments" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil" "sigs.k8s.io/kustomize/kyaml/kio/filters" @@ -33,8 +32,6 @@ type Filter struct { Path string runtimeutil.FunctionFilter - - ids map[string]*yaml.RNode } func (sf *Filter) String() string { @@ -128,23 +125,6 @@ func (sf *Filter) readResourceList(reader io.Reader) (starlark.Value, error) { return nil, errors.Wrap(err) } - // set the id on each node to map inputs to outputs - var id int - sf.ids = map[string]*yaml.RNode{} - items, err := rn.Pipe(yaml.Lookup("items")) - if err != nil { - return nil, errors.Wrap(err) - } - err = items.VisitElements(func(node *yaml.RNode) error { - id++ - idStr := fmt.Sprintf("%v", id) - sf.ids[idStr] = node - return node.PipeE(yaml.SetAnnotation("config.k8s.io/id", idStr)) - }) - if err != nil { - return nil, errors.Wrap(err) - } - // convert to a starlark value b, err := yaml.Marshal(rn.Document()) // convert to bytes if err != nil { @@ -182,33 +162,10 @@ func (sf *Filter) writeResourceList(value starlark.Value, writer io.Writer) erro return errors.Wrap(err) } err = items.VisitElements(func(node *yaml.RNode) error { - anID, err := node.Pipe(yaml.GetAnnotation("config.k8s.io/id")) - if err != nil { - return errors.Wrap(err) - } - if anID == nil { - return nil - } - - var in *yaml.RNode - var found bool - if in, found = sf.ids[anID.YNode().Value]; !found { - return nil - } - if err := node.PipeE(yaml.ClearAnnotation("config.k8s.io/id")); err != nil { - return errors.Wrap(err) - } - if err := comments.CopyComments(in, node); err != nil { - return errors.Wrap(err) - } - // starlark will serialize the resources sorting the fields alphabetically, // format them to have a better ordering - fmtFltr := filters.FormatFilter{} - if _, err := fmtFltr.Filter([]*yaml.RNode{node}); err != nil { - return errors.Wrap(err) - } - return nil + _, err := filters.FormatFilter{}.Filter([]*yaml.RNode{node}) + return err }) if err != nil { return errors.Wrap(err)