- support AllowMissingPathOnRemove and EnsurePathExistsOnAdd in patchesJSON6902

- upgrade to evanphx/json-patch/v5

Signed-off-by: Shuting Zhao <shutting06@gmail.com>

go.mod

@@ -5,7 +5,7 @@ go 1.14
 require (
 	github.com/cenkalti/backoff v2.2.1+incompatible
 	github.com/cornelk/hashmap v1.0.1
-	github.com/evanphx/json-patch v4.9.0+incompatible
+	github.com/evanphx/json-patch/v5 v5.2.0
 	github.com/fatih/color v1.9.0
 	github.com/fsnotify/fsnotify v1.4.9 // indirect
 	github.com/gardener/controller-manager-library v0.2.0

go.sum

@@ -188,6 +188,8 @@ github.com/evanphx/json-patch v4.5.0+incompatible h1:ouOWdg56aJriqS0huScTkVXPC5I
 github.com/evanphx/json-patch v4.5.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
 github.com/evanphx/json-patch v4.9.0+incompatible h1:kLcOMZeuLAJvL2BPWLMIj5oaZQobrkAqrL+WFZwQses=
 github.com/evanphx/json-patch v4.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
+github.com/evanphx/json-patch/v5 v5.2.0 h1:8ozOH5xxoMYDt5/u+yMTsVXydVCbTORFnOOoq2lumco=
+github.com/evanphx/json-patch/v5 v5.2.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
 github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
 github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=

pkg/engine/context/context.go

@@ -6,7 +6,7 @@ import (
 	"strings"
 	"sync"
 
-	jsonpatch "github.com/evanphx/json-patch"
+	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
 	"k8s.io/api/admission/v1beta1"

pkg/engine/mutate/overlay.go

@@ -10,14 +10,13 @@ import (
 	"strings"
 	"time"
 
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"sigs.k8s.io/controller-runtime/pkg/log"
-
-	jsonpatch "github.com/evanphx/json-patch"
+	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/go-logr/logr"
 	commonAnchors "github.com/kyverno/kyverno/pkg/engine/anchor/common"
 	"github.com/kyverno/kyverno/pkg/engine/response"
 	"github.com/kyverno/kyverno/pkg/engine/utils"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 // ProcessOverlay processes mutation overlay on the resource

pkg/engine/mutate/overlayCondition.go

@@ -26,6 +26,10 @@ func checkConditions(log logr.Logger, resource, overlay interface{}, path string
 	// return false if anchor exists in overlay
 	// condition never be true in this case
 	if reflect.TypeOf(resource) != reflect.TypeOf(overlay) {
+		if resource == nil {
+			return "", overlayError{}
+		}
+
 		if hasNestedAnchors(overlay) {
 			log.V(4).Info(fmt.Sprintf("element type mismatch at path %s: overlay %T, resource %T", path, overlay, resource))
 			return path, newOverlayError(conditionFailure,

pkg/engine/mutate/overlay_test.go

@@ -5,7 +5,7 @@ import (
 	"reflect"
 	"testing"
 
-	jsonpatch "github.com/evanphx/json-patch"
+	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/kyverno/kyverno/pkg/engine/utils"
 	"gotest.tools/assert"
 	"sigs.k8s.io/controller-runtime/pkg/log"

pkg/engine/mutate/patchJson6902.go

@@ -1,12 +1,11 @@
 package mutate
 
 import (
-	"encoding/json"
 	"fmt"
 	"time"
 
+	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/go-logr/logr"
-	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
 	"github.com/kyverno/kyverno/pkg/engine/response"
 	"github.com/kyverno/kyverno/pkg/engine/utils"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -33,15 +32,26 @@ func ProcessPatchJSON6902(ruleName string, patchesJSON6902 []byte, resource unst
 		return resp, resource
 	}
 
-	patchedResourceRaw, err := utils.ApplyPatchNew(resourceRaw, patchesJSON6902)
-	// patchedResourceRaw, err := patchJSON6902(string(resourceRaw), mutation.PatchesJSON6902)
+	patchedResourceRaw, err := applyPatchesWithOptions(resourceRaw, patchesJSON6902)
 	if err != nil {
 		resp.Success = false
-		logger.Error(err, "failed to process JSON6902 patches")
-		resp.Message = fmt.Sprintf("failed to process JSON6902 patches: %v", err)
+		logger.Error(err, "unable to apply RFC 6902 patches")
+		resp.Message = fmt.Sprintf("unable to apply RFC 6902 patches: %v", err)
 		return resp, resource
 	}
 
+	patchesBytes, err := generatePatches(resourceRaw, patchedResourceRaw)
+	if err != nil {
+		resp.Success = false
+		logger.Error(err, "unable generate patch bytes from base and patched document, apply patchesJSON6902 directly")
+		resp.Message = fmt.Sprintf("unable generate patch bytes from base and patched document, apply patchesJSON6902 directly: %v", err)
+		return resp, resource
+	}
+
+	for _, p := range patchesBytes {
+		log.V(4).Info("generated RFC 6902 patches", "patch", string(p))
+	}
+
 	err = patchedResource.UnmarshalJSON(patchedResourceRaw)
 	if err != nil {
 		logger.Error(err, "failed to unmarshal resource")
@@ -50,34 +60,32 @@ func ProcessPatchJSON6902(ruleName string, patchesJSON6902 []byte, resource unst
 		return resp, resource
 	}
 
-	var decodedPatch []kyverno.Patch
-	err = json.Unmarshal(patchesJSON6902, &decodedPatch)
-	if err != nil {
-		resp.Success = false
-		resp.Message = err.Error()
-		return resp, resource
-	}
-
-	patchesBytes, err := utils.TransformPatches(decodedPatch)
-	if err != nil {
-		logger.Error(err, "failed to marshal patches to bytes array")
-		resp.Success = false
-		resp.Message = fmt.Sprintf("failed to marshal patches to bytes array: %v", err)
-		return resp, resource
-	}
-
-	for _, p := range patchesBytes {
-		log.V(6).Info("", "patches", string(p))
-	}
-
-	// JSON patches processed successfully
 	resp.Success = true
 	resp.Message = fmt.Sprintf("successfully process JSON6902 patches")
 	resp.Patches = patchesBytes
 	return resp, patchedResource
 }
 
+func applyPatchesWithOptions(resource, patch []byte) ([]byte, error) {
+	patches, err := jsonpatch.DecodePatch(patch)
+	if err != nil {
+		return resource, fmt.Errorf("failed to decode patches: %v", err)
+	}
+
+	options := &jsonpatch.ApplyOptions{AllowMissingPathOnRemove: true, EnsurePathExistsOnAdd: true}
+	patchedResource, err := patches.ApplyWithOptions(resource, options)
+	if err != nil {
+		return resource, err
+	}
+
+	return patchedResource, nil
+}
+
 func convertPatchesToJSON(patchesJSON6902 string) ([]byte, error) {
+	if len(patchesJSON6902) == 0 {
+		return []byte(patchesJSON6902), nil
+	}
+
 	if patchesJSON6902[0] != '[' {
 		// If the patch doesn't look like a JSON6902 patch, we
 		// try to parse it to json.

pkg/engine/mutate/patchJson6902_test.go

@@ -1,32 +1,15 @@
 package mutate
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/ghodss/yaml"
-	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
 	assert "github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-const input = `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: myDeploy
-spec:
-  replica: 2
-  template:
-    metadata:
-      labels:
-        old-label: old-value
-    spec:
-      containers:
-      - image: nginx
-        name: nginx
-`
-
 var inputBytes = []byte(`
 apiVersion: apps/v1
 kind: Deployment
@@ -45,34 +28,34 @@ spec:
 `)
 
 func TestTypeConversion(t *testing.T) {
-
-	mutateRule := kyverno.Mutation{
-		PatchesJSON6902: `
+	patchesJSON6902 := []byte(`
 - op: replace
   path: /spec/template/spec/containers/0/name
   value: my-nginx
-`,
-	}
+`)
 
 	expectedPatches := [][]byte{
-		[]byte(`{"path":"/spec/template/spec/containers/0/name","op":"replace","value":"my-nginx"}`),
+		[]byte(`{"op":"replace","path":"/spec/template/spec/containers/0/name","value":"my-nginx"}`),
 	}
 
 	// serialize resource
-	inputJSONgo, err := yaml.YAMLToJSON(inputBytes)
+	inputJSON, err := yaml.YAMLToJSON(inputBytes)
 	assert.Nil(t, err)
 
 	var resource unstructured.Unstructured
-	err = resource.UnmarshalJSON(inputJSONgo)
+	err = resource.UnmarshalJSON(inputJSON)
 	assert.Nil(t, err)
 
+	jsonPatches, err := yaml.YAMLToJSON(patchesJSON6902)
+	assert.Nil(t, err)
 	// apply patches
-	resp, _ := ProcessPatchJSON6902("type-conversion", mutateRule, resource, log.Log)
+	resp, _ := ProcessPatchJSON6902("type-conversion", jsonPatches, resource, log.Log)
 	if !assert.Equal(t, true, resp.Success) {
 		t.Fatal(resp.Message)
 	}
 
-	assert.Equal(t, expectedPatches, resp.Patches)
+	assert.Equal(t, expectedPatches, resp.Patches,
+		fmt.Sprintf("expectedPatches: %s\ngeneratedPatches: %s", string(expectedPatches[0]), string(resp.Patches[0])))
 }
 
 func TestJsonPatch(t *testing.T) {
@@ -166,11 +149,11 @@ spec:
   path: /spec/replica
   value: 999
 - op: add
-  path: /spec/template/spec/containers/0/command
+  path: /spec/template/spec/volumes
   value:
-  - arg1
-  - arg2
-  - arg3
+  - emptyDir:
+      medium: Memory
+    name: vault-secret
 `,
 			expected: []byte(`
 apiVersion: apps/v1
@@ -185,12 +168,12 @@ spec:
         old-label: old-value
     spec:
       containers:
-      - command:
-        - arg1
-        - arg2
-        - arg3
-        image: nginx
+      - image: nginx
         name: my-nginx
+      volumes:
+      - emptyDir:
+          medium: Memory
+        name: vault-secret
 `),
 		},
 	}
@@ -201,9 +184,16 @@ spec:
 			expectedBytes, err := yaml.YAMLToJSON(testCase.expected)
 			assert.Nil(t, err)
 
-			out, err := patchJSON6902(input, testCase.patches)
+			inputBytes, err := yaml.YAMLToJSON(inputBytes)
+			assert.Nil(t, err)
 
-			if !assert.Equal(t, string(expectedBytes), string(out)) {
+			patches, err := yaml.YAMLToJSON([]byte(testCase.patches))
+			assert.Nil(t, err)
+
+			out, err := applyPatchesWithOptions(inputBytes, patches)
+			assert.Nil(t, err)
+
+			if !assert.Equal(t, string(expectedBytes), string(out), testCase.testName) {
 				t.FailNow()
 			}
 		})

pkg/engine/mutate/patchesUtils.go

@@ -8,7 +8,7 @@ import (
 	"strconv"
 	"strings"
 
-	evanjsonpatch "github.com/evanphx/json-patch"
+	evanjsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/go-logr/logr"
 	"github.com/mattbaird/jsonpatch"
 	"github.com/minio/minio/pkg/wildcard"
@@ -18,8 +18,11 @@ import (
 func generatePatches(src, dst []byte) ([][]byte, error) {
 	var patchesBytes [][]byte
 	pp, err := jsonpatch.CreatePatch(src, dst)
-	sortedPatches := filterAndSortPatches(pp)
+	if err != nil {
+		return nil, err
+	}
 
+	sortedPatches := filterAndSortPatches(pp)
 	for _, p := range sortedPatches {
 		pbytes, err := p.MarshalJSON()
 		if err != nil {
@@ -186,13 +189,13 @@ func preProcessJSONPatches(patchesJSON6902 []byte, resource unstructured.Unstruc
 
 		resourceObj, err := getObject(path, resource.UnstructuredContent())
 		if err != nil {
-			log.V(4).Info("failed to get object by the given path", "path", path, "error", err.Error())
+			log.V(4).Info("unable to get object by the given path, proceed patchesJson6902 without preprocessing", "path", path, "error", err.Error())
 			continue
 		}
 
 		val, err := patch.ValueInterface()
 		if err != nil {
-			log.V(4).Info("failed to get value by the given path", "path", path, "error", err.Error())
+			log.V(4).Info("unable to get value by the given path, proceed patchesJson6902 without preprocessing", "path", path, "error", err.Error())
 			continue
 		}
 

pkg/engine/mutate/patchesUtils_test.go

@@ -5,8 +5,6 @@ import (
 	"fmt"
 	"testing"
 
-	v1 "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/engine/utils"
 	"github.com/mattbaird/jsonpatch"
 	assertnew "github.com/stretchr/testify/assert"
 	"gotest.tools/assert"
@@ -20,28 +18,17 @@ func Test_GeneratePatches(t *testing.T) {
 	out, err := strategicMergePatch(string(baseBytes), string(overlayBytes))
 	assert.NilError(t, err)
 
+	expectedPatches := [][]byte{
+		[]byte(`{"op":"remove","path":"/spec/template/spec/containers/0"}`),
+		[]byte(`{"op":"add","path":"/spec/template/spec/containers/0","value":{"image":"nginx","name":"nginx"}}`),
+		[]byte(`{"op":"add","path":"/spec/template/spec/containers/1","value":{"env":[{"name":"WORDPRESS_DB_HOST","value":"$(MYSQL_SERVICE)"},{"name":"WORDPRESS_DB_PASSWORD","valueFrom":{"secretKeyRef":{"key":"password","name":"mysql-pass"}}}],"image":"wordpress:4.8-apache","name":"wordpress","ports":[{"containerPort":80,"name":"wordpress"}],"volumeMounts":[{"mountPath":"/var/www/html","name":"wordpress-persistent-storage"}]}}`),
+		[]byte(`{"op":"add","path":"/spec/template/spec/initContainers","value":[{"command":["echo $(WORDPRESS_SERVICE)","echo $(MYSQL_SERVICE)"],"image":"debian","name":"init-command"}]}`),
+	}
 	patches, err := generatePatches(baseBytes, out)
 	assert.NilError(t, err)
 
-	var overlay unstructured.Unstructured
-	err = json.Unmarshal(baseBytes, &overlay)
-	assert.NilError(t, err)
-
-	bb, err := json.Marshal(overlay.Object)
-	assert.NilError(t, err)
-
-	res, err := utils.ApplyPatches(bb, patches)
-	assert.NilError(t, err)
-
-	var ep unstructured.Unstructured
-	err = json.Unmarshal(expectBytes, &ep)
-	assert.NilError(t, err)
-
-	eb, err := json.Marshal(ep.Object)
-	assert.NilError(t, err)
-
-	if !assertnew.Equal(t, string(eb), string(res)) {
-		t.FailNow()
+	for i, expect := range expectedPatches {
+		assertnew.Equal(t, string(expect), string(patches[i]))
 	}
 }
 
@@ -175,79 +162,36 @@ var podBytes = []byte(`
 `)
 
 func Test_preProcessJSONPatches_skip(t *testing.T) {
-	var policyBytes = []byte(`
-{
-  "apiVersion": "kyverno.io/v1",
-  "kind": "ClusterPolicy",
-  "metadata": {
-      "name": "insert-container"
-  },
-  "spec": {
-      "rules": [
-          {
-              "name": "insert-container",
-              "match": {
-                  "resources": {
-                      "kinds": [
-                          "Pod"
-                      ]
-                  }
-              },
-              "mutate": {
-                  "patchesJson6902": "- op: add\n  path: /spec/containers/1\n  value: {\"name\":\"nginx-new\",\"image\":\"nginx:latest\"}"
-              }
-          }
-      ]
-  }
-}
+	patchesJSON6902 := []byte(`
+- op: add
+  path: /spec/containers/1
+  value: {"name":"nginx-new","image":"nginx:latest"}
 `)
-
 	var pod unstructured.Unstructured
-	var policy v1.ClusterPolicy
-
 	assertnew.Nil(t, json.Unmarshal(podBytes, &pod))
-	assertnew.Nil(t, yaml.Unmarshal(policyBytes, &policy))
 
-	skip, err := preProcessJSONPatches(policy.Spec.Rules[0].Mutation, pod, log.Log)
+	patches, err := yaml.YAMLToJSON(patchesJSON6902)
+	assertnew.Nil(t, err)
+
+	skip, err := preProcessJSONPatches(patches, pod, log.Log)
 	assertnew.Nil(t, err)
 	assertnew.Equal(t, true, skip)
 }
 
 func Test_preProcessJSONPatches_not_skip(t *testing.T) {
-	var policyBytes = []byte(`
-{
-  "apiVersion": "kyverno.io/v1",
-  "kind": "ClusterPolicy",
-  "metadata": {
-      "name": "insert-container"
-  },
-  "spec": {
-      "rules": [
-          {
-              "name": "insert-container",
-              "match": {
-                  "resources": {
-                      "kinds": [
-                          "Pod"
-                      ]
-                  }
-              },
-              "mutate": {
-                  "patchesJson6902": "- op: add\n  path: /spec/containers/1\n  value: {\"name\":\"my-new-container\",\"image\":\"nginx:latest\"}"
-              }
-          }
-      ]
-  }
-}
+	patchesJSON6902 := []byte(`
+- op: add
+  path: /spec/containers/1
+  value: {"name":"my-new-container","image":"nginx:latest"}
 `)
 
+	patches, err := yaml.YAMLToJSON(patchesJSON6902)
+	assertnew.Nil(t, err)
+
 	var pod unstructured.Unstructured
-	var policy v1.ClusterPolicy
-
 	assertnew.Nil(t, json.Unmarshal(podBytes, &pod))
-	assertnew.Nil(t, yaml.Unmarshal(policyBytes, &policy))
 
-	skip, err := preProcessJSONPatches(policy.Spec.Rules[0].Mutation, pod, log.Log)
+	skip, err := preProcessJSONPatches(patches, pod, log.Log)
 	assertnew.Nil(t, err)
 	assertnew.Equal(t, false, skip)
 }

pkg/engine/utils/utils.go

@@ -1,10 +1,7 @@
 package utils
 
 import (
-	"encoding/json"
-
-	jsonpatch "github.com/evanphx/json-patch"
-	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
+	jsonpatch "github.com/evanphx/json-patch/v5"
 	commonAnchor "github.com/kyverno/kyverno/pkg/engine/anchor/common"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -57,12 +54,12 @@ func ApplyPatches(resource []byte, patches [][]byte) ([]byte, error) {
 func ApplyPatchNew(resource, patch []byte) ([]byte, error) {
 	jsonpatch, err := jsonpatch.DecodePatch(patch)
 	if err != nil {
-		return nil, err
+		return resource, err
 	}
 
 	patchedResource, err := jsonpatch.Apply(resource)
 	if err != nil {
-		return nil, err
+		return resource, err
 	}
 
 	return patchedResource, err
@@ -87,19 +84,6 @@ func JoinPatches(patches [][]byte) []byte {
 	return result
 }
 
-// TransformPatches converts mutation.Patches to bytes array
-func TransformPatches(patches []kyverno.Patch) (patchesBytes [][]byte, err error) {
-	for _, patch := range patches {
-		patchRaw, err := json.Marshal(patch)
-		if err != nil {
-			return nil, err
-		}
-		patchesBytes = append(patchesBytes, patchRaw)
-	}
-
-	return patchesBytes, nil
-}
-
 //ConvertToUnstructured converts the resource to unstructured format
 func ConvertToUnstructured(data []byte) (*unstructured.Unstructured, error) {
 	resource := &unstructured.Unstructured{}

pkg/kyverno/common/common.go

@@ -12,7 +12,7 @@ import (
 	"path/filepath"
 	"strings"
 
-	jsonpatch "github.com/evanphx/json-patch"
+	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-logr/logr"
 	v1 "github.com/kyverno/kyverno/pkg/api/kyverno/v1"

pkg/policy/apply.go

@@ -7,7 +7,7 @@ import (
 	"strings"
 	"time"
 
-	jsonpatch "github.com/evanphx/json-patch"
+	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
 	client "github.com/kyverno/kyverno/pkg/dclient"

pkg/policymutation/policymutation.go

@@ -8,7 +8,7 @@ import (
 	"strconv"
 	"strings"
 
-	jsonpatch "github.com/evanphx/json-patch"
+	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
 	"github.com/kyverno/kyverno/pkg/common"

pkg/webhooks/annotations.go

@@ -4,7 +4,7 @@ import (
 	"encoding/json"
 	"strings"
 
-	jsonpatch "github.com/evanphx/json-patch"
+	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/go-logr/logr"
 	"github.com/kyverno/kyverno/pkg/engine/response"
 	yamlv2 "gopkg.in/yaml.v2"