fix: load policy and add tests (#4515)

* fix: load policy and add tests

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

* fix callers

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
Co-authored-by: Vyankatesh Kudtarkar <vyankateshkd@gmail.com>

cmd/cli/kubectl-kyverno/utils/common/common.go

@@ -30,7 +30,7 @@ import (
 	"github.com/kyverno/kyverno/pkg/engine/variables"
 	"github.com/kyverno/kyverno/pkg/policymutation"
 	"github.com/kyverno/kyverno/pkg/policyreport"
-	"github.com/kyverno/kyverno/pkg/utils"
+	yamlutils "github.com/kyverno/kyverno/pkg/utils/yaml"
 	yamlv2 "gopkg.in/yaml.v2"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -164,7 +164,7 @@ func GetPolicies(paths []string) (policies []kyvernov1.PolicyInterface, errors [
 				}
 			}
 
-			policiesFromFile, errFromFile := utils.GetPolicy(fileBytes)
+			policiesFromFile, errFromFile := yamlutils.GetPolicy(fileBytes)
 			if errFromFile != nil {
 				err := fmt.Errorf("failed to process %s: %v", path, errFromFile.Error())
 				errors = append(errors, err)
@@ -631,7 +631,7 @@ func GetPoliciesFromPaths(fs billy.Filesystem, dirPath []string, isGit bool, pol
 				fmt.Printf("failed to convert to JSON: %v", err)
 				continue
 			}
-			policiesFromFile, errFromFile := utils.GetPolicy(policyBytes)
+			policiesFromFile, errFromFile := yamlutils.GetPolicy(policyBytes)
 			if errFromFile != nil {
 				fmt.Printf("failed to process : %v", errFromFile.Error())
 				continue
@@ -647,7 +647,7 @@ func GetPoliciesFromPaths(fs billy.Filesystem, dirPath []string, isGit bool, pol
 					policyStr = policyStr + scanner.Text() + "\n"
 				}
 				yamlBytes := []byte(policyStr)
-				policies, err = utils.GetPolicy(yamlBytes)
+				policies, err = yamlutils.GetPolicy(yamlBytes)
 				if err != nil {
 					return nil, sanitizederror.NewWithError("failed to extract the resources", err)
 				}

cmd/cli/kubectl-kyverno/utils/common/common_test.go

@@ -5,7 +5,7 @@ import (
 
 	v1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1"
 	"github.com/kyverno/kyverno/pkg/toggle"
-	ut "github.com/kyverno/kyverno/pkg/utils"
+	yamlutils "github.com/kyverno/kyverno/pkg/utils/yaml"
 	"gotest.tools/assert"
 )
 
@@ -98,7 +98,7 @@ func Test_NamespaceSelector(t *testing.T) {
 
 	rc := &ResultCounts{}
 	for _, tc := range testcases {
-		policyArray, _ := ut.GetPolicy(tc.policy)
+		policyArray, _ := yamlutils.GetPolicy(tc.policy)
 		resourceArray, _ := GetResource(tc.resource)
 		ApplyPolicyOnResource(policyArray[0], resourceArray[0], "", false, nil, v1beta1.RequestInfo{}, false, tc.namespaceSelectorMap, false, rc, false, nil)
 		assert.Equal(t, int64(rc.Pass), int64(tc.result.Pass))

pkg/autogen/autogen_test.go

@@ -10,7 +10,7 @@ import (
 	"testing"
 
 	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/utils"
+	yamlutils "github.com/kyverno/kyverno/pkg/utils/yaml"
 	"gotest.tools/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -298,7 +298,7 @@ func Test_Any(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -336,7 +336,7 @@ func Test_All(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -375,7 +375,7 @@ func Test_Exclude(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -409,7 +409,7 @@ func Test_CronJobOnly(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -439,7 +439,7 @@ func Test_ForEachPod(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -474,7 +474,7 @@ func Test_CronJob_hasExclude(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -511,7 +511,7 @@ func Test_CronJobAndDeployment(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -542,7 +542,7 @@ func Test_UpdateVariablePath(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -572,7 +572,7 @@ func Test_Deny(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -610,7 +610,7 @@ func Test_ValidatePodSecurity(t *testing.T) {
 	if err != nil {
 		t.Log(err)
 	}
-	policies, err := utils.GetPolicy(file)
+	policies, err := yamlutils.GetPolicy(file)
 	if err != nil {
 		t.Log(err)
 	}
@@ -810,7 +810,7 @@ kA==
 	}
 
 	for _, test := range testCases {
-		policies, err := utils.GetPolicy([]byte(test.policy))
+		policies, err := yamlutils.GetPolicy([]byte(test.policy))
 		assert.NilError(t, err)
 		assert.Equal(t, 1, len(policies))
 		rules := computeRules(policies[0])

pkg/policy/allowed_vars_test.go

@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"testing"
 
-	ut "github.com/kyverno/kyverno/pkg/utils"
+	yamlutils "github.com/kyverno/kyverno/pkg/utils/yaml"
 	"gotest.tools/assert"
 	"k8s.io/apimachinery/pkg/util/yaml"
 )
@@ -54,7 +54,7 @@ func TestNotAllowedVars_MatchSection(t *testing.T) {
     }
   `)
 
-	policy, err := ut.GetPolicy(policyWithVarInMatch)
+	policy, err := yamlutils.GetPolicy(policyWithVarInMatch)
 	assert.NilError(t, err)
 
 	err = hasInvalidVariables(policy[0], false)
@@ -106,7 +106,7 @@ func TestNotAllowedVars_ExcludeSection(t *testing.T) {
     }
   `)
 
-	policy, err := ut.GetPolicy(policyWithVarInExclude)
+	policy, err := yamlutils.GetPolicy(policyWithVarInExclude)
 	assert.NilError(t, err)
 
 	err = hasInvalidVariables(policy[0], false)
@@ -159,7 +159,7 @@ func TestNotAllowedVars_ExcludeSection_PositiveCase(t *testing.T) {
     }
   `)
 
-	policy, err := ut.GetPolicy(policyWithVarInExclude)
+	policy, err := yamlutils.GetPolicy(policyWithVarInExclude)
 	assert.NilError(t, err)
 
 	err = hasInvalidVariables(policy[0], false)
@@ -193,7 +193,7 @@ func TestNotAllowedVars_JSONPatchPath(t *testing.T) {
     }
     }`)
 
-	policy, err := ut.GetPolicy(policyWithVarInExclude)
+	policy, err := yamlutils.GetPolicy(policyWithVarInExclude)
 	assert.NilError(t, err)
 
 	err = hasInvalidVariables(policy[0], false)
@@ -236,7 +236,7 @@ func TestNotAllowedVars_JSONPatchPath_ContextPositive(t *testing.T) {
     }
     }`)
 
-	policy, err := ut.GetPolicy(policyWithVarInExclude)
+	policy, err := yamlutils.GetPolicy(policyWithVarInExclude)
 	assert.NilError(t, err)
 
 	err = hasInvalidVariables(policy[0], false)
@@ -270,7 +270,7 @@ func TestNotAllowedVars_JSONPatchPath_PositiveCase(t *testing.T) {
     }
     }`)
 
-	policy, err := ut.GetPolicy(policyWithVarInExclude)
+	policy, err := yamlutils.GetPolicy(policyWithVarInExclude)
 	assert.NilError(t, err)
 
 	err = hasInvalidVariables(policy[0], false)
@@ -302,7 +302,7 @@ spec:
 	policyJSON, err := yaml.ToJSON(policyYAML)
 	assert.NilError(t, err)
 
-	policy, err := ut.GetPolicy(policyJSON)
+	policy, err := yamlutils.GetPolicy(policyJSON)
 	assert.NilError(t, err)
 
 	err = hasInvalidVariables(policy[0], false)
@@ -389,7 +389,7 @@ func TestNotAllowedVars_VariableFormats(t *testing.T) {
               value: "foo.com"
     `, tc.input))
 
-		policy, err := ut.GetPolicy(policyYAML)
+		policy, err := yamlutils.GetPolicy(policyYAML)
 		assert.NilError(t, err)
 
 		err = hasInvalidVariables(policy[0], false)
@@ -435,7 +435,7 @@ spec:
 	policyJSON, err := yaml.ToJSON(policyYAML)
 	assert.NilError(t, err)
 
-	policy, err := ut.GetPolicy(policyJSON)
+	policy, err := yamlutils.GetPolicy(policyJSON)
 	assert.NilError(t, err)
 
 	err = hasInvalidVariables(policy[0], false)

pkg/utils/yaml/loadpolicy.go

@@ -1,18 +1,17 @@
-package utils
+package yaml
 
 import (
 	"encoding/json"
 	"fmt"
 
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
-	yamlutils "github.com/kyverno/kyverno/pkg/utils/yaml"
 	"k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-// GetPolicy - extracts policies from YAML bytes
+// GetPolicy extracts policies from YAML bytes
 func GetPolicy(bytes []byte) (policies []kyvernov1.PolicyInterface, err error) {
-	documents, err := yamlutils.SplitDocuments(bytes)
+	documents, err := SplitDocuments(bytes)
 	if err != nil {
 		return nil, err
 	}
@@ -32,11 +31,12 @@ func GetPolicy(bytes []byte) (policies []kyvernov1.PolicyInterface, err error) {
 		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.Namespace != "" || (policy.Namespace == "" && policy.Kind == "Policy") {
+		if policy.Kind == "Policy" {
 			if policy.Namespace == "" {
 				policy.Namespace = "default"
 			}
-			policy.Kind = "ClusterPolicy"
+		} else {
+			policy.Namespace = ""
 		}
 		policies = append(policies, policy)
 	}

pkg/utils/yaml/loadpolicy_test.go

@@ -0,0 +1,250 @@
+package yaml
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetPolicy(t *testing.T) {
+	type args struct {
+		bytes []byte
+	}
+	type policy struct {
+		kind      string
+		namespace string
+	}
+	tests := []struct {
+		name         string
+		args         args
+		wantPolicies []policy
+		wantErr      bool
+	}{{
+		name: "policy",
+		args: args{
+			[]byte(`
+apiVersion: kyverno.io/v1
+kind: Policy
+metadata:
+  name: generate-policy
+  namespace: ns-1
+spec:
+  rules:
+  - name: copy-game-demo
+    match:
+      resources:
+        kinds:
+        - Namespace
+    exclude:
+      resources:
+        namespaces:
+        - kube-system
+        - default
+        - kube-public
+        - kyverno
+    generate:
+      kind: ConfigMap
+      name: game-demo
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      clone:
+        namespace: default
+        name: game-demo
+`),
+		},
+		wantPolicies: []policy{
+			{"Policy", "ns-1"},
+		},
+		wantErr: false,
+	}, {
+		name: "policy without ns",
+		args: args{
+			[]byte(`
+apiVersion: kyverno.io/v1
+kind: Policy
+metadata:
+  name: generate-policy
+spec:
+  rules:
+  - name: copy-game-demo
+    match:
+      resources:
+        kinds:
+        - Namespace
+    exclude:
+      resources:
+        namespaces:
+        - kube-system
+        - default
+        - kube-public
+        - kyverno
+    generate:
+      kind: ConfigMap
+      name: game-demo
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      clone:
+        namespace: default
+        name: game-demo
+`),
+		},
+		wantPolicies: []policy{
+			{"Policy", "default"},
+		},
+		wantErr: false,
+	}, {
+		name: "cluster policy",
+		args: args{
+			[]byte(`
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: generate-policy
+spec:
+  rules:
+  - name: copy-game-demo
+    match:
+      resources:
+        kinds:
+        - Namespace
+    exclude:
+      resources:
+        namespaces:
+        - kube-system
+        - default
+        - kube-public
+        - kyverno
+    generate:
+      kind: ConfigMap
+      name: game-demo
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      clone:
+        namespace: default
+        name: game-demo
+`),
+		},
+		wantPolicies: []policy{
+			{"ClusterPolicy", ""},
+		},
+		wantErr: false,
+	}, {
+		name: "cluster policy with ns",
+		args: args{
+			[]byte(`
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: generate-policy
+  namespace: ns-1
+spec:
+  rules:
+  - name: copy-game-demo
+    match:
+      resources:
+        kinds:
+        - Namespace
+    exclude:
+      resources:
+        namespaces:
+        - kube-system
+        - default
+        - kube-public
+        - kyverno
+    generate:
+      kind: ConfigMap
+      name: game-demo
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      clone:
+        namespace: default
+        name: game-demo
+`),
+		},
+		wantPolicies: []policy{
+			{"ClusterPolicy", ""},
+		},
+		wantErr: false,
+	}, {
+		name: "policy and cluster policy",
+		args: args{
+			[]byte(`
+apiVersion: kyverno.io/v1
+kind: Policy
+metadata:
+  name: generate-policy
+  namespace: ns-1
+spec:
+  rules:
+  - name: copy-game-demo
+    match:
+      resources:
+        kinds:
+        - Namespace
+    exclude:
+      resources:
+        namespaces:
+        - kube-system
+        - default
+        - kube-public
+        - kyverno
+    generate:
+      kind: ConfigMap
+      name: game-demo
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      clone:
+        namespace: default
+        name: game-demo
+---
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: generate-policy
+spec:
+  rules:
+  - name: copy-game-demo
+    match:
+      resources:
+        kinds:
+        - Namespace
+    exclude:
+      resources:
+        namespaces:
+        - kube-system
+        - default
+        - kube-public
+        - kyverno
+    generate:
+      kind: ConfigMap
+      name: game-demo
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      clone:
+        namespace: default
+        name: game-demo
+`),
+		},
+		wantPolicies: []policy{
+			{"Policy", "ns-1"},
+			{"ClusterPolicy", ""},
+		},
+		wantErr: false,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotPolicies, err := GetPolicy(tt.args.bytes)
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				if assert.Equal(t, len(tt.wantPolicies), len(gotPolicies)) {
+					for i := range tt.wantPolicies {
+						assert.Equal(t, tt.wantPolicies[i].kind, gotPolicies[i].GetKind())
+						assert.Equal(t, tt.wantPolicies[i].namespace, gotPolicies[i].GetNamespace())
+					}
+				}
+			}
+		})
+	}
+}