Merge pull request #49259 from dixudx/fix_jsonpatch_nil_value_merge

Automatic merge from submit-queue (batch tested with PRs 49259, 49350)

update json-patch to fix nil value issue when creating mergepatch

**What this PR does / why we need it**:
When [creating a patch for merge](https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/annotate.go#L255), nil value will be considered as different value. This has been fixed and merged in [evanphx/json-patch #45](https://github.com/evanphx/json-patch/pull/45).

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #49044

**Special notes for your reviewer**:
/cc @MikeSpreitzer @mengqiy 

**Release note**:

```release-note
Fix nil value issue when creating json patch for merge
```

Godeps/Godeps.json

@@ -1090,7 +1090,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "ba18e35c5c1b36ef6334cad706eb681153d2d379"
+			"Rev": "944e07253867aacae43c04b2e6a239005443f33a"
 		},
 		{
 			"ImportPath": "github.com/exponent-io/jsonpath",

pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/3.request

@@ -1,7 +1,7 @@
 {
     "metadata": {
         "annotations": {
-            "kubectl.kubernetes.io~1last-applied-configuration": "{\"apiVersion\":\"v1\",\"data\":{\"baz\":\"qux\",\"foo\":\"changed-value\",\"new-data\":\"new-value\",\"new-data2\":\"new-value\",\"new-data3\":\"newivalue\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"cm1\",\"namespace\":\"myproject\"}}\n"
+            "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"data\":{\"baz\":\"qux\",\"foo\":\"changed-value\",\"new-data\":\"new-value\",\"new-data2\":\"new-value\",\"new-data3\":\"newivalue\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"cm1\",\"namespace\":\"myproject\"}}\n"
         }
     }
 }

pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list/4.request

@@ -1,7 +1,7 @@
 {
     "metadata": {
         "annotations": {
-            "kubectl.kubernetes.io~1last-applied-configuration": "{\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"svc1\",\"new-label\":\"foo\",\"new-label2\":\"foo2\"},\"name\":\"svc1\",\"namespace\":\"myproject\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":82,\"protocol\":\"TCP\",\"targetPort\":81}],\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
+            "kubectl.kubernetes.io/last-applied-configuration": "{\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"svc1\",\"new-label\":\"foo\",\"new-label2\":\"foo2\"},\"name\":\"svc1\",\"namespace\":\"myproject\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":82,\"protocol\":\"TCP\",\"targetPort\":81}],\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
         }
     }
 }

pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-syntax-error/3.request

@@ -1,7 +1,7 @@
 {
     "metadata": {
         "annotations": {
-            "kubectl.kubernetes.io~1last-applied-configuration": "{\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"svc1\",\"new-label\":\"foo\",\"new-label1\":\"foo1\"},\"name\":\"svc1\",\"namespace\":\"myproject\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":81,\"protocol\":\"TCP\",\"targetPort\":81}],\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
+            "kubectl.kubernetes.io/last-applied-configuration": "{\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"svc1\",\"new-label\":\"foo\",\"new-label1\":\"foo1\"},\"name\":\"svc1\",\"namespace\":\"myproject\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":81,\"protocol\":\"TCP\",\"targetPort\":81}],\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
         }
     }
 }

pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied/2.request

@@ -1,7 +1,7 @@
 {
     "metadata": {
         "annotations": {
-            "kubectl.kubernetes.io~1last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"creationTimestamp\":\"2017-02-01T21:14:09Z\",\"labels\":{\"app\":\"svc1\",\"new-label\":\"new-value\"},\"name\":\"svc1\",\"namespace\":\"myproject\",\"resourceVersion\":\"20820\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":81,\"protocol\":\"TCP\",\"targetPort\":92}],\"selector\":{\"app\":\"svc1\"},\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
+            "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Service\",\"metadata\":{\"annotations\":{},\"creationTimestamp\":\"2017-02-01T21:14:09Z\",\"labels\":{\"app\":\"svc1\",\"new-label\":\"new-value\"},\"name\":\"svc1\",\"namespace\":\"myproject\",\"resourceVersion\":\"20820\"},\"spec\":{\"ports\":[{\"name\":\"80\",\"port\":81,\"protocol\":\"TCP\",\"targetPort\":92}],\"selector\":{\"app\":\"svc1\"},\"sessionAffinity\":\"None\",\"type\":\"ClusterIP\"},\"status\":{\"loadBalancer\":{}}}\n"
         }
     }
 }

staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json

@@ -104,7 +104,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "ba18e35c5c1b36ef6334cad706eb681153d2d379"
+			"Rev": "944e07253867aacae43c04b2e6a239005443f33a"
 		},
 		{
 			"ImportPath": "github.com/ghodss/yaml",

staging/src/k8s.io/apimachinery/Godeps/Godeps.json

@@ -40,7 +40,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "ba18e35c5c1b36ef6334cad706eb681153d2d379"
+			"Rev": "944e07253867aacae43c04b2e6a239005443f33a"
 		},
 		{
 			"ImportPath": "github.com/ghodss/yaml",

staging/src/k8s.io/apiserver/Godeps/Godeps.json

@@ -324,7 +324,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "ba18e35c5c1b36ef6334cad706eb681153d2d379"
+			"Rev": "944e07253867aacae43c04b2e6a239005443f33a"
 		},
 		{
 			"ImportPath": "github.com/ghodss/yaml",

staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json

@@ -112,7 +112,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "ba18e35c5c1b36ef6334cad706eb681153d2d379"
+			"Rev": "944e07253867aacae43c04b2e6a239005443f33a"
 		},
 		{
 			"ImportPath": "github.com/ghodss/yaml",

staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json

@@ -104,7 +104,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "ba18e35c5c1b36ef6334cad706eb681153d2d379"
+			"Rev": "944e07253867aacae43c04b2e6a239005443f33a"
 		},
 		{
 			"ImportPath": "github.com/ghodss/yaml",

vendor/github.com/evanphx/json-patch/.travis.yml

@@ -1,13 +1,15 @@
 language: go
 
 go:
-  - 1.4
+  - 1.8
-  - 1.3
+  - 1.7
 
 install:
   - if ! go get code.google.com/p/go.tools/cmd/cover; then go get golang.org/x/tools/cmd/cover; fi
+  - go get github.com/jessevdk/go-flags
 
 script:
+  - go get
   - go test -cover ./...
 
 notifications:

vendor/github.com/evanphx/json-patch/merge.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
-	"strings"
 )
 
 func merge(cur, patch *lazyNode, mergeMerge bool) *lazyNode {
@@ -28,7 +27,6 @@ func merge(cur, patch *lazyNode, mergeMerge bool) *lazyNode {
 
 func mergeDocs(doc, patch *partialDoc, mergeMerge bool) {
 	for k, v := range *patch {
-		k := decodePatchKey(k)
 		if v == nil {
 			if mergeMerge {
 				(*doc)[k] = nil
@@ -226,6 +224,9 @@ func matchesValue(av, bv interface{}) bool {
 		if bt == at {
 			return true
 		}
+	case nil:
+		// Both nil, fine.
+		return true
 	case map[string]interface{}:
 		bt := bv.(map[string]interface{})
 		for key := range at {
@@ -250,16 +251,15 @@ func matchesValue(av, bv interface{}) bool {
 func getDiff(a, b map[string]interface{}) (map[string]interface{}, error) {
 	into := map[string]interface{}{}
 	for key, bv := range b {
-		escapedKey := encodePatchKey(key)
 		av, ok := a[key]
 		// value was added
 		if !ok {
-			into[escapedKey] = bv
+			into[key] = bv
 			continue
 		}
 		// If types have changed, replace completely
 		if reflect.TypeOf(av) != reflect.TypeOf(bv) {
-			into[escapedKey] = bv
+			into[key] = bv
 			continue
 		}
 		// Types are the same, compare values
@@ -272,23 +272,23 @@ func getDiff(a, b map[string]interface{}) (map[string]interface{}, error) {
 				return nil, err
 			}
 			if len(dst) > 0 {
-				into[escapedKey] = dst
+				into[key] = dst
 			}
 		case string, float64, bool:
 			if !matchesValue(av, bv) {
-				into[escapedKey] = bv
+				into[key] = bv
 			}
 		case []interface{}:
 			bt := bv.([]interface{})
 			if !matchesArray(at, bt) {
-				into[escapedKey] = bv
+				into[key] = bv
 			}
 		case nil:
 			switch bv.(type) {
 			case nil:
 				// Both nil, fine.
 			default:
-				into[escapedKey] = bv
+				into[key] = bv
 			}
 		default:
 			panic(fmt.Sprintf("Unknown type:%T in key %s", av, key))
@@ -303,23 +303,3 @@ func getDiff(a, b map[string]interface{}) (map[string]interface{}, error) {
 	}
 	return into, nil
 }
-
-// From http://tools.ietf.org/html/rfc6901#section-4 :
-//
-// Evaluation of each reference token begins by decoding any escaped
-// character sequence.  This is performed by first transforming any
-// occurrence of the sequence '~1' to '/', and then transforming any
-// occurrence of the sequence '~0' to '~'.
-
-var (
-	rfc6901Encoder = strings.NewReplacer("~", "~0", "/", "~1")
-	rfc6901Decoder = strings.NewReplacer("~1", "/", "~0", "~")
-)
-
-func decodePatchKey(k string) string {
-	return rfc6901Decoder.Replace(k)
-}
-
-func encodePatchKey(k string) string {
-	return rfc6901Encoder.Replace(k)
-}

vendor/github.com/evanphx/json-patch/patch.go

@@ -369,6 +369,15 @@ func (d *partialArray) add(key string, val *lazyNode) error {
 
 	cur := *d
 
+	if idx < 0 {
+		idx *= -1
+
+		if idx > len(ary) {
+			return fmt.Errorf("Unable to access invalid index: %d", idx)
+		}
+		idx = len(ary) - idx
+	}
+
 	copy(ary[0:idx], cur[0:idx])
 	ary[idx] = val
 	copy(ary[idx+1:], cur[idx:])
@@ -446,6 +455,11 @@ func (p Patch) replace(doc *container, op operation) error {
 		return fmt.Errorf("jsonpatch replace operation does not apply: doc is missing path: %s", path)
 	}
 
+	val, ok := con.get(key)
+	if val == nil || ok != nil {
+		return fmt.Errorf("jsonpatch replace operation does not apply: doc is missing key: %s", path)
+	}
+
 	return con.set(key, op.value())
 }
 
@@ -508,6 +522,31 @@ func (p Patch) test(doc *container, op operation) error {
 	return fmt.Errorf("Testing value %s failed", path)
 }
 
+func (p Patch) copy(doc *container, op operation) error {
+	from := op.from()
+
+	con, key := findObject(doc, from)
+
+	if con == nil {
+		return fmt.Errorf("jsonpatch copy operation does not apply: doc is missing from path: %s", from)
+	}
+
+	val, err := con.get(key)
+	if err != nil {
+		return err
+	}
+
+	path := op.path()
+
+	con, key = findObject(doc, path)
+
+	if con == nil {
+		return fmt.Errorf("jsonpatch copy operation does not apply: doc is missing destination path: %s", path)
+	}
+
+	return con.set(key, val)
+}
+
 // Equal indicates if 2 JSON documents have the same structural equality.
 func Equal(a, b []byte) bool {
 	ra := make(json.RawMessage, len(a))
@@ -570,6 +609,8 @@ func (p Patch) ApplyIndent(doc []byte, indent string) ([]byte, error) {
 			err = p.move(&pd, op)
 		case "test":
 			err = p.test(&pd, op)
+		case "copy":
+			err = p.copy(&pd, op)
 		default:
 			err = fmt.Errorf("Unexpected kind: %s", op.kind())
 		}
@@ -585,3 +626,18 @@ func (p Patch) ApplyIndent(doc []byte, indent string) ([]byte, error) {
 
 	return json.Marshal(pd)
 }
+
+// From http://tools.ietf.org/html/rfc6901#section-4 :
+//
+// Evaluation of each reference token begins by decoding any escaped
+// character sequence.  This is performed by first transforming any
+// occurrence of the sequence '~1' to '/', and then transforming any
+// occurrence of the sequence '~0' to '~'.
+
+var (
+	rfc6901Decoder = strings.NewReplacer("~1", "/", "~0", "~")
+)
+
+func decodePatchKey(k string) string {
+	return rfc6901Decoder.Replace(k)
+}