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

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

Closed
MenD32 opened this issue Aug 9, 2024 · 12 comments · Fixed by #1035
Closed

Comments

@MenD32
Copy link

MenD32 commented Aug 9, 2024

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:

@MenD32 MenD32 changed the title Error when trying to create a CRD that contains ContainerPort Error applying a CRD generated by controller-tools that contains the ContainerPort struct Aug 9, 2024
@aerfio
Copy link

aerfio commented Aug 13, 2024

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
Copy link
Member

Another case was reported here: #1034

@sbueringer
Copy link
Member

I assume the relevant change since v0.15.0 is this one: #938

@sbueringer
Copy link
Member

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
Copy link
Member

Oh my...

"k8s.io/api/core/v1": func(p *Parser, pkg *loader.Package) {
// Explicit defaulting for the corev1.Protocol type in lieu of https://github.com/kubernetes/enhancements/pull/1928
p.Schemata[TypeIdent{Name: "Protocol", Package: pkg}] = apiext.JSONSchemaProps{
Type: "string",
Default: &apiext.JSON{Raw: []byte(`"TCP"`)},
}
p.AddPackage(pkg)
},

@mbobrovskyi
Copy link

Oh my...

"k8s.io/api/core/v1": func(p *Parser, pkg *loader.Package) {
// Explicit defaulting for the corev1.Protocol type in lieu of https://github.com/kubernetes/enhancements/pull/1928
p.Schemata[TypeIdent{Name: "Protocol", Package: pkg}] = apiext.JSONSchemaProps{
Type: "string",
Default: &apiext.JSON{Raw: []byte(`"TCP"`)},
}
p.AddPackage(pkg)
},

Good catch!

@mbobrovskyi
Copy link

@sbueringer do you want to fix it?

@sbueringer
Copy link
Member

Already on it

@sbueringer
Copy link
Member

sbueringer commented Aug 14, 2024

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

@mbobrovskyi
Copy link

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
Copy link
Contributor

pmalek commented Aug 14, 2024

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
Copy link
Member

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

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