Skip to content

Commit 054ea69

Browse files
authored
Merge pull request #279 from stefanprodan/test-nested-spec
Fix `x-kubernetes-preserve-unknown-fields` path mismatch
2 parents 6e82434 + c1d602d commit 054ea69

File tree

2 files changed

+66
-3
lines changed

2 files changed

+66
-3
lines changed

Diff for: internal/engine/importer.go

+17-3
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,8 @@ func convertCRD(crd cue.Value) (*IntermediateCRD, error) {
252252
// all the way to the file root
253253
err = walkfn(defpath.Selectors(), rootosch)
254254

255-
// First pass of astutil.Apply to remove ellipses for fields not marked with x-kubernetes-embedded-resource: true
255+
// First pass of astutil.Apply to remove ellipses for fields not marked with
256+
// 'x-kubernetes-preserve-unknown-fields: true'.
256257
// Note that this implementation is only correct for CUE inputs that do not contain references.
257258
// It is safe to use in this context because CRDs already have that invariant.
258259
var stack []ast.Node
@@ -278,7 +279,20 @@ func convertCRD(crd cue.Value) (*IntermediateCRD, error) {
278279
}
279280
stack = append(stack[:i], pc.Node())
280281
pathstack = append(pathstack[:i], psel)
281-
if !preserve[cue.MakePath(pathstack...).String()] {
282+
283+
// Risk not closing up fields that are not marked with 'x-kubernetes-preserve-unknown-fields: true'
284+
// if the current field name matches a path that is marked with preserve unknown.
285+
// TODO: find a way to fix parentPath when arrays are involved.
286+
currentPath := cue.MakePath(pathstack...).String()
287+
found := false
288+
for k, ok := range preserve {
289+
if strings.HasSuffix(k, currentPath) && ok {
290+
found = true
291+
break
292+
}
293+
}
294+
295+
if !found {
282296
newlist := make([]ast.Decl, 0, len(x.Elts))
283297
for _, elt := range x.Elts {
284298
if _, is := elt.(*ast.Ellipsis); !is {
@@ -366,7 +380,7 @@ func convertCRD(crd cue.Value) (*IntermediateCRD, error) {
366380
// - *ast.ListLit (index is the path)
367381
// - *ast.Field (label is the path)
368382
//
369-
// If the there exceptions for the above two items, or the list should properly
383+
// If there are exceptions for the above two items, or the list should properly
370384
// have more items, this func will be buggy
371385
func parentPath(c astutil.Cursor) (cue.Selector, astutil.Cursor) {
372386
p, prior := c.Parent(), c

Diff for: internal/engine/importer_test.go

+49
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,52 @@ func TestConvertCRD(t *testing.T) {
293293
innerField?: string
294294
}
295295
...
296+
}`,
297+
},
298+
{
299+
name: "array-xk-preserve",
300+
spec: `type: "object"
301+
properties: {
302+
resources: {
303+
properties: claims: {
304+
items: {
305+
properties: name: type: "string"
306+
required: ["name"]
307+
type: "object"
308+
}
309+
type: "array"
310+
"x-kubernetes-list-map-keys": ["name"]
311+
"x-kubernetes-list-type": "map"
312+
}
313+
type: "object"
314+
}
315+
spec: {
316+
properties: template: {
317+
properties: values: {
318+
description: "Preserve unknown fields."
319+
type: "object"
320+
"x-kubernetes-preserve-unknown-fields": true
321+
}
322+
type: "object"
323+
}
324+
type: "object"
325+
}
326+
}
327+
`,
328+
expect: `{
329+
resources?: {
330+
claims?: [...{
331+
name: string
332+
}]
333+
}
334+
spec?: {
335+
template?: {
336+
// Preserve unknown fields.
337+
values?: {
338+
...
339+
}
340+
}
341+
}
296342
}`,
297343
},
298344
}
@@ -314,6 +360,9 @@ func TestConvertCRD(t *testing.T) {
314360
// remove the _#def injected by CUE's syntax formatter
315361
fn, err := format.Node(specNode.(*ast.StructLit).Elts[1].(*ast.Field).Value)
316362
g.Expect(err).ToNot(HaveOccurred())
363+
364+
t.Log(string(fn))
365+
317366
diff := cmp.Diff(tt.expect, string(fn), multiline)
318367
if diff != "" {
319368
t.Fatal(diff)

0 commit comments

Comments
 (0)