From 5a6b3c86f6dc14f726410ca00f998bbba69f0f43 Mon Sep 17 00:00:00 2001
From: shuting <shuting@nirmata.com>
Date: Mon, 24 Apr 2023 00:22:29 +0800
Subject: [PATCH] fix background variables validation (#6978)

Signed-off-by: ShutingZhao <shuting@nirmata.com>
---
 pkg/policy/validate.go                        | 18 ++++++++++++++++
 pkg/policy/validate_test.go                   |  2 +-
 pkg/utils/yaml/loadpolicy.go                  | 21 ++++++++++++-------
 .../01-policy.yaml                            |  6 ++++++
 .../02-policy.yaml                            |  5 +++++
 .../background-variables-update/README.md     | 11 ++++++++++
 .../policy-assert.yaml                        |  9 ++++++++
 .../policy-update.yaml                        | 20 ++++++++++++++++++
 .../background-variables-update/policy.yaml   | 20 ++++++++++++++++++
 9 files changed, 103 insertions(+), 9 deletions(-)
 create mode 100644 test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/01-policy.yaml
 create mode 100644 test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/02-policy.yaml
 create mode 100644 test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/README.md
 create mode 100644 test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy-assert.yaml
 create mode 100644 test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy-update.yaml
 create mode 100644 test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy.yaml

diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go
index d3817d4d27..689efe030a 100644
--- a/pkg/policy/validate.go
+++ b/pkg/policy/validate.go
@@ -505,11 +505,29 @@ func ruleForbiddenSectionsHaveVariables(rule *kyvernov1.Rule) error {
 
 // hasVariables - check for variables in the policy
 func hasVariables(policy kyvernov1.PolicyInterface) [][]string {
+	policy = cleanup(policy)
 	policyRaw, _ := json.Marshal(policy)
 	matches := regex.RegexVariables.FindAllStringSubmatch(string(policyRaw), -1)
 	return matches
 }
 
+func cleanup(policy kyvernov1.PolicyInterface) kyvernov1.PolicyInterface {
+	ann := policy.GetAnnotations()
+	if ann != nil {
+		ann["kubectl.kubernetes.io/last-applied-configuration"] = ""
+		policy.SetAnnotations(ann)
+	}
+	if policy.GetNamespace() == "" {
+		pol := policy.(*kyvernov1.ClusterPolicy)
+		pol.Status.Autogen.Rules = nil
+		return pol
+	} else {
+		pol := policy.(*kyvernov1.Policy)
+		pol.Status.Autogen.Rules = nil
+		return pol
+	}
+}
+
 func jsonPatchPathHasVariables(patch string) error {
 	jsonPatch, err := yaml.ToJSON([]byte(patch))
 	if err != nil {
diff --git a/pkg/policy/validate_test.go b/pkg/policy/validate_test.go
index 58fd74f583..2e3aec08a7 100644
--- a/pkg/policy/validate_test.go
+++ b/pkg/policy/validate_test.go
@@ -1085,7 +1085,7 @@ func Test_Namespced_Policy(t *testing.T) {
 	  }
 	`)
 
-	var policy *kyverno.ClusterPolicy
+	var policy *kyverno.Policy
 	err := json.Unmarshal(rawPolicy, &policy)
 	assert.NilError(t, err)
 
diff --git a/pkg/utils/yaml/loadpolicy.go b/pkg/utils/yaml/loadpolicy.go
index 1cf79c7f2c..3087e081bb 100644
--- a/pkg/utils/yaml/loadpolicy.go
+++ b/pkg/utils/yaml/loadpolicy.go
@@ -49,26 +49,31 @@ func GetPolicy(bytes []byte) (policies []kyvernov1.PolicyInterface, err error) {
 }
 
 func addPolicy(policies []kyvernov1.PolicyInterface, us *unstructured.Unstructured) ([]kyvernov1.PolicyInterface, error) {
-	policy := &kyvernov1.ClusterPolicy{}
+	var policy kyvernov1.PolicyInterface
+	if us.GetKind() == "ClusterPolicy" {
+		policy = &kyvernov1.ClusterPolicy{}
+	} else {
+		policy = &kyvernov1.Policy{}
+	}
 
 	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(us.Object, policy); err != nil {
 		return nil, fmt.Errorf("failed to decode policy: %v", err)
 	}
 
-	if policy.TypeMeta.Kind == "" {
+	if policy.GetKind() == "" {
 		log.V(3).Info("skipping file as policy.TypeMeta.Kind not found")
 		return policies, nil
 	}
-	if policy.TypeMeta.Kind != "ClusterPolicy" && policy.TypeMeta.Kind != "Policy" {
-		return nil, fmt.Errorf("resource %s/%s is not a Policy or a ClusterPolicy", policy.Kind, policy.Name)
+	if policy.GetKind() != "ClusterPolicy" && policy.GetKind() != "Policy" {
+		return nil, fmt.Errorf("resource %s/%s is not a Policy or a ClusterPolicy", policy.GetKind(), policy.GetName())
 	}
 
-	if policy.Kind == "Policy" {
-		if policy.Namespace == "" {
-			policy.Namespace = "default"
+	if policy.GetKind() == "Policy" {
+		if policy.GetNamespace() == "" {
+			policy.SetNamespace("default")
 		}
 	} else {
-		policy.Namespace = ""
+		policy.SetNamespace("")
 	}
 	policies = append(policies, policy)
 	return policies, nil
diff --git a/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/01-policy.yaml b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/01-policy.yaml
new file mode 100644
index 0000000000..b20ef0bd7d
--- /dev/null
+++ b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/01-policy.yaml
@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- policy.yaml
+assert:
+- policy-assert.yaml
\ No newline at end of file
diff --git a/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/02-policy.yaml b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/02-policy.yaml
new file mode 100644
index 0000000000..585b404599
--- /dev/null
+++ b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/02-policy.yaml
@@ -0,0 +1,5 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- file: policy-update.yaml
+  shouldFail: false
diff --git a/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/README.md b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/README.md
new file mode 100644
index 0000000000..3a5e1c3007
--- /dev/null
+++ b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/README.md
@@ -0,0 +1,11 @@
+## Description
+
+This test ensures the background policy update that does not contain admission userinfo variables should be allowed.
+
+## Expected Behavior
+
+The policy update should pass through.
+
+## Related Issue
+
+https://github.com/kyverno/kyverno/issues/6938
diff --git a/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy-assert.yaml b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy-assert.yaml
new file mode 100644
index 0000000000..277982879c
--- /dev/null
+++ b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy-assert.yaml
@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: background-variables-update
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready
\ No newline at end of file
diff --git a/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy-update.yaml b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy-update.yaml
new file mode 100644
index 0000000000..3d67a52e6f
--- /dev/null
+++ b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy-update.yaml
@@ -0,0 +1,20 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: background-variables-update
+spec:
+  validationFailureAction: Audit
+  background: true
+  rules:
+  - name: ns-vars-userinfo
+    match:
+      any:
+      - resources:
+          kinds:
+            - Pod
+    validate:
+      message: The `owner` label is required for all Namespaces.
+      pattern:
+        metadata:
+          labels:
+            owner: foo
\ No newline at end of file
diff --git a/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy.yaml b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy.yaml
new file mode 100644
index 0000000000..90e89fba89
--- /dev/null
+++ b/test/conformance/kuttl/policy-validation/cluster-policy/background-variables-update/policy.yaml
@@ -0,0 +1,20 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: background-variables-update
+spec:
+  validationFailureAction: Audit
+  background: false
+  rules:
+  - name: ns-vars-userinfo
+    match:
+      any:
+      - resources:
+          kinds:
+            - Pod
+    validate:
+      message: The `owner` label is required for all Namespaces.
+      pattern:
+        metadata:
+          labels:
+            owner: "{{request.userInfo}}"
\ No newline at end of file