fix: mutation unit test not working as expected (#8188)

* fix: mutation unit test not working as expected

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* package and unit tests

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

---------

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

cmd/cli/kubectl-kyverno/test/test.go

@@ -6,7 +6,6 @@ import (
 	"regexp"
 	"strings"
 
-	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/go-git/go-billy/v5"
 	"github.com/kyverno/kyverno/api/kyverno"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
@@ -18,6 +17,7 @@ import (
 	pathutils "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/path"
 	sanitizederror "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/sanitizedError"
 	"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/store"
+	unstructuredutils "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/unstructured"
 	"github.com/kyverno/kyverno/pkg/autogen"
 	"github.com/kyverno/kyverno/pkg/background/generate"
 	"github.com/kyverno/kyverno/pkg/clients/dclient"
@@ -600,37 +600,6 @@ func isNamespacedPolicy(policyNames string) (bool, error) {
 	return regexp.MatchString("^[a-z]*/[a-z]*", policyNames)
 }
 
-func tidyObject(obj interface{}) interface{} {
-	switch typedPatternElement := obj.(type) {
-	case map[string]interface{}:
-		tidy := map[string]interface{}{}
-		for k, v := range typedPatternElement {
-			v = tidyObject(v)
-			if v != nil {
-				tidy[k] = v
-			}
-		}
-		if len(tidy) == 0 {
-			return nil
-		}
-		return tidy
-	case []interface{}:
-		var tidy []interface{}
-		for _, v := range typedPatternElement {
-			v = tidyObject(v)
-			if v != nil {
-				tidy = append(tidy, v)
-			}
-		}
-		if len(tidy) == 0 {
-			return nil
-		}
-		return tidy
-	default:
-		return obj
-	}
-}
-
 // getAndCompareResource --> Get the patchedResource or generatedResource from the path provided by user
 // And compare this resource with engine generated resource.
 func getAndCompareResource(path string, engineResource unstructured.Unstructured, isGit bool, policyResourcePath string, fs billy.Filesystem, isGenerate bool) string {
@@ -653,28 +622,13 @@ func getAndCompareResource(path string, engineResource unstructured.Unstructured
 			status = "pass"
 		}
 	} else {
-		userResource = unstructured.Unstructured{Object: tidyObject(userResource.UnstructuredContent()).(map[string]interface{})}
-		expected, err := userResource.MarshalJSON()
-		if err != nil {
-			fmt.Printf("Error: failed to convert patched resource to json (%s)\n", err)
-			return status
-		}
-		engineResource = unstructured.Unstructured{Object: tidyObject(engineResource.UnstructuredContent()).(map[string]interface{})}
-		actual, err := engineResource.MarshalJSON()
-		if err != nil {
-			fmt.Printf("Error: failed to convert engine resource to json (%s)\n", err)
-			return status
-		}
-		patch, err := jsonpatch.CreateMergePatch(actual, expected)
-		if err != nil {
-			fmt.Printf("Error: failed to calculate diff between patched and engine resources (%s)\n", err)
-			return status
-		}
-		if len(patch) > 2 {
-			log.Log.V(3).Info("patchedResource mismatch", "actual", string(actual), "expected", string(expected), "patch", string(patch))
-			status = "fail"
-		} else {
-			status = "pass"
+		equals, err := unstructuredutils.Compare(engineResource, userResource, true)
+		if err == nil {
+			if !equals {
+				status = "fail"
+			} else {
+				status = "pass"
+			}
 		}
 	}
 	return status

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

@@ -0,0 +1,66 @@
+package unstructured
+
+import (
+	jsonpatch "github.com/evanphx/json-patch/v5"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+func TidyObject(obj interface{}) interface{} {
+	switch typedPatternElement := obj.(type) {
+	case map[string]interface{}:
+		tidy := map[string]interface{}{}
+		for k, v := range typedPatternElement {
+			v = TidyObject(v)
+			if v != nil {
+				tidy[k] = v
+			}
+		}
+		if len(tidy) == 0 {
+			return nil
+		}
+		return tidy
+	case []interface{}:
+		var tidy []interface{}
+		for _, v := range typedPatternElement {
+			v = TidyObject(v)
+			if v != nil {
+				tidy = append(tidy, v)
+			}
+		}
+		if len(tidy) == 0 {
+			return nil
+		}
+		return tidy
+	default:
+		return obj
+	}
+}
+
+func Tidy(obj unstructured.Unstructured) unstructured.Unstructured {
+	if obj.Object == nil {
+		return obj
+	}
+	return unstructured.Unstructured{
+		Object: TidyObject(obj.UnstructuredContent()).(map[string]interface{}),
+	}
+}
+
+func Compare(a, b unstructured.Unstructured, tidy bool) (bool, error) {
+	if tidy {
+		a = Tidy(a)
+		b = Tidy(b)
+	}
+	expected, err := a.MarshalJSON()
+	if err != nil {
+		return false, err
+	}
+	actual, err := b.MarshalJSON()
+	if err != nil {
+		return false, err
+	}
+	patch, err := jsonpatch.CreateMergePatch(actual, expected)
+	if err != nil {
+		return false, err
+	}
+	return len(patch) == 2, nil
+}

cmd/cli/kubectl-kyverno/utils/unstructured/unstructured_test.go

@@ -0,0 +1,186 @@
+package unstructured
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+func TestTidyObject(t *testing.T) {
+	tests := []struct {
+		name string
+		obj  interface{}
+		want interface{}
+	}{{
+		obj:  "string",
+		want: "string",
+	}, {
+		obj:  map[string]interface{}{},
+		want: nil,
+	}, {
+		obj:  nil,
+		want: nil,
+	}, {
+		obj:  []interface{}{},
+		want: nil,
+	}, {
+		obj: map[string]interface{}{
+			"map": nil,
+		},
+		want: nil,
+	}, {
+		obj: map[string]interface{}{
+			"map": map[string]interface{}{},
+		},
+		want: nil,
+	}, {
+		obj: map[string]interface{}{
+			"map": map[string]interface{}{
+				"foo": "bar",
+			},
+		},
+		want: map[string]interface{}{
+			"map": map[string]interface{}{
+				"foo": "bar",
+			},
+		},
+	}, {
+		obj:  []interface{}{[]interface{}{}},
+		want: nil,
+	}, {
+		obj:  []interface{}{1},
+		want: []interface{}{1},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := TidyObject(tt.obj); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("TidyObject() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestTidy(t *testing.T) {
+	tests := []struct {
+		name string
+		obj  unstructured.Unstructured
+		want unstructured.Unstructured
+	}{{
+		obj:  unstructured.Unstructured{},
+		want: unstructured.Unstructured{},
+	}, {
+		obj: unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"map": map[string]interface{}{
+					"foo": "bar",
+				},
+			},
+		},
+		want: unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"map": map[string]interface{}{
+					"foo": "bar",
+				},
+			},
+		},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := Tidy(tt.obj); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Tidy() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestCompare(t *testing.T) {
+	tests := []struct {
+		name    string
+		a       unstructured.Unstructured
+		b       unstructured.Unstructured
+		tidy    bool
+		want    bool
+		wantErr bool
+	}{{
+		a:       unstructured.Unstructured{},
+		b:       unstructured.Unstructured{},
+		tidy:    true,
+		want:    true,
+		wantErr: false,
+	}, {
+		a:       unstructured.Unstructured{},
+		b:       unstructured.Unstructured{},
+		tidy:    false,
+		want:    true,
+		wantErr: false,
+	}, {
+		a: unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"map": map[string]interface{}{
+					"foo": "bar",
+				},
+			},
+		},
+		b: unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"map": map[string]interface{}{
+					"foo": "bar",
+				},
+			},
+		},
+		tidy:    true,
+		want:    true,
+		wantErr: false,
+	}, {
+		a: unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"map": map[string]interface{}{
+					"foo": "bar",
+					"bar": map[string]interface{}{},
+				},
+			},
+		},
+		b: unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"map": map[string]interface{}{
+					"foo": "bar",
+				},
+			},
+		},
+		tidy:    true,
+		want:    true,
+		wantErr: false,
+	}, {
+		a: unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"map": map[string]interface{}{
+					"foo": "bar",
+					"bar": map[string]interface{}{},
+				},
+			},
+		},
+		b: unstructured.Unstructured{
+			Object: map[string]interface{}{
+				"map": map[string]interface{}{
+					"foo": "bar",
+				},
+			},
+		},
+		tidy:    false,
+		want:    false,
+		wantErr: false,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := Compare(tt.a, tt.b, tt.tidy)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Compare() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if got != tt.want {
+				t.Errorf("Compare() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

test/cli/test-mutate/patched-resource/kyverno-test.yaml

@@ -0,0 +1,20 @@
+name: add-default-resources-test
+policies:
+  - policy.yaml
+resources:
+  - resource.yaml
+variables: variables.yaml
+results:
+  - policy: add-default-resources
+    rule: add-default-requests
+    resource: nginx-demo
+    patchedResource: patched-resource.yaml
+    kind: Pod
+    result: pass
+values:
+  policies:
+  - name: add-default-resources
+    resources:
+    - name: nginx-demo
+      values:
+        request.operation: CREATE

test/cli/test-mutate/patched-resource/patched-resource.yaml

@@ -0,0 +1,12 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: nginx-demo
+spec:
+  containers:
+  - name: nginx
+    image: nginx:1.14.2
+    resources:
+      requests:
+        memory: "100Mi"
+        cpu: "100m"

test/cli/test-mutate/patched-resource/policy.yaml

@@ -0,0 +1,29 @@
+apiVersion : kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: add-default-resources
+spec:
+  background: false
+  rules:
+  - name: add-default-requests
+    match:
+      any:
+      - resources:
+          kinds:
+          - Pod
+    preconditions:
+      any:
+      - key: "{{request.operation}}"
+        operator: In
+        value:
+        - CREATE
+        - UPDATE
+    mutate:
+      patchStrategicMerge:
+        spec:
+          containers:
+            - (name): "*"
+              resources:
+                requests:
+                  +(memory): "100Mi"
+                  +(cpu): "100m"

test/cli/test-mutate/patched-resource/resource.yaml

@@ -0,0 +1,8 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: nginx-demo
+spec:
+  containers:
+  - name: nginx
+    image: nginx:1.14.2