What happened?
The CEL rules on spec.taint check the key thoroughly (prefix, a single /, the name part's character set), but the only rule on value is the 63-character cap (api/v1alpha1/nodereadinessrule_types.go, mirrored in config/crd/bases), and the webhook's validateSpec only looks at the node selector. The API server validates a node taint's value as a label value, so a rule whose value is not a valid value! (a space, a /, a leading -, anything outside (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?) is admitted, and then every node patch it triggers fails with Invalid. RetryOnConflict does not retry an Invalid error, so each matching node lands in status.failedNodes with EvaluationError and the reconciler returns the error. Because taint.value is immutable the rule cannot be repaired in place; it has to be deleted and recreated. A dry-run rule never patches nodes, so it gives no warning either.
Steps to Reproduce
- Apply a NodeReadinessRule whose
spec.taint.value is not a valid value! and whose selector matches a node.
- Read the rule status.
Reproduced on main (80f59f3) in envtest (Kubernetes 1.36.2), calling the webhook's ValidateCreate, creating the rule through the API server, then running processNodeAgainstAllRules for a matching node:
webhook ValidateCreate: <nil>
CRD create: <nil>
processNodeAgainstAllRules: failed to add taint: Node "verify-n1-node" is invalid: metadata.taints[1].value: Invalid value: "not a valid value!": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (...)
rule.status.failedNodes: 1 entry, reason EvaluationError
Expected Behavior
The rule is rejected at admission with a message that says what a taint value may contain, the same way an invalid key is rejected today.
Controller Version / Image Tag
main (commit 80f59f3)
Kubernetes Version
envtest 1.36.2
Controller Logs
Not applicable; reproduced at the reconciler call.
Additional Environment Details
The fix looks like one CEL marker next to the existing value-length rule, mirroring the key rule: !has(self.value) || self.value.matches('^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$'), then make manifests, plus a CEL test of the kind #112 added for the key. I can send that once #466 is through, so there is one PR from me open here at a time.
What happened?
The CEL rules on
spec.taintcheck the key thoroughly (prefix, a single/, the name part's character set), but the only rule onvalueis the 63-character cap (api/v1alpha1/nodereadinessrule_types.go, mirrored inconfig/crd/bases), and the webhook'svalidateSpeconly looks at the node selector. The API server validates a node taint's value as a label value, so a rule whose value isnot a valid value!(a space, a/, a leading-, anything outside(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?) is admitted, and then every node patch it triggers fails withInvalid.RetryOnConflictdoes not retry anInvaliderror, so each matching node lands instatus.failedNodeswithEvaluationErrorand the reconciler returns the error. Becausetaint.valueis immutable the rule cannot be repaired in place; it has to be deleted and recreated. A dry-run rule never patches nodes, so it gives no warning either.Steps to Reproduce
spec.taint.valueisnot a valid value!and whose selector matches a node.Reproduced on
main(80f59f3) in envtest (Kubernetes 1.36.2), calling the webhook'sValidateCreate, creating the rule through the API server, then runningprocessNodeAgainstAllRulesfor a matching node:Expected Behavior
The rule is rejected at admission with a message that says what a taint value may contain, the same way an invalid key is rejected today.
Controller Version / Image Tag
main (commit 80f59f3)
Kubernetes Version
envtest 1.36.2
Controller Logs
Not applicable; reproduced at the reconciler call.
Additional Environment Details
The fix looks like one CEL marker next to the existing value-length rule, mirroring the key rule:
!has(self.value) || self.value.matches('^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$'), thenmake manifests, plus a CEL test of the kind #112 added for the key. I can send that once #466 is through, so there is one PR from me open here at a time.