Skip to content

[BUG] Helm chart suggests nodeSelector: {} but the webhook rejects it #403

Description

@tejassinghbhati

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

  1. Install the chart with webhook.enabled=true and validatingWebhook.enabled=true.
  2. Add a rule under nodeReadinessRules with nodeSelector: {}, as the chart's message suggests.
  3. The rule is rejected with nodeSelector must not be empty.
  4. 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions