diff --git a/pkg/kubectl/cmd/BUILD b/pkg/kubectl/cmd/BUILD index 327a8d120cc88..390a91eb28d5d 100644 --- a/pkg/kubectl/cmd/BUILD +++ b/pkg/kubectl/cmd/BUILD @@ -114,6 +114,7 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/util/intstr", "//vendor:k8s.io/apimachinery/pkg/util/json", "//vendor:k8s.io/apimachinery/pkg/util/jsonmergepatch", + "//vendor:k8s.io/apimachinery/pkg/util/mergepatch", "//vendor:k8s.io/apimachinery/pkg/util/sets", "//vendor:k8s.io/apimachinery/pkg/util/strategicpatch", "//vendor:k8s.io/apimachinery/pkg/util/validation", diff --git a/pkg/kubectl/cmd/apply.go b/pkg/kubectl/cmd/apply.go index 72cf981c3e535..7d5d2e0007070 100644 --- a/pkg/kubectl/cmd/apply.go +++ b/pkg/kubectl/cmd/apply.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/jsonmergepatch" + "k8s.io/apimachinery/pkg/util/mergepatch" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/wait" @@ -566,10 +567,13 @@ func (p *patcher) patchSimple(obj runtime.Object, modified []byte, source, names case runtime.IsNotRegisteredError(err): // fall back to generic JSON merge patch patchType = types.MergePatchType - preconditions := []strategicpatch.PreconditionFunc{strategicpatch.RequireKeyUnchanged("apiVersion"), - strategicpatch.RequireKeyUnchanged("kind"), strategicpatch.RequireMetadataKeyUnchanged("name")} + preconditions := []mergepatch.PreconditionFunc{mergepatch.RequireKeyUnchanged("apiVersion"), + mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name")} patch, err = jsonmergepatch.CreateThreeWayJSONMergePatch(original, modified, current, preconditions...) if err != nil { + if mergepatch.IsPreconditionFailed(err) { + return nil, fmt.Errorf("%s", "At least one of apiVersion, kind and name was changed") + } return nil, cmdutil.AddSourceToErr(fmt.Sprintf(createPatchErrFormat, original, modified, current), source, err) } case err != nil: diff --git a/pkg/kubectl/cmd/edit.go b/pkg/kubectl/cmd/edit.go index 504b988ec8e7e..3009ca708c912 100644 --- a/pkg/kubectl/cmd/edit.go +++ b/pkg/kubectl/cmd/edit.go @@ -33,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/mergepatch" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/yaml" @@ -489,12 +490,12 @@ func visitToPatch( return nil } - preconditions := []strategicpatch.PreconditionFunc{strategicpatch.RequireKeyUnchanged("apiVersion"), - strategicpatch.RequireKeyUnchanged("kind"), strategicpatch.RequireMetadataKeyUnchanged("name")} + preconditions := []mergepatch.PreconditionFunc{mergepatch.RequireKeyUnchanged("apiVersion"), + mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name")} patch, err := strategicpatch.CreateTwoWayMergePatch(originalJS, editedJS, currOriginalObj, preconditions...) if err != nil { glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err) - if strategicpatch.IsPreconditionFailed(err) { + if mergepatch.IsPreconditionFailed(err) { return fmt.Errorf("%s", "At least one of apiVersion, kind and name was changed") } return err diff --git a/pkg/kubectl/cmd/util/jsonmerge/BUILD b/pkg/kubectl/cmd/util/jsonmerge/BUILD index 70ab48cfd35d1..524fe65b8db1a 100644 --- a/pkg/kubectl/cmd/util/jsonmerge/BUILD +++ b/pkg/kubectl/cmd/util/jsonmerge/BUILD @@ -14,7 +14,7 @@ go_library( deps = [ "//vendor:github.com/evanphx/json-patch", "//vendor:github.com/golang/glog", - "//vendor:k8s.io/apimachinery/pkg/util/strategicpatch", + "//vendor:k8s.io/apimachinery/pkg/util/mergepatch", "//vendor:k8s.io/apimachinery/pkg/util/yaml", ], ) diff --git a/pkg/kubectl/cmd/util/jsonmerge/jsonmerge.go b/pkg/kubectl/cmd/util/jsonmerge/jsonmerge.go index ca8c9fbe303ec..beebc7f052a2d 100644 --- a/pkg/kubectl/cmd/util/jsonmerge/jsonmerge.go +++ b/pkg/kubectl/cmd/util/jsonmerge/jsonmerge.go @@ -23,7 +23,7 @@ import ( "github.com/evanphx/json-patch" "github.com/golang/glog" - "k8s.io/apimachinery/pkg/util/strategicpatch" + "k8s.io/apimachinery/pkg/util/mergepatch" "k8s.io/apimachinery/pkg/util/yaml" ) @@ -161,7 +161,7 @@ func (d *Delta) Apply(latest []byte) ([]byte, error) { } glog.V(6).Infof("Testing for conflict between:\n%s\n%s", string(d.edit), string(changes)) - hasConflicts, err := strategicpatch.HasConflicts(diff1, diff2) + hasConflicts, err := mergepatch.HasConflicts(diff1, diff2) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apimachinery/pkg/util/jsonmergepatch/patch.go b/staging/src/k8s.io/apimachinery/pkg/util/jsonmergepatch/patch.go index 5c6adf2f83d4b..5583daa3330ad 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/jsonmergepatch/patch.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/jsonmergepatch/patch.go @@ -22,13 +22,13 @@ import ( "k8s.io/apimachinery/pkg/util/json" "github.com/evanphx/json-patch" - "k8s.io/apimachinery/pkg/util/strategicpatch" + "k8s.io/apimachinery/pkg/util/mergepatch" ) // Create a 3-way merge patch based-on JSON merge patch. // Calculate addition-and-change patch between current and modified. // Calculate deletion patch between original and modified. -func CreateThreeWayJSONMergePatch(original, modified, current []byte, fns ...strategicpatch.PreconditionFunc) ([]byte, error) { +func CreateThreeWayJSONMergePatch(original, modified, current []byte, fns ...mergepatch.PreconditionFunc) ([]byte, error) { if len(original) == 0 { original = []byte(`{}`) } @@ -59,12 +59,12 @@ func CreateThreeWayJSONMergePatch(original, modified, current []byte, fns ...str return nil, err } - hasConflicts, err := strategicpatch.HasConflicts(addAndChangePatchObj, deletePatchObj) + hasConflicts, err := mergepatch.HasConflicts(addAndChangePatchObj, deletePatchObj) if err != nil { return nil, err } if hasConflicts { - return nil, strategicpatch.NewErrConflict(strategicpatch.ToYAMLOrError(addAndChangePatchObj), strategicpatch.ToYAMLOrError(deletePatchObj)) + return nil, mergepatch.NewErrConflict(mergepatch.ToYAMLOrError(addAndChangePatchObj), mergepatch.ToYAMLOrError(deletePatchObj)) } patch, err := jsonpatch.MergePatch(deletePatch, addAndChangePatch) if err != nil { @@ -81,7 +81,7 @@ func CreateThreeWayJSONMergePatch(original, modified, current []byte, fns ...str return nil, err } if !meetPreconditions { - return nil, strategicpatch.NewErrPreconditionFailed(patchMap) + return nil, mergepatch.NewErrPreconditionFailed(patchMap) } return patch, nil @@ -133,7 +133,7 @@ func keepOrDeleteNullInObj(m map[string]interface{}, keepNull bool) (map[string] return filteredMap, nil } -func meetPreconditions(patchObj map[string]interface{}, fns ...strategicpatch.PreconditionFunc) (bool, error) { +func meetPreconditions(patchObj map[string]interface{}, fns ...mergepatch.PreconditionFunc) (bool, error) { // Apply the preconditions to the patch, and return an error if any of them fail. for _, fn := range fns { if !fn(patchObj) { diff --git a/staging/src/k8s.io/apimachinery/pkg/util/mergepatch/errors.go b/staging/src/k8s.io/apimachinery/pkg/util/mergepatch/errors.go new file mode 100644 index 0000000000000..b9d3e6017fd56 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/util/mergepatch/errors.go @@ -0,0 +1,66 @@ +/* +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 mergepatch + +import ( + "errors" + "fmt" +) + +var ErrBadJSONDoc = errors.New("Invalid JSON document") +var ErrNoListOfLists = errors.New("Lists of lists are not supported") +var ErrBadPatchFormatForPrimitiveList = errors.New("Invalid patch format of primitive list") + +// IsPreconditionFailed returns true if the provided error indicates +// a precondition failed. +func IsPreconditionFailed(err error) bool { + _, ok := err.(ErrPreconditionFailed) + return ok +} + +type ErrPreconditionFailed struct { + message string +} + +func NewErrPreconditionFailed(target map[string]interface{}) ErrPreconditionFailed { + s := fmt.Sprintf("precondition failed for: %v", target) + return ErrPreconditionFailed{s} +} + +func (err ErrPreconditionFailed) Error() string { + return err.message +} + +type ErrConflict struct { + message string +} + +func NewErrConflict(patch, current string) ErrConflict { + s := fmt.Sprintf("patch:\n%s\nconflicts with changes made from original to current:\n%s\n", patch, current) + return ErrConflict{s} +} + +func (err ErrConflict) Error() string { + return err.message +} + +// IsConflict returns true if the provided error indicates +// a conflict between the patch and the current configuration. +func IsConflict(err error) bool { + _, ok := err.(ErrConflict) + return ok +} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/mergepatch/util.go b/staging/src/k8s.io/apimachinery/pkg/util/mergepatch/util.go new file mode 100644 index 0000000000000..591884e5c8ccb --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/util/mergepatch/util.go @@ -0,0 +1,126 @@ +/* +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 mergepatch + +import ( + "fmt" + "reflect" + + "github.com/davecgh/go-spew/spew" + "github.com/ghodss/yaml" +) + +// PreconditionFunc asserts that an incompatible change is not present within a patch. +type PreconditionFunc func(interface{}) bool + +// RequireKeyUnchanged returns a precondition function that fails if the provided key +// is present in the patch (indicating that its value has changed). +func RequireKeyUnchanged(key string) PreconditionFunc { + return func(patch interface{}) bool { + patchMap, ok := patch.(map[string]interface{}) + if !ok { + return true + } + + // The presence of key means that its value has been changed, so the test fails. + _, ok = patchMap[key] + return !ok + } +} + +// RequireMetadataKeyUnchanged creates a precondition function that fails +// if the metadata.key is present in the patch (indicating its value +// has changed). +func RequireMetadataKeyUnchanged(key string) PreconditionFunc { + return func(patch interface{}) bool { + patchMap, ok := patch.(map[string]interface{}) + if !ok { + return true + } + patchMap1, ok := patchMap["metadata"] + if !ok { + return true + } + patchMap2, ok := patchMap1.(map[string]interface{}) + if !ok { + return true + } + _, ok = patchMap2[key] + return !ok + } +} + +func ToYAMLOrError(v interface{}) string { + y, err := toYAML(v) + if err != nil { + return err.Error() + } + + return y +} + +func toYAML(v interface{}) (string, error) { + y, err := yaml.Marshal(v) + if err != nil { + return "", fmt.Errorf("yaml marshal failed:%v\n%v\n", err, spew.Sdump(v)) + } + + return string(y), nil +} + +// HasConflicts returns true if the left and right JSON interface objects overlap with +// different values in any key. All keys are required to be strings. Since patches of the +// same Type have congruent keys, this is valid for multiple patch types. This method +// supports JSON merge patch semantics. +func HasConflicts(left, right interface{}) (bool, error) { + switch typedLeft := left.(type) { + case map[string]interface{}: + switch typedRight := right.(type) { + case map[string]interface{}: + for key, leftValue := range typedLeft { + rightValue, ok := typedRight[key] + if !ok { + return false, nil + } + return HasConflicts(leftValue, rightValue) + } + + return false, nil + default: + return true, nil + } + case []interface{}: + switch typedRight := right.(type) { + case []interface{}: + if len(typedLeft) != len(typedRight) { + return true, nil + } + + for i := range typedLeft { + return HasConflicts(typedLeft[i], typedRight[i]) + } + + return false, nil + default: + return true, nil + } + case string, float64, bool, int, int64, nil: + return !reflect.DeepEqual(left, right), nil + default: + return true, fmt.Errorf("unknown type: %v", reflect.TypeOf(left)) + } +} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/mergepatch/util_test.go b/staging/src/k8s.io/apimachinery/pkg/util/mergepatch/util_test.go new file mode 100644 index 0000000000000..9e761c005ba97 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/util/mergepatch/util_test.go @@ -0,0 +1,84 @@ +/* +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 mergepatch + +import ( + "testing" +) + +func TestHasConflicts(t *testing.T) { + testCases := []struct { + A interface{} + B interface{} + Ret bool + }{ + {A: "hello", B: "hello", Ret: false}, // 0 + {A: "hello", B: "hell", Ret: true}, + {A: "hello", B: nil, Ret: true}, + {A: "hello", B: 1, Ret: true}, + {A: "hello", B: float64(1.0), Ret: true}, + {A: "hello", B: false, Ret: true}, + {A: 1, B: 1, Ret: false}, + {A: false, B: false, Ret: false}, + {A: float64(3), B: float64(3), Ret: false}, + + {A: "hello", B: []interface{}{}, Ret: true}, // 6 + {A: []interface{}{1}, B: []interface{}{}, Ret: true}, + {A: []interface{}{}, B: []interface{}{}, Ret: false}, + {A: []interface{}{1}, B: []interface{}{1}, Ret: false}, + {A: map[string]interface{}{}, B: []interface{}{1}, Ret: true}, + + {A: map[string]interface{}{}, B: map[string]interface{}{"a": 1}, Ret: false}, // 11 + {A: map[string]interface{}{"a": 1}, B: map[string]interface{}{"a": 1}, Ret: false}, + {A: map[string]interface{}{"a": 1}, B: map[string]interface{}{"a": 2}, Ret: true}, + {A: map[string]interface{}{"a": 1}, B: map[string]interface{}{"b": 2}, Ret: false}, + + { // 15 + A: map[string]interface{}{"a": []interface{}{1}}, + B: map[string]interface{}{"a": []interface{}{1}}, + Ret: false, + }, + { + A: map[string]interface{}{"a": []interface{}{1}}, + B: map[string]interface{}{"a": []interface{}{}}, + Ret: true, + }, + { + A: map[string]interface{}{"a": []interface{}{1}}, + B: map[string]interface{}{"a": 1}, + Ret: true, + }, + } + + for i, testCase := range testCases { + out, err := HasConflicts(testCase.A, testCase.B) + if err != nil { + t.Errorf("%d: unexpected error: %v", i, err) + } + if out != testCase.Ret { + t.Errorf("%d: expected %t got %t", i, testCase.Ret, out) + continue + } + out, err = HasConflicts(testCase.B, testCase.A) + if err != nil { + t.Errorf("%d: unexpected error: %v", i, err) + } + if out != testCase.Ret { + t.Errorf("%d: expected reversed %t got %t", i, testCase.Ret, out) + } + } +} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go index 7ea55e2fb2bf7..544108b6926b4 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go @@ -24,9 +24,7 @@ import ( "k8s.io/apimachinery/pkg/util/json" forkedjson "k8s.io/apimachinery/third_party/forked/golang/json" - - "github.com/davecgh/go-spew/spew" - "github.com/ghodss/yaml" + "k8s.io/apimachinery/pkg/util/mergepatch" ) // An alternate implementation of JSON Merge Patch @@ -55,110 +53,26 @@ const ( // json marshaling and/or unmarshaling operations. type JSONMap map[string]interface{} -// IsPreconditionFailed returns true if the provided error indicates -// a precondition failed. -func IsPreconditionFailed(err error) bool { - _, ok := err.(ErrPreconditionFailed) - return ok -} - -type ErrPreconditionFailed struct { - message string -} - -func NewErrPreconditionFailed(target map[string]interface{}) ErrPreconditionFailed { - s := fmt.Sprintf("precondition failed for: %v", target) - return ErrPreconditionFailed{s} -} - -func (err ErrPreconditionFailed) Error() string { - return err.message -} - -type ErrConflict struct { - message string -} - -func NewErrConflict(patch, current string) ErrConflict { - s := fmt.Sprintf("patch:\n%s\nconflicts with changes made from original to current:\n%s\n", patch, current) - return ErrConflict{s} -} - -func (err ErrConflict) Error() string { - return err.message -} - -// IsConflict returns true if the provided error indicates -// a conflict between the patch and the current configuration. -func IsConflict(err error) bool { - _, ok := err.(ErrConflict) - return ok -} - -var errBadJSONDoc = fmt.Errorf("Invalid JSON document") -var errNoListOfLists = fmt.Errorf("Lists of lists are not supported") -var errBadPatchFormatForPrimitiveList = fmt.Errorf("Invalid patch format of primitive list") - // The following code is adapted from github.com/openshift/origin/pkg/util/jsonmerge. // Instead of defining a Delta that holds an original, a patch and a set of preconditions, // the reconcile method accepts a set of preconditions as an argument. -// PreconditionFunc asserts that an incompatible change is not present within a patch. -type PreconditionFunc func(interface{}) bool - -// RequireKeyUnchanged returns a precondition function that fails if the provided key -// is present in the patch (indicating that its value has changed). -func RequireKeyUnchanged(key string) PreconditionFunc { - return func(patch interface{}) bool { - patchMap, ok := patch.(map[string]interface{}) - if !ok { - return true - } - - // The presence of key means that its value has been changed, so the test fails. - _, ok = patchMap[key] - return !ok - } -} - -// RequireMetadataKeyUnchanged creates a precondition function that fails -// if the metadata.key is present in the patch (indicating its value -// has changed). -func RequireMetadataKeyUnchanged(key string) PreconditionFunc { - return func(patch interface{}) bool { - patchMap, ok := patch.(map[string]interface{}) - if !ok { - return true - } - patchMap1, ok := patchMap["metadata"] - if !ok { - return true - } - patchMap2, ok := patchMap1.(map[string]interface{}) - if !ok { - return true - } - _, ok = patchMap2[key] - return !ok - } -} - // CreateTwoWayMergePatch creates a patch that can be passed to StrategicMergePatch from an original // document and a modified document, which are passed to the method as json encoded content. It will // return a patch that yields the modified document when applied to the original document, or an error // if either of the two documents is invalid. -func CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}, fns ...PreconditionFunc) ([]byte, error) { +func CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}, fns ...mergepatch.PreconditionFunc) ([]byte, error) { originalMap := map[string]interface{}{} if len(original) > 0 { if err := json.Unmarshal(original, &originalMap); err != nil { - return nil, errBadJSONDoc + return nil, mergepatch.ErrBadJSONDoc } } modifiedMap := map[string]interface{}{} if len(modified) > 0 { if err := json.Unmarshal(modified, &modifiedMap); err != nil { - return nil, errBadJSONDoc + return nil, mergepatch.ErrBadJSONDoc } } @@ -173,7 +87,7 @@ func CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}, f // CreateTwoWayMergeMapPatch creates a patch from an original and modified JSON objects, // encoded JSONMap. // The serialized version of the map can then be passed to StrategicMergeMapPatch. -func CreateTwoWayMergeMapPatch(original, modified JSONMap, dataStruct interface{}, fns ...PreconditionFunc) (JSONMap, error) { +func CreateTwoWayMergeMapPatch(original, modified JSONMap, dataStruct interface{}, fns ...mergepatch.PreconditionFunc) (JSONMap, error) { t, err := getTagStructType(dataStruct) if err != nil { return nil, err @@ -187,7 +101,7 @@ func CreateTwoWayMergeMapPatch(original, modified JSONMap, dataStruct interface{ // Apply the preconditions to the patch, and return an error if any of them fail. for _, fn := range fns { if !fn(patchMap) { - return nil, NewErrPreconditionFailed(patchMap) + return nil, mergepatch.NewErrPreconditionFailed(patchMap) } } @@ -332,7 +246,7 @@ func diffLists(original, modified []interface{}, t reflect.Type, mergeKey string return patchList, nil, err case reflect.Slice: // Lists of Lists are not permitted by the api - return nil, nil, errNoListOfLists + return nil, nil, mergepatch.ErrNoListOfLists default: return diffListsOfScalars(original, modified, ignoreChangesAndAdditions, ignoreDeletions) } @@ -523,13 +437,13 @@ func StrategicMergePatch(original, patch []byte, dataStruct interface{}) ([]byte originalMap := map[string]interface{}{} err := json.Unmarshal(original, &originalMap) if err != nil { - return nil, errBadJSONDoc + return nil, mergepatch.ErrBadJSONDoc } patchMap := map[string]interface{}{} err = json.Unmarshal(patch, &patchMap) if err != nil { - return nil, errBadJSONDoc + return nil, mergepatch.ErrBadJSONDoc } result, err := StrategicMergeMapPatch(originalMap, patchMap, dataStruct) @@ -615,7 +529,7 @@ func mergeMap(original, patch map[string]interface{}, t reflect.Type, mergeDelet } substrings := strings.SplitN(k, "/", 2) if len(substrings) <= 1 { - return nil, errBadPatchFormatForPrimitiveList + return nil, mergepatch.ErrBadPatchFormatForPrimitiveList } isDeleteList = true k = substrings[1] @@ -871,7 +785,7 @@ func sortMergeListsByNameMap(s map[string]interface{}, t reflect.Type) (map[stri if strings.HasPrefix(k, deleteFromPrimitiveListDirectivePrefix) { typedV, ok := v.([]interface{}) if !ok { - return nil, errBadPatchFormatForPrimitiveList + return nil, mergepatch.ErrBadPatchFormatForPrimitiveList } v = uniqifyAndSortScalars(typedV) } else if k != directiveMarker { @@ -1052,7 +966,7 @@ func sliceElementType(slices ...[]interface{}) (reflect.Type, error) { prevType = currentType // We don't support lists of lists yet. if prevType.Kind() == reflect.Slice { - return nil, errNoListOfLists + return nil, mergepatch.ErrNoListOfLists } } else { if prevType != currentType { @@ -1070,49 +984,6 @@ func sliceElementType(slices ...[]interface{}) (reflect.Type, error) { return prevType, nil } -// HasConflicts returns true if the left and right JSON interface objects overlap with -// different values in any key. All keys are required to be strings. Since patches of the -// same Type have congruent keys, this is valid for multiple patch types. This method -// supports JSON merge patch semantics. -func HasConflicts(left, right interface{}) (bool, error) { - switch typedLeft := left.(type) { - case map[string]interface{}: - switch typedRight := right.(type) { - case map[string]interface{}: - for key, leftValue := range typedLeft { - rightValue, ok := typedRight[key] - if !ok { - return false, nil - } - return HasConflicts(leftValue, rightValue) - } - - return false, nil - default: - return true, nil - } - case []interface{}: - switch typedRight := right.(type) { - case []interface{}: - if len(typedLeft) != len(typedRight) { - return true, nil - } - - for i := range typedLeft { - return HasConflicts(typedLeft[i], typedRight[i]) - } - - return false, nil - default: - return true, nil - } - case string, float64, bool, int, int64, nil: - return !reflect.DeepEqual(left, right), nil - default: - return true, fmt.Errorf("unknown type: %v", reflect.TypeOf(left)) - } -} - // MergingMapsHaveConflicts returns true if the left and right JSON interface // objects overlap with different values in any key. All keys are required to be // strings. Since patches of the same Type have congruent keys, this is valid @@ -1291,25 +1162,25 @@ func mapsOfMapsHaveConflicts(typedLeft, typedRight map[string]interface{}, struc // in a way that is different from how it is changed in current (e.g., deleting it, changing its // value). We also propagate values fields that do not exist in original but are explicitly // defined in modified. -func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct interface{}, overwrite bool, fns ...PreconditionFunc) ([]byte, error) { +func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct interface{}, overwrite bool, fns ...mergepatch.PreconditionFunc) ([]byte, error) { originalMap := map[string]interface{}{} if len(original) > 0 { if err := json.Unmarshal(original, &originalMap); err != nil { - return nil, errBadJSONDoc + return nil, mergepatch.ErrBadJSONDoc } } modifiedMap := map[string]interface{}{} if len(modified) > 0 { if err := json.Unmarshal(modified, &modifiedMap); err != nil { - return nil, errBadJSONDoc + return nil, mergepatch.ErrBadJSONDoc } } currentMap := map[string]interface{}{} if len(current) > 0 { if err := json.Unmarshal(current, ¤tMap); err != nil { - return nil, errBadJSONDoc + return nil, mergepatch.ErrBadJSONDoc } } @@ -1340,7 +1211,7 @@ func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct int // Apply the preconditions to the patch, and return an error if any of them fail. for _, fn := range fns { if !fn(patchMap) { - return nil, NewErrPreconditionFailed(patchMap) + return nil, mergepatch.NewErrPreconditionFailed(patchMap) } } @@ -1358,27 +1229,9 @@ func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct int } if hasConflicts { - return nil, NewErrConflict(ToYAMLOrError(patchMap), ToYAMLOrError(changedMap)) + return nil, mergepatch.NewErrConflict(mergepatch.ToYAMLOrError(patchMap), mergepatch.ToYAMLOrError(changedMap)) } } return json.Marshal(patchMap) } - -func ToYAMLOrError(v interface{}) string { - y, err := toYAML(v) - if err != nil { - return err.Error() - } - - return y -} - -func toYAML(v interface{}) (string, error) { - y, err := yaml.Marshal(v) - if err != nil { - return "", fmt.Errorf("yaml marshal failed:%v\n%v\n", err, spew.Sdump(v)) - } - - return string(y), nil -} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch_test.go b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch_test.go index b29714af719a3..d5c1e59f8d0c1 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch_test.go @@ -25,6 +25,7 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/ghodss/yaml" + "k8s.io/apimachinery/pkg/util/mergepatch" ) type SortMergeListTestCases struct { @@ -266,7 +267,7 @@ func TestSortMergeLists(t *testing.T) { sorted := testObjectToJSONOrFail(t, c.Sorted, c.Description) if !reflect.DeepEqual(original, sorted) { t.Errorf("error in test case: %s\ncannot sort object:\n%s\nexpected:\n%s\ngot:\n%s\n", - c.Description, ToYAMLOrError(c.Original), ToYAMLOrError(c.Sorted), jsonToYAMLOrError(original)) + c.Description, mergepatch.ToYAMLOrError(c.Original), mergepatch.ToYAMLOrError(c.Sorted), jsonToYAMLOrError(original)) } } } @@ -1990,9 +1991,9 @@ mergingIntList: func TestStrategicMergePatch(t *testing.T) { testStrategicMergePatchWithCustomArguments(t, "bad original", - "", "{}", mergeItem, errBadJSONDoc) + "", "{}", mergeItem, mergepatch.ErrBadJSONDoc) testStrategicMergePatchWithCustomArguments(t, "bad patch", - "{}", "", mergeItem, errBadJSONDoc) + "{}", "", mergeItem, mergepatch.ErrBadJSONDoc) testStrategicMergePatchWithCustomArguments(t, "bad struct", "{}", "{}", []byte(""), fmt.Errorf(errBadArgTypeFmt, "struct", "slice")) testStrategicMergePatchWithCustomArguments(t, "nil struct", @@ -2037,7 +2038,7 @@ func testTwoWayPatch(t *testing.T, c StrategicMergePatchTestCase) { actualPatch, err := CreateTwoWayMergePatch(original, modified, mergeItem) if err != nil { t.Errorf("error: %s\nin test case: %s\ncannot create two way patch: %s:\n%s\n", - err, c.Description, original, ToYAMLOrError(c.StrategicMergePatchTestCaseData)) + err, c.Description, original, mergepatch.ToYAMLOrError(c.StrategicMergePatchTestCaseData)) return } @@ -2085,15 +2086,15 @@ func testThreeWayPatch(t *testing.T, c StrategicMergePatchTestCase) { original, modified, current, expected, result := threeWayTestCaseToJSONOrFail(t, c) actual, err := CreateThreeWayMergePatch(original, modified, current, mergeItem, false) if err != nil { - if !IsConflict(err) { + if !mergepatch.IsConflict(err) { t.Errorf("error: %s\nin test case: %s\ncannot create three way patch:\n%s\n", - err, c.Description, ToYAMLOrError(c.StrategicMergePatchTestCaseData)) + err, c.Description, mergepatch.ToYAMLOrError(c.StrategicMergePatchTestCaseData)) return } if !strings.Contains(c.Description, "conflict") { t.Errorf("unexpected conflict: %s\nin test case: %s\ncannot create three way patch:\n%s\n", - err, c.Description, ToYAMLOrError(c.StrategicMergePatchTestCaseData)) + err, c.Description, mergepatch.ToYAMLOrError(c.StrategicMergePatchTestCaseData)) return } @@ -2101,7 +2102,7 @@ func testThreeWayPatch(t *testing.T, c StrategicMergePatchTestCase) { actual, err := CreateThreeWayMergePatch(original, modified, current, mergeItem, true) if err != nil { t.Errorf("error: %s\nin test case: %s\ncannot force three way patch application:\n%s\n", - err, c.Description, ToYAMLOrError(c.StrategicMergePatchTestCaseData)) + err, c.Description, mergepatch.ToYAMLOrError(c.StrategicMergePatchTestCaseData)) return } @@ -2114,7 +2115,7 @@ func testThreeWayPatch(t *testing.T, c StrategicMergePatchTestCase) { if strings.Contains(c.Description, "conflict") || len(c.Result) < 1 { t.Errorf("error in test case: %s\nexpected conflict did not occur:\n%s\n", - c.Description, ToYAMLOrError(c.StrategicMergePatchTestCaseData)) + c.Description, mergepatch.ToYAMLOrError(c.StrategicMergePatchTestCaseData)) return } @@ -2126,7 +2127,7 @@ func testThreeWayPatchForRawTestCase(t *testing.T, c StrategicMergePatchRawTestC original, modified, current, expected, result := threeWayRawTestCaseToJSONOrFail(t, c) actual, err := CreateThreeWayMergePatch(original, modified, current, mergeItem, false) if err != nil { - if !IsConflict(err) { + if !mergepatch.IsConflict(err) { t.Errorf("error: %s\nin test case: %s\ncannot create three way patch:\noriginal:%s\ntwoWay:%s\nmodified:%s\ncurrent:%s\nthreeWay:%s\nresult:%s\n", err, c.Description, c.Original, c.TwoWay, c.Modified, c.Current, c.ThreeWay, c.Result) return @@ -2282,69 +2283,6 @@ func yamlToJSONOrError(t *testing.T, y []byte) []byte { return j } -func TestHasConflicts(t *testing.T) { - testCases := []struct { - A interface{} - B interface{} - Ret bool - }{ - {A: "hello", B: "hello", Ret: false}, // 0 - {A: "hello", B: "hell", Ret: true}, - {A: "hello", B: nil, Ret: true}, - {A: "hello", B: 1, Ret: true}, - {A: "hello", B: float64(1.0), Ret: true}, - {A: "hello", B: false, Ret: true}, - {A: 1, B: 1, Ret: false}, - {A: false, B: false, Ret: false}, - {A: float64(3), B: float64(3), Ret: false}, - - {A: "hello", B: []interface{}{}, Ret: true}, // 6 - {A: []interface{}{1}, B: []interface{}{}, Ret: true}, - {A: []interface{}{}, B: []interface{}{}, Ret: false}, - {A: []interface{}{1}, B: []interface{}{1}, Ret: false}, - {A: map[string]interface{}{}, B: []interface{}{1}, Ret: true}, - - {A: map[string]interface{}{}, B: map[string]interface{}{"a": 1}, Ret: false}, // 11 - {A: map[string]interface{}{"a": 1}, B: map[string]interface{}{"a": 1}, Ret: false}, - {A: map[string]interface{}{"a": 1}, B: map[string]interface{}{"a": 2}, Ret: true}, - {A: map[string]interface{}{"a": 1}, B: map[string]interface{}{"b": 2}, Ret: false}, - - { // 15 - A: map[string]interface{}{"a": []interface{}{1}}, - B: map[string]interface{}{"a": []interface{}{1}}, - Ret: false, - }, - { - A: map[string]interface{}{"a": []interface{}{1}}, - B: map[string]interface{}{"a": []interface{}{}}, - Ret: true, - }, - { - A: map[string]interface{}{"a": []interface{}{1}}, - B: map[string]interface{}{"a": 1}, - Ret: true, - }, - } - - for i, testCase := range testCases { - out, err := HasConflicts(testCase.A, testCase.B) - if err != nil { - t.Errorf("%d: unexpected error: %v", i, err) - } - if out != testCase.Ret { - t.Errorf("%d: expected %t got %t", i, testCase.Ret, out) - continue - } - out, err = HasConflicts(testCase.B, testCase.A) - if err != nil { - t.Errorf("%d: unexpected error: %v", i, err) - } - if out != testCase.Ret { - t.Errorf("%d: expected reversed %t got %t", i, testCase.Ret, out) - } - } -} - type PrecisionItem struct { Name string Int32 int32 diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index a1f36ed7ffeb6..c1fdeeb602ff0 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -39,6 +39,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/mergepatch" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" @@ -658,7 +659,7 @@ func patchResource( } } - hasConflicts, err := strategicpatch.HasConflicts(originalPatchMap, currentPatchMap) + hasConflicts, err := mergepatch.HasConflicts(originalPatchMap, currentPatchMap) if err != nil { return nil, err } diff --git a/vendor/BUILD b/vendor/BUILD index 8c84fa19ab085..e795e3e6ff415 100644 --- a/vendor/BUILD +++ b/vendor/BUILD @@ -13508,6 +13508,7 @@ go_test( deps = [ "//vendor:github.com/davecgh/go-spew/spew", "//vendor:github.com/ghodss/yaml", + "//vendor:k8s.io/apimachinery/pkg/util/mergepatch", ], ) @@ -13516,9 +13517,8 @@ go_library( srcs = ["k8s.io/apimachinery/pkg/util/strategicpatch/patch.go"], tags = ["automanaged"], deps = [ - "//vendor:github.com/davecgh/go-spew/spew", - "//vendor:github.com/ghodss/yaml", "//vendor:k8s.io/apimachinery/pkg/util/json", + "//vendor:k8s.io/apimachinery/pkg/util/mergepatch", "//vendor:k8s.io/apimachinery/third_party/forked/golang/json", ], ) @@ -14269,7 +14269,7 @@ go_library( deps = [ "//vendor:github.com/evanphx/json-patch", "//vendor:k8s.io/apimachinery/pkg/util/json", - "//vendor:k8s.io/apimachinery/pkg/util/strategicpatch", + "//vendor:k8s.io/apimachinery/pkg/util/mergepatch", ], ) @@ -14450,6 +14450,7 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/runtime/serializer/streaming", "//vendor:k8s.io/apimachinery/pkg/types", "//vendor:k8s.io/apimachinery/pkg/util/httpstream", + "//vendor:k8s.io/apimachinery/pkg/util/mergepatch", "//vendor:k8s.io/apimachinery/pkg/util/net", "//vendor:k8s.io/apimachinery/pkg/util/runtime", "//vendor:k8s.io/apimachinery/pkg/util/strategicpatch", @@ -15171,3 +15172,23 @@ go_library( "//vendor:k8s.io/client-go/pkg/api/install", ], ) + +go_test( + name = "k8s.io/apimachinery/pkg/util/mergepatch_test", + srcs = ["k8s.io/apimachinery/pkg/util/mergepatch/util_test.go"], + library = ":k8s.io/apimachinery/pkg/util/mergepatch", + tags = ["automanaged"], +) + +go_library( + name = "k8s.io/apimachinery/pkg/util/mergepatch", + srcs = [ + "k8s.io/apimachinery/pkg/util/mergepatch/errors.go", + "k8s.io/apimachinery/pkg/util/mergepatch/util.go", + ], + tags = ["automanaged"], + deps = [ + "//vendor:github.com/davecgh/go-spew/spew", + "//vendor:github.com/ghodss/yaml", + ], +)