From f21415ab24633889c8b82ad7fffb7a163959a82b Mon Sep 17 00:00:00 2001 From: itsvishalyadav Date: Tue, 10 Nov 2026 16:29:32 +0530 Subject: [PATCH] fix(admission): evaluate DELETE requests against the deleted object 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 exposes the deleted object as oldObject. A rule on event.Operation == "DELETE" never fired. Only drop a request that has neither an object nor an old object, and look rules up for the old object when there is no new one. Co-Authored-By: Claude Opus 5.5 Signed-off-by: itsvishalyadav --- admission/webhook/validator.go | 10 +++- admission/webhook/validator_delete_test.go | 57 ++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 admission/webhook/validator_delete_test.go diff --git a/admission/webhook/validator.go b/admission/webhook/validator.go index 5a878ed0..867a011a 100644 --- a/admission/webhook/validator.go +++ b/admission/webhook/validator.go @@ -199,7 +199,8 @@ func (av *AdmissionValidator) Validate(ctx context.Context, attrs admission.Attr telemetry.Admission(ctx, "unmatched_kind", 0) return nil } - if attrs.GetObject() == nil { + // A DELETE carries the deleted object as the old object and no object. + if attrs.GetObject() == nil && attrs.GetOldObject() == nil { telemetry.Admission(ctx, "empty", 0) return nil } @@ -283,7 +284,12 @@ func (av *AdmissionValidator) evaluate(ctx context.Context, attrs admission.Attr return } } else { - un, ok := attrs.GetObject().(*unstructured.Unstructured) + admitted := attrs.GetObject() + if admitted == nil { + // DELETE: the rules apply to the object being deleted. + admitted = attrs.GetOldObject() + } + un, ok := admitted.(*unstructured.Unstructured) if !ok { logger.L().Warning("admission worker: object is not unstructured", helpers.String("kind", attrs.GetKind().Kind)) diff --git a/admission/webhook/validator_delete_test.go b/admission/webhook/validator_delete_test.go new file mode 100644 index 00000000..fa2e7bc5 --- /dev/null +++ b/admission/webhook/validator_delete_test.go @@ -0,0 +1,57 @@ +package webhook + +import ( + "context" + "testing" + "time" + + "github.com/kubescape/operator/admission/rules" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authentication/user" +) + +// recordingRuleBindingCache keeps the object each evaluation asked rules for. +type recordingRuleBindingCache struct { + objects chan *unstructured.Unstructured +} + +func (c *recordingRuleBindingCache) ListRulesForObject(_ context.Context, object *unstructured.Unstructured) []rules.RuleEvaluator { + c.objects <- object + return nil +} + +// TestValidator_DeleteIsEvaluatedWithTheDeletedObject checks that a DELETE request reaches +// rule evaluation. The API server sends the deleted object as the old object and no object, +// so dropping every request without an object dropped every DELETE, although the chart +// subscribes the webhook to DELETE and the CEL event exposes it as oldObject. +func TestValidator_DeleteIsEvaluatedWithTheDeletedObject(t *testing.T) { + cache := &recordingRuleBindingCache{objects: make(chan *unstructured.Unstructured, 1)} + av := newTestValidator(cache) + av.SetWorkerPool(1, 4) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + av.Start(ctx) + + gvk := schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRoleBinding"} + gvr := schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "clusterrolebindings"} + deleted := &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": map[string]interface{}{"name": "admins"}, + }} + attrs := admission.NewAttributesRecord(nil, deleted, gvk, "", "admins", gvr, "", + admission.Delete, nil, false, &user.DefaultInfo{Name: "kubernetes-admin"}) + + require.NoError(t, av.Validate(ctx, attrs, nil)) + + select { + case object := <-cache.objects: + assert.Equal(t, "admins", object.GetName(), "rules must be looked up for the deleted object") + case <-time.After(2 * time.Second): + t.Fatal("the DELETE request never reached rule evaluation") + } +}