feat: add validation to prevent anyOf misconfiguration - #418
feat: add validation to prevent anyOf misconfiguration#418vishnukothakapu wants to merge 3 commits into
Conversation
✅ Deploy Preview for node-readiness-controller ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
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 Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
/assign @ajaysundark |
|
/test pull-node-readiness-controller-junit-report-canary |
|
@vishnukothakapu: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
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. |
|
/ok-to-test |
9cb3968 to
1f55410
Compare
faf4a7d to
5fd41e0
Compare
f1b193e to
9d9062b
Compare
|
/retitle feat: add validation to prevent anyOf misconfiguration |
|
@vishnukothakapu please update release notes and description to keep it in sync with the implementation |
9d9062b to
f85f431
Compare
f85f431 to
2c0f43b
Compare
2c0f43b to
95c0b0e
Compare
AnuragThePathak
left a comment
There was a problem hiding this comment.
Approving it from my side. @ajaysundark PTAL at your convenience.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: AnuragThePathak, vishnukothakapu The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Description
This PR adds CEL validation to the CRD to explicitly reject
NodeReadinessRuleconfigurations where theconditionPolicyis set toanyOfand any of the defined conditions has adefaultStatusequal to itsrequiredStatus.As discussed with @ajaysundark & @AnuragThePathak in PR #315, allowing
defaultStatus == requiredStatuswithin ananyOfrule creates a silent logical short-circuit, causing the rule to be considered satisfied even if the condition is never reported by the Node.By enforcing this natively via CEL XValidation, we keep the webhook lean while still preventing the misconfiguration. This PR also adds a warning to the concepts documentation.
Related Issue
Fixes #414
Type of Change
/kind feature
/kind documentation
Testing
nodereadinessrule_controller_test.goto includeenvtestcoverage verifying that the API server actively rejectsanyOf+defaultStatusconfigurations via CEL.Checklist
make testpassesmake lintpassesDoes this PR introduce a user-facing change?