fix: CEL policies aren't applied to deleted resources

Signed-off-by: Mariam Fahmy <[email protected]>

pkg/engine/handlers/validation/validate_cel.go

@@ -47,11 +47,6 @@ func (h validateCELHandler) Process(
 	_ engineapi.EngineContextLoader,
 	exceptions []kyvernov2beta1.PolicyException,
 ) (unstructured.Unstructured, []engineapi.RuleResponse) {
-	if engineutils.IsDeleteRequest(policyContext) {
-		logger.V(3).Info("skipping CEL validation on deleted resource")
-		return resource, nil
-	}
-
 	// check if there is a policy exception matches the incoming resource
 	exception := engineutils.MatchesException(exceptions, policyContext, logger)
 	if exception != nil {
@@ -76,23 +71,31 @@ func (h validateCELHandler) Process(
 
 	// get resource's name, namespace, GroupVersionResource, and GroupVersionKind
 	gvr := schema.GroupVersionResource(policyContext.RequestResource())
-	gvk := resource.GroupVersionKind()
-	namespaceName := resource.GetNamespace()
-	resourceName := resource.GetName()
-	resourceKind, _ := policyContext.ResourceKind()
+	gvk, _ := policyContext.ResourceKind()
 	policyKind := policyContext.Policy().GetKind()
 	policyName := policyContext.Policy().GetName()
 
-	object := resource.DeepCopyObject()
-	// in case of update request, set the oldObject to the current resource before it gets updated
-	var oldObject runtime.Object
+	// in case of UPDATE requests, set the oldObject to the current resource before it gets updated
+	var object, oldObject runtime.Object
 	oldResource := policyContext.OldResource()
 	if oldResource.Object == nil {
 		oldObject = nil
 	} else {
 		oldObject = oldResource.DeepCopyObject()
 	}
 
+	var ns, name string
+	// in case of DELETE request, get the name and the namespace from the old object
+	if resource.Object == nil {
+		ns = oldResource.GetNamespace()
+		name = oldResource.GetName()
+		object = nil
+	} else {
+		ns = resource.GetNamespace()
+		name = resource.GetName()
+		object = resource.DeepCopyObject()
+	}
+
 	// check if the rule uses parameter resources
 	hasParam := rule.Validation.CEL.HasParam()
 	// extract preconditions written as CEL expressions
@@ -129,11 +132,11 @@ func (h validateCELHandler) Process(
 	// Special case, the namespace object has the namespace of itself.
 	// unset it if the incoming object is a namespace
 	if gvk.Kind == "Namespace" && gvk.Version == "v1" && gvk.Group == "" {
-		namespaceName = ""
+		ns = ""
 	}
-	if namespaceName != "" {
+	if ns != "" {
 		if h.client != nil {
-			namespace, err = h.client.GetNamespace(ctx, namespaceName, metav1.GetOptions{})
+			namespace, err = h.client.GetNamespace(ctx, ns, metav1.GetOptions{})
 			if err != nil {
 				return resource, handlers.WithResponses(
 					engineapi.RuleError(rule.Name, engineapi.Validation, "Error getting the resource's namespace", err),
@@ -142,24 +145,28 @@ func (h validateCELHandler) Process(
 		} else {
 			namespace = &corev1.Namespace{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: namespaceName,
+					Name: ns,
 				},
 			}
 		}
 	}
 
 	requestInfo := policyContext.AdmissionInfo()
 	userInfo := internal.NewUser(requestInfo.AdmissionUserInfo.Username, requestInfo.AdmissionUserInfo.UID, requestInfo.AdmissionUserInfo.Groups)
-	admissionAttributes := admission.NewAttributesRecord(object, oldObject, gvk, namespaceName, resourceName, gvr, "", admission.Operation(policyContext.Operation()), nil, false, &userInfo)
-	versionedAttr, _ := admission.NewVersionedAttributes(admissionAttributes, admissionAttributes.GetKind(), nil)
-	authorizer := internal.NewAuthorizer(h.client, resourceKind)
+	attr := admission.NewAttributesRecord(object, oldObject, gvk, ns, name, gvr, "", admission.Operation(policyContext.Operation()), nil, false, &userInfo)
+	o := admission.NewObjectInterfacesFromScheme(runtime.NewScheme())
+	versionedAttr, err := admission.NewVersionedAttributes(attr, attr.GetKind(), o)
+	if err != nil {
+		return resource, handlers.WithError(rule, engineapi.Validation, "error while creating versioned attributes", err)
+	}
+	authorizer := internal.NewAuthorizer(h.client, gvk)
 	// validate the incoming object against the rule
 	var validationResults []validatingadmissionpolicy.ValidateResult
 	if hasParam {
 		paramKind := rule.Validation.CEL.ParamKind
 		paramRef := rule.Validation.CEL.ParamRef
 
-		params, err := collectParams(ctx, h.client, paramKind, paramRef, namespaceName)
+		params, err := collectParams(ctx, h.client, paramKind, paramRef, ns)
 		if err != nil {
 			return resource, handlers.WithResponses(
 				engineapi.RuleError(rule.Name, engineapi.Validation, "error in parameterized resource", err),

test/conformance/chainsaw/validate/clusterpolicy/cornercases/cel-messages-upon-resource-failure/policy.yaml

@@ -14,6 +14,9 @@ spec:
         - resources:
             kinds:
               - Pod
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         message: "hostPort must either be unset or set to 0"
         cel:

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/authorizor-checks/with-permissions/policy.yaml

@@ -12,6 +12,9 @@ spec:
         - resources:
             kinds:
               - Pod
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         cel:
           expressions:

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/authorizor-checks/without-permissions/policy.yaml

@@ -12,6 +12,9 @@ spec:
         - resources:
             kinds:
               - Deployment
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         cel:
           expressions:

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/cel-preconditions/policy.yaml

@@ -12,6 +12,9 @@ spec:
         - resources:
             kinds:
               - Pod
+            operations:
+              - CREATE
+              - UPDATE
       celPreconditions:
           - name: "first match condition in CEL"
             expression: "object.metadata.name.matches('nginx-pod')"

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/cel-variables/policy.yaml

@@ -12,6 +12,9 @@ spec:
         - resources:
             kinds:
               - Deployment
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         cel:
           variables:

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/check-statefulset-namespace/policy.yaml

@@ -12,6 +12,9 @@ spec:
         - resources:
             kinds:
               - StatefulSet
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         cel:
           expressions:

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/deny/README.md

@@ -0,0 +1,11 @@
+## Description
+
+This test creates a policy that uses CEL expressions to deny the creation/deletion/update of any pod.
+
+## Expected Behavior
+
+Any pod creation/deletion/update is blocked.
+
+## Reference Issue(s)
+
+10576

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/deny/chainsaw-test.yaml

@@ -0,0 +1,32 @@
+apiVersion: chainsaw.kyverno.io/v1alpha1
+kind: Test
+metadata:
+  creationTimestamp: null
+  name: deny
+spec:
+  steps:
+  - name: step-01
+    try:
+    - script:
+        content: kubectl run nginx --image=nginx
+  - name: step-02
+    try:
+    - apply:
+        file: policy.yaml
+    - assert:
+        file: policy-assert.yaml
+  - name: step-03
+    try:
+    - script:
+        content: "if kubectl run --image=busybox foo\nthen \n  exit 1\nelse \n  exit
+          0\nfi\n"
+  - name: step-04
+    try:
+    - script:
+        content: "if kubectl label pod nginx app=nginx\nthen \n  exit 1\nelse \n  exit
+          0\nfi\n"
+  - name: step-05
+    try:
+    - script:
+        content: "if kubectl delete pod nginx\nthen \n  exit 1\nelse \n  exit
+          0\nfi\n"

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/deny/policy-assert.yaml

@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: restrict-operations-on-pod
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/deny/policy.yaml

@@ -0,0 +1,19 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: restrict-operations-on-pod
+spec:
+  validationFailureAction: Enforce
+  background: true
+  rules:
+  - name: rule-1
+    match:
+      any:
+      - resources:
+          kinds:
+          - Pod
+    validate:
+      cel:
+        expressions:
+          - expression: "false"
+            message: Create, update and delete on pods is not allowed

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/disallow-host-port/pod-fail.yaml

@@ -1,10 +1,11 @@
 apiVersion: v1
 kind: Pod
 metadata:
-  name: webserver
+  name: webserver-2
 spec:
   containers:
   - name: webserver
     image: nginx:latest
     ports:
     - hostPort: 80
+      containerPort: 80

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/disallow-host-port/pod-pass.yaml

@@ -1,7 +1,7 @@
 apiVersion: v1
 kind: Pod
 metadata:
-  name: webserver
+  name: webserver-1
 spec:
   containers:
   - name: webserver

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/disallow-host-port/policy.yaml

@@ -14,6 +14,9 @@ spec:
         - resources:
             kinds:
               - Pod
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         cel:
           expressions:

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/parameter-resources/clusterscoped/policy.yaml

@@ -12,6 +12,9 @@ spec:
         - resources:
             kinds:
               - Namespace
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         cel:
           paramKind: 

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/parameter-resources/namespaced/match-clusterscoped-resource/policy.yaml

@@ -12,6 +12,9 @@ spec:
         - resources:
             kinds:
               - Namespace
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         cel:
           paramKind: 

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/parameter-resources/namespaced/set-paramref-namespace/policy.yaml

@@ -12,6 +12,9 @@ spec:
         - resources:
             kinds:
               - Deployment
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         cel:
           paramKind: 

test/conformance/chainsaw/validate/clusterpolicy/standard/cel/parameter-resources/namespaced/unset-paramref-namespace/policy.yaml

@@ -12,6 +12,9 @@ spec:
         - resources:
             kinds:
               - StatefulSet
+            operations:
+              - CREATE
+              - UPDATE
       validate:
         cel:
           paramKind: