From f07007f8649e6cb0315abaa4e6fc4aa60dd8136b Mon Sep 17 00:00:00 2001
From: Khaled Emara <khaled.emara@nirmata.com>
Date: Tue, 5 Nov 2024 13:59:09 +0200
Subject: [PATCH] fix(validate): custom match conditions errors (#11461)

* fix(validate): custom match conditions errors

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* test(webhook): failing match conditions

---------

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>
Co-authored-by: Mariam Fahmy <mariam.fahmy@nirmata.com>
---
 pkg/validation/policy/validate.go             | 22 +++++++++++++++++++
 .../match-conditions-fail/README.md           |  7 ++++++
 .../match-conditions-fail/chainsaw-test.yaml  | 13 +++++++++++
 .../match-conditions-fail/policy.yaml         | 21 ++++++++++++++++++
 4 files changed, 63 insertions(+)
 create mode 100644 test/conformance/chainsaw/webhook-configurations/match-conditions-fail/README.md
 create mode 100755 test/conformance/chainsaw/webhook-configurations/match-conditions-fail/chainsaw-test.yaml
 create mode 100644 test/conformance/chainsaw/webhook-configurations/match-conditions-fail/policy.yaml

diff --git a/pkg/validation/policy/validate.go b/pkg/validation/policy/validate.go
index fb27fbd108..9fb8cfbb8d 100644
--- a/pkg/validation/policy/validate.go
+++ b/pkg/validation/policy/validate.go
@@ -27,15 +27,18 @@ import (
 	"github.com/kyverno/kyverno/pkg/engine/variables/operator"
 	"github.com/kyverno/kyverno/pkg/engine/variables/regex"
 	"github.com/kyverno/kyverno/pkg/logging"
+	celutils "github.com/kyverno/kyverno/pkg/utils/cel"
 	datautils "github.com/kyverno/kyverno/pkg/utils/data"
 	kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
 	vaputils "github.com/kyverno/kyverno/pkg/validatingadmissionpolicy"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/apiserver/pkg/admission/plugin/cel"
 	"k8s.io/apiserver/pkg/admission/plugin/policy/validating"
 	"k8s.io/apiserver/pkg/cel/openapi/resolver"
 	"k8s.io/client-go/discovery"
@@ -148,6 +151,13 @@ func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interf
 	var errs field.ErrorList
 	specPath := field.NewPath("spec")
 
+	mc := spec.GetMatchConditions()
+	if mc != nil {
+		if err := ValidateCustomWebhookMatchConditions(spec.GetMatchConditions()); err != nil {
+			return warnings, err
+		}
+	}
+
 	err := ValidateVariables(policy, background)
 	if err != nil {
 		return warnings, err
@@ -531,6 +541,18 @@ func isGlobalContextEntryReady(name string, gctxentries *kyvernov2alpha1.GlobalC
 	return false
 }
 
+func ValidateCustomWebhookMatchConditions(wc []admissionregistrationv1.MatchCondition) error {
+	c, err := celutils.NewCompiler(nil, nil, wc, nil)
+	if err != nil {
+		return err
+	}
+	f := c.CompileMatchExpressions(cel.OptionalVariableDeclarations{})
+	if len(f.CompilationErrors()) > 0 {
+		return fmt.Errorf("match conditions compilation errors: %v", f.CompilationErrors())
+	}
+	return nil
+}
+
 func ValidateVariables(p kyvernov1.PolicyInterface, backgroundMode bool) error {
 	vars, err := hasVariables(p)
 	if err != nil {
diff --git a/test/conformance/chainsaw/webhook-configurations/match-conditions-fail/README.md b/test/conformance/chainsaw/webhook-configurations/match-conditions-fail/README.md
new file mode 100644
index 0000000000..a8cba2c033
--- /dev/null
+++ b/test/conformance/chainsaw/webhook-configurations/match-conditions-fail/README.md
@@ -0,0 +1,7 @@
+## Description
+
+This test checks whether a Policy with failing match conditions will be accepted or not.
+
+## Expected Behavior
+
+Policy creation should be blocked if the match conditions are failing.
diff --git a/test/conformance/chainsaw/webhook-configurations/match-conditions-fail/chainsaw-test.yaml b/test/conformance/chainsaw/webhook-configurations/match-conditions-fail/chainsaw-test.yaml
new file mode 100755
index 0000000000..381554c7aa
--- /dev/null
+++ b/test/conformance/chainsaw/webhook-configurations/match-conditions-fail/chainsaw-test.yaml
@@ -0,0 +1,13 @@
+apiVersion: chainsaw.kyverno.io/v1alpha1
+kind: Test
+metadata:
+  name: failing-match-conditions
+spec:
+  steps:
+    - name: expect-policy-failure
+      try:
+        - apply:
+            expect:
+              - check:
+                  ($error != null): true
+            file: policy.yaml
diff --git a/test/conformance/chainsaw/webhook-configurations/match-conditions-fail/policy.yaml b/test/conformance/chainsaw/webhook-configurations/match-conditions-fail/policy.yaml
new file mode 100644
index 0000000000..11e67ad352
--- /dev/null
+++ b/test/conformance/chainsaw/webhook-configurations/match-conditions-fail/policy.yaml
@@ -0,0 +1,21 @@
+apiVersion: kyverno.io/v2beta1
+kind: ClusterPolicy
+metadata:
+  name: refresh-env-var-in-pods
+spec:
+  webhookConfiguration:
+    matchConditions:
+      - name: "exclude-managed-pod"
+        expression: '!("ownerReferences" in request.object.metadata.keys(@))'
+  rules:
+    - name: refresh-from-secret-env
+      match:
+        any:
+          - resources:
+              kinds:
+                - Secret
+      validate:
+        pattern:
+          metadata:
+            labels:
+              foo: bar