From 481a126f5fc237a07b2150a9b4a09b2a23d4f797 Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Tue, 31 Jan 2017 13:36:32 +0100 Subject: [PATCH 1/2] UPSTREAM: 41043: allow setting replace patchStrategy for structs --- .../pkg/util/strategicpatch/patch.go | 13 +- .../pkg/util/strategicpatch/patch_test.go | 332 +++++++++++++++++- 2 files changed, 334 insertions(+), 11 deletions(-) diff --git a/vendor/k8s.io/kubernetes/pkg/util/strategicpatch/patch.go b/vendor/k8s.io/kubernetes/pkg/util/strategicpatch/patch.go index 66a33f7ad08a..72bb44d27231 100644 --- a/vendor/k8s.io/kubernetes/pkg/util/strategicpatch/patch.go +++ b/vendor/k8s.io/kubernetes/pkg/util/strategicpatch/patch.go @@ -225,11 +225,18 @@ func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreC switch originalValueTyped := originalValue.(type) { case map[string]interface{}: modifiedValueTyped := modifiedValue.(map[string]interface{}) - fieldType, _, _, err := forkedjson.LookupPatchMetadata(t, key) + fieldType, fieldPatchStrategy, _, err := forkedjson.LookupPatchMetadata(t, key) if err != nil { return nil, err } + if fieldPatchStrategy == replaceDirective { + if !ignoreChangesAndAdditions { + patch[key] = modifiedValue + } + continue + } + patchValue, err := diffMaps(originalValueTyped, modifiedValueTyped, fieldType, ignoreChangesAndAdditions, ignoreDeletions) if err != nil { return nil, err @@ -1037,6 +1044,10 @@ func mergingMapFieldsHaveConflicts( } } + if fieldPatchStrategy == replaceDirective { + return false, nil + } + // Check the individual keys. return mapsHaveConflicts(leftType, rightType, fieldType) default: diff --git a/vendor/k8s.io/kubernetes/pkg/util/strategicpatch/patch_test.go b/vendor/k8s.io/kubernetes/pkg/util/strategicpatch/patch_test.go index 0aa7c37cb222..be3859866d50 100644 --- a/vendor/k8s.io/kubernetes/pkg/util/strategicpatch/patch_test.go +++ b/vendor/k8s.io/kubernetes/pkg/util/strategicpatch/patch_test.go @@ -25,6 +25,8 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/ghodss/yaml" + + "k8s.io/kubernetes/pkg/runtime" ) type SortMergeListTestCases struct { @@ -46,6 +48,11 @@ type StrategicMergePatchTestCase struct { StrategicMergePatchTestCaseData } +type StrategicMergePatchRawTestCase struct { + Description string + StrategicMergePatchRawTestCaseData +} + type StrategicMergePatchTestCaseData struct { Original map[string]interface{} TwoWay map[string]interface{} @@ -55,6 +62,16 @@ type StrategicMergePatchTestCaseData struct { Result map[string]interface{} } +type StrategicMergePatchRawTestCaseData struct { + Original []byte + Modified []byte + Current []byte + TwoWay []byte + ThreeWay []byte + Result []byte + TwoWayResult []byte +} + type MergeItem struct { Name string Value string @@ -65,6 +82,7 @@ type MergeItem struct { NonMergingIntList []int MergeItemPtr *MergeItem `patchStrategy:"merge" patchMergeKey:"name"` SimpleMap map[string]string + ReplacingItem runtime.RawExtension `patchStrategy:"replace"` } var mergeItem MergeItem @@ -491,7 +509,7 @@ testCases: threeWay: name: null value: null - result: + result: other: a - description: delete all fields from map with conflict original: @@ -508,7 +526,7 @@ testCases: threeWay: name: null value: null - result: + result: other: a - description: add field and delete all fields from map original: @@ -636,12 +654,12 @@ testCases: mergingList: - name: 3 value: 3 - - name: 4 - value: 4 + - name: 4 + value: 4 modified: mergingList: - - name: 4 - value: 4 + - name: 4 + value: 4 - name: 1 - name: 2 value: 2 @@ -658,8 +676,8 @@ testCases: mergingList: - name: 3 value: 3 - - name: 4 - value: 4 + - name: 4 + value: 4 result: mergingList: - name: 1 @@ -669,8 +687,8 @@ testCases: other: b - name: 3 value: 3 - - name: 4 - value: 4 + - name: 4 + value: 4 - description: merge lists of maps with conflict original: mergingList: @@ -1836,6 +1854,17 @@ func twoWayTestCaseToJSONOrFail(t *testing.T, c StrategicMergePatchTestCase) ([] testObjectToJSONOrFail(t, c.Modified, c.Description) } +func twoWayRawTestCaseToJSONOrFail(t *testing.T, c StrategicMergePatchRawTestCase) ([]byte, []byte, []byte, []byte) { + expectedResult := c.TwoWayResult + if expectedResult == nil { + expectedResult = c.Modified + } + return yamlToJSONOrError(t, c.Original), + yamlToJSONOrError(t, c.TwoWay), + yamlToJSONOrError(t, c.Modified), + yamlToJSONOrError(t, expectedResult) +} + func testThreeWayPatch(t *testing.T, c StrategicMergePatchTestCase) { original, modified, current, expected, result := threeWayTestCaseToJSONOrFail(t, c) actual, err := CreateThreeWayMergePatch(original, modified, current, mergeItem, false) @@ -1885,6 +1914,14 @@ func threeWayTestCaseToJSONOrFail(t *testing.T, c StrategicMergePatchTestCase) ( testObjectToJSONOrFail(t, c.Result, c.Description) } +func threeWayRawTestCaseToJSONOrFail(t *testing.T, c StrategicMergePatchRawTestCase) ([]byte, []byte, []byte, []byte, []byte) { + return yamlToJSONOrError(t, c.Original), + yamlToJSONOrError(t, c.Modified), + yamlToJSONOrError(t, c.Current), + yamlToJSONOrError(t, c.ThreeWay), + yamlToJSONOrError(t, c.Result) +} + func testPatchCreation(t *testing.T, expected, actual []byte, description string) { sorted, err := sortMergeListsByName(actual, mergeItem) if err != nil { @@ -1970,6 +2007,24 @@ func jsonToYAML(j []byte) ([]byte, error) { return y, nil } +func yamlToJSON(y []byte) ([]byte, error) { + j, err := yaml.YAMLToJSON(y) + if err != nil { + return nil, fmt.Errorf("yaml to json failed: %v\n%v\n", err, y) + } + + return j, nil +} + +func yamlToJSONOrError(t *testing.T, y []byte) []byte { + j, err := yamlToJSON(y) + if err != nil { + t.Errorf("%v", err) + } + + return j +} + func TestHasConflicts(t *testing.T) { testCases := []struct { A interface{} @@ -2116,3 +2171,260 @@ func TestNumberConversion(t *testing.T) { } } } + +var replaceRawExtensionPatchTestCases = []StrategicMergePatchRawTestCase{ + { + Description: "replace RawExtension field, rest unchanched", + StrategicMergePatchRawTestCaseData: StrategicMergePatchRawTestCaseData{ + Original: []byte(` +name: my-object +value: some-value +other: current-other +replacingItem: + Some: Generic + Yaml: Inside + The: RawExtension + Field: Period +`), + Current: []byte(` +name: my-object +value: some-value +other: current-other +merginglist: + - name: 1 + - name: 2 + - name: 3 +replacingItem: + Some: Generic + Yaml: Inside + The: RawExtension + Field: Period +`), + Modified: []byte(` +name: my-object +value: some-value +other: current-other +merginglist: + - name: 1 + - name: 2 + - name: 3 +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + TwoWay: []byte(` +merginglist: + - name: 1 + - name: 2 + - name: 3 +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + TwoWayResult: []byte(` +name: my-object +value: some-value +other: current-other +merginglist: + - name: 1 + - name: 2 + - name: 3 +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + ThreeWay: []byte(` +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + Result: []byte(` +name: my-object +value: some-value +other: current-other +merginglist: + - name: 1 + - name: 2 + - name: 3 +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + }, + }, + { + Description: "replace RawExtension field and merge list", + StrategicMergePatchRawTestCaseData: StrategicMergePatchRawTestCaseData{ + Original: []byte(` +name: my-object +value: some-value +other: current-other +merginglist: + - name: 1 +replacingItem: + Some: Generic + Yaml: Inside + The: RawExtension + Field: Period +`), + Current: []byte(` +name: my-object +value: some-value +other: current-other +merginglist: + - name: 1 + - name: 3 +replacingItem: + Some: Generic + Yaml: Inside + The: RawExtension + Field: Period +`), + Modified: []byte(` +name: my-object +value: some-value +other: current-other +merginglist: + - name: 1 + - name: 2 +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + TwoWay: []byte(` +merginglist: + - name: 2 +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + TwoWayResult: []byte(` +name: my-object +value: some-value +other: current-other +merginglist: + - name: 1 + - name: 2 +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + ThreeWay: []byte(` +merginglist: + - name: 2 +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + Result: []byte(` +name: my-object +value: some-value +other: current-other +merginglist: + - name: 1 + - name: 3 + - name: 2 +replacingItem: + Newly: Modified + Yaml: Inside + The: RawExtension +`), + }, + }, +} + +func TestReplaceWithRawExtension(t *testing.T) { + for _, c := range replaceRawExtensionPatchTestCases { + testTwoWayPatchWithoutSorting(t, c) + testThreeWayPatchWithoutSorting(t, c) + } +} + +func testTwoWayPatchWithoutSorting(t *testing.T, c StrategicMergePatchRawTestCase) { + original, expectedPatch, modified, expectedResult := twoWayRawTestCaseToJSONOrFail(t, c) + + actualPatch, err := CreateTwoWayMergePatch(original, modified, mergeItem) + if err != nil { + t.Errorf("error: %s\nin test case: %s\ncannot create two 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 + } + + testPatchCreationWithoutSorting(t, expectedPatch, actualPatch, c.Description) + testPatchApplicationWithoutSorting(t, original, actualPatch, expectedResult, c.Description) +} + +func testThreeWayPatchWithoutSorting(t *testing.T, c StrategicMergePatchRawTestCase) { + original, modified, current, expected, result := threeWayRawTestCaseToJSONOrFail(t, c) + actual, err := CreateThreeWayMergePatch(original, modified, current, mergeItem, false) + if err != nil { + if !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 + } + + if !strings.Contains(c.Description, "conflict") { + t.Errorf("unexpected conflict: %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 + } + + if len(c.Result) > 0 { + 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:\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 + } + + testPatchCreationWithoutSorting(t, expected, actual, c.Description) + testPatchApplicationWithoutSorting(t, current, actual, result, c.Description) + } + + return + } + + if strings.Contains(c.Description, "conflict") || len(c.Result) < 1 { + t.Errorf("error: %s\nin test case: %s\nexpected conflict did not occur:\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 + } + + testPatchCreationWithoutSorting(t, expected, actual, c.Description) + testPatchApplicationWithoutSorting(t, current, actual, result, c.Description) +} + +func testPatchCreationWithoutSorting(t *testing.T, expected, actual []byte, description string) { + if !reflect.DeepEqual(actual, expected) { + t.Errorf("error in test case: %s\nexpected patch:\n%s\ngot:\n%s\n", + description, jsonToYAMLOrError(expected), jsonToYAMLOrError(actual)) + return + } +} + +func testPatchApplicationWithoutSorting(t *testing.T, original, patch, expected []byte, description string) { + result, err := StrategicMergePatch(original, patch, mergeItem) + if err != nil { + t.Errorf("error: %s\nin test case: %s\ncannot apply patch:\n%s\nto original:\n%s\n", + err, description, jsonToYAMLOrError(patch), jsonToYAMLOrError(original)) + return + } + + if !reflect.DeepEqual(result, expected) { + format := "error in test case: %s\npatch application failed:\noriginal:\n%s\npatch:\n%s\nexpected:\n%s\ngot:\n%s\n" + t.Errorf(format, description, + jsonToYAMLOrError(original), jsonToYAMLOrError(patch), + jsonToYAMLOrError(expected), jsonToYAMLOrError(result)) + return + } +} From e208952c702135009e49ab32f6a9b50ef1ab77e5 Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Mon, 6 Feb 2017 18:28:36 +0100 Subject: [PATCH 2/2] Add replace patch strategy for DockerImageMetadata and cmd tests for oc edit istag --- pkg/image/api/v1/types.go | 2 +- test/cmd/edit.sh | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/image/api/v1/types.go b/pkg/image/api/v1/types.go index e47e9e914f67..73895331f3c0 100644 --- a/pkg/image/api/v1/types.go +++ b/pkg/image/api/v1/types.go @@ -27,7 +27,7 @@ type Image struct { // DockerImageReference is the string that can be used to pull this image. DockerImageReference string `json:"dockerImageReference,omitempty" protobuf:"bytes,2,opt,name=dockerImageReference"` // DockerImageMetadata contains metadata about this image - DockerImageMetadata runtime.RawExtension `json:"dockerImageMetadata,omitempty" protobuf:"bytes,3,opt,name=dockerImageMetadata"` + DockerImageMetadata runtime.RawExtension `json:"dockerImageMetadata,omitempty" patchStrategy:"replace" protobuf:"bytes,3,opt,name=dockerImageMetadata"` // DockerImageMetadataVersion conveys the version of the object, which if empty defaults to "1.0" DockerImageMetadataVersion string `json:"dockerImageMetadataVersion,omitempty" protobuf:"bytes,4,opt,name=dockerImageMetadataVersion"` // DockerImageManifest is the raw JSON of the manifest diff --git a/test/cmd/edit.sh b/test/cmd/edit.sh index f853f7da61e8..affa62f2130c 100755 --- a/test/cmd/edit.sh +++ b/test/cmd/edit.sh @@ -22,5 +22,17 @@ os::cmd::expect_success_and_not_text 'OC_EDITOR=cat oc edit --windows-line-endin os::cmd::expect_success 'oc create -f test/testdata/services.yaml' os::cmd::expect_success_and_text 'OC_EDITOR=cat oc edit svc' 'kind: List' + +os::cmd::expect_success 'oc create imagestream test' +os::cmd::expect_success 'oc create -f test/testdata/mysql-image-stream-mapping.yaml' +os::cmd::expect_success_and_not_text 'oc get istag/test:new -o jsonpath={.metadata.annotations}' "tags:hidden" +editorfile="$(mktemp -d)/tmp-editor.sh" +echo '#!/bin/bash' > ${editorfile} +echo 'sed -i "s/^tag: null/tag:\n referencePolicy:\n type: Source/g" $1' >> ${editorfile} +echo 'sed -i "s/^metadata:$/metadata:\n annotations:\n tags: hidden/g" $1' >> ${editorfile} +chmod +x ${editorfile} +os::cmd::expect_success "EDITOR=${editorfile} oc edit istag/test:new" +os::cmd::expect_success_and_text 'oc get istag/test:new -o jsonpath={.metadata.annotations}' "tags:hidden" + echo "edit: ok" os::test::junit::declare_suite_end