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") + } +}