From 05e84fafbe5418b2c54f665229cbb32771a602e4 Mon Sep 17 00:00:00 2001 From: deads2k Date: Wed, 30 Sep 2015 10:39:08 -0400 Subject: [PATCH] move HasConflicts to shareable location --- pkg/apiserver/resthandler.go | 46 ++---------- pkg/kubectl/cmd/util/jsonmerge/jsonmerge.go | 48 ++---------- .../cmd/util/jsonmerge/jsonmerge_test.go | 75 ------------------- pkg/util/strategicpatch/patch.go | 41 ++++++++++ pkg/util/strategicpatch/patch_test.go | 63 ++++++++++++++++ 5 files changed, 116 insertions(+), 157 deletions(-) delete mode 100644 pkg/kubectl/cmd/util/jsonmerge/jsonmerge_test.go diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index 8c313262c3e..200150e4cc2 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -22,7 +22,6 @@ import ( "net/http" "net/url" gpath "path" - "reflect" "strings" "time" @@ -497,7 +496,11 @@ func patchResource(ctx api.Context, timeout time.Duration, versionedObj runtime. if err := json.Unmarshal(currentPatch, &diff2); err != nil { return nil, err } - if hasConflicts(diff1, diff2) { + hasConflicts, err := strategicpatch.HasConflicts(diff1, diff2) + if err != nil { + return nil, err + } + if hasConflicts { return updateObject, updateErr } @@ -516,45 +519,6 @@ func patchResource(ctx api.Context, timeout time.Duration, versionedObj runtime. }) } -// hasConflicts returns true if the left and right JSON interface objects overlap with -// different values in any key. The code will panic if an unrecognized type is passed -// (anything not returned by a JSON decode). All keys are required to be strings. -func hasConflicts(left, right interface{}) bool { - switch typedLeft := left.(type) { - case map[string]interface{}: - switch typedRight := right.(type) { - case map[string]interface{}: - for key, leftValue := range typedLeft { - if rightValue, ok := typedRight[key]; ok && hasConflicts(leftValue, rightValue) { - return true - } - } - return false - default: - return true - } - case []interface{}: - switch typedRight := right.(type) { - case []interface{}: - if len(typedLeft) != len(typedRight) { - return true - } - for i := range typedLeft { - if hasConflicts(typedLeft[i], typedRight[i]) { - return true - } - } - return false - default: - return true - } - case string, float64, bool, int, int64, nil: - return !reflect.DeepEqual(left, right) - default: - panic(fmt.Sprintf("unknown type: %v", reflect.TypeOf(left))) - } -} - // UpdateResource returns a function that will handle a resource update func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectTyper, admit admission.Interface) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { diff --git a/pkg/kubectl/cmd/util/jsonmerge/jsonmerge.go b/pkg/kubectl/cmd/util/jsonmerge/jsonmerge.go index c3b2244dbfb..84a76da9541 100644 --- a/pkg/kubectl/cmd/util/jsonmerge/jsonmerge.go +++ b/pkg/kubectl/cmd/util/jsonmerge/jsonmerge.go @@ -19,11 +19,11 @@ package jsonmerge import ( "encoding/json" "fmt" - "reflect" "github.com/evanphx/json-patch" "github.com/golang/glog" + "k8s.io/kubernetes/pkg/util/strategicpatch" "k8s.io/kubernetes/pkg/util/yaml" ) @@ -161,9 +161,14 @@ 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)) - if hasConflicts(diff1, diff2) { + hasConflicts, err := strategicpatch.HasConflicts(diff1, diff2) + if err != nil { + return nil, err + } + if hasConflicts { return nil, ErrConflict } + return jsonpatch.MergePatch(base, d.edit) } @@ -183,45 +188,6 @@ func IsPreconditionFailed(err error) bool { var ErrPreconditionFailed = fmt.Errorf("a precondition failed") var ErrConflict = fmt.Errorf("changes are in conflict") -// hasConflicts returns true if the left and right JSON interface objects overlap with -// different values in any key. The code will panic if an unrecognized type is passed -// (anything not returned by a JSON decode). All keys are required to be strings. -func hasConflicts(left, right interface{}) bool { - switch typedLeft := left.(type) { - case map[string]interface{}: - switch typedRight := right.(type) { - case map[string]interface{}: - for key, leftValue := range typedLeft { - if rightValue, ok := typedRight[key]; ok && hasConflicts(leftValue, rightValue) { - return true - } - } - return false - default: - return true - } - case []interface{}: - switch typedRight := right.(type) { - case []interface{}: - if len(typedLeft) != len(typedRight) { - return true - } - for i := range typedLeft { - if hasConflicts(typedLeft[i], typedRight[i]) { - return true - } - } - return false - default: - return true - } - case string, float64, bool, int, int64, nil: - return !reflect.DeepEqual(left, right) - default: - panic(fmt.Sprintf("unknown type: %v", reflect.TypeOf(left))) - } -} - func (d *Delta) Edit() []byte { return d.edit } diff --git a/pkg/kubectl/cmd/util/jsonmerge/jsonmerge_test.go b/pkg/kubectl/cmd/util/jsonmerge/jsonmerge_test.go deleted file mode 100644 index 2c9559a70f0..00000000000 --- a/pkg/kubectl/cmd/util/jsonmerge/jsonmerge_test.go +++ /dev/null @@ -1,75 +0,0 @@ -/* -Copyright 2015 The Kubernetes Authors 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 jsonmerge - -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: "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 := hasConflicts(testCase.A, testCase.B) - if out != testCase.Ret { - t.Errorf("%d: expected %t got %t", i, testCase.Ret, out) - continue - } - out = hasConflicts(testCase.B, testCase.A) - if out != testCase.Ret { - t.Errorf("%d: expected reversed %t got %t", i, testCase.Ret, out) - } - } -} diff --git a/pkg/util/strategicpatch/patch.go b/pkg/util/strategicpatch/patch.go index 6a0861091bc..9e66d62e1cb 100644 --- a/pkg/util/strategicpatch/patch.go +++ b/pkg/util/strategicpatch/patch.go @@ -827,3 +827,44 @@ 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. The code will panic if an unrecognized type is passed +// (anything not returned by a JSON decode). All keys are required to be strings. +// Since patches of the same Type have congruent keys, this is valid for multiple patch +// types. +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/pkg/util/strategicpatch/patch_test.go b/pkg/util/strategicpatch/patch_test.go index 08411114d04..be1b382138a 100644 --- a/pkg/util/strategicpatch/patch_test.go +++ b/pkg/util/strategicpatch/patch_test.go @@ -764,3 +764,66 @@ func jsonToYAML(j []byte) ([]byte, error) { return y, nil } + +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) + } + } +}