kubernetes-sigs / controller-tools

Tools to use with the controller-runtime libraries
Apache License 2.0
736 stars 420 forks source link

Error applying a CRD generated by controller-tools that contains the ContainerPort struct #1027

Closed MenD32 closed 3 months ago

MenD32 commented 3 months ago

I have the following types.go file:

// types.go

type ExampleSpec struct {
    apiv1.ContainerPort `json:",inline"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// +kubebuilder:subresource:status
// +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[0].message"
// +kubebuilder:printcolumn:name="Synced",type="string",JSONPath=".status.conditions[0].status"
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
// +genclient

type Example struct {
    metav1.TypeMeta   `json:",inline"`
    metav1.ObjectMeta `json:"metadata,omitempty"`

    Spec ExampleSpec    `json:"spec"`
}

after generating a CRD from this code with controller-gen like so: controller-gen crd:generateEmbeddedObjectMeta=true paths="./pkg/apis/..." output:stdout | tail -n +2 > manifests/crd.yaml

and trying to apply with kubectl create I get the following error(s):

* spec.validation.openAPIV3Schema.properties[spec].properties[protocol].allOf[0].default: Forbidden: must be undefined to be structural
* spec.validation.openAPIV3Schema.properties[spec].properties[protocol].allOf[1].default: Forbidden: must be undefined to be structural

after some messing around i found that removing the // +default="TCP" comment from ContainerPort.Protocol in k8s.io/api, fixes the issue while maintaining the default value of TCP in the CRD itself.

versions:

aerfio commented 3 months ago

It affects https://github.com/kubernetes-sigs/controller-tools/releases/tag/v0.16.0-beta.0, works with v0.15.0 in my case

sbueringer commented 3 months ago

Another case was reported here: https://github.com/kubernetes-sigs/controller-tools/issues/1034

sbueringer commented 3 months ago

I assume the relevant change since v0.15.0 is this one: https://github.com/kubernetes-sigs/controller-tools/pull/938

sbueringer commented 3 months ago

Not sure what's going on, clearly looks like a bug:

    // Protocol for port. Must be UDP, TCP, or SCTP.
    // Defaults to "TCP".
    // +optional
    // +default="TCP"
    Protocol corev1.Protocol `json:"protocol,omitempty" protobuf:"bytes,4,opt,name=protocol,casttype=Protocol"`

leads to

              protocol:
                allOf:
                - default: TCP
                - default: TCP
                description: |-
                  Protocol for port. Must be UDP, TCP, or SCTP.
                  Defaults to "TCP".
                type: string
sbueringer commented 3 months ago

Oh my...

https://github.com/kubernetes-sigs/controller-tools/blob/0adb7e1adec45c82ede9266fc4ef0279717c207c/pkg/crd/known_types.go#L28-L35

mbobrovskyi commented 3 months ago

Oh my...

https://github.com/kubernetes-sigs/controller-tools/blob/0adb7e1adec45c82ede9266fc4ef0279717c207c/pkg/crd/known_types.go#L28-L35

Good catch!

mbobrovskyi commented 3 months ago

@sbueringer do you want to fix it?

sbueringer commented 3 months ago

Already on it

sbueringer commented 3 months ago

PR is open: https://github.com/kubernetes-sigs/controller-tools/pull/1035

If possible, please also test in your use cases to ensure it fixes the entire issue (although I'm pretty confident)

Once the PR merges I would cut a patch release

mbobrovskyi commented 3 months ago

If possible, please also test in your use cases to ensure it fixes the entire issue (although I'm pretty confident)

Yes, it's working fine. Thank you so much!

pmalek commented 3 months ago

PR is open: #1035

If possible, please also test in your use cases to ensure it fixes the entire issue (although I'm pretty confident)

Once the PR merges I would cut a patch release

It does look like it doesn't produce the defaults for container port. 👍

sbueringer commented 3 months ago

Thx for reporting everyone! v0.16.1 with the fix is now available: https://github.com/kubernetes-sigs/controller-tools/releases/tag/v0.16.1