Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading from v0.16.5 -> v0.17.0 causes controller-gen crd generator to panic #1123

Closed
kate-osborn opened this issue Jan 8, 2025 · 15 comments

Comments

@kate-osborn
Copy link

We are getting the following panic after upgrading from 0.16.5 to 0.17.0 when running the crd generator:

go run sigs.k8s.io/controller-tools/cmd/[email protected] crd object paths=./apis/... output:crd:artifacts:config=config/crd/bases
go: downloading sigs.k8s.io/controller-tools v0.17.0
panic: interface conversion: *types.Struct is not interface { Obj() *types.TypeName }: missing method Obj

goroutine 1 [running]:
sigs.k8s.io/controller-tools/pkg/crd.localNamedToSchema(0xc000fd3aa0, 0xc0005f26e0)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:299 +0x3eb
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0xc000fd3aa0, {0xec6900, 0xc0005f26e0})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:220 +0xd9
sigs.k8s.io/controller-tools/pkg/crd.structToSchema(0xc000a5d568, 0xc000714120)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:477 +0x872
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0xc000a5d568, {0xec68d0, 0xc000714120})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:230 +0x93
sigs.k8s.io/controller-tools/pkg/crd.infoToSchema(0xc000a5d568)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:139 +0x165
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedSchemaFor(0xc000[55](https://github.com/nginx/nginx-gateway-fabric/actions/runs/12660268000/job/35281123021?pr=2962#step:8:56)e0c0, {0xc0004070a0, {0xc000410810, 0x14}})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/parser.go:193 +0x290
sigs.k8s.io/controller-tools/pkg/crd.(*schemaContext).requestSchema(0xc00030cf30?, {0x0?, 0xd73825?}, {0xc000410810?, 0x0?})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:108 +0xdb
sigs.k8s.io/controller-tools/pkg/crd.localNamedToSchema(0xc000a5d9a0, 0xc000706320)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:305 +0x4ae
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0xc000a5d9a0, {0xec6900, 0xc000706320})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:220 +0xd9
sigs.k8s.io/controller-tools/pkg/crd.arrayToSchema(0xc000fd3a10, 0xc0007112c0)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:343 +0x10a
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0xc000fd3a10, {0xec6960, 0xc0007112c0})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:224 +0x51
sigs.k8s.io/controller-tools/pkg/crd.structToSchema(0xc000a5de[58](https://github.com/nginx/nginx-gateway-fabric/actions/runs/12660268000/job/35281123021?pr=2962#step:8:59), 0xc000714480)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:477 +0x872
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0xc000a5de58, {0xec68d0, 0xc000714480})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:230 +0x93
sigs.k8s.io/controller-tools/pkg/crd.infoToSchema(0xc000a5de58)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:139 +0x165
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedSchemaFor(0xc00055e0c0, {0xc0004070a0, {0xc0003[60](https://github.com/nginx/nginx-gateway-fabric/actions/runs/12660268000/job/35281123021?pr=2962#step:8:61)8f0, 0xc}})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/parser.go:193 +0x290
sigs.k8s.io/controller-tools/pkg/crd.(*schemaContext).requestSchema(0xc000fd3950?, {0xc00030cf30?, 0xd73825?}, {0xc0003608f0?, 0xc?})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:108 +0xdb
sigs.k8s.io/controller-tools/pkg/crd.namedToSchema(0xc000fd3950, 0xc000012858)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:322 +0x234
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0xc000fd3950, {0xec6930, 0xc000012858})
/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:222 +0xf6
sigs.k8s.io/controller-tools/pkg/crd.structToSchema(0xc000a5e650, 0xc000012870)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:477 +0x872
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0xc000a5e650, {0xec68d0, 0xc000012870})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:230 +0x[93](https://github.com/nginx/nginx-gateway-fabric/actions/runs/12660268000/job/35281123021?pr=2962#step:8:94)
sigs.k8s.io/controller-tools/pkg/crd.infoToSchema(0xc000a5e650)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:139 +0x165
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedSchemaFor(0xc00055e0c0, {0xc000407080, {0xc0004f2018, 0x14}})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/parser.go:193 +0x290
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedFlattenedSchemaFor(0xc00055e0c0, {0xc000407080, {0xc0004f2018, 0x14}})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/parser.go:205 +0xcd
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedCRDFor(0xc00055e0c0, {{0xc00003e3ce, 0x11}, {0xc0004f2018, 0x14}}, 0x0)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/spec.go:93 +0x57a
sigs.k8s.io/controller-tools/pkg/crd.Generator.Generate({0x0, 0x0, 0x0, {0x0, 0x0, 0x0}, 0x0, {0x0, 0x0}, {0x0, ...}, ...}, ...)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/gen.go:182 +0x566
sigs.k8s.io/controller-tools/pkg/genall.(*Runtime).Run(0xc0001823f0)
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/genall/genall.go:272 +0x234
main.main.func1(0xc0001f4200?, {0xc00024e280?, 0x4?, 0xd7193d?})
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/cmd/controller-gen/main.go:176 +0x6a
github.com/spf13/cobra.(*Command).execute(0xc000204c08, {0xc000036060, 0x4, 0x4})
	/home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:[98](https://github.com/nginx/nginx-gateway-fabric/actions/runs/12660268000/job/35281123021?pr=2962#step:8:99)5 +0xaaa
github.com/spf13/cobra.(*Command).ExecuteC(0xc000204c08)
	/home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
	/home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:1041
main.main()
	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/cmd/controller-gen/main.go:200 +0x2f6
exit status 2

Link to failed workflow: https://github.com/nginx/nginx-gateway-fabric/actions/runs/12660268000/job/35281123021?pr=2962

We are running the following command:

go run sigs.k8s.io/controller-tools/cmd/[email protected] crd object paths=./apis/... output:crd:artifacts:config=config/crd/bases
@camilamacedo86
Copy link
Member

What is your go-version?

@abhay-krishna
Copy link

@camilamacedo86 I am running into the same issue and my Go version is v1.23.4.

@camilamacedo86
Copy link
Member

Hi @abhay-krishna

I can see

panic: interface conversion: *types.Struct is not interface { Obj() *types.TypeName }: missing method Obj

So, could you give an example of how we can reproduce the scenario?
It seems related to some specific definition of your APIs

PS. We updated the version of controller-gen in Kubebuilder, and we did not find any issues in the tests and samples, which leads us to think that it is related to a specific use case bug or problems with your solution itself.

Therefore, having the steps to reproduce the scenario would be nice.

@kate-osborn
Copy link
Author

@camilamacedo86 our go version is 1.23.0. We are running controller-gen on the types in this folder: https://github.com/nginx/nginx-gateway-fabric/tree/main/apis/v1alpha1

@abhay-krishna
Copy link

abhay-krishna commented Jan 9, 2025

@camilamacedo86 We hit this issue when running the generate-manifests Make target, aws/eks-anywhere or by directly running go run with version controller-gen v0.17.0. Our types are stored here and we're passing this in the paths argument.

$ go version                                                                             
go version go1.23.4 darwin/arm64

$ go run sigs.k8s.io/controller-tools/cmd/[email protected] paths=./pkg/api/... \ 
    paths=./controllers/... \
    paths=./manager/... \
    crd:crdVersions=v1 \
    rbac:roleName=manager-role \
    output:crd:dir=./config/crd/bases \
    output:webhook:dir=./config/webhook \
    webhook

panic: interface conversion: *types.Struct is not interface { Obj() *types.TypeName }: missing method Obj

goroutine 1 [running]:
sigs.k8s.io/controller-tools/pkg/crd.localNamedToSchema(0x14005c4d8f8, 0x1400145aa00)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:299 +0x37c
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0x14005c4d8f8, {0x1013c9558, 0x1400145aa00})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:220 +0xd0
sigs.k8s.io/controller-tools/pkg/crd.arrayToSchema(0x14005150bd0, 0x14001451d40)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:343 +0x11c
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0x14005150bd0, {0x1013c95b8, 0x14001451d40})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:224 +0x58
sigs.k8s.io/controller-tools/pkg/crd.structToSchema(0x14005c4ddb8, 0x14001458a20)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:477 +0x7d8
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0x14005c4ddb8, {0x1013c9528, 0x14001458a20})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:230 +0x90
sigs.k8s.io/controller-tools/pkg/crd.infoToSchema(0x14005c4ddb8)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:139 +0x150
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedSchemaFor(0x14000102900, {0x140009923c0, {0x1400037f6d0, 0xd}})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/parser.go:193 +0x1e8
sigs.k8s.io/controller-tools/pkg/crd.(*schemaContext).requestSchema(0x140004ec060?, {0x0?, 0x1010f1524?}, {0x1400037f6d0?, 0x0?})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:108 +0xd8
sigs.k8s.io/controller-tools/pkg/crd.localNamedToSchema(0x14005150900, 0x14001689280)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:305 +0x42c
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0x14005150900, {0x1013c9558, 0x14001689280})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:220 +0xd0
sigs.k8s.io/controller-tools/pkg/crd.structToSchema(0x14005c4e5f8, 0x14001692300)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:477 +0x7d8
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0x14005c4e5f8, {0x1013c9528, 0x14001692300})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:230 +0x90
sigs.k8s.io/controller-tools/pkg/crd.infoToSchema(0x14005c4e5f8)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/schema.go:139 +0x150
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedSchemaFor(0x14000102900, {0x140009923c0, {0x140006d5dd0, 0x7}})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/parser.go:193 +0x1e8
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedFlattenedSchemaFor(0x14000102900, {0x140009923c0, {0x140006d5dd0, 0x7}})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/parser.go:205 +0x9c
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedCRDFor(0x14000102900, {{0x14000f5e1be, 0x1a}, {0x140006d5dd0, 0x7}}, 0x0)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/spec.go:93 +0x3d8
sigs.k8s.io/controller-tools/pkg/crd.Generator.Generate({0x0, 0x0, 0x0, {0x14000190050, 0x1, 0x1}, 0x0, {0x0, 0x0}, {0x0, ...}, ...}, ...)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/crd/gen.go:182 +0x464
sigs.k8s.io/controller-tools/pkg/genall.(*Runtime).Run(0x140011a9320)
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/genall/genall.go:272 +0x21c
main.main.func1(0x140002d8200?, {0x140000b3380?, 0x4?, 0x1010ef714?})
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/cmd/controller-gen/main.go:176 +0x64
github.com/spf13/cobra.(*Command).execute(0x14000032f08, {0x140001b6010, 0x8, 0x8})
        /Users/arnchlm/go/pkg/mod/github.com/spf13/[email protected]/command.go:985 +0x834
github.com/spf13/cobra.(*Command).ExecuteC(0x14000032f08)
        /Users/arnchlm/go/pkg/mod/github.com/spf13/[email protected]/command.go:1117 +0x344
github.com/spf13/cobra.(*Command).Execute(...)
        /Users/arnchlm/go/pkg/mod/github.com/spf13/[email protected]/command.go:1041
main.main()
        /Users/arnchlm/go/pkg/mod/sigs.k8s.io/[email protected]/cmd/controller-gen/main.go:200 +0x290
exit status 2

@sbueringer
Copy link
Member

@kate-osborn @abhay-krishna This seems to be either the same or similar as #1088 which was fixed just now in #1122.

Can you please check if controller-gen including #1122 works for you?

@abhay-krishna
Copy link

@sbueringer Yes indeed, including that commit worked! 🚀 Thank you very much! Will wait for it to be included in a new release to avoid the pseudo-versioning.

controller-gen.kubebuilder.io/version: v0.17.1-0.20250106111415-a7ed77794ca8

@sbueringer
Copy link
Member

Perfect, thx for the quick feedback! I'll wait a bit so @kate-osborn has a chance to try it out and then cut a release soon'ish (in a few days)

@abhay-krishna
Copy link

abhay-krishna commented Jan 10, 2025

Sounds good, thank you very much!

On another note, we are upgrading controller-gen from v0.8.0 to v0.16.5 (a huge bump I know!) and we saw that the generated RBAC role YAML had a large diff with a lot of removals, I wanted to make sure if this is intended behavior.

Also CRD struct fields which were previously (i.e. have always been) marked as required (example) with //+kubebuilder:validation:Required were so far (somehow) not included in the required section of the generated CRD but after the 0.16.5 bump, they did go under the required (above fields after generation), which caused a whole bunch of unit tests to break with validation errors.

Do you have any insights into this?

@sbueringer
Copy link
Member

sbueringer commented Jan 10, 2025

On another note, we are upgrading controller-gen from v0.8.0 to v0.16.5 (a huge bump I know!) and we saw that the generated RBAC role YAML had a large diff with a lot of removals, I wanted to make sure if this is intended behavior.

This is intended. We did multiple iterations of implementing deduplications for RBAC (if you diff the RBAC you'll see that it's only deduplicated)

Also CRD struct fields which were previously (i.e. have always been) marked as required (example) with //+kubebuilder:validation:Required were so far (somehow) not included in the required section of the generated CRD but after the 0.16.5 bump, they did go under the required (above fields after generation), which caused a whole bunch of unit tests to break with validation errors.

This PR implemented support for the +required marker: #944 (in addition to the kubebuilder specific marker). It also included a fix that the markers are evaluated with the right "priority". IIRC somehow "omitempty" was previously prioritized above explicit required markers

@kate-osborn
Copy link
Author

@sbueringer #1122 works for me, too! Thanks for the quick response.

@abhay-krishna
Copy link

On another note, we are upgrading controller-gen from v0.8.0 to v0.16.5 (a huge bump I know!) and we saw that the generated RBAC role YAML had a large diff with a lot of removals, I wanted to make sure if this is intended behavior.

This is intended. We did multiple iterations of implementing deduplications for RBAC (if you diff the RBAC you'll see that it's only deduplicated)

Also CRD struct fields which were previously (i.e. have always been) marked as required (example) with //+kubebuilder:validation:Required were so far (somehow) not included in the required section of the generated CRD but after the 0.16.5 bump, they did go under the required (above fields after generation), which caused a whole bunch of unit tests to break with validation errors.

This PR implemented support for the +required marker: #944 (in addition to the kubebuilder specific marker). It also included a fix that the markers are evaluated with the right "priority". IIRC somehow "omitempty" was previously prioritized above explicit required markers

@sbueringer Thanks for clarifiying! Looking closer, I did see that they were only being deduplicated by being incorporated into existing lists so avoiding the need for separate sections, which then got removed. Awesome optimizations! Also thanks for the reference to the +required marker implementation. Just a quick question - are the +required and +kubebuilder:validation:Required markers equivalent?

@sbueringer
Copy link
Member

Yes they are

@sbueringer
Copy link
Member

Just released v0.17.1. Thx for reporting the issue and thx to @mtardy for fixing it! :)

/close

@k8s-ci-robot
Copy link
Contributor

@sbueringer: Closing this issue.

In response to this:

Just released v0.17.1. Thx for reporting the issue and thx to @mtardy for fixing it! :)

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants