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

controller-gen doesn't work with k8s.io/apiextensions-apiserver #291

Closed
jiachengxu opened this issue Jul 31, 2019 · 26 comments · Fixed by #528
Closed

controller-gen doesn't work with k8s.io/apiextensions-apiserver #291

jiachengxu opened this issue Jul 31, 2019 · 26 comments · Fixed by #528
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@jiachengxu
Copy link
Contributor

Hi folks,

I use controller-gen integrated with Kubebuilder to generate RBAC, webhook and CRDs for my project, there is a command called make manifests to run:
controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
to generate everything, but it doesn't work for me since I have k8s.io/apiextensions-apiserver in my go.mod as a dependency, the following error messages are poping out:
/Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:63:2: enountered struct field "Raw" without JSON tag in type "JSON" /Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:29:24: unsupported type "float64" /Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:31:24: unsupported type "float64" /Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:39:24: unsupported type "float64" /Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:85:2: enountered struct field "Schema" without JSON tag in type "JSONSchemaPropsOrArray" /Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:86:2: enountered struct field "JSONSchemas" without JSON tag in type "JSONSchemaPropsOrArray" /Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:105:2: enountered struct field "Allows" without JSON tag in type "JSONSchemaPropsOrBool" /Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:106:2: enountered struct field "Schema" without JSON tag in type "JSONSchemaPropsOrBool" /Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:127:2: enountered struct field "Schema" without JSON tag in type "JSONSchemaPropsOrStringArray" /Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:128:2: enountered struct field "Property" without JSON tag in type "JSONSchemaPropsOrStringArray" Error: not all generators ran successfully
Any ideas about why this happened?

Thanks,
Jiacheng

@munnerz
Copy link
Member

munnerz commented Aug 1, 2019

Can you share your types.go file containing your API definitions? It looks like you are depending on something in the apiextensions-apiserver API group 🤔 fwiw, #277 is a fix for:

/Users/maxwell/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.0.0-20190409022649-727a075fdec8/pkg/apis/apiextensions/v1beta1/types_jsonschema.go:63:2: enountered struct field "Raw" without JSON tag in type "JSON"

@jiachengxu
Copy link
Contributor Author

@munnerz , yes, you are right, my API definitions depend on the apiextensions-apiserver API group, and thanks for the info :)!

@munnerz
Copy link
Member

munnerz commented Aug 1, 2019

It looks like similar PRs for the JSONSchemaPropsOrArray, JSONSchemaPropsOrBool and JSONSchemaPropsOrStringArray need to be made in order to make controller-tools work for you too!

@jiachengxu
Copy link
Contributor Author

Hi @munnerz, yes, you are right, would you mind me working on this? I think your PR is a good example for me to start and I can send the similar PRs for JSONSchemaPropsOrArray, JSONSchemaPropsOrBool and JSONSchemaPropsOrStringArray.

@DirectXMan12
Copy link
Contributor

DirectXMan12 commented Sep 30, 2019

I'm curious as to why your types depend on apiextensions. Are you embedding a CRD spec in your types? Just want to make sure we're solving the right problems here

@maxsmythe
Copy link
Contributor

maxsmythe commented Nov 12, 2019

@thetechnick
Copy link

@jiachengxu and I are also embedding the CRD Spec into one of our resources.
We are working on tooling that manages CRDs dynamically.

@DirectXMan12
Copy link
Contributor

We might want to better generically detect when something implements the JSON interfaces manually -- we have the typechecking info, so it should be possilbe.

/help

@k8s-ci-robot
Copy link
Contributor

@DirectXMan12:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

We might want to better generically detect when something implements the JSON interfaces manually -- we have the typechecking info, so it should be possilbe.

/help

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/test-infra repository.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 13, 2020
@maxsmythe
Copy link
Contributor

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 13, 2020
@muvaf
Copy link
Contributor

muvaf commented Apr 8, 2020

I have a type that embeds CustomResourceDefinitionSpec type which suffers from this issue.

We might want to better generically detect when something implements the JSON interfaces manually -- we have the typechecking info, so it should be possilbe.

@DirectXMan12 If you can give me some pointers, I can work on a PR to make it happen.

@DirectXMan12
Copy link
Contributor

I think someone posted something recently: #427

just waiting on a test

@maxsmythe
Copy link
Contributor

Great news! I look forward to ditching the old, old version of controller-gen

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 19, 2020
@maxsmythe
Copy link
Contributor

/remove-lifecycle stale

How is this looking? I notice #427 is merged, but this isn't resolved. Is this just an oversight?

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 20, 2020
@DirectXMan12
Copy link
Contributor

I think this is mostly resolved. Technically, we should have type info for the other types, but we at least have a workaround, so I think we can probably close this for now.

@tshak
Copy link

tshak commented Nov 19, 2020

I'm able to repro this issue on v0.4.1. It does not appear to be resolved. 🙏

@julianKatz
Copy link

@tshak did you use the following CRD option?

controller-gen crd:allowDangerousTypes=true

That got it going for me. I only figured that out by looking at the PR. Further documentation would make this a lot friendlier.

@tshak
Copy link

tshak commented Nov 21, 2020

Thanks Julian. Sadly that does not work for me:

/Users/tshak/code/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.18.12/pkg/apis/apiextensions/v1/types_jsonschema.go:191:2: encountered struct field "Schema" without JSON tag in type "JSONSchemaPropsOrArray"
/Users/tshak/code/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.18.12/pkg/apis/apiextensions/v1/types_jsonschema.go:192:2: encountered struct field "JSONSchemas" without JSON tag in type "JSONSchemaPropsOrArray"
/Users/tshak/code/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.18.12/pkg/apis/apiextensions/v1/types_jsonschema.go:211:2: encountered struct field "Allows" without JSON tag in type "JSONSchemaPropsOrBool"
/Users/tshak/code/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.18.12/pkg/apis/apiextensions/v1/types_jsonschema.go:212:2: encountered struct field "Schema" without JSON tag in type "JSONSchemaPropsOrBool"
/Users/tshak/code/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.18.12/pkg/apis/apiextensions/v1/types_jsonschema.go:233:2: encountered struct field "Schema" without JSON tag in type "JSONSchemaPropsOrStringArray"
/Users/tshak/code/go/pkg/mod/k8s.io/apiextensions-apiserver@v0.18.12/pkg/apis/apiextensions/v1/types_jsonschema.go:234:2: encountered struct field "Property" without JSON tag in type "JSONSchemaPropsOrStringArray"

I'm running controller-gen as part of go code generation:

//go:generate go run -tags generate sigs.k8s.io/controller-tools/cmd/controller-gen object:headerFile=../../hack/boilerplate.go.txt paths=./... crd:trivialVersions=true,crdVersions=v1,allowDangerousTypes=true output:artifacts:config=../../config/base

@maxsmythe
Copy link
Contributor

Is the following code being vendored in?

https://github.com/kubernetes/apiextensions-apiserver/blob/a7ee1efe41fc19623ce57f047c7595d624df4c17/pkg/apis/apiextensions/v1/marshal.go#L28-L37

If gomod is pruning that code for some reason, then the implementsJSONMarshaler interface wouldn't be satisfied, which means the fix in #427 wouldn't be triggered.

@tshak
Copy link

tshak commented Nov 24, 2020

I'm using Go Modules but nevertheless I confirmed that the code exists when I Go to Definition in VS code. I'm on v0.18.12 of k8s.io/apiextensions-apiserver. I'm not sure if any of this is relevant though as the fix in #427 was reverted in #505.

@maxsmythe
Copy link
Contributor

It looks like that reversion is in v0.4.1, but not v0.4.0.

I can confirm that generating CRDs that embed JSONSchema is broken due to the reversion in #505

@DirectXMan12 Should the reversion be un-reverted? Are there any plans to accommodate embedding JSONSchema in other ways?

@maxsmythe
Copy link
Contributor

/reopen

Re-opening as this issue has been de-mitigated

@k8s-ci-robot
Copy link
Contributor

@maxsmythe: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

/reopen

Re-opening as this issue has been de-mitigated

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/test-infra repository.

@maxsmythe
Copy link
Contributor

Looks like I don't have permissions to re-open. Can it be re-opened?

maxsmythe added a commit to maxsmythe/controller-tools that referenced this issue Nov 26, 2020
This marker will avoid trying to do any type detection
on any struct field on which it is set. This gives
users a safety valve when they hit an edge case where
type inference does the wrong thing for them.

This fixes kubernetes-sigs#291, which was recently re-broken by fixing kubernetes-sigs#502

Signed-off-by: Max Smythe <smythe@google.com>
maxsmythe added a commit to maxsmythe/controller-tools that referenced this issue Nov 26, 2020
This marker will avoid trying to do any type detection
on any struct field on which it is set. This gives
users a safety valve when they hit an edge case where
type inference does the wrong thing for them.

This fixes kubernetes-sigs#291, which was recently re-broken by fixing kubernetes-sigs#502

Signed-off-by: Max Smythe <smythe@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
None yet
10 participants