Deprecate redundant markers that duplicate +optional and +required

[camilamacedo86]

controller-tools currently allows both:

• +required / +optional
• +kubebuilder:validation:Required / +kubebuilder:validation:Optional

These perform the same function. Maintaining both causes confusion and inconsistent API definitions.

Proposal:

• Deprecate +kubebuilder:validation:Required and +kubebuilder:validation:Optional
• Recommend +required and +optional
• (Optional) Warn when deprecated markers are used

Motivation, context came from: kubernetes-sigs/kube-api-linter#112

[JoelSpeed]

I know on fields that these are equivalent, but I believe they are also valid at the package level right? Do we know if +required or +optional work at the package level in place of the kubebuilder versions?

[camilamacedo86]

If that is true, then we have a reason for a project to have both.
That means we should keep it and probably re-open kubernetes-sigs/kube-api-linter#112 ;-)

[camilamacedo86]

I am checking and see that you are right @JoelSpeed
At the package level, +kubebuilder:validation:Required and +kubebuilder:validation:Optional are supported (e.g., to influence default behavior for fields in the package). However, +required / +optional are shorthand markers intended only for struct fields.

So, it would not make sense to deprecate those.
Instead, KAL should accept both, IMO.
I have reopened kubernetes-sigs/kube-api-linter#112 for us to discuss it there.

[sbueringer]

Not sure. The recommendation is to have optional or required on every single field. Why should then there be a specific fallback behavior on the package level?

I think depending on a package level fallback to decide if a field is optional or required is not a good idea

[sbueringer]

If I'm not missing anything, I would be in favor of:

• Deprecation with a hint that optional or required should be set on all fields instead
• Keep the markers forever for compatibility

[camilamacedo86]

I think depending on a package level fallback to decide if a field is optional or required is not a good idea

IHMO: That is a very valid point.

Btw, from 4.7.0 Kubebuilder release, users will no longer find in the samples and etc the ref: +kubebuilder:validation:Required all has been replaced by +required or +optional

The reference now only exists in the CRD marker docs since it is a valid marker so far.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale