Implement Strategic Merge Patch in apiserver

cmd/integration/integration.go

@@ -598,15 +598,14 @@ func runAtomicPutTest(c *client.Client) {
 
 func runPatchTest(c *client.Client) {
 	name := "patchservice"
+	resource := "services"
 	svcBody := api.Service{
 		TypeMeta: api.TypeMeta{
 			APIVersion: c.APIVersion(),
 		},
 		ObjectMeta: api.ObjectMeta{
 			Name:   name,
-			Labels: map[string]string{
-				"name": name,
-			},
+			Labels: map[string]string{},
 		},
 		Spec: api.ServiceSpec{
 			// This is here because validation requires it.
@@ -625,44 +624,68 @@ func runPatchTest(c *client.Client) {
 	if err != nil {
 		glog.Fatalf("Failed creating patchservice: %v", err)
 	}
-	if len(svc.Labels) != 1 {
-		glog.Fatalf("Original length does not equal one")
+
+	patchBodies := map[api.PatchType]struct {
+		AddLabelBody        []byte
+		RemoveLabelBody     []byte
+		RemoveAllLabelsBody []byte
+	}{
+		api.JSONPatchType: {
+			[]byte(`[{"op":"add","path":"/labels","value":{"foo":"bar","baz":"qux"}}]`),
+			[]byte(`[{"op":"remove","path":"/labels/foo"}]`),
+			[]byte(`[{"op":"remove","path":"/labels"}]`),
+		},
+		api.MergePatchType: {
+			[]byte(`{"labels":{"foo":"bar","baz":"qux"}}`),
+			[]byte(`{"labels":{"foo":null}}`),
+			[]byte(`{"labels":null}`),
+		},
+		api.StrategicMergePatchType: {
+			[]byte(`{"labels":{"foo":"bar","baz":"qux"}}`),
+			[]byte(`{"labels":{"foo":null}}`),
+			[]byte(`{"labels":{"$patch":"replace"}}`),
+		},
 	}
 
+	for k, v := range patchBodies {
 		// add label
-	svc.Labels["foo"] = "bar"
-	if _, err = services.Update(svc); err != nil {
-		glog.Fatalf("Failed updating patchservice: %v", err)
+		_, err := c.Patch(k).Resource(resource).Name(name).Body(v.AddLabelBody).Do().Get()
+		if err != nil {
+			glog.Fatalf("Failed updating patchservice with patch type %s: %v", k, err)
 		}
-	if svc, err = services.Get(name); err != nil {
+		svc, err = services.Get(name)
+		if err != nil {
 			glog.Fatalf("Failed getting patchservice: %v", err)
 		}
-	if len(svc.Labels) != 2 || svc.Labels["foo"] != "bar" {
-		glog.Fatalf("Failed updating patchservice, labels are: %v", svc.Labels)
+		if len(svc.Labels) != 2 || svc.Labels["foo"] != "bar" || svc.Labels["baz"] != "qux" {
+			glog.Fatalf("Failed updating patchservice with patch type %s: labels are: %v", k, svc.Labels)
 		}
 
 		// remove one label
-	delete(svc.Labels, "name")
-	if _, err = services.Update(svc); err != nil {
-		glog.Fatalf("Failed updating patchservice: %v", err)
+		_, err = c.Patch(k).Resource(resource).Name(name).Body(v.RemoveLabelBody).Do().Get()
+		if err != nil {
+			glog.Fatalf("Failed updating patchservice with patch type %s: %v", k, err)
 		}
-	if svc, err = services.Get(name); err != nil {
+		svc, err = services.Get(name)
+		if err != nil {
 			glog.Fatalf("Failed getting patchservice: %v", err)
 		}
-	if len(svc.Labels) != 1 || svc.Labels["foo"] != "bar" {
-		glog.Fatalf("Failed updating patchservice, labels are: %v", svc.Labels)
+		if len(svc.Labels) != 1 || svc.Labels["baz"] != "qux" {
+			glog.Fatalf("Failed updating patchservice with patch type %s: labels are: %v", k, svc.Labels)
 		}
 
 		// remove all labels
-	svc.Labels = nil
-	if _, err = services.Update(svc); err != nil {
-		glog.Fatalf("Failed updating patchservice: %v", err)
+		_, err = c.Patch(k).Resource(resource).Name(name).Body(v.RemoveAllLabelsBody).Do().Get()
+		if err != nil {
+			glog.Fatalf("Failed updating patchservice with patch type %s: %v", k, err)
 		}
-	if svc, err = services.Get(name); err != nil {
+		svc, err = services.Get(name)
+		if err != nil {
 			glog.Fatalf("Failed getting patchservice: %v", err)
 		}
 		if svc.Labels != nil {
-		glog.Fatalf("Failed remove all labels from patchservice: %v", svc.Labels)
+			glog.Fatalf("Failed remove all labels from patchservice with patch type %s: %v", k, svc.Labels)
+		}
 	}
 
 	glog.Info("PATCHs work.")

docs/api-conventions.md

@@ -146,6 +146,7 @@ API resources should use the traditional REST pattern:
 * GET /<resourceNamePlural>/<name> - Retrieves a single resource with the given name, e.g. GET /pods/first returns a Pod named 'first'.
 * DELETE /<resourceNamePlural>/<name>  - Delete the single resource with the given name.
 * PUT /<resourceNamePlural>/<name> - Update or create the resource with the given name with the JSON object provided by the client.
+* PATCH /<resourceNamePlural>/<name> - Selectively modify the specified fields of the resource. See more information [below](#patch).
 
 Kubernetes by convention exposes additional verbs as new root endpoints with singular names. Examples:
 
@@ -160,6 +161,87 @@ When resources wish to expose alternative actions that are closely coupled to a
 
 TODO: more documentation of Watch
 
+### PATCH operations
+
+The API supports three different PATCH operations, determined by their corresponding Content-Type header:
+
+* JSON Patch, `Content-Type: application/json-patch+json`
+ * As defined in [RFC6902](https://tools.ietf.org/html/rfc6902), a JSON Patch is a sequence of operations that are executed on the resource, e.g. `{"op": "add", "path": "/a/b/c", "value": [ "foo", "bar" ]}`. For more details on how to use JSON Patch, see the RFC.
+* Merge Patch, `Content-Type: application/merge-json-patch+json`
+ * As defined in [RFC7386](https://tools.ietf.org/html/rfc7386), a Merge Patch is essentially a partial representation of the resource. The submitted JSON is "merged" with the current resource to create a new one, then the new one is saved. For more details on how to use Merge Patch, see the RFC.
+* Strategic Merge Patch, `Content-Type: application/strategic-merge-json-patch+json`
+ * Strategic Merge Patch is a custom implementation of Merge Patch. For a detailed explanation of how it works and why it needed to be introduced, see below.
+
+#### Strategic Merge Patch
+
+In the standard JSON merge patch, JSON objects are always merged but lists are always replaced. Often that isn't what we want. Let's say we start with the following Pod:
+
+```yaml
+spec:
+  containers:
+    - name: nginx
+      image: nginx-1.0
+```
+
+...and we POST that to the server (as JSON). Then let's say we want to *add* a container to this Pod.
+
+```yaml
+PATCH /v1beta1/pod
+spec:
+  containers:
+    - name: log-tailer
+      image: log-tailer-1.0
+```
+
+If we were to use standard Merge Patch, the entire container list would be replaced with the single log-tailer container. However, our intent is for the container lists to merge together based on the `name` field.
+
+To solve this problem, Strategic Merge Patch uses metadata attached to the API objects to determine what lists should be merged and which ones should not. Currently the metadata is available as struct tags on the API objects themselves, but will become available to clients as Swagger annotations in the future. In the above example, the `patchStrategy` metadata for the `containers` field would be `merge` and the `patchMergeKey` would be `name`.
+
+Note: If the patch results in merging two lists of scalars, the scalars are first deduplicated and then merged.
+
+Strategic Merge Patch also supports special operations as listed below.
+
+### List Operations
+
+To override the container list to be strictly replaced, regardless of the default:
+
+```yaml
+containers:
+  - name: nginx
+    image: nginx-1.0
+  - $patch: replace   # any further $patch operations nested in this list will be ignored
+```
+
+To delete an element of a list that should be merged:
+
+```yaml
+containers:
+  - name: nginx
+    image: nginx-1.0
+  - $patch: delete
+    name: log-tailer  # merge key and value goes here
+```
+
+### Map Operations
+
+To indicate that a map should not be merged and instead should be taken literally:
+
+```yaml
+$patch: replace  # recursive and applies to all fields of the map it's in
+containers:
+- name: nginx
+  image: nginx-1.0
+```
+
+To delete a field of a map:
+
+```yaml
+name: nginx
+image: nginx-1.0
+labels:
+  live: null  # set the value of the map key to null
+```
+
 
 Idempotency
 -----------

pkg/api/types.go

@@ -1688,6 +1688,17 @@ const (
 	PortHeader = "port"
 )
 
+// Similarly to above, these are constants to support HTTP PATCH utilized by
+// both the client and server that didn't make sense for a whole package to be
+// dedicated to.
+type PatchType string
+
+const (
+	JSONPatchType           PatchType = "application/json-patch+json"
+	MergePatchType          PatchType = "application/merge-patch+json"
+	StrategicMergePatchType PatchType = "application/strategic-merge-patch+json"
+)
+
 // Appends the NodeAddresses to the passed-by-pointer slice, only if they do not already exist
 func AddToNodeAddresses(addresses *[]NodeAddress, addAddresses ...NodeAddress) {
 	for _, add := range addAddresses {

pkg/api/v1beta3/types.go

@@ -526,10 +526,10 @@ type Container struct {
 	Args []string `json:"args,omitempty" description:"command array; the docker image's cmd is used if this is not provided; arguments to the entrypoint; cannot be updated"`
 	// Optional: Defaults to Docker's default.
 	WorkingDir     string               `json:"workingDir,omitempty" description:"container's working directory; defaults to image's default; cannot be updated"`
-	Ports          []ContainerPort      `json:"ports,omitempty" description:"list of ports to expose from the container; cannot be updated"`
-	Env            []EnvVar             `json:"env,omitempty" description:"list of environment variables to set in the container; cannot be updated"`
+	Ports          []ContainerPort      `json:"ports,omitempty" description:"list of ports to expose from the container; cannot be updated" patchStrategy:"merge" patchMergeKey:"containerPort"`
+	Env            []EnvVar             `json:"env,omitempty" description:"list of environment variables to set in the container; cannot be updated" patchStrategy:"merge" patchMergeKey:"name"`
 	Resources      ResourceRequirements `json:"resources,omitempty" description:"Compute Resources required by this container; cannot be updated"`
-	VolumeMounts   []VolumeMount        `json:"volumeMounts,omitempty" description:"pod volumes to mount into the container's filesyste; cannot be updated"`
+	VolumeMounts   []VolumeMount        `json:"volumeMounts,omitempty" description:"pod volumes to mount into the container's filesyste; cannot be updated" patchStrategy:"merge" patchMergeKey:"name"`
 	LivenessProbe  *Probe               `json:"livenessProbe,omitempty" description:"periodic probe of container liveness; container will be restarted if the probe fails; cannot be updated"`
 	ReadinessProbe *Probe               `json:"readinessProbe,omitempty" description:"periodic probe of container service readiness; container will be removed from service endpoints if the probe fails; cannot be updated"`
 	Lifecycle      *Lifecycle           `json:"lifecycle,omitempty" description:"actions that the management system should take in response to container lifecycle events; cannot be updated"`
@@ -695,9 +695,9 @@ const (
 
 // PodSpec is a description of a pod
 type PodSpec struct {
-	Volumes []Volume `json:"volumes" description:"list of volumes that can be mounted by containers belonging to the pod"`
+	Volumes []Volume `json:"volumes" description:"list of volumes that can be mounted by containers belonging to the pod" patchStrategy:"merge" patchMergeKey:"name"`
 	// Required: there must be at least one container in a pod.
-	Containers    []Container   `json:"containers" description:"list of containers belonging to the pod; cannot be updated; containers cannot currently be added or removed; there must be at least one container in a Pod"`
+	Containers    []Container   `json:"containers" description:"list of containers belonging to the pod; cannot be updated; containers cannot currently be added or removed; there must be at least one container in a Pod" patchStrategy:"merge" patchMergeKey:"name"`
 	RestartPolicy RestartPolicy `json:"restartPolicy,omitempty" description:"restart policy for all containers within the pod; one of RestartPolicyAlways, RestartPolicyOnFailure, RestartPolicyNever"`
 	// Optional: Set DNS policy.  Defaults to "ClusterFirst"
 	DNSPolicy DNSPolicy `json:"dnsPolicy,omitempty" description:"DNS policy for containers within the pod; one of 'ClusterFirst' or 'Default'"`
@@ -718,7 +718,7 @@ type PodSpec struct {
 // state of a system.
 type PodStatus struct {
 	Phase      PodPhase       `json:"phase,omitempty" description:"current condition of the pod."`
-	Conditions []PodCondition `json:"Condition,omitempty" description:"current service state of pod"`
+	Conditions []PodCondition `json:"Condition,omitempty" description:"current service state of pod" patchStrategy:"merge" patchMergeKey:"type"`
 	// A human readable message indicating details about why the pod is in this state.
 	Message string `json:"message,omitempty" description:"human readable message indicating details about why the pod is in this condition"`
 
@@ -1029,9 +1029,9 @@ type NodeStatus struct {
 	// NodePhase is the current lifecycle phase of the node.
 	Phase NodePhase `json:"phase,omitempty" description:"most recently observed lifecycle phase of the node"`
 	// Conditions is an array of current node conditions.
-	Conditions []NodeCondition `json:"conditions,omitempty" description:"list of node conditions observed"`
+	Conditions []NodeCondition `json:"conditions,omitempty" description:"list of node conditions observed" patchStrategy:"merge" patchMergeKey:"type"`
 	// Queried from cloud provider, if available.
-	Addresses []NodeAddress `json:"addresses,omitempty" description:"list of addresses reachable to the node"`
+	Addresses []NodeAddress `json:"addresses,omitempty" description:"list of addresses reachable to the node" patchStrategy:"merge" patchMergeKey:"type"`
 	// NodeSystemInfo is a set of ids/uuids to uniquely identify the node
 	NodeInfo NodeSystemInfo `json:"nodeInfo,omitempty"`
 }

pkg/apiserver/api_installer.go

@@ -346,11 +346,10 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 			addParams(route, action.Params)
 			ws.Route(route)
 		case "PATCH": // Partially update a resource
-			route := ws.PATCH(action.Path).To(PatchResource(patcher, reqScope, a.group.Typer, admit)).
+			route := ws.PATCH(action.Path).To(PatchResource(patcher, reqScope, a.group.Typer, admit, mapping.ObjectConvertor)).
 				Filter(m).
-				Doc("partially update the specified " + kind).
-				// TODO: toggle patch strategy by content type
-				// Consumes("application/merge-patch+json", "application/json-patch+json").
+				Doc("partially update the specified "+kind).
+				Consumes(string(api.JSONPatchType), string(api.MergePatchType), string(api.StrategicMergePatchType)).
 				Operation("patch" + kind).
 				Produces(append(storageMeta.ProducesMIMETypes(action.Verb), "application/json")...).
 				Reads(versionedObject)

pkg/apiserver/apiserver_test.go

@@ -1159,6 +1159,7 @@ func TestPatch(t *testing.T) {
 
 	client := http.Client{}
 	request, err := http.NewRequest("PATCH", server.URL+"/api/version/simple/"+ID, bytes.NewReader([]byte(`{"labels":{"foo":"bar"}}`)))
+	request.Header.Set("Content-Type", "application/merge-patch+json")
 	_, err = client.Do(request)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -1190,6 +1191,7 @@ func TestPatchRequiresMatchingName(t *testing.T) {
 
 	client := http.Client{}
 	request, err := http.NewRequest("PATCH", server.URL+"/api/version/simple/"+ID, bytes.NewReader([]byte(`{"metadata":{"name":"idbar"}}`)))
+	request.Header.Set("Content-Type", "application/merge-patch+json")
 	response, err := client.Do(request)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)

pkg/apiserver/resthandler.go

@@ -28,6 +28,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/strategicpatch"
 
 	"github.com/emicklei/go-restful"
 	"github.com/evanphx/json-patch"
@@ -210,11 +211,13 @@ func CreateResource(r rest.Creater, scope RequestScope, typer runtime.ObjectType
 
 // PatchResource returns a function that will handle a resource patch
 // TODO: Eventually PatchResource should just use AtomicUpdate and this routine should be a bit cleaner
-func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper, admit admission.Interface) restful.RouteFunction {
+func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper, admit admission.Interface, converter runtime.ObjectConvertor) restful.RouteFunction {
 	return func(req *restful.Request, res *restful.Response) {
 		w := res.ResponseWriter
 
-		// TODO: we either want to remove timeout or document it (if we document, move timeout out of this function and declare it in api_installer)
+		// TODO: we either want to remove timeout or document it (if we
+		// document, move timeout out of this function and declare it in
+		// api_installer)
 		timeout := parseTimeout(req.Request.URL.Query().Get("timeout"))
 
 		namespace, name, err := scope.Namer.Name(req)
@@ -234,29 +237,36 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper
 		ctx := scope.ContextFunc(req)
 		ctx = api.WithNamespace(ctx, namespace)
 
+		versionedObj, err := converter.ConvertToVersion(obj, scope.APIVersion)
+		if err != nil {
+			errorJSON(err, scope.Codec, w)
+			return
+		}
+
 		original, err := r.Get(ctx, name)
 		if err != nil {
 			errorJSON(err, scope.Codec, w)
 			return
 		}
 
-		originalObjJs, err := scope.Codec.Encode(original)
+		originalObjJS, err := scope.Codec.Encode(original)
 		if err != nil {
 			errorJSON(err, scope.Codec, w)
 			return
 		}
-		patchJs, err := readBody(req.Request)
+		patchJS, err := readBody(req.Request)
 		if err != nil {
 			errorJSON(err, scope.Codec, w)
 			return
 		}
-		patchedObjJs, err := jsonpatch.MergePatch(originalObjJs, patchJs)
+		contentType := req.HeaderParameter("Content-Type")
+		patchedObjJS, err := getPatchedJS(contentType, originalObjJS, patchJS, versionedObj)
 		if err != nil {
 			errorJSON(err, scope.Codec, w)
 			return
 		}
 
-		if err := scope.Codec.DecodeInto(patchedObjJs, obj); err != nil {
+		if err := scope.Codec.DecodeInto(patchedObjJS, obj); err != nil {
 			errorJSON(err, scope.Codec, w)
 			return
 		}
@@ -502,12 +512,17 @@ func setSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer) err
 // checkName checks the provided name against the request
 func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) error {
 	if objNamespace, objName, err := namer.ObjectName(obj); err == nil {
+		if err != nil {
+			return err
+		}
 		if objName != name {
-			return errors.NewBadRequest("the name of the object does not match the name on the URL")
+			return errors.NewBadRequest(fmt.Sprintf(
+				"the name of the object (%s) does not match the name on the URL (%s)", objName, name))
 		}
 		if len(namespace) > 0 {
 			if len(objNamespace) > 0 && objNamespace != namespace {
-				return errors.NewBadRequest("the namespace of the object does not match the namespace on the request")
+				return errors.NewBadRequest(fmt.Sprintf(
+					"the namespace of the object (%s) does not match the namespace on the request (%s)", objNamespace, namespace))
 			}
 		}
 	}
@@ -548,3 +563,22 @@ func setListSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer)
 	return runtime.SetList(obj, items)
 
 }
+
+func getPatchedJS(contentType string, originalJS, patchJS []byte, obj runtime.Object) ([]byte, error) {
+	patchType := api.PatchType(contentType)
+	switch patchType {
+	case api.JSONPatchType:
+		patchObj, err := jsonpatch.DecodePatch(patchJS)
+		if err != nil {
+			return nil, err
+		}
+		return patchObj.Apply(originalJS)
+	case api.MergePatchType:
+		return jsonpatch.MergePatch(originalJS, patchJS)
+	case api.StrategicMergePatchType:
+		return strategicpatch.StrategicMergePatchData(originalJS, patchJS, obj)
+	default:
+		// only here as a safety net - go-restful filters content-type
+		return nil, fmt.Errorf("unknown Content-Type header for patch: %s", contentType)
+	}
+}

pkg/client/request.go

@@ -101,6 +101,7 @@ type Request struct {
 	path    string
 	subpath string
 	params  url.Values
+	headers http.Header
 
 	// structural elements of the request that are part of the Kubernetes API conventions
 	namespace    string
@@ -343,6 +344,14 @@ func (r *Request) setParam(paramName, value string) *Request {
 	return r
 }
 
+func (r *Request) SetHeader(key, value string) *Request {
+	if r.headers == nil {
+		r.headers = http.Header{}
+	}
+	r.headers.Set(key, value)
+	return r
+}
+
 // Timeout makes the request use the given duration as a timeout. Sets the "timeout"
 // parameter.
 func (r *Request) Timeout(d time.Duration) *Request {
@@ -561,6 +570,7 @@ func (r *Request) DoRaw() ([]byte, error) {
 		if err != nil {
 			return nil, err
 		}
+		r.req.Header = r.headers
 		r.resp, err = client.Do(r.req)
 		if err != nil {
 			return nil, err

pkg/client/restclient.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 )
 
@@ -106,8 +107,8 @@ func (c *RESTClient) Put() *Request {
 }
 
 // Patch begins a PATCH request. Short for c.Verb("Patch").
-func (c *RESTClient) Patch() *Request {
-	return c.Verb("PATCH")
+func (c *RESTClient) Patch(pt api.PatchType) *Request {
+	return c.Verb("PATCH").SetHeader("Content-Type", string(pt))
 }
 
 // Get begins a GET request. Short for c.Verb("GET").

pkg/util/strategicpatch/patch_test.go

+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package strategicpatch
+
+import (
+	"encoding/json"
+	"fmt"
+	"reflect"
+	"testing"
+
+	"github.com/davecgh/go-spew/spew"
+	"github.com/ghodss/yaml"
+)
+
+type TestCases struct {
+	StrategicMergePatchCases []StrategicMergePatchCase
+	SortMergeListTestCases   []SortMergeListCase
+}
+
+type StrategicMergePatchCase struct {
+	Description string
+	Patch       map[string]interface{}
+	Original    map[string]interface{}
+	Result      map[string]interface{}
+}
+
+type SortMergeListCase struct {
+	Description string
+	Original    map[string]interface{}
+	Sorted      map[string]interface{}
+}
+
+type MergeItem struct {
+	Name              string
+	Value             string
+	MergingList       []MergeItem `patchStrategy:"merge" patchMergeKey:"name"`
+	NonMergingList    []MergeItem
+	MergingIntList    []int `patchStrategy:"merge"`
+	NonMergingIntList []int
+	SimpleMap         map[string]string
+}
+
+var testCaseData = []byte(`
+strategicMergePatchCases:
+  - description: add new field
+    original:
+      name: 1
+    patch:
+      value: 1
+    result:
+      name: 1
+      value: 1
+  - description: remove field and add new field
+    original:
+      name: 1
+    patch:
+      name: null
+      value: 1
+    result:
+      value: 1
+  - description: merge arrays of scalars
+    original:
+      mergingIntList:
+        - 1
+        - 2
+    patch:
+      mergingIntList:
+        - 2
+        - 3
+    result:
+      mergingIntList:
+        - 1
+        - 2
+        - 3
+  - description: replace arrays of scalars
+    original:
+      nonMergingIntList:
+        - 1
+        - 2
+    patch:
+      nonMergingIntList:
+        - 2
+        - 3
+    result:
+      nonMergingIntList:
+        - 2
+        - 3
+  - description: update param of list that should be merged but had element added serverside
+    original:
+      mergingList:
+        - name: 1
+          value: 1
+        - name: 2
+          value: 2
+    patch:
+      mergingList:
+        - name: 1
+          value: a
+    result:
+      mergingList:
+        - name: 1
+          value: a
+        - name: 2
+          value: 2
+  - description: delete field when field is nested in a map
+    original:
+      simpleMap:
+        key1: 1
+        key2: 1
+    patch:
+      simpleMap:
+        key2: null
+    result:
+      simpleMap:
+        key1: 1
+  - description: update nested list when nested list should not be merged
+    original:
+      mergingList:
+        - name: 1
+          nonMergingList:
+            - name: 1
+            - name: 2
+              value: 2
+        - name: 2
+    patch:
+      mergingList:
+        - name: 1
+          nonMergingList:
+            - name: 1
+              value: 1
+    result:
+      mergingList:
+        - name: 1
+          nonMergingList:
+            - name: 1
+              value: 1
+        - name: 2
+  - description: update nested list when nested list should be merged
+    original:
+      mergingList:
+        - name: 1
+          mergingList:
+            - name: 1
+            - name: 2
+              value: 2
+        - name: 2
+    patch:
+      mergingList:
+        - name: 1
+          mergingList:
+            - name: 1
+              value: 1
+    result:
+      mergingList:
+        - name: 1
+          mergingList:
+            - name: 1
+              value: 1
+            - name: 2
+              value: 2
+        - name: 2
+  - description: update map when map should be replaced
+    original:
+      name: 1
+      value: 1
+    patch:
+      value: 1
+      $patch: replace
+    result:
+      value: 1
+  - description: merge empty merge lists
+    original:
+      mergingList: []
+    patch:
+      mergingList: []
+    result:
+      mergingList: []
+  - description: delete others in a map
+    original:
+      name: 1
+      value: 1
+    patch:
+      $patch: replace
+    result: {}
+  - description: delete item from a merge list
+    original:
+      mergingList:
+        - name: 1
+        - name: 2
+    patch:
+      mergingList:
+        - $patch: delete
+          name: 1
+    result:
+      mergingList:
+        - name: 2
+  - description: add and delete item from a merge list
+    original:
+      merginglist:
+        - name: 1
+        - name: 2
+    patch:
+      merginglist:
+        - name: 3
+        - $patch: delete
+          name: 1
+    result:
+      merginglist:
+        - name: 2
+        - name: 3
+  - description: delete all items from a merge list
+    original:
+      mergingList:
+        - name: 1
+        - name: 2
+    patch:
+      mergingList:
+        - $patch: replace
+    result:
+      mergingList: []
+sortMergeListTestCases:
+  - description: sort one list of maps
+    original:
+      mergingList:
+        - name: 1
+        - name: 3
+        - name: 2
+    sorted:
+      mergingList:
+        - name: 1
+        - name: 2
+        - name: 3
+  - description: sort lists of maps but not nested lists of maps
+    original:
+      mergingList:
+        - name: 2
+          nonMergingList:
+            - name: 1
+            - name: 3
+            - name: 2
+        - name: 1
+          nonMergingList:
+            - name: 2
+            - name: 1
+    sorted:
+      mergingList:
+        - name: 1
+          nonMergingList:
+            - name: 2
+            - name: 1
+        - name: 2
+          nonMergingList:
+            - name: 1
+            - name: 3
+            - name: 2
+  - description: sort lists of maps and nested lists of maps
+    fieldTypes:
+    original:
+      mergingList:
+        - name: 2
+          mergingList:
+            - name: 1
+            - name: 3
+            - name: 2
+        - name: 1
+          mergingList:
+            - name: 2
+            - name: 1
+    sorted:
+      mergingList:
+        - name: 1
+          mergingList:
+            - name: 1
+            - name: 2
+        - name: 2
+          mergingList:
+            - name: 1
+            - name: 2
+            - name: 3
+  - description: merging list should NOT sort when nested in a non merging list
+    original:
+      nonMergingList:
+        - name: 2
+          mergingList:
+            - name: 1
+            - name: 3
+            - name: 2
+        - name: 1
+          mergingList:
+            - name: 2
+            - name: 1
+    sorted:
+      nonMergingList:
+        - name: 2
+          mergingList:
+            - name: 1
+            - name: 3
+            - name: 2
+        - name: 1
+          mergingList:
+            - name: 2
+            - name: 1
+  - description: sort a very nested list of maps
+    fieldTypes:
+    original:
+      mergingList:
+        - mergingList:
+            - mergingList:
+                - name: 2
+                - name: 1
+    sorted:
+      mergingList:
+        - mergingList:
+            - mergingList:
+                - name: 1
+                - name: 2
+  - description: sort nested lists of ints
+    original:
+      mergingList:
+        - name: 2
+          mergingIntList:
+            - 1
+            - 3
+            - 2
+        - name: 1
+          mergingIntList:
+            - 2
+            - 1
+    sorted:
+      mergingList:
+        - name: 1
+          mergingIntList:
+            - 1
+            - 2
+        - name: 2
+          mergingIntList:
+            - 1
+            - 2
+            - 3
+`)
+
+func TestStrategicMergePatch(t *testing.T) {
+	tc := TestCases{}
+	err := yaml.Unmarshal(testCaseData, &tc)
+	if err != nil {
+		t.Errorf("can't unmarshal test cases: %v", err)
+		return
+	}
+
+	var e MergeItem
+	for _, c := range tc.StrategicMergePatchCases {
+		result, err := StrategicMergePatchData(toJSON(c.Original), toJSON(c.Patch), e)
+		if err != nil {
+			t.Errorf("error patching: %v:\noriginal:\n%s\npatch:\n%s",
+				err, toYAML(c.Original), toYAML(c.Patch))
+		}
+
+		// Sort the lists that have merged maps, since order is not significant.
+		result, err = sortMergeListsByName(result, e)
+		if err != nil {
+			t.Errorf("error sorting result object: %v", err)
+		}
+		cResult, err := sortMergeListsByName(toJSON(c.Result), e)
+		if err != nil {
+			t.Errorf("error sorting result object: %v", err)
+		}
+
+		if !reflect.DeepEqual(result, cResult) {
+			t.Errorf("patching failed: %s\noriginal:\n%s\npatch:\n%s\nexpected result:\n%s\ngot result:\n%s",
+				c.Description, toYAML(c.Original), toYAML(c.Patch), jsonToYAML(cResult), jsonToYAML(result))
+		}
+	}
+}
+
+func TestSortMergeLists(t *testing.T) {
+	tc := TestCases{}
+	err := yaml.Unmarshal(testCaseData, &tc)
+	if err != nil {
+		t.Errorf("can't unmarshal test cases: %v", err)
+		return
+	}
+
+	var e MergeItem
+	for _, c := range tc.SortMergeListTestCases {
+		sorted, err := sortMergeListsByName(toJSON(c.Original), e)
+		if err != nil {
+			t.Errorf("sort arrays returned error: %v", err)
+		}
+
+		if !reflect.DeepEqual(sorted, toJSON(c.Sorted)) {
+			t.Errorf("sorting failed: %s\ntried to sort:\n%s\nexpected:\n%s\ngot:\n%s",
+				c.Description, toYAML(c.Original), toYAML(c.Sorted), jsonToYAML(sorted))
+		}
+	}
+}
+
+func toYAML(v interface{}) string {
+	y, err := yaml.Marshal(v)
+	if err != nil {
+		panic(fmt.Sprintf("yaml marshal failed: %v", err))
+	}
+	return string(y)
+}
+
+func toJSON(v interface{}) []byte {
+	j, err := json.Marshal(v)
+	if err != nil {
+		panic(fmt.Sprintf("json marshal failed: %s", spew.Sdump(v)))
+	}
+	return j
+}
+
+func jsonToYAML(j []byte) []byte {
+	y, err := yaml.JSONToYAML(j)
+	if err != nil {
+		panic(fmt.Sprintf("json to yaml failed: %v", err))
+	}
+	return y
+}

test/integration/auth_test.go

@@ -328,7 +328,7 @@ func TestAuthModeAlwaysAllow(t *testing.T) {
 		var bodyStr string
 		if r.body != "" {
 			sub := ""
-			if r.verb == "PUT" && r.body != "" {
+			if r.verb == "PUT" {
 				// For update operations, insert previous resource version
 				if resVersion := previousResourceVersion[getPreviousResourceVersionKey(r.URL, "")]; resVersion != 0 {
 					sub += fmt.Sprintf(",\r\n\"resourceVersion\": %v", resVersion)
@@ -345,6 +345,9 @@ func TestAuthModeAlwaysAllow(t *testing.T) {
 			t.Logf("case %v", r)
 			t.Fatalf("unexpected error: %v", err)
 		}
+		if r.verb == "PATCH" {
+			req.Header.Set("Content-Type", "application/merge-patch+json")
+		}
 		func() {
 			resp, err := transport.RoundTrip(req)
 			defer resp.Body.Close()
@@ -500,7 +503,7 @@ func TestAliceNotForbiddenOrUnauthorized(t *testing.T) {
 		var bodyStr string
 		if r.body != "" {
 			sub := ""
-			if r.verb == "PUT" && r.body != "" {
+			if r.verb == "PUT" {
 				// For update operations, insert previous resource version
 				if resVersion := previousResourceVersion[getPreviousResourceVersionKey(r.URL, "")]; resVersion != 0 {
 					sub += fmt.Sprintf(",\r\n\"resourceVersion\": %v", resVersion)
@@ -517,6 +520,9 @@ func TestAliceNotForbiddenOrUnauthorized(t *testing.T) {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
+		if r.verb == "PATCH" {
+			req.Header.Set("Content-Type", "application/merge-patch+json")
+		}
 
 		func() {
 			resp, err := transport.RoundTrip(req)