Merge pull request #2307 from phanimarupaka/RearrangeComments

Rearrange comments for yaml/V3 bug fix

kyaml/kio/byteio_writer.go

@@ -45,6 +45,7 @@ type ByteWriter struct {
 var _ Writer = ByteWriter{}
 
 func (w ByteWriter) Write(nodes []*yaml.RNode) error {
+	yaml.DoSerializationHacksOnNodes(nodes)
 	if w.Sort {
 		if err := kioutil.SortNodes(nodes); err != nil {
 			return errors.Wrap(err)
@@ -117,5 +118,7 @@ func (w ByteWriter) Write(nodes []*yaml.RNode) error {
 	for i := range nodes {
 		items.Content = append(items.Content, nodes[i].YNode())
 	}
-	return errors.Wrap(encoder.Encode(doc))
+	err := errors.Wrap(encoder.Encode(doc))
+	yaml.UndoSerializationHacksOnNodes(nodes)
+	return err
 }

kyaml/kio/byteio_writer_test.go

@@ -75,8 +75,10 @@ func TestByteWriter_Write_withoutAnnotations(t *testing.T) {
 	node3, err := yaml.Parse(`e: f
 g:
   h:
-  - i # has a list
-  - j
+  # has a list
+  - i : [i1, i2] # line comment
+  # has a list 2
+  - j : j1
 `)
 	if !assert.NoError(t, err) {
 		return
@@ -93,8 +95,10 @@ g:
 e: f
 g:
   h:
-  - i # has a list
-  - j
+  # has a list
+  - i: [i1, i2] # line comment
+  # has a list 2
+  - j: j1
 ---
 a: b #first
 `, buff.String())

kyaml/kio/filters/fmtr_test.go

@@ -411,8 +411,8 @@ spec:
         app: nginx
     spec:
       containers:
-      - # this is another container
-        name: a-nginx
+      # this is another container
+      - name: a-nginx
         image: nginx:1.7.9
         ports:
         - containerPort: 80

kyaml/yaml/serialization.go

@@ -0,0 +1,76 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package yaml
+
+import "gopkg.in/yaml.v3"
+
+func DoSerializationHacksOnNodes(nodes []*RNode) {
+	for _, node := range nodes {
+		DoSerializationHacks(node.YNode())
+	}
+}
+
+// DoSerializationHacks addresses a bug in yaml V3 upstream, it parses the yaml node,
+// and rearranges the head comments of the children of sequence node.
+// Refer to https://github.com/go-yaml/yaml/issues/587 for more details
+func DoSerializationHacks(node *yaml.Node) {
+	switch node.Kind {
+	case DocumentNode:
+		for _, node := range node.Content {
+			DoSerializationHacks(node)
+		}
+
+	case MappingNode:
+		for _, node := range node.Content {
+			DoSerializationHacks(node)
+		}
+
+	case SequenceNode:
+		for _, node := range node.Content {
+			// for each child mapping node, transfer the head comment of it's
+			// first child scalar node to the head comment of itself
+			// This is necessary to address serialization issue
+			// https://github.com/go-yaml/yaml/issues/587 in go-yaml.v3
+			// Remove this hack when the issue has been resolved
+			if len(node.Content) > 0 && node.Content[0].Kind == ScalarNode {
+				node.HeadComment = node.Content[0].HeadComment
+				node.Content[0].HeadComment = ""
+			}
+		}
+	}
+}
+
+func UndoSerializationHacksOnNodes(nodes []*RNode) {
+	for _, node := range nodes {
+		UndoSerializationHacks(node.YNode())
+	}
+}
+
+// UndoSerializationHacks reverts the changes made by DoSerializationHacks
+// Refer to https://github.com/go-yaml/yaml/issues/587 for more details
+func UndoSerializationHacks(node *yaml.Node) {
+	switch node.Kind {
+	case DocumentNode:
+		for _, node := range node.Content {
+			DoSerializationHacks(node)
+		}
+
+	case MappingNode:
+		for _, node := range node.Content {
+			DoSerializationHacks(node)
+		}
+
+	case SequenceNode:
+		for _, node := range node.Content {
+			// revert the changes made in DoSerializationHacks
+			// This is necessary to address serialization issue
+			// https://github.com/go-yaml/yaml/issues/587 in go-yaml.v3
+			// Remove this hack when the issue has been resolved
+			if len(node.Content) > 0 && node.Content[0].Kind == ScalarNode {
+				node.Content[0].HeadComment = node.HeadComment
+				node.HeadComment = ""
+			}
+		}
+	}
+}

kyaml/yaml/types.go

@@ -496,6 +496,7 @@ func String(node *yaml.Node, opts ...string) (string, error) {
 	if node == nil {
 		return "", nil
 	}
+	DoSerializationHacks(node)
 	optsSet := sets.String{}
 	optsSet.Insert(opts...)
 	if optsSet.Has(Flow) {
@@ -514,6 +515,7 @@ func String(node *yaml.Node, opts ...string) (string, error) {
 	if optsSet.Has(Trim) {
 		val = strings.TrimSpace(val)
 	}
+	UndoSerializationHacks(node)
 	return val, errors.Wrap(err)
 }