[itsvishalyadav]

Overview

This PR fixes #490

Signed Commits

• Yes, I signed my commits.

The API server sends a DELETE with the deleted object as the old object and no object. Validate dropped every request without an object, so every DELETE was dropped before any rule saw it, although the chart subscribes the webhook to DELETE for pods and RBAC objects and the CEL event already exposes the deleted object as oldObject. A rule on event.Operation == "DELETE" never fired.

• Validate now only drops a request that has neither an object nor an old object.
• evaluate looks rules up for the old object when there is no new one.

NewAdmissionCelEvent, the alert builder and the v1 rule helpers already check GetObject() for nil, so nothing downstream needs to change. CREATE, UPDATE and CONNECT are handled as before.

How to Test

go test ./admission/webhook/ -run 'TestValidator' -count=1 -v

The new TestValidator_DeleteIsEvaluatedWithTheDeletedObject sends a DELETE of a ClusterRoleBinding (old object only) through Validate and waits for rule evaluation. On main it fails with the DELETE request never reached rule evaluation; with the fix the rules are looked up for the deleted admins binding. The existing TestValidator_NilObjectIsDropped (no object and no old object) still passes.

go test ./admission/webhook/ -skip TestRun_ReloadsSecretSymlinkSwap -count=1 passes (that test needs symlinks and fails on Windows on main too), go test ./admission/cel/... ./admission/rules/... -count=1 passes, and gofmt, goimports, go vet and golangci-lint report nothing new. I haven't run it on a live cluster.

Checklist before requesting a review

• My code follows the style guidelines of this project
• I have commented on my code, particularly in hard-to-understand areas
• I have performed a self-review of my code
• If it is a core feature, I have added thorough tests.
• New and existing unit tests pass locally with my changes

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 55 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0b46ac52-d46b-4760-9a9f-cd6128440cad

Commits

Reviewing files that changed from the base of the PR and between 47e9ae6 and f21415a.

Files selected for processing: 2

• admission/webhook/validator.go
• admission/webhook/validator_delete_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[matthyx]

Approve — reviewed head f21415a against main @ 47e9ae6.

• Need: confirmed. On main, Validate drops every request with GetObject()==nil, which is all DELETEs (API server sends the deleted object as old object only), although the webhook subscribes to DELETE and cel/event.go exposes oldObject. Fixes #490; I found no duplicate or previously rejected PRs touching this.
• Soundness: the guard now only drops when both object and old object are nil, and evaluate falls back to the old object for rule lookup. Downstream (cel/event.go, rules/cel/evaluator.go, v1/helpers.go) already nil-checks GetObject(). Pod DELETE (Kind Pod) takes the same branch; the non-Pod pods subresource path is unchanged. CREATE/UPDATE/CONNECT behave as before.
• Validation: go test ./admission/webhook/ -run TestValidator passes on the PR. With validator.go reverted to base, the new TestValidator_DeleteIsEvaluatedWithTheDeletedObject fails ("never reached rule evaluation"), so the test is a real regression test. CI: build, DCO, CodeQL, GitGuardian pass; Basic-Test was still running when I checked. Not run on a live cluster.
• Blockers: none. Nonblocking: rules that use event.Object now see it nil on DELETE (expected; they should use oldObject).