Merge pull request #3965 from mengqiy/listcomments

handle comments in list correctly

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 copy the comments recursively on each node in the list.
+			if err := CopyComments(yaml.NewRNode(dest), yaml.NewRNode(origin)); err != nil {
+				return nil, err
+			}
 		}
 	}
 

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

kyaml/kio/byteio_writer.go

@@ -50,7 +50,9 @@ type ByteWriter struct {
 
 var _ Writer = ByteWriter{}
 
-func (w ByteWriter) Write(nodes []*yaml.RNode) error {
+func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
+	// Copy the nodes to prevent writer from mutating the original nodes.
+	nodes := copyRNodes(inputNodes)
 	yaml.DoSerializationHacksOnNodes(nodes)
 	if w.Sort {
 		if err := kioutil.SortNodes(nodes); err != nil {
@@ -130,9 +132,15 @@ func (w ByteWriter) Write(nodes []*yaml.RNode) error {
 	for i := range nodes {
 		items.Content = append(items.Content, nodes[i].YNode())
 	}
-	err := encoder.Encode(doc)
+	return encoder.Encode(doc)
-	yaml.UndoSerializationHacksOnNodes(nodes)
+}
-	return err
+
+func copyRNodes(in []*yaml.RNode) []*yaml.RNode {
+	out := make([]*yaml.RNode, len(in))
+	for i := range in {
+		out[i] = in[i].Copy()
+	}
+	return out
 }
 
 // shouldJSONEncodeSingleBareNode determines if nodes contain a single node that should not be

kyaml/kio/byteio_writer_test.go

@@ -91,6 +91,127 @@ a: b #first
 `,
 		},
 
+		//
+		// Test Case
+		//
+		{
+			name: "handle_comments",
+			items: []string{
+				`# comment 0
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: my-nginx
+  namespace: my-space
+  labels:
+    env: dev
+    foo: bar
+spec:
+  # comment 1
+  replicas: 3
+  selector:
+    # comment 2
+    matchLabels: # comment 3
+      # comment 4
+      app: nginx # comment 5
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      # comment 6
+      containers:
+        # comment 7
+        - name: nginx
+          image: nginx:1.14.2 # comment 8
+          ports:
+            # comment 9
+            - containerPort: 80 # comment 10
+`,
+				`apiVersion: v1
+kind: Service
+metadata:
+  name: my-service
+spec:
+  ports:
+    # comment 1
+    - name: etcd-server-ssl
+      port: 2380
+    # comment 2
+    - name: etcd-client-ssl
+      port: 2379
+`,
+				`apiVersion: constraints.gatekeeper.sh/v1beta1
+kind: EnforceFoo
+metadata:
+  name: enforce-foo
+spec:
+  parameters:
+    naming_rules:
+      - kind: Folder
+        patterns:
+          # comment 1
+          - ^(dev|prod|staging|qa|shared)$
+`,
+			},
+			expectedOutput: `# comment 0
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: my-nginx
+  namespace: my-space
+  labels:
+    env: dev
+    foo: bar
+spec:
+  # comment 1
+  replicas: 3
+  selector:
+    # comment 2
+    matchLabels: # comment 3
+      # comment 4
+      app: nginx # comment 5
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      # comment 6
+      containers:
+      # comment 7
+      - name: nginx
+        image: nginx:1.14.2 # comment 8
+        ports:
+        # comment 9
+        - containerPort: 80 # comment 10
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: my-service
+spec:
+  ports:
+  # comment 1
+  - name: etcd-server-ssl
+    port: 2380
+  # comment 2
+  - name: etcd-client-ssl
+    port: 2379
+---
+apiVersion: constraints.gatekeeper.sh/v1beta1
+kind: EnforceFoo
+metadata:
+  name: enforce-foo
+spec:
+  parameters:
+    naming_rules:
+    - kind: Folder
+      patterns:
+      # comment 1
+      - ^(dev|prod|staging|qa|shared)$
+`,
+		},
+
 		//
 		// Test Case
 		//

kyaml/kio/filters/fmtr_test.go

@@ -420,8 +420,8 @@ spec:
         image: nginx:1.7.9
         # this is a container
         ports:
-        - # this is a port
+        # this is a port
-          containerPort: 80
+        - containerPort: 80
 `
 	s, err := FormatInput(strings.NewReader(y))
 	assert.NoError(t, err)

kyaml/yaml/serialization.go

@@ -34,43 +34,13 @@ 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 {
+				// 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 = ""
-			}
-		}
 	}
 }