handle comments in list correctly

kubernetes-sigs · Jun 4, 2021 · 6a9d8aa · 6a9d8aa
1 parent 5993eae
commit 6a9d8aa
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 39 deletions.
diff --git a/kyaml/comments/comments.go b/kyaml/comments/comments.go
@@ -54,7 +54,10 @@ func (c *copier) VisitList(s walk.Sources, _ *openapi.ResourceSchema, _ walk.Lis
 		origin := originItems[i]
 
 		if dest.Value == origin.Value {
-			copy(yaml.NewRNode(dest), yaml.NewRNode(origin))
+			// We should do it recursively on each node in the list.
+			if err := CopyComments(yaml.NewRNode(dest), yaml.NewRNode(origin)); err != nil {
+				return nil, err
+			}
 		}
 	}
 

diff --git a/kyaml/comments/comments_test.go b/kyaml/comments/comments_test.go
@@ -85,6 +85,48 @@ spec:
 `,
 		},
 
+		{
+			name: "associative_list_2",
+			from: `
+apiVersion: constraints.gatekeeper.sh/v1beta1
+kind: EnforceFoo
+metadata:
+  name: enforce-foo
+spec:
+  parameters:
+    naming_rules:
+    - kind: Bar
+      patterns:
+      # comment 1
+      - ^(dev|prod|staging|qa|shared)$
+`,
+			to: `
+apiVersion: constraints.gatekeeper.sh/v1beta1
+kind: EnforceFoo
+metadata:
+  name: enforce-foo
+spec:
+  parameters:
+    naming_rules:
+    - kind: Bar
+      patterns:
+      - ^(dev|prod|staging|qa|shared)$
+`,
+			expected: `
+apiVersion: constraints.gatekeeper.sh/v1beta1
+kind: EnforceFoo
+metadata:
+  name: enforce-foo
+spec:
+  parameters:
+    naming_rules:
+    - kind: Bar
+      patterns:
+      # comment 1
+      - ^(dev|prod|staging|qa|shared)$
+`,
+		},
+
 		{
 			name: "keep_comments",
 			from: `# A

diff --git a/kyaml/go.mod b/kyaml/go.mod
@@ -17,7 +17,7 @@ require (
 	github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca
 	go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5
 	gopkg.in/yaml.v2 v2.4.0
-	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
+	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
 	k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e
 )
 

diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go
@@ -51,6 +51,10 @@ type ByteWriter struct {
 var _ Writer = ByteWriter{}
 
 func (w ByteWriter) Write(nodes []*yaml.RNode) error {
+	var originalNodes []*yaml.RNode
+	for i := range nodes {
+		originalNodes = append(originalNodes, nodes[i].Copy())
+	}
 	yaml.DoSerializationHacksOnNodes(nodes)
 	if w.Sort {
 		if err := kioutil.SortNodes(nodes); err != nil {
@@ -131,7 +135,8 @@ func (w ByteWriter) Write(nodes []*yaml.RNode) error {
 		items.Content = append(items.Content, nodes[i].YNode())
 	}
 	err := encoder.Encode(doc)
-	yaml.UndoSerializationHacksOnNodes(nodes)
+
+	nodes = originalNodes
 	return err
 }
 
@@ -157,4 +162,4 @@ func (w ByteWriter) shouldJSONEncodeSingleBareNode(nodes []*yaml.RNode) bool {
 		}
 	}
 	return false
-}
+}
diff --git a/kyaml/yaml/serialization.go b/kyaml/yaml/serialization.go
@@ -34,43 +34,12 @@ func DoSerializationHacks(node *yaml.Node) {
 			// 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
+				// Don't clobber the head comment if it's not empty.
+				if node.HeadComment == "" && node.Content[0].HeadComment != "" {
+					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 = ""
-			}
-		}
-	}
-}