-
Notifications
You must be signed in to change notification settings - Fork 399
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
🐛 Fix incompatible CRDs #203
🐛 Fix incompatible CRDs #203
Conversation
The CRD status field doesn't have omitempty on its fields, which means that they get serialized as null. However, the fields aren't marked as optional or nullable in the OpenAPI schema, which means they fail the client-side OpenAPI validation. This populates the known ones so that this doesn't happen. In the long term, we should figure out a better solution.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DirectXMan12 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
e310df3
to
192b99d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one nit
This adds a flag to CRD gen (`trivialVersions=<bool>`), which, when enabled, moves the storage schema into the root CRD, and wipes all version-level schemas. This makes generated CRDs compatible with pre-Kubernetes-1.13 clusters.
the printcolumn marker had some fields that are optional in kubernetes which were not marked as optional in the marker. This fixes that.
192b99d
to
92086f2
Compare
/lgtm |
// NB(directxman12): CRD's status doesn't have omitempty markers, which means things | ||
// get serialized as null, which causes the validator to freak out. Manually set | ||
// these to empty till we get a better solution. | ||
crd.Status.Conditions = []apiext.CustomResourceDefinitionCondition{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An upstream PR would be helpful.
This introduces two fixes for generated CRDs:
printcolumn
fields optional when appropriate