Skip to content

Commit 294070b

Browse files
author
Mengqi Yu
committed
address comments
1 parent cabbea0 commit 294070b

File tree

5 files changed

+135
-9
lines changed

5 files changed

+135
-9
lines changed

kyaml/comments/comments.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (c *copier) VisitList(s walk.Sources, _ *openapi.ResourceSchema, _ walk.Lis
5454
origin := originItems[i]
5555

5656
if dest.Value == origin.Value {
57-
// We should do it recursively on each node in the list.
57+
// We copy the comments recursively on each node in the list.
5858
if err := CopyComments(yaml.NewRNode(dest), yaml.NewRNode(origin)); err != nil {
5959
return nil, err
6060
}

kyaml/kio/byteio_writer.go

+10-6
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,7 @@ var _ Writer = ByteWriter{}
5252

5353
func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
5454
// Copy the nodes to prevent writer from mutating the original nodes.
55-
var nodes []*yaml.RNode
56-
for i := range inputNodes {
57-
nodes = append(nodes, inputNodes[i].Copy())
58-
}
55+
nodes := copyRNodes(inputNodes)
5956
yaml.DoSerializationHacksOnNodes(nodes)
6057
if w.Sort {
6158
if err := kioutil.SortNodes(nodes); err != nil {
@@ -135,8 +132,15 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
135132
for i := range nodes {
136133
items.Content = append(items.Content, nodes[i].YNode())
137134
}
138-
err := encoder.Encode(doc)
139-
return err
135+
return encoder.Encode(doc)
136+
}
137+
138+
func copyRNodes(in []*yaml.RNode) []*yaml.RNode {
139+
out := make([]*yaml.RNode, len(in))
140+
for i := range in {
141+
out[i] = in[i].Copy()
142+
}
143+
return out
140144
}
141145

142146
// shouldJSONEncodeSingleBareNode determines if nodes contain a single node that should not be

kyaml/kio/byteio_writer_test.go

+121
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,127 @@ a: b #first
9191
`,
9292
},
9393

94+
//
95+
// Test Case
96+
//
97+
{
98+
name: "handle_comments",
99+
items: []string{
100+
`# comment 0
101+
apiVersion: apps/v1
102+
kind: Deployment
103+
metadata:
104+
name: my-nginx
105+
namespace: my-space
106+
labels:
107+
env: dev
108+
foo: bar
109+
spec:
110+
# comment 1
111+
replicas: 3
112+
selector:
113+
# comment 2
114+
matchLabels: # comment 3
115+
# comment 4
116+
app: nginx # comment 5
117+
template:
118+
metadata:
119+
labels:
120+
app: nginx
121+
spec:
122+
# comment 6
123+
containers:
124+
# comment 7
125+
- name: nginx
126+
image: nginx:1.14.2 # comment 8
127+
ports:
128+
# comment 9
129+
- containerPort: 80 # comment 10
130+
`,
131+
`apiVersion: v1
132+
kind: Service
133+
metadata:
134+
name: my-service
135+
spec:
136+
ports:
137+
# comment 1
138+
- name: etcd-server-ssl
139+
port: 2380
140+
# comment 2
141+
- name: etcd-client-ssl
142+
port: 2379
143+
`,
144+
`apiVersion: constraints.gatekeeper.sh/v1beta1
145+
kind: EnforceFoo
146+
metadata:
147+
name: enforce-foo
148+
spec:
149+
parameters:
150+
naming_rules:
151+
- kind: Folder
152+
patterns:
153+
# comment 1
154+
- ^(dev|prod|staging|qa|shared)$
155+
`,
156+
},
157+
expectedOutput: `# comment 0
158+
apiVersion: apps/v1
159+
kind: Deployment
160+
metadata:
161+
name: my-nginx
162+
namespace: my-space
163+
labels:
164+
env: dev
165+
foo: bar
166+
spec:
167+
# comment 1
168+
replicas: 3
169+
selector:
170+
# comment 2
171+
matchLabels: # comment 3
172+
# comment 4
173+
app: nginx # comment 5
174+
template:
175+
metadata:
176+
labels:
177+
app: nginx
178+
spec:
179+
# comment 6
180+
containers:
181+
# comment 7
182+
- name: nginx
183+
image: nginx:1.14.2 # comment 8
184+
ports:
185+
# comment 9
186+
- containerPort: 80 # comment 10
187+
---
188+
apiVersion: v1
189+
kind: Service
190+
metadata:
191+
name: my-service
192+
spec:
193+
ports:
194+
# comment 1
195+
- name: etcd-server-ssl
196+
port: 2380
197+
# comment 2
198+
- name: etcd-client-ssl
199+
port: 2379
200+
---
201+
apiVersion: constraints.gatekeeper.sh/v1beta1
202+
kind: EnforceFoo
203+
metadata:
204+
name: enforce-foo
205+
spec:
206+
parameters:
207+
naming_rules:
208+
- kind: Folder
209+
patterns:
210+
# comment 1
211+
- ^(dev|prod|staging|qa|shared)$
212+
`,
213+
},
214+
94215
//
95216
// Test Case
96217
//

kyaml/kio/filters/fmtr_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -420,8 +420,8 @@ spec:
420420
image: nginx:1.7.9
421421
# this is a container
422422
ports:
423-
- # this is a port
424-
containerPort: 80
423+
# this is a port
424+
- containerPort: 80
425425
`
426426
s, err := FormatInput(strings.NewReader(y))
427427
assert.NoError(t, err)

kyaml/yaml/serialization.go

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ func DoSerializationHacks(node *yaml.Node) {
4040
}
4141
node.Content[0].HeadComment = ""
4242
}
43+
DoSerializationHacks(node)
4344
}
4445
}
4546
}

0 commit comments

Comments
 (0)