🐛 Remove defaults from crd v1beta1 #446
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tamalsaha The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/M
Signed-off-by: Tamal Saha <tamal@appscode.com>
|
It seems a little strange to me that we'd silently remove the defaults markers for an unsupported type, this could lead into confusion, thoughts @alvaroaleman @DirectXMan12 ? |
There was a problem hiding this comment.
left comment, needs a test case, also we should probably warn about this, or maybe just error out.
I'd lean towards just erroring out, but I wouldn't be surprised if people have workarounds (deploying different controller versions or webhooks but same YAML), so a warning is probably better.
| @@ -182,6 +189,59 @@ func toTrivialVersions(crd *apiextlegacy.CustomResourceDefinition) { | |||
| crd.Spec.AdditionalPrinterColumns = canonicalColumns | |||
| } | |||
|
|
|||
| // removeDefaults removes defaults from apiextensions.k8s.io/v1beta1 CRD definition. | |||
| // To use defaulting, CustomResourceDefinitions must use API version apiextensions.k8s.io/v1 | |||
| func removeDefaults(schema *apiextlegacy.JSONSchemaProps) { | |||
There was a problem hiding this comment.
we've got a schema visitor for this purpose, so you shouldn't have to manually write a schema visitor.
|
Closed by #481, but that'll need some fixups. |
Fixes #445
Signed-off-by: Tamal Saha tamal@appscode.com