feat: migrate bootstrap-only validations to CEL by vishnukothakapu · Pull Request #451 · kubernetes-sigs/node-readiness-controller

vishnukothakapu · 2026-08-24T16:47:18Z

Description

This PR completes the migration of the remaining bootstrap-only enforcement validations from the admission webhook to native CEL XValidation markers on the NodeReadinessRuleSpec.

Specifically, it blocks two invalid configurations directly at the API level:

enforcementMode: bootstrap-only combined with defaultStatus
enforcementMode: bootstrap-only combined with conditionPolicy: anyOf

This aligns with the clean hybrid approach established in PR #418, ensuring standard user templates using allOf continue to work while safely short-circuiting unsupported bootstrap-only configurations without the need for a webhook round-trip.

Changes include:

Addition of two CEL XValidation markers to nodereadinessrule_types.go
Regeneration of the CRD schema
Removal of the legacy Go validation logic and associated unit tests from nodereadinessgaterule_webhook.go
Addition of mirroring CEL controller envtest test cases
Minor documentation update reflecting that the API server (not the webhook) now handles these rejections.

Related Issue

Fixes #449

Type of Change

/kind feature
/kind api-change
/kind cleanup

Testing

Automated envtest cases were added to nodereadinessrule_controller_test.go to explicitly verify that the API server rejects these invalid manifests during creation.
Webhook unit tests were cleaned up and verified to pass.
Manual verification of the generated CRD schema confirmed the CEL markers propagated correctly.

Checklist

make test passes
make lint passes

Does this PR introduce a user-facing change?

Migrated the remaining `bootstrap-only` validation logic from the admission webhook to native CRD CEL validations, improving API server efficiency when creating or updating NodeReadinessRules.

netlify · 2026-08-24T16:47:24Z

✅ Deploy Preview for node-readiness-controller ready!

Name	Link
🔨 Latest commit	`4a22f0c`
🔍 Latest deploy log	https://app.netlify.com/projects/node-readiness-controller/deploys/6a940a49566b6e0008132451
😎 Deploy Preview	https://deploy-preview-451--node-readiness-controller.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

kubernetes-prow · 2026-08-24T16:47:30Z

Hi @vishnukothakapu. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

tejassinghbhati · 2026-08-25T11:54:26Z

Heads up on a collision with #455, which migrates the nodeSelector check to CEL. Different constraint to yours, but we overlap on four files.

Two concrete spots. You delete the accumulate errors across nodeSelector, defaultStatus, and conditionPolicy violations spec, I edit it, so that will conflict.

The one worth knowing about is in should perform comprehensive validation. Your diff leaves this as context.

Expect(allErrs).To(HaveLen(1)) // Empty nodeSelector validation
Expect(allErrs[0].Field).To(Equal("spec.nodeSelector"))

That holds today but stops holding once #455 removes the empty check from the webhook, so whichever of ours merges second needs that repointed at an unparseable selector rather than an empty one.

Happy to rebase mine on yours if you land first, no preference.

yindia · 2026-08-25T13:58:22Z

Same thing I flagged on #455: the chart CRD didn't get regenerated. Both new bootstrap-only rules are in config/crd/bases/ but not in charts/node-readiness-controller/crds/. So verify-chart-drift.sh will fail and Helm installs won't actually enforce them. Could you sync the chart CRD?

vishnukothakapu · 2026-08-25T20:35:14Z

/cc @ajaysundark

ajaysundark · 2026-08-30T08:27:14Z

/ok-to-test

ajaysundark

Thanks!

/lgtm

ajaysundark · 2026-08-30T08:38:21Z

/ok-to-test

ajaysundark

/lgtm
/approve

kubernetes-prow · 2026-08-30T16:47:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ajaysundark, vishnukothakapu, yindia

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [ajaysundark]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

An empty nodeSelector matches every Node in the cluster, so a rule carrying one applies its taint fleet wide. The only thing stopping that today is validateSpec in the validating webhook, and the webhook is optional and off by default, so the guard is absent on a default install. That gap is what kubernetes-sigs#403 documented from the chart side. Move the constraint onto the CRD as CEL, where it applies on every cluster regardless of whether the webhook is deployed. This continues the migration described in kubernetes-sigs#449, and does not overlap kubernetes-sigs#451, which covers the two bootstrap-only constraints. CEL cannot express whether a selector parses, so the LabelSelectorAsSelector error check stays in the webhook. The empty check is removed rather than left alongside it: CRD validation runs before validating webhooks, so that branch is now unreachable. An absent nodeSelector is still caught by the required marker rather than by CEL, because the field is omitempty/omitzero and serialises away. Both reject the object, they just report it differently, and the tests cover each path. Signed-off-by: tejassinghbhati <tejassinghbhati077@gmail.com>

kubernetes-prow Bot requested review from ajaysundark and haircommander August 24, 2026 16:47

kubernetes-prow Bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 24, 2026

kubernetes-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 24, 2026

This was referenced Aug 25, 2026

Migrate the non-empty nodeSelector validation from the admission webhook to CEL #454

Open

feat: enforce non-empty nodeSelector with CEL instead of the webhook #455

Open

Migrate remaining bootstrap-only validations from admission webhook to CEL #449

Closed

yindia reviewed Aug 25, 2026

View reviewed changes

Comment thread internal/webhook/nodereadinessgaterule_webhook.go

vishnukothakapu force-pushed the feat/cel-bootstrap-only branch from 39834ad to 20889e9 Compare August 25, 2026 14:14

kubernetes-prow Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 25, 2026

vishnukothakapu requested a review from yindia August 25, 2026 14:14

yindia approved these changes Aug 25, 2026

View reviewed changes

yindia reviewed Aug 25, 2026

View reviewed changes

Comment thread internal/webhook/nodereadinessgaterule_webhook.go

vishnukothakapu force-pushed the feat/cel-bootstrap-only branch from 20889e9 to 6115d07 Compare August 25, 2026 14:52

kubernetes-prow Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 30, 2026

ajaysundark reviewed Aug 30, 2026

View reviewed changes

Comment thread docs/book/src/user-guide/concepts.md Outdated

kubernetes-prow Bot assigned ajaysundark Aug 30, 2026

kubernetes-prow Bot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 30, 2026

feat: migrate bootstrap-only validations to CEL

4a22f0c

vishnukothakapu force-pushed the feat/cel-bootstrap-only branch from 6115d07 to 4a22f0c Compare August 30, 2026 10:47

kubernetes-prow Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 30, 2026

vishnukothakapu requested a review from ajaysundark August 30, 2026 10:50

ajaysundark approved these changes Aug 30, 2026

View reviewed changes

kubernetes-prow Bot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 30, 2026

kubernetes-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 30, 2026

kubernetes-prow Bot merged commit 62d1b77 into kubernetes-sigs:main Aug 30, 2026
11 checks passed

Navigation Menu

Uh oh!

feat: migrate bootstrap-only validations to CEL - #451

feat: migrate bootstrap-only validations to CEL#451
kubernetes-prow[bot] merged 1 commit into
kubernetes-sigs:mainfrom
vishnukothakapu:feat/cel-bootstrap-only

vishnukothakapu commented Aug 24, 2026

Uh oh!

netlify Bot commented Aug 24, 2026 •

edited

Loading

Uh oh!

kubernetes-prow Bot commented Aug 24, 2026

Uh oh!

tejassinghbhati commented Aug 25, 2026

Uh oh!

yindia commented Aug 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

vishnukothakapu commented Aug 25, 2026

Uh oh!

ajaysundark commented Aug 30, 2026

Uh oh!

ajaysundark left a comment

Uh oh!

Uh oh!

ajaysundark commented Aug 30, 2026

Uh oh!

ajaysundark left a comment

Uh oh!

kubernetes-prow Bot commented Aug 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

vishnukothakapu commented Aug 24, 2026

Description

Related Issue

Type of Change

Testing

Checklist

Does this PR introduce a user-facing change?

Uh oh!

netlify Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for node-readiness-controller ready!

Uh oh!

kubernetes-prow Bot commented Aug 24, 2026

Uh oh!

tejassinghbhati commented Aug 25, 2026

Uh oh!

yindia commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vishnukothakapu commented Aug 25, 2026

Uh oh!

ajaysundark commented Aug 30, 2026

Uh oh!

ajaysundark left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ajaysundark commented Aug 30, 2026

Uh oh!

ajaysundark left a comment

Choose a reason for hiding this comment

Uh oh!

kubernetes-prow Bot commented Aug 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

netlify Bot commented Aug 24, 2026 •

edited

Loading

yindia commented Aug 25, 2026 •

edited

Loading