Skip to content

Commit

Permalink
Merge pull request #279 from stefanprodan/test-nested-spec
Browse files Browse the repository at this point in the history
Fix `x-kubernetes-preserve-unknown-fields` path mismatch
  • Loading branch information
stefanprodan authored Dec 21, 2023
2 parents 6e82434 + c1d602d commit 054ea69
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 3 deletions.
20 changes: 17 additions & 3 deletions internal/engine/importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ func convertCRD(crd cue.Value) (*IntermediateCRD, error) {
// all the way to the file root
err = walkfn(defpath.Selectors(), rootosch)

// First pass of astutil.Apply to remove ellipses for fields not marked with x-kubernetes-embedded-resource: true
// First pass of astutil.Apply to remove ellipses for fields not marked with
// 'x-kubernetes-preserve-unknown-fields: true'.
// Note that this implementation is only correct for CUE inputs that do not contain references.
// It is safe to use in this context because CRDs already have that invariant.
var stack []ast.Node
Expand All @@ -278,7 +279,20 @@ func convertCRD(crd cue.Value) (*IntermediateCRD, error) {
}
stack = append(stack[:i], pc.Node())
pathstack = append(pathstack[:i], psel)
if !preserve[cue.MakePath(pathstack...).String()] {

// Risk not closing up fields that are not marked with 'x-kubernetes-preserve-unknown-fields: true'
// if the current field name matches a path that is marked with preserve unknown.
// TODO: find a way to fix parentPath when arrays are involved.
currentPath := cue.MakePath(pathstack...).String()
found := false
for k, ok := range preserve {
if strings.HasSuffix(k, currentPath) && ok {
found = true
break
}
}

if !found {
newlist := make([]ast.Decl, 0, len(x.Elts))
for _, elt := range x.Elts {
if _, is := elt.(*ast.Ellipsis); !is {
Expand Down Expand Up @@ -366,7 +380,7 @@ func convertCRD(crd cue.Value) (*IntermediateCRD, error) {
// - *ast.ListLit (index is the path)
// - *ast.Field (label is the path)
//
// If the there exceptions for the above two items, or the list should properly
// If there are exceptions for the above two items, or the list should properly
// have more items, this func will be buggy
func parentPath(c astutil.Cursor) (cue.Selector, astutil.Cursor) {
p, prior := c.Parent(), c
Expand Down
49 changes: 49 additions & 0 deletions internal/engine/importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,52 @@ func TestConvertCRD(t *testing.T) {
innerField?: string
}
...
}`,
},
{
name: "array-xk-preserve",
spec: `type: "object"
properties: {
resources: {
properties: claims: {
items: {
properties: name: type: "string"
required: ["name"]
type: "object"
}
type: "array"
"x-kubernetes-list-map-keys": ["name"]
"x-kubernetes-list-type": "map"
}
type: "object"
}
spec: {
properties: template: {
properties: values: {
description: "Preserve unknown fields."
type: "object"
"x-kubernetes-preserve-unknown-fields": true
}
type: "object"
}
type: "object"
}
}
`,
expect: `{
resources?: {
claims?: [...{
name: string
}]
}
spec?: {
template?: {
// Preserve unknown fields.
values?: {
...
}
}
}
}`,
},
}
Expand All @@ -314,6 +360,9 @@ func TestConvertCRD(t *testing.T) {
// remove the _#def injected by CUE's syntax formatter
fn, err := format.Node(specNode.(*ast.StructLit).Elts[1].(*ast.Field).Value)
g.Expect(err).ToNot(HaveOccurred())

t.Log(string(fn))

diff := cmp.Diff(tt.expect, string(fn), multiline)
if diff != "" {
t.Fatal(diff)
Expand Down

0 comments on commit 054ea69

Please sign in to comment.