Skip to content

✨ Add k8s:required and k8s:optional markers #1247

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lalitc375
Copy link

This commit introduces two new markers, k8s:required and k8s:optional, to allow marking fields in CRDs as required or
optional.

These IDL tags are introduced as part of the KEP for declarative validation: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/5073-declarative-validation-with-validation-gen/README.md

Adding this in controller-gen will allow understanding native types much better in CRDs.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 28, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lalitc375
Once this PR has been reviewed and has the lgtm label, please assign joelanford for approval. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 28, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @lalitc375. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 28, 2025
This commit introduces `k8s:required` and `k8s:optional` markers to align with the declarative validation KEP. These markers serve as a Kubernetes-style alternative to the existing `+required` and `+optional` markers for marking CRD fields.

This change allows controller-gen to better understand native types in CRDs, improving the declarative validation process.

Reference: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/5073-declarative-validation-with-validation-gen/README.md
@sbueringer
Copy link
Member

/assign @JoelSpeed
/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 29, 2025
@JoelSpeed
Copy link
Contributor

Adding this in controller-gen will allow understanding native types much better in CRDs.

Is the intention in native types that this will replace the existing +optional and +required tags, or are they intended to supplement them?

Do we expect to see only +k8s:optional and that to be sufficient, or, would we also expect +optional alongside?

I'm keen to see that the community settle on choices that bring consistency, but that also includes among codebases. In the [kube-api-linter](https://github.com/kubernetes-sigs/kube-api-linter] project, we recommend folks to replace +kubebuilder:validation:Optional with +optional at present so that their codebase is consistent. I'd like to know how this would fit in, if we decide to start supporting it here.

CC @yongruilin

@lalitc375
Copy link
Author

Adding this in controller-gen will allow understanding native types much better in CRDs.

Is the intention in native types that this will replace the existing +optional and +required tags, or are they intended to supplement them?

+optional and +required tags for native type are used to set/unset the openapi required field, it doesn't do any server side validation to enforce it. +k8s:required and +k8s:optional as of today, generate go code to perform server side validation. The working group also has a plan to set/unset the openAPI required property on seeing these new tags. 

So, +optional/+required are not the same as of +k8s:optional/+k8s:required for native types. But these are the same for CRD's.

We will the same situation while adding support for k8s:listType and k8s:listMapKey

Do we expect to see only +k8s:optional and that to be sufficient, or, would we also expect +optional alongside?

For CRD's, both are the same thing, so one is sufficient. We can recommend devs to use the new ones only. For native types, we have to make sure the +k8s:optional/+k8s:required has the same behaviour as of existing ones for the openAPI required property, before making that recommendation. 

I'm keen to see that the community settle on choices that bring consistency, but that also includes among codebases. In the [kube-api-linter]([https://github.com/kubernetes-sigs/kube-api-linter]](https://github.com/kubernetes-sigs/kube-api-linter%5D) project, we recommend folks to replace +kubebuilder:validation:Optional with +optional at present so that their codebase is consistent. I'd like to know how this would fit in, if we decide to start supporting it here.

I totally agree with you on the consistency part, Kubelinter should recommend the new ones, once the features are complete. 

@JoelSpeed
Copy link
Contributor

I totally agree with you on the consistency part, Kubelinter should recommend the new ones, once the features are complete.

Do we have an estimated timeline for the declarative validation project to be stable enough that there's no going back? I understand at the moment that it's all gated and could all be taken out should the project stall.

If we add these markers to controller-tools now, I'd expect we do something similar where we have an easy way to back out of the declarative validation markers in the future, could we put them behind a feature flag for now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants