What happened?
The Helm chart tells users to use nodeSelector: {} to match all nodes, but the validating webhook rejects exactly that.
The chart's error message for nodeReadinessRules says:
spec.nodeSelector is required and immutable. Set it explicitly; use 'nodeSelector: {}' only if the rule is intended to match ALL nodes.
The webhook does this in validateSpec:
if selector != nil && selector.Empty() {
allErrs = append(allErrs, field.Required(field.NewPath("spec", "nodeSelector"), "nodeSelector must not be empty"))
}
So a user who follows the chart's own guidance and enables the webhook gets the rule rejected on create. The chart also has a test named renders full rule spec with explicit all-nodes selector that renders nodeSelector: {} and asserts it comes out empty, so the chart clearly treats it as supported while the webhook treats it as invalid.
The other half is what happens with the webhook off, which is the default. Nothing else blocks an empty selector. The CRD has no CEL rule for it, and metav1.LabelSelectorAsSelector turns {} into labels.Everything(), so the rule matches every node in the cluster. With NoExecute that evicts pods everywhere that lack a toleration.
So the same manifest either fails admission or taints the whole cluster, depending on a flag that is off by default.
Steps to Reproduce
- Install the chart with
webhook.enabled=true and validatingWebhook.enabled=true.
- Add a rule under
nodeReadinessRules with nodeSelector: {}, as the chart's message suggests.
- The rule is rejected with
nodeSelector must not be empty.
- Install without the webhook and the same rule is accepted, and applies to every node.
Expected Behavior
The chart should not point users at a configuration the webhook forbids.
Beyond fixing the message, there is a question I do not think I should answer on my own: should an empty selector be blocked for everyone rather than only when the optional webhook is running? Today the only guard against a rule that taints every node lives in a component that is off by default. Moving it into the CRD as a CEL rule would make it consistent, but it is an API change and would reject any existing rule that relies on it, so it needs a maintainer call.
Controller Version / Image Tag
main (commit d74dabd)
Kubernetes Version
Not version specific.
Additional Environment Details
I checked before filing. No existing issue covers empty node selectors, and neither #315 nor #352 touches the chart template or the webhook's empty-selector check, #315 only shows that line as unchanged context.
Sending a PR for the message. Happy to follow up on the CRD question separately if there is appetite.
What happened?
The Helm chart tells users to use
nodeSelector: {}to match all nodes, but the validating webhook rejects exactly that.The chart's error message for
nodeReadinessRulessays:The webhook does this in
validateSpec:So a user who follows the chart's own guidance and enables the webhook gets the rule rejected on create. The chart also has a test named
renders full rule spec with explicit all-nodes selectorthat rendersnodeSelector: {}and asserts it comes out empty, so the chart clearly treats it as supported while the webhook treats it as invalid.The other half is what happens with the webhook off, which is the default. Nothing else blocks an empty selector. The CRD has no CEL rule for it, and
metav1.LabelSelectorAsSelectorturns{}intolabels.Everything(), so the rule matches every node in the cluster. WithNoExecutethat evicts pods everywhere that lack a toleration.So the same manifest either fails admission or taints the whole cluster, depending on a flag that is off by default.
Steps to Reproduce
webhook.enabled=trueandvalidatingWebhook.enabled=true.nodeReadinessRuleswithnodeSelector: {}, as the chart's message suggests.nodeSelector must not be empty.Expected Behavior
The chart should not point users at a configuration the webhook forbids.
Beyond fixing the message, there is a question I do not think I should answer on my own: should an empty selector be blocked for everyone rather than only when the optional webhook is running? Today the only guard against a rule that taints every node lives in a component that is off by default. Moving it into the CRD as a CEL rule would make it consistent, but it is an API change and would reject any existing rule that relies on it, so it needs a maintainer call.
Controller Version / Image Tag
main(commitd74dabd)Kubernetes Version
Not version specific.
Additional Environment Details
I checked before filing. No existing issue covers empty node selectors, and neither #315 nor #352 touches the chart template or the webhook's empty-selector check, #315 only shows that line as unchanged context.
Sending a PR for the message. Happy to follow up on the CRD question separately if there is appetite.