- support AllowMissingPathOnRemove and EnsurePathExistsOnAdd in patch…

…esJSON6902

- 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,51 +32,60 @@ 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
 	}
 
-	err = patchedResource.UnmarshalJSON(patchedResourceRaw)
+	patchesBytes, err := generatePatches(resourceRaw, patchedResourceRaw)
 	if err != nil {
-		logger.Error(err, "failed to unmarshal resource")
 		resp.Success = false
-		resp.Message = fmt.Sprintf("failed to unmarshal resource: %v", err)
+		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
 	}
 
-	var decodedPatch []kyverno.Patch
-	err = json.Unmarshal(patchesJSON6902, &decodedPatch)
-	if err != nil {
-		resp.Success = false
-		resp.Message = err.Error()
-		return resp, resource
+	for _, p := range patchesBytes {
+		log.V(4).Info("generated RFC 6902 patches", "patch", string(p))
 	}
 
-	patchesBytes, err := utils.TransformPatches(decodedPatch)
+	err = patchedResource.UnmarshalJSON(patchedResourceRaw)
 	if err != nil {
-		logger.Error(err, "failed to marshal patches to bytes array")
+		logger.Error(err, "failed to unmarshal resource")
 		resp.Success = false
-		resp.Message = fmt.Sprintf("failed to marshal patches to bytes array: %v", err)
+		resp.Message = fmt.Sprintf("failed to unmarshal resource: %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)
+
+			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)) {
+			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
 		}