diff --git a/pkg/genericapiserver/api/handlers/BUILD b/pkg/genericapiserver/api/handlers/BUILD index 6e9ca9f875e..d44cda9d934 100644 --- a/pkg/genericapiserver/api/handlers/BUILD +++ b/pkg/genericapiserver/api/handlers/BUILD @@ -36,6 +36,7 @@ go_library( srcs = [ "discovery.go", "doc.go", + "patch.go", "proxy.go", "rest.go", "watch.go", diff --git a/pkg/genericapiserver/api/handlers/patch.go b/pkg/genericapiserver/api/handlers/patch.go new file mode 100644 index 00000000000..d117f14c28c --- /dev/null +++ b/pkg/genericapiserver/api/handlers/patch.go @@ -0,0 +1,131 @@ +/* +Copyright 2017 The Kubernetes Authors. + +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 handlers + +import ( + "encoding/json" + "fmt" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/kubernetes/pkg/util/strategicpatch" + + "github.com/evanphx/json-patch" +) + +// patchObjectJSON patches the with and stores +// the result in . +// Currently it also returns the original and patched objects serialized to +// JSONs (this may not be needed once we can apply patches at the +// map[string]interface{} level). +func patchObjectJSON( + patchType types.PatchType, + codec runtime.Codec, + originalObject runtime.Object, + patchJS []byte, + objToUpdate runtime.Object, + versionedObj runtime.Object, +) (originalObjJS []byte, patchedObjJS []byte, retErr error) { + js, err := runtime.Encode(codec, originalObject) + if err != nil { + return nil, nil, err + } + originalObjJS = js + + switch patchType { + case types.JSONPatchType: + patchObj, err := jsonpatch.DecodePatch(patchJS) + if err != nil { + return nil, nil, err + } + if patchedObjJS, err = patchObj.Apply(originalObjJS); err != nil { + return nil, nil, err + } + case types.MergePatchType: + if patchedObjJS, err = jsonpatch.MergePatch(originalObjJS, patchJS); err != nil { + return nil, nil, err + } + case types.StrategicMergePatchType: + if patchedObjJS, err = strategicpatch.StrategicMergePatch(originalObjJS, patchJS, versionedObj); err != nil { + return nil, nil, err + } + default: + // only here as a safety net - go-restful filters content-type + return nil, nil, fmt.Errorf("unknown Content-Type header for patch: %v", patchType) + } + if err := runtime.DecodeInto(codec, patchedObjJS, objToUpdate); err != nil { + return nil, nil, err + } + return +} + +// strategicPatchObject applies a strategic merge patch of to +// and stores the result in . +// It additionally returns the map[string]interface{} representation of the +// and . +func strategicPatchObject( + codec runtime.Codec, + originalObject runtime.Object, + patchJS []byte, + objToUpdate runtime.Object, + versionedObj runtime.Object, +) (originalObjMap map[string]interface{}, patchMap map[string]interface{}, retErr error) { + // TODO: This should be one-step conversion that doesn't require + // json marshaling and unmarshaling once #39017 is fixed. + data, err := runtime.Encode(codec, originalObject) + if err != nil { + return nil, nil, err + } + originalObjMap = make(map[string]interface{}) + if err := json.Unmarshal(data, &originalObjMap); err != nil { + return nil, nil, err + } + + patchMap = make(map[string]interface{}) + if err := json.Unmarshal(patchJS, &patchMap); err != nil { + return nil, nil, err + } + + if err := applyPatchToObject(codec, originalObjMap, patchMap, objToUpdate, versionedObj); err != nil { + return nil, nil, err + } + return +} + +// applyPatchToObject applies a strategic merge patch of to +// and stores the result in , though it operates +// on versioned map[string]interface{} representations. +func applyPatchToObject( + codec runtime.Codec, + originalMap map[string]interface{}, + patchMap map[string]interface{}, + objToUpdate runtime.Object, + versionedObj runtime.Object, +) error { + patchedObjMap, err := strategicpatch.StrategicMergeMapPatch(originalMap, patchMap, versionedObj) + if err != nil { + return err + } + + // TODO: This should be one-step conversion that doesn't require + // json marshaling and unmarshaling once #39017 is fixed. + data, err := json.Marshal(patchedObjMap) + if err != nil { + return err + } + return runtime.DecodeInto(codec, data, objToUpdate) +} diff --git a/pkg/genericapiserver/api/handlers/rest.go b/pkg/genericapiserver/api/handlers/rest.go index e621d340b9b..211d800c6e9 100644 --- a/pkg/genericapiserver/api/handlers/rest.go +++ b/pkg/genericapiserver/api/handlers/rest.go @@ -45,7 +45,6 @@ import ( "k8s.io/kubernetes/pkg/util/strategicpatch" "github.com/emicklei/go-restful" - "github.com/evanphx/json-patch" "github.com/golang/glog" ) @@ -556,6 +555,8 @@ func patchResource( var ( originalObjJS []byte originalPatchedObjJS []byte + originalObjMap map[string]interface{} + originalPatchMap map[string]interface{} lastConflictErr error ) @@ -570,25 +571,30 @@ func patchResource( } switch { - case len(originalObjJS) == 0 || len(originalPatchedObjJS) == 0: + case originalObjJS == nil && originalObjMap == nil: // first time through, // 1. apply the patch - // 2. save the originalJS and patchedJS to detect whether there were conflicting changes on retries - if js, err := runtime.Encode(codec, currentObject); err != nil { - return nil, err - } else { - originalObjJS = js - } - - if js, err := getPatchedJS(patchType, originalObjJS, patchJS, versionedObj); err != nil { - return nil, err - } else { - originalPatchedObjJS = js - } + // 2. save the original and patched to detect whether there were conflicting changes on retries objToUpdate := patcher.New() - if err := runtime.DecodeInto(codec, originalPatchedObjJS, objToUpdate); err != nil { - return nil, err + + // For performance reasons, in case of strategicpatch, we avoid json + // marshaling and unmarshaling and operate just on map[string]interface{}. + // In case of other patch types, we still have to operate on JSON + // representations. + switch patchType { + case types.JSONPatchType, types.MergePatchType: + originalJS, patchedJS, err := patchObjectJSON(patchType, codec, currentObject, patchJS, objToUpdate, versionedObj) + if err != nil { + return nil, err + } + originalObjJS, originalPatchedObjJS = originalJS, patchedJS + case types.StrategicMergePatchType: + originalMap, patchMap, err := strategicPatchObject(codec, currentObject, patchJS, objToUpdate, versionedObj) + if err != nil { + return nil, err + } + originalObjMap, originalPatchMap = originalMap, patchMap } if err := checkName(objToUpdate, name, namespace, namer); err != nil { return nil, err @@ -603,33 +609,62 @@ func patchResource( // 2. build a strategic merge patch from originalJS and the currentJS // 3. ensure no conflicts between the two patches // 4. apply the #1 patch to the currentJS object - currentObjectJS, err := runtime.Encode(codec, currentObject) + + // TODO: This should be one-step conversion that doesn't require + // json marshaling and unmarshaling once #39017 is fixed. + data, err := runtime.Encode(codec, currentObject) if err != nil { return nil, err } - currentPatch, err := strategicpatch.CreateTwoWayMergePatch(originalObjJS, currentObjectJS, versionedObj) - if err != nil { - return nil, err - } - originalPatch, err := strategicpatch.CreateTwoWayMergePatch(originalObjJS, originalPatchedObjJS, versionedObj) - if err != nil { + currentObjMap := make(map[string]interface{}) + if err := json.Unmarshal(data, ¤tObjMap); err != nil { return nil, err } - diff1 := make(map[string]interface{}) - if err := json.Unmarshal(originalPatch, &diff1); err != nil { - return nil, err + var currentPatchMap map[string]interface{} + if originalObjMap != nil { + var err error + currentPatchMap, err = strategicpatch.CreateTwoWayMergeMapPatch(originalObjMap, currentObjMap, versionedObj) + if err != nil { + return nil, err + } + } else { + if originalPatchMap == nil { + // Compute original patch, if we already didn't do this in previous retries. + originalPatch, err := strategicpatch.CreateTwoWayMergePatch(originalObjJS, originalPatchedObjJS, versionedObj) + if err != nil { + return nil, err + } + originalPatchMap = make(map[string]interface{}) + if err := json.Unmarshal(originalPatch, &originalPatchMap); err != nil { + return nil, err + } + } + // Compute current patch. + currentObjJS, err := runtime.Encode(codec, currentObject) + if err != nil { + return nil, err + } + currentPatch, err := strategicpatch.CreateTwoWayMergePatch(originalObjJS, currentObjJS, versionedObj) + if err != nil { + return nil, err + } + currentPatchMap = make(map[string]interface{}) + if err := json.Unmarshal(currentPatch, ¤tPatchMap); err != nil { + return nil, err + } } - diff2 := make(map[string]interface{}) - if err := json.Unmarshal(currentPatch, &diff2); err != nil { - return nil, err - } - hasConflicts, err := strategicpatch.HasConflicts(diff1, diff2) + + hasConflicts, err := strategicpatch.HasConflicts(originalPatchMap, currentPatchMap) if err != nil { return nil, err } if hasConflicts { - glog.V(4).Infof("patchResource failed for resource %s, because there is a meaningful conflict.\n diff1=%v\n, diff2=%v\n", name, diff1, diff2) + if glog.V(4) { + diff1, _ := json.Marshal(currentPatchMap) + diff2, _ := json.Marshal(originalPatchMap) + glog.Infof("patchResource failed for resource %s, because there is a meaningful conflict.\n diff1=%v\n, diff2=%v\n", name, diff1, diff2) + } // Return the last conflict error we got if we have one if lastConflictErr != nil { return nil, lastConflictErr @@ -638,14 +673,11 @@ func patchResource( return nil, errors.NewConflict(resource.GroupResource(), name, nil) } - newlyPatchedObjJS, err := getPatchedJS(types.StrategicMergePatchType, currentObjectJS, originalPatch, versionedObj) - if err != nil { - return nil, err - } objToUpdate := patcher.New() - if err := runtime.DecodeInto(codec, newlyPatchedObjJS, objToUpdate); err != nil { + if err := applyPatchToObject(codec, currentObjMap, originalPatchMap, objToUpdate, versionedObj); err != nil { return nil, err } + return objToUpdate, nil } } @@ -1079,24 +1111,6 @@ func setListSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer) return count, err } -func getPatchedJS(patchType types.PatchType, originalJS, patchJS []byte, obj runtime.Object) ([]byte, error) { - switch patchType { - case types.JSONPatchType: - patchObj, err := jsonpatch.DecodePatch(patchJS) - if err != nil { - return nil, err - } - return patchObj.Apply(originalJS) - case types.MergePatchType: - return jsonpatch.MergePatch(originalJS, patchJS) - case types.StrategicMergePatchType: - return strategicpatch.StrategicMergePatch(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: %v", patchType) - } -} - func summarizeData(data []byte, maxLength int) string { switch { case len(data) == 0: diff --git a/pkg/genericapiserver/api/handlers/rest_test.go b/pkg/genericapiserver/api/handlers/rest_test.go index 12e5fa617c1..9dda1d29e25 100644 --- a/pkg/genericapiserver/api/handlers/rest_test.go +++ b/pkg/genericapiserver/api/handlers/rest_test.go @@ -55,16 +55,27 @@ type TestPatchSubType struct { func (obj *testPatchType) GetObjectKind() schema.ObjectKind { return &obj.TypeMeta } func TestPatchAnonymousField(t *testing.T) { - originalJS := `{"kind":"testPatchType","theField":"my-value"}` - patch := `{"theField": "changed!"}` - expectedJS := `{"kind":"testPatchType","theField":"changed!"}` + testGV := schema.GroupVersion{Group: "", Version: "v"} + api.Scheme.AddKnownTypes(testGV, &testPatchType{}) + codec := api.Codecs.LegacyCodec(testGV) - actualBytes, err := getPatchedJS(types.StrategicMergePatchType, []byte(originalJS), []byte(patch), &testPatchType{}) + original := &testPatchType{ + TypeMeta: metav1.TypeMeta{Kind: "testPatchType", APIVersion: "v"}, + TestPatchSubType: TestPatchSubType{StringField: "my-value"}, + } + patch := `{"theField": "changed!"}` + expected := &testPatchType{ + TypeMeta: metav1.TypeMeta{Kind: "testPatchType", APIVersion: "v"}, + TestPatchSubType: TestPatchSubType{StringField: "changed!"}, + } + + actual := &testPatchType{} + _, _, err := strategicPatchObject(codec, original, []byte(patch), actual, &testPatchType{}) if err != nil { t.Fatalf("unexpected error: %v", err) } - if string(actualBytes) != expectedJS { - t.Errorf("expected %v, got %v", expectedJS, string(actualBytes)) + if !api.Semantic.DeepEqual(actual, expected) { + t.Errorf("expected %#v, got %#v", expected, actual) } }