Is your feature request related to a problem or existing issue? Please describe.
Following the direction in #449, moving validation out of the Go admission webhook and onto the CRD as native CEL.
One constraint sits outside that issue's scope. validateSpec rejects an empty spec.nodeSelector:
if selector != nil && selector.Empty() {
allErrs = append(allErrs, field.Required(field.NewPath("spec", "nodeSelector"), "nodeSelector must not be empty"))
}
metav1.LabelSelectorAsSelector turns an empty selector into labels.Everything(), so a rule carrying one applies its taint to every Node in the cluster. With NoExecute that evicts pods everywhere that lack a toleration.
The webhook is optional and off by default (--enable-webhook=false, webhook.enabled: false in the chart), so on a default install nothing stops it. #403 hit this from the other side: the chart's own error message suggested nodeSelector: {} for matching all Nodes, which the webhook forbids and the CRD allows.
Describe the solution you'd like
Express it as CEL on the field, so it applies on every cluster whether or not the webhook is deployed:
// +kubebuilder:validation:XValidation:rule="(has(self.matchLabels) && size(self.matchLabels) > 0) || (has(self.matchExpressions) && size(self.matchExpressions) > 0)",message="nodeSelector must not be empty"
NodeSelector metav1.LabelSelector `json:"nodeSelector,omitempty,omitzero"`
Then drop the empty check from validateSpec. CRD validation runs before validating webhooks, so the branch becomes unreachable once the rule is in place.
Two details worth stating, since both surfaced while testing this against envtest:
The LabelSelectorAsSelector error check has to stay in the webhook. CEL can see that matchExpressions is non-empty but not whether an operator is one the selector parser accepts, so an entry like {key: k, operator: NotARealOperator} still needs Go.
An absent nodeSelector is caught by the +required marker, not by this rule. The field is omitempty,omitzero, so an empty struct serialises away entirely and the CEL rule never evaluates. The API server reports spec.nodeSelector: Required value for that case and nodeSelector must not be empty for a selector that is present but empty. Both are rejected, they just report differently, and it is worth covering both paths in tests.
After this, the only validation left in the webhook is cross-object taint conflict detection, which genuinely cannot move to CEL since it has to list other rules.
Describe alternatives you've considered
Leaving it in the webhook and documenting that the guard requires the webhook. That is what happens today and it is what made #403 possible, so it does not seem worth keeping.
Adding it to the CRD while leaving the webhook copy in place. Harmless but dead, since CRD validation runs first.
Additional context
Checked for overlap before filing. #451 covers the two bootstrap-only constraints from #449 and does not touch nodeSelector. No other open issue or PR references the empty selector check.
PR to follow.
Is your feature request related to a problem or existing issue? Please describe.
Following the direction in #449, moving validation out of the Go admission webhook and onto the CRD as native CEL.
One constraint sits outside that issue's scope.
validateSpecrejects an emptyspec.nodeSelector:metav1.LabelSelectorAsSelectorturns an empty selector intolabels.Everything(), so a rule carrying one applies its taint to every Node in the cluster. WithNoExecutethat evicts pods everywhere that lack a toleration.The webhook is optional and off by default (
--enable-webhook=false,webhook.enabled: falsein the chart), so on a default install nothing stops it. #403 hit this from the other side: the chart's own error message suggestednodeSelector: {}for matching all Nodes, which the webhook forbids and the CRD allows.Describe the solution you'd like
Express it as CEL on the field, so it applies on every cluster whether or not the webhook is deployed:
Then drop the empty check from
validateSpec. CRD validation runs before validating webhooks, so the branch becomes unreachable once the rule is in place.Two details worth stating, since both surfaced while testing this against envtest:
The
LabelSelectorAsSelectorerror check has to stay in the webhook. CEL can see thatmatchExpressionsis non-empty but not whether an operator is one the selector parser accepts, so an entry like{key: k, operator: NotARealOperator}still needs Go.An absent
nodeSelectoris caught by the+requiredmarker, not by this rule. The field isomitempty,omitzero, so an empty struct serialises away entirely and the CEL rule never evaluates. The API server reportsspec.nodeSelector: Required valuefor that case andnodeSelector must not be emptyfor a selector that is present but empty. Both are rejected, they just report differently, and it is worth covering both paths in tests.After this, the only validation left in the webhook is cross-object taint conflict detection, which genuinely cannot move to CEL since it has to list other rules.
Describe alternatives you've considered
Leaving it in the webhook and documenting that the guard requires the webhook. That is what happens today and it is what made #403 possible, so it does not seem worth keeping.
Adding it to the CRD while leaving the webhook copy in place. Harmless but dead, since CRD validation runs first.
Additional context
Checked for overlap before filing. #451 covers the two bootstrap-only constraints from #449 and does not touch
nodeSelector. No other open issue or PR references the empty selector check.PR to follow.