diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 403c03d201..7b9494c548 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -1259,17 +1259,7 @@ __EOF__ kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" '' # Test that we can create a new resource of type Foo - kubectl "${kube_flags[@]}" create -f - "${kube_flags[@]}" << __EOF__ -{ - "kind": "Foo", - "apiVersion": "company.com/v1", - "metadata": { - "name": "test" - }, - "some-field": "field1", - "other-field": "field2" -} -__EOF__ + kubectl "${kube_flags[@]}" create -f hack/testdata/TPR/foo.yaml "${kube_flags[@]}" # Test that we can list this new third party resource kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" 'test:' @@ -1291,10 +1281,10 @@ __EOF__ kubectl "${kube_flags[@]}" get foos/test -o json kubectl "${kube_flags[@]}" get foos -o yaml kubectl "${kube_flags[@]}" get foos/test -o yaml - kubectl "${kube_flags[@]}" get foos -o "jsonpath={.items[*].some-field}" --allow-missing-template-keys=false - kubectl "${kube_flags[@]}" get foos/test -o "jsonpath={.some-field}" --allow-missing-template-keys=false - kubectl "${kube_flags[@]}" get foos -o "go-template={{range .items}}{{index . \"some-field\"}}{{end}}" --allow-missing-template-keys=false - kubectl "${kube_flags[@]}" get foos/test -o "go-template={{index . \"some-field\"}}" --allow-missing-template-keys=false + kubectl "${kube_flags[@]}" get foos -o "jsonpath={.items[*].someField}" --allow-missing-template-keys=false + kubectl "${kube_flags[@]}" get foos/test -o "jsonpath={.someField}" --allow-missing-template-keys=false + kubectl "${kube_flags[@]}" get foos -o "go-template={{range .items}}{{.someField}}{{end}}" --allow-missing-template-keys=false + kubectl "${kube_flags[@]}" get foos/test -o "go-template={{.someField}}" --allow-missing-template-keys=false # Test patching kube::log::status "Testing ThirdPartyResource patching" @@ -1348,17 +1338,7 @@ __EOF__ kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" '' # Test that we can create a new resource of type Bar - kubectl "${kube_flags[@]}" create -f - "${kube_flags[@]}" << __EOF__ -{ - "kind": "Bar", - "apiVersion": "company.com/v1", - "metadata": { - "name": "test" - }, - "some-field": "field1", - "other-field": "field2" -} -__EOF__ + kubectl "${kube_flags[@]}" create -f hack/testdata/TPR/bar.yaml "${kube_flags[@]}" # Test that we can list this new third party resource kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" 'test:' @@ -1369,6 +1349,136 @@ __EOF__ # Make sure it's gone kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" '' + # Test that we can create single item via apply + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/foo.yaml + + # Test that we have create a foo named test + kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" 'test:' + + # Test that the field has the expected value + kube::test::get_object_assert foos/test '{{.someField}}' 'field1' + + # Test that apply an empty patch doesn't change fields + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/foo.yaml + + # Test that the field has the same value after re-apply + kube::test::get_object_assert foos/test '{{.someField}}' 'field1' + + # Test that apply has updated the subfield + kube::test::get_object_assert foos/test '{{.nestedField.someSubfield}}' 'subfield1' + + # Update a subfield and then apply the change + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/foo-updated-subfield.yaml + + # Test that apply has updated the subfield + kube::test::get_object_assert foos/test '{{.nestedField.someSubfield}}' 'modifiedSubfield' + + # Test that the field has the expected value + kube::test::get_object_assert foos/test '{{.nestedField.otherSubfield}}' 'subfield2' + + # Delete a subfield and then apply the change + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/foo-deleted-subfield.yaml + + # Test that apply has deleted the field + kube::test::get_object_assert foos/test '{{.nestedField.otherSubfield}}' '' + + # Test that the field does not exist + kube::test::get_object_assert foos/test '{{.nestedField.newSubfield}}' '' + + # Add a field and then apply the change + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/foo-added-subfield.yaml + + # Test that apply has added the field + kube::test::get_object_assert foos/test '{{.nestedField.newSubfield}}' 'subfield3' + + # Delete the resource + kubectl "${kube_flags[@]}" delete -f hack/testdata/TPR/foo.yaml + + # Make sure it's gone + kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" '' + + # Test that we can create list via apply + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/multi-tpr-list.yaml + + # Test that we have create a foo and a bar from a list + kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" 'test-list:' + kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" 'test-list:' + + # Test that the field has the expected value + kube::test::get_object_assert foos/test-list '{{.someField}}' 'field1' + kube::test::get_object_assert bars/test-list '{{.someField}}' 'field1' + + # Test that re-apply an list doesn't change anything + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/multi-tpr-list.yaml + + # Test that the field has the same value after re-apply + kube::test::get_object_assert foos/test-list '{{.someField}}' 'field1' + kube::test::get_object_assert bars/test-list '{{.someField}}' 'field1' + + # Test that the fields have the expected value + kube::test::get_object_assert foos/test-list '{{.someField}}' 'field1' + kube::test::get_object_assert bars/test-list '{{.someField}}' 'field1' + + # Update fields and then apply the change + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/multi-tpr-list-updated-field.yaml + + # Test that apply has updated the fields + kube::test::get_object_assert foos/test-list '{{.someField}}' 'modifiedField' + kube::test::get_object_assert bars/test-list '{{.someField}}' 'modifiedField' + + # Test that the field has the expected value + kube::test::get_object_assert foos/test-list '{{.otherField}}' 'field2' + kube::test::get_object_assert bars/test-list '{{.otherField}}' 'field2' + + # Delete fields and then apply the change + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/multi-tpr-list-deleted-field.yaml + + # Test that apply has deleted the fields + kube::test::get_object_assert foos/test-list '{{.otherField}}' '' + kube::test::get_object_assert bars/test-list '{{.otherField}}' '' + + # Test that the fields does not exist + kube::test::get_object_assert foos/test-list '{{.newField}}' '' + kube::test::get_object_assert bars/test-list '{{.newField}}' '' + + # Add a field and then apply the change + kubectl "${kube_flags[@]}" apply -f hack/testdata/TPR/multi-tpr-list-added-field.yaml + + # Test that apply has added the field + kube::test::get_object_assert foos/test-list '{{.newField}}' 'field3' + kube::test::get_object_assert bars/test-list '{{.newField}}' 'field3' + + # Delete the resource + kubectl "${kube_flags[@]}" delete -f hack/testdata/TPR/multi-tpr-list.yaml + + # Make sure it's gone + kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" '' + kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" '' + + ## kubectl apply --prune + # Test that no foo or bar exist + kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" '' + kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" '' + + # apply --prune on foo.yaml that has foo/test + kubectl apply --prune -l pruneGroup=true -f hack/testdata/TPR/foo.yaml "${kube_flags[@]}" --prune-whitelist=company.com/v1/Foo --prune-whitelist=company.com/v1/Bar + # check right tprs exist + kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" 'test:' + kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" '' + + # apply --prune on bar.yaml that has bar/test + kubectl apply --prune -l pruneGroup=true -f hack/testdata/TPR/bar.yaml "${kube_flags[@]}" --prune-whitelist=company.com/v1/Foo --prune-whitelist=company.com/v1/Bar + # check right tprs exist + kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" '' + kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" 'test:' + + # Delete the resource + kubectl "${kube_flags[@]}" delete -f hack/testdata/TPR/bar.yaml + + # Make sure it's gone + kube::test::get_object_assert foos "{{range.items}}{{$id_field}}:{{end}}" '' + kube::test::get_object_assert bars "{{range.items}}{{$id_field}}:{{end}}" '' + # teardown kubectl delete thirdpartyresources foo.company.com "${kube_flags[@]}" kubectl delete thirdpartyresources bar.company.com "${kube_flags[@]}" diff --git a/hack/testdata/TPR/bar.yaml b/hack/testdata/TPR/bar.yaml new file mode 100644 index 0000000000..6707f45d42 --- /dev/null +++ b/hack/testdata/TPR/bar.yaml @@ -0,0 +1,8 @@ +kind: Bar +apiVersion: company.com/v1 +metadata: + name: test + labels: + pruneGroup: "true" +someField: field1 +otherField: field2 diff --git a/hack/testdata/TPR/foo-added-subfield.yaml b/hack/testdata/TPR/foo-added-subfield.yaml new file mode 100644 index 0000000000..cc0439fda3 --- /dev/null +++ b/hack/testdata/TPR/foo-added-subfield.yaml @@ -0,0 +1,11 @@ +kind: Foo +apiVersion: company.com/v1 +metadata: + name: test + labels: + pruneGroup: "true" +someField: field1 +otherField: field2 +nestedField: + someSubfield: modifiedSubfield + newSubfield: subfield3 diff --git a/hack/testdata/TPR/foo-deleted-subfield.yaml b/hack/testdata/TPR/foo-deleted-subfield.yaml new file mode 100644 index 0000000000..0adb8c67f5 --- /dev/null +++ b/hack/testdata/TPR/foo-deleted-subfield.yaml @@ -0,0 +1,10 @@ +kind: Foo +apiVersion: company.com/v1 +metadata: + name: test + labels: + pruneGroup: "true" +someField: field1 +otherField: field2 +nestedField: + someSubfield: modifiedSubfield diff --git a/hack/testdata/TPR/foo-updated-subfield.yaml b/hack/testdata/TPR/foo-updated-subfield.yaml new file mode 100644 index 0000000000..a497fdb383 --- /dev/null +++ b/hack/testdata/TPR/foo-updated-subfield.yaml @@ -0,0 +1,11 @@ +kind: Foo +apiVersion: company.com/v1 +metadata: + name: test + labels: + pruneGroup: "true" +someField: field1 +otherField: field2 +nestedField: + someSubfield: modifiedSubfield + otherSubfield: subfield2 diff --git a/hack/testdata/TPR/foo.yaml b/hack/testdata/TPR/foo.yaml new file mode 100644 index 0000000000..0a436e2d34 --- /dev/null +++ b/hack/testdata/TPR/foo.yaml @@ -0,0 +1,11 @@ +kind: Foo +apiVersion: company.com/v1 +metadata: + name: test + labels: + pruneGroup: "true" +someField: field1 +otherField: field2 +nestedField: + someSubfield: subfield1 + otherSubfield: subfield2 diff --git a/hack/testdata/TPR/multi-tpr-list-added-field.yaml b/hack/testdata/TPR/multi-tpr-list-added-field.yaml new file mode 100644 index 0000000000..ab37130ad1 --- /dev/null +++ b/hack/testdata/TPR/multi-tpr-list-added-field.yaml @@ -0,0 +1,19 @@ +apiVersion: v1 +kind: List +items: +- kind: Foo + apiVersion: company.com/v1 + metadata: + name: test-list + labels: + pruneGroup: "true" + someField: modifiedField + newField: field3 +- kind: Bar + apiVersion: company.com/v1 + metadata: + name: test-list + labels: + pruneGroup: "true" + someField: modifiedField + newField: field3 diff --git a/hack/testdata/TPR/multi-tpr-list-deleted-field.yaml b/hack/testdata/TPR/multi-tpr-list-deleted-field.yaml new file mode 100644 index 0000000000..30c802e0cd --- /dev/null +++ b/hack/testdata/TPR/multi-tpr-list-deleted-field.yaml @@ -0,0 +1,17 @@ +apiVersion: v1 +kind: List +items: +- kind: Foo + apiVersion: company.com/v1 + metadata: + name: test-list + labels: + pruneGroup: "true" + someField: modifiedField +- kind: Bar + apiVersion: company.com/v1 + metadata: + name: test-list + labels: + pruneGroup: "true" + someField: modifiedField diff --git a/hack/testdata/TPR/multi-tpr-list-updated-field.yaml b/hack/testdata/TPR/multi-tpr-list-updated-field.yaml new file mode 100644 index 0000000000..aefdd0b823 --- /dev/null +++ b/hack/testdata/TPR/multi-tpr-list-updated-field.yaml @@ -0,0 +1,19 @@ +apiVersion: v1 +kind: List +items: +- kind: Foo + apiVersion: company.com/v1 + metadata: + name: test-list + labels: + pruneGroup: "true" + someField: modifiedField + otherField: field2 +- kind: Bar + apiVersion: company.com/v1 + metadata: + name: test-list + labels: + pruneGroup: "true" + someField: modifiedField + otherField: field2 diff --git a/hack/testdata/TPR/multi-tpr-list.yaml b/hack/testdata/TPR/multi-tpr-list.yaml new file mode 100644 index 0000000000..5b2160835c --- /dev/null +++ b/hack/testdata/TPR/multi-tpr-list.yaml @@ -0,0 +1,19 @@ +apiVersion: v1 +kind: List +items: +- kind: Foo + apiVersion: company.com/v1 + metadata: + name: test-list + labels: + pruneGroup: "true" + someField: field1 + otherField: field2 +- kind: Bar + apiVersion: company.com/v1 + metadata: + name: test-list + labels: + pruneGroup: "true" + someField: field1 + otherField: field2 diff --git a/pkg/kubectl/cmd/BUILD b/pkg/kubectl/cmd/BUILD index 6378a5859a..327a8d120c 100644 --- a/pkg/kubectl/cmd/BUILD +++ b/pkg/kubectl/cmd/BUILD @@ -113,6 +113,7 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/util/errors", "//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/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 68d7e6a1b8..72cf981c3e 100644 --- a/pkg/kubectl/cmd/apply.go +++ b/pkg/kubectl/cmd/apply.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/jsonmergepatch" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/wait" @@ -144,7 +145,7 @@ func validatePruneAll(prune, all bool, selector string) error { return nil } -func parsePruneResources(gvks []string) ([]pruneResource, error) { +func parsePruneResources(mapper meta.RESTMapper, gvks []string) ([]pruneResource, error) { pruneResources := []pruneResource{} for _, groupVersionKind := range gvks { gvk := strings.Split(groupVersionKind, "/") @@ -152,15 +153,24 @@ func parsePruneResources(gvks []string) ([]pruneResource, error) { return nil, fmt.Errorf("invalid GroupVersionKind format: %v, please follow ", groupVersionKind) } - namespaced := true - if gvk[2] == "Namespace" || - gvk[2] == "Node" || - gvk[2] == "PersistentVolume" { - namespaced = false - } if gvk[0] == "core" { gvk[0] = "" } + mapping, err := mapper.RESTMapping(schema.GroupKind{Group: gvk[0], Kind: gvk[2]}, gvk[1]) + if err != nil { + return pruneResources, err + } + var namespaced bool + namespaceScope := mapping.Scope.Name() + switch namespaceScope { + case meta.RESTScopeNameNamespace: + namespaced = true + case meta.RESTScopeNameRoot: + namespaced = false + default: + return pruneResources, fmt.Errorf("Unknown namespace scope: %q", namespaceScope) + } + pruneResources = append(pruneResources, pruneResource{gvk[0], gvk[1], gvk[2], namespaced}) } return pruneResources, nil @@ -177,17 +187,18 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti return err } + mapper, typer, err := f.UnstructuredObject() + if err != nil { + return err + } + if options.Prune { - options.PruneResources, err = parsePruneResources(cmdutil.GetFlagStringArray(cmd, "prune-whitelist")) + options.PruneResources, err = parsePruneResources(mapper, cmdutil.GetFlagStringArray(cmd, "prune-whitelist")) if err != nil { return err } } - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } r := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), unstructured.UnstructuredJSONScheme). Schema(schema). ContinueOnError(). @@ -335,7 +346,7 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti } p := pruner{ mapper: mapper, - clientFunc: f.ClientForMapping, + clientFunc: f.UnstructuredClientForMapping, clientsetFunc: f.ClientSet, selector: selector, @@ -348,7 +359,7 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti out: out, } - namespacedRESTMappings, nonNamespacedRESTMappings, err := getRESTMappings(&(options.PruneResources)) + namespacedRESTMappings, nonNamespacedRESTMappings, err := getRESTMappings(mapper, &(options.PruneResources)) if err != nil { return fmt.Errorf("error retrieving RESTMappings to prune: %v", err) } @@ -380,7 +391,7 @@ func (pr pruneResource) String() string { return fmt.Sprintf("%v/%v, Kind=%v, Namespaced=%v", pr.group, pr.version, pr.kind, pr.namespaced) } -func getRESTMappings(pruneResources *[]pruneResource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) { +func getRESTMappings(mapper meta.RESTMapper, pruneResources *[]pruneResource) (namespaced, nonNamespaced []*meta.RESTMapping, err error) { if len(*pruneResources) == 0 { // default whitelist // TODO: need to handle the older api versions - e.g. v1beta1 jobs. Github issue: #35991 @@ -402,9 +413,9 @@ func getRESTMappings(pruneResources *[]pruneResource) (namespaced, nonNamespaced {"apps", "v1beta1", "StatefulSet", true}, } } - registeredMapper := api.Registry.RESTMapper() + for _, resource := range *pruneResources { - addedMapping, err := registeredMapper.RESTMapping(schema.GroupKind{Group: resource.group, Kind: resource.kind}, resource.version) + addedMapping, err := mapper.RESTMapping(schema.GroupKind{Group: resource.group, Kind: resource.kind}, resource.version) if err != nil { return nil, nil, fmt.Errorf("invalid resource %v: %v", resource, err) } @@ -548,18 +559,31 @@ func (p *patcher) patchSimple(obj runtime.Object, modified []byte, source, names // Create the versioned struct from the type defined in the restmapping // (which is the API version we'll be submitting the patch to) versionedObject, err := api.Scheme.New(p.mapping.GroupVersionKind) - if err != nil { + var patchType types.PatchType + var patch []byte + createPatchErrFormat := "creating patch with:\noriginal:\n%s\nmodified:\n%s\ncurrent:\n%s\nfor:" + switch { + 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")} + patch, err = jsonmergepatch.CreateThreeWayJSONMergePatch(original, modified, current, preconditions...) + if err != nil { + return nil, cmdutil.AddSourceToErr(fmt.Sprintf(createPatchErrFormat, original, modified, current), source, err) + } + case err != nil: return nil, cmdutil.AddSourceToErr(fmt.Sprintf("getting instance of versioned object for %v:", p.mapping.GroupVersionKind), source, err) + case err == nil: + // Compute a three way strategic merge patch to send to server. + patchType = types.StrategicMergePatchType + patch, err = strategicpatch.CreateThreeWayMergePatch(original, modified, current, versionedObject, p.overwrite) + if err != nil { + return nil, cmdutil.AddSourceToErr(fmt.Sprintf(createPatchErrFormat, original, modified, current), source, err) + } } - // Compute a three way strategic merge patch to send to server. - patch, err := strategicpatch.CreateThreeWayMergePatch(original, modified, current, versionedObject, p.overwrite) - if err != nil { - format := "creating patch with:\noriginal:\n%s\nmodified:\n%s\ncurrent:\n%s\nfor:" - return nil, cmdutil.AddSourceToErr(fmt.Sprintf(format, original, modified, current), source, err) - } - - _, err = p.helper.Patch(namespace, name, types.StrategicMergePatchType, patch) + _, err = p.helper.Patch(namespace, name, patchType, patch) return patch, 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 new file mode 100644 index 0000000000..5c6adf2f83 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/util/jsonmergepatch/patch.go @@ -0,0 +1,144 @@ +/* +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 jsonmergepatch + +import ( + "fmt" + "reflect" + + "k8s.io/apimachinery/pkg/util/json" + "github.com/evanphx/json-patch" + "k8s.io/apimachinery/pkg/util/strategicpatch" +) + +// 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) { + if len(original) == 0 { + original = []byte(`{}`) + } + if len(modified) == 0 { + modified = []byte(`{}`) + } + if len(current) == 0 { + current = []byte(`{}`) + } + + addAndChangePatch, err := jsonpatch.CreateMergePatch(current, modified) + if err != nil { + return nil, err + } + // Only keep addition and changes + addAndChangePatch, addAndChangePatchObj, err := keepOrDeleteNullInJsonPatch(addAndChangePatch, false) + if err != nil { + return nil, err + } + + deletePatch, err := jsonpatch.CreateMergePatch(original, modified) + if err != nil { + return nil, err + } + // Only keep deletion + deletePatch, deletePatchObj, err := keepOrDeleteNullInJsonPatch(deletePatch, true) + if err != nil { + return nil, err + } + + hasConflicts, err := strategicpatch.HasConflicts(addAndChangePatchObj, deletePatchObj) + if err != nil { + return nil, err + } + if hasConflicts { + return nil, strategicpatch.NewErrConflict(strategicpatch.ToYAMLOrError(addAndChangePatchObj), strategicpatch.ToYAMLOrError(deletePatchObj)) + } + patch, err := jsonpatch.MergePatch(deletePatch, addAndChangePatch) + if err != nil { + return nil, err + } + + var patchMap map[string]interface{} + err = json.Unmarshal(patch, &patchMap) + if err != nil { + return nil, fmt.Errorf("Failed to unmarshal patch for precondition check: %s", patch) + } + meetPreconditions, err := meetPreconditions(patchMap, fns...) + if err != nil { + return nil, err + } + if !meetPreconditions { + return nil, strategicpatch.NewErrPreconditionFailed(patchMap) + } + + return patch, nil +} + +// keepOrDeleteNullInJsonPatch takes a json-encoded byte array and a boolean. +// It returns a filtered object and its corresponding json-encoded byte array. +// It is a wrapper of func keepOrDeleteNullInObj +func keepOrDeleteNullInJsonPatch(patch []byte, keepNull bool) ([]byte, map[string]interface{}, error) { + var patchMap map[string]interface{} + err := json.Unmarshal(patch, &patchMap) + if err != nil { + return nil, nil, err + } + filteredMap, err := keepOrDeleteNullInObj(patchMap, keepNull) + if err != nil { + return nil, nil, err + } + o, err := json.Marshal(filteredMap) + return o, filteredMap, err +} + +// keepOrDeleteNullInObj will keep only the null value and delete all the others, +// if keepNull is true. Otherwise, it will delete all the null value and keep the others. +func keepOrDeleteNullInObj(m map[string]interface{}, keepNull bool) (map[string]interface{}, error) { + filteredMap := make(map[string]interface{}) + var err error + for key, val := range m { + switch { + case keepNull && val == nil: + filteredMap[key] = nil + case val != nil: + switch typedVal := val.(type) { + case map[string]interface{}: + filteredMap[key], err = keepOrDeleteNullInObj(typedVal, keepNull) + if err != nil { + return nil, err + } + case []interface{}, string, float64, bool, int, int64, nil: + // Lists are always replaced in Json, no need to check each entry in the list. + if !keepNull { + filteredMap[key] = val + } + default: + return nil, fmt.Errorf("unknown type: %v", reflect.TypeOf(typedVal)) + } + } + } + return filteredMap, nil +} + +func meetPreconditions(patchObj map[string]interface{}, fns ...strategicpatch.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) { + return false, fmt.Errorf("precondition failed for: %v", patchObj) + } + } + return true, nil +} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/jsonmergepatch/patch_test.go b/staging/src/k8s.io/apimachinery/pkg/util/jsonmergepatch/patch_test.go new file mode 100644 index 0000000000..49967151dd --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/util/jsonmergepatch/patch_test.go @@ -0,0 +1,664 @@ +/* +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 jsonmergepatch + +import ( + "fmt" + "reflect" + "testing" + + "github.com/evanphx/json-patch" + "github.com/ghodss/yaml" + "github.com/davecgh/go-spew/spew" + "k8s.io/apimachinery/pkg/util/json" +) + +type FilterNullTestCases struct { + TestCases []FilterNullTestCase +} + +type FilterNullTestCase struct { + Description string + OriginalObj map[string]interface{} + ExpectedWithNull map[string]interface{} + ExpectedWithoutNull map[string]interface{} +} + +var filterNullTestCaseData = []byte(` +testCases: + - description: nil original + originalObj: {} + expectedWithNull: {} + expectedWithoutNull: {} + - description: simple map + originalObj: + nilKey: null + nonNilKey: foo + expectedWithNull: + nilKey: null + expectedWithoutNull: + nonNilKey: foo + - description: simple map with all nil values + originalObj: + nilKey1: null + nilKey2: null + expectedWithNull: + nilKey1: null + nilKey2: null + expectedWithoutNull: {} + - description: simple map with all non-nil values + originalObj: + nonNilKey: foo + nonNilKey: bar + expectedWithNull: {} + expectedWithoutNull: + nonNilKey: foo + nonNilKey: bar + - description: nested map + originalObj: + mapKey: + nilKey: null + nonNilKey: foo + expectedWithNull: + mapKey: + nilKey: null + expectedWithoutNull: + mapKey: + nonNilKey: foo + - description: nested map that all subkeys are nil + originalObj: + mapKey: + nilKey1: null + nilKey2: null + expectedWithNull: + mapKey: + nilKey1: null + nilKey2: null + expectedWithoutNull: + mapKey: {} + - description: nested map that all subkeys are non-nil + originalObj: + mapKey: + nonNilKey: foo + nonNilKey: bar + expectedWithNull: + mapKey: {} + expectedWithoutNull: + mapKey: + nonNilKey: foo + nonNilKey: bar + - description: empty list + originalObj: + listKey: [] + expectedWithNull: {} + expectedWithoutNull: + listKey: [] + - description: list of primitives + originalObj: + listKey: + - 1 + - 2 + expectedWithNull: {} + expectedWithoutNull: + listKey: + - 1 + - 2 + - description: list of maps + originalObj: + listKey: + - k1: v1 + - k2: null + - k3: v3 + k4: null + expectedWithNull: {} + expectedWithoutNull: + listKey: + - k1: v1 + - k2: null + - k3: v3 + k4: null + - description: list of different types + originalObj: + listKey: + - k1: v1 + - k2: null + - v3 + expectedWithNull: {} + expectedWithoutNull: + listKey: + - k1: v1 + - k2: null + - v3 +`) + +func TestKeepOrDeleteNullInObj(t *testing.T) { + tc := FilterNullTestCases{} + err := yaml.Unmarshal(filterNullTestCaseData, &tc) + if err != nil { + t.Fatalf("can't unmarshal test cases: %s\n", err) + } + + for _, test := range tc.TestCases { + resultWithNull, err := keepOrDeleteNullInObj(test.OriginalObj, true) + if err != nil { + t.Errorf("Failed in test case %q when trying to keep null values: %s", test.Description, err) + } + if !reflect.DeepEqual(test.ExpectedWithNull, resultWithNull) { + t.Errorf("Failed in test case %q when trying to keep null values:\nexpected expectedWithNull:\n%+v\nbut got:\n%+v\n", test.Description, test.ExpectedWithNull, resultWithNull) + } + + resultWithoutNull, err := keepOrDeleteNullInObj(test.OriginalObj, false) + if err != nil { + t.Errorf("Failed in test case %q when trying to keep non-null values: %s", test.Description, err) + } + if !reflect.DeepEqual(test.ExpectedWithoutNull, resultWithoutNull) { + t.Errorf("Failed in test case %q when trying to keep non-null values:\n expected expectedWithoutNull:\n%+v\nbut got:\n%+v\n", test.Description, test.ExpectedWithoutNull, resultWithoutNull) + } + } +} + + +type JSONMergePatchTestCases struct { + TestCases []JSONMergePatchTestCase +} + +type JSONMergePatchTestCase struct { + Description string + JSONMergePatchTestCaseData +} + +type JSONMergePatchTestCaseData struct { + // Original is the original object (last-applied config in annotation) + Original map[string]interface{} + // Modified is the modified object (new config we want) + Modified map[string]interface{} + // Current is the current object (live config in the server) + Current map[string]interface{} + // ThreeWay is the expected three-way merge patch + ThreeWay map[string]interface{} + // Result is the expected object after applying the three-way patch on current object. + Result map[string]interface{} +} + +var createJSONMergePatchTestCaseData = []byte(` +testCases: + - description: nil original + modified: + name: 1 + value: 1 + current: + name: 1 + other: a + threeWay: + value: 1 + result: + name: 1 + value: 1 + other: a + - description: nil patch + original: + name: 1 + modified: + name: 1 + current: + name: 1 + threeWay: + {} + result: + name: 1 + - description: add field to map + original: + name: 1 + modified: + name: 1 + value: 1 + current: + name: 1 + other: a + threeWay: + value: 1 + result: + name: 1 + value: 1 + other: a + - description: add field to map with conflict + original: + name: 1 + modified: + name: 1 + value: 1 + current: + name: a + other: a + threeWay: + name: 1 + value: 1 + result: + name: 1 + value: 1 + other: a + - description: add field and delete field from map + original: + name: 1 + modified: + value: 1 + current: + name: 1 + other: a + threeWay: + name: null + value: 1 + result: + value: 1 + other: a + - description: add field and delete field from map with conflict + original: + name: 1 + modified: + value: 1 + current: + name: a + other: a + threeWay: + name: null + value: 1 + result: + value: 1 + other: a + - description: delete field from nested map + original: + simpleMap: + key1: 1 + key2: 1 + modified: + simpleMap: + key1: 1 + current: + simpleMap: + key1: 1 + key2: 1 + other: a + threeWay: + simpleMap: + key2: null + result: + simpleMap: + key1: 1 + other: a + - description: delete field from nested map with conflict + original: + simpleMap: + key1: 1 + key2: 1 + modified: + simpleMap: + key1: 1 + current: + simpleMap: + key1: a + key2: 1 + other: a + threeWay: + simpleMap: + key1: 1 + key2: null + result: + simpleMap: + key1: 1 + other: a + - description: delete all fields from map + original: + name: 1 + value: 1 + modified: {} + current: + name: 1 + value: 1 + other: a + threeWay: + name: null + value: null + result: + other: a + - description: delete all fields from map with conflict + original: + name: 1 + value: 1 + modified: {} + current: + name: 1 + value: a + other: a + threeWay: + name: null + value: null + result: + other: a + - description: add field and delete all fields from map + original: + name: 1 + value: 1 + modified: + other: a + current: + name: 1 + value: 1 + other: a + threeWay: + name: null + value: null + result: + other: a + - description: add field and delete all fields from map with conflict + original: + name: 1 + value: 1 + modified: + other: a + current: + name: 1 + value: 1 + other: b + threeWay: + name: null + value: null + other: a + result: + other: a + - description: replace list of scalars + original: + intList: + - 1 + - 2 + modified: + intList: + - 2 + - 3 + current: + intList: + - 1 + - 2 + threeWay: + intList: + - 2 + - 3 + result: + intList: + - 2 + - 3 + - description: replace list of scalars with conflict + original: + intList: + - 1 + - 2 + modified: + intList: + - 2 + - 3 + current: + intList: + - 1 + - 4 + threeWay: + intList: + - 2 + - 3 + result: + intList: + - 2 + - 3 + - description: patch with different scalar type + original: + foo: 1 + modified: + foo: true + current: + foo: 1 + bar: 2 + threeWay: + foo: true + result: + foo: true + bar: 2 + - description: patch from scalar to list + original: + foo: 0 + modified: + foo: + - 1 + - 2 + current: + foo: 0 + bar: 2 + threeWay: + foo: + - 1 + - 2 + result: + foo: + - 1 + - 2 + bar: 2 + - description: patch from list to scalar + original: + foo: + - 1 + - 2 + modified: + foo: 0 + current: + foo: + - 1 + - 2 + bar: 2 + threeWay: + foo: 0 + result: + foo: 0 + bar: 2 + - description: patch from scalar to map + original: + foo: 0 + modified: + foo: + baz: 1 + current: + foo: 0 + bar: 2 + threeWay: + foo: + baz: 1 + result: + foo: + baz: 1 + bar: 2 + - description: patch from map to scalar + original: + foo: + baz: 1 + modified: + foo: 0 + current: + foo: + baz: 1 + bar: 2 + threeWay: + foo: 0 + result: + foo: 0 + bar: 2 + - description: patch from map to list + original: + foo: + baz: 1 + modified: + foo: + - 1 + - 2 + current: + foo: + baz: 1 + bar: 2 + threeWay: + foo: + - 1 + - 2 + result: + foo: + - 1 + - 2 + bar: 2 + - description: patch from list to map + original: + foo: + - 1 + - 2 + modified: + foo: + baz: 0 + current: + foo: + - 1 + - 2 + bar: 2 + threeWay: + foo: + baz: 0 + result: + foo: + baz: 0 + bar: 2 + - description: patch with different nested types + original: + foo: + - a: true + - 2 + - false + modified: + foo: + - 1 + - false + - b: 1 + current: + foo: + - a: true + - 2 + - false + bar: 0 + threeWay: + foo: + - 1 + - false + - b: 1 + result: + foo: + - 1 + - false + - b: 1 + bar: 0 +`) + +func TestCreateThreeWayJSONMergePatch(t *testing.T) { + tc := JSONMergePatchTestCases{} + err := yaml.Unmarshal(createJSONMergePatchTestCaseData, &tc) + if err != nil { + t.Errorf("can't unmarshal test cases: %s\n", err) + return + } + + for _, c := range tc.TestCases { + testThreeWayPatch(t, c) + } +} + +func testThreeWayPatch(t *testing.T, c JSONMergePatchTestCase) { + original, modified, current, expected, result := threeWayTestCaseToJSONOrFail(t, c) + actual, err := CreateThreeWayJSONMergePatch(original, modified, current) + if err != nil { + t.Fatalf("error: %s", err) + } + testPatchCreation(t, expected, actual, c.Description) + testPatchApplication(t, current, actual, result, c.Description) +} + +func testPatchCreation(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 testPatchApplication(t *testing.T, original, patch, expected []byte, description string) { + result, err := jsonpatch.MergePatch(original, patch) + 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 + } +} + +func threeWayTestCaseToJSONOrFail(t *testing.T, c JSONMergePatchTestCase) ([]byte, []byte, []byte, []byte, []byte) { + return testObjectToJSONOrFail(t, c.Original), + testObjectToJSONOrFail(t, c.Modified), + testObjectToJSONOrFail(t, c.Current), + testObjectToJSONOrFail(t, c.ThreeWay), + testObjectToJSONOrFail(t, c.Result) +} + +func testObjectToJSONOrFail(t *testing.T, o map[string]interface{}) []byte { + if o == nil { + return nil + } + j, err := toJSON(o) + if err != nil { + t.Error(err) + } + return j +} + +func jsonToYAMLOrError(j []byte) string { + y, err := jsonToYAML(j) + if err != nil { + return err.Error() + } + return string(y) +} + +func toJSON(v interface{}) ([]byte, error) { + j, err := json.Marshal(v) + if err != nil { + return nil, fmt.Errorf("json marshal failed: %v\n%v\n", err, spew.Sdump(v)) + } + return j, nil +} + +func jsonToYAML(j []byte) ([]byte, error) { + y, err := yaml.JSONToYAML(j) + if err != nil { + return nil, fmt.Errorf("json to yaml failed: %v\n%v\n", err, j) + } + return y, nil +} \ No newline at end of file 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 1694aea337..7ea55e2fb2 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go @@ -58,40 +58,40 @@ type JSONMap map[string]interface{} // IsPreconditionFailed returns true if the provided error indicates // a precondition failed. func IsPreconditionFailed(err error) bool { - _, ok := err.(errPreconditionFailed) + _, ok := err.(ErrPreconditionFailed) return ok } -type errPreconditionFailed struct { +type ErrPreconditionFailed struct { message string } -func newErrPreconditionFailed(target map[string]interface{}) errPreconditionFailed { +func NewErrPreconditionFailed(target map[string]interface{}) ErrPreconditionFailed { s := fmt.Sprintf("precondition failed for: %v", target) - return errPreconditionFailed{s} + return ErrPreconditionFailed{s} } -func (err errPreconditionFailed) Error() string { +func (err ErrPreconditionFailed) Error() string { return err.message } -type errConflict struct { +type ErrConflict struct { message string } -func newErrConflict(patch, current string) errConflict { +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} + return ErrConflict{s} } -func (err errConflict) Error() string { +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) + _, ok := err.(ErrConflict) return ok } @@ -187,7 +187,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, NewErrPreconditionFailed(patchMap) } } @@ -1340,7 +1340,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, NewErrPreconditionFailed(patchMap) } } @@ -1358,14 +1358,14 @@ func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct int } if hasConflicts { - return nil, newErrConflict(toYAMLOrError(patchMap), toYAMLOrError(changedMap)) + return nil, NewErrConflict(ToYAMLOrError(patchMap), ToYAMLOrError(changedMap)) } } return json.Marshal(patchMap) } -func toYAMLOrError(v interface{}) string { +func ToYAMLOrError(v interface{}) string { y, err := toYAML(v) if err != nil { return err.Error() 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 ad5bd235a6..b29714af71 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 @@ -266,7 +266,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, ToYAMLOrError(c.Original), ToYAMLOrError(c.Sorted), jsonToYAMLOrError(original)) } } } @@ -2037,7 +2037,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, ToYAMLOrError(c.StrategicMergePatchTestCaseData)) return } @@ -2087,13 +2087,13 @@ func testThreeWayPatch(t *testing.T, c StrategicMergePatchTestCase) { if err != nil { if !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, 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, ToYAMLOrError(c.StrategicMergePatchTestCaseData)) return } @@ -2101,7 +2101,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, ToYAMLOrError(c.StrategicMergePatchTestCaseData)) return } @@ -2114,7 +2114,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, ToYAMLOrError(c.StrategicMergePatchTestCaseData)) return } diff --git a/vendor/BUILD b/vendor/BUILD index 234d2d18c5..6bd0232ccf 100644 --- a/vendor/BUILD +++ b/vendor/BUILD @@ -14192,3 +14192,27 @@ go_library( "//vendor:k8s.io/client-go/tools/clientcmd", ], ) + +go_test( + name = "k8s.io/apimachinery/pkg/util/jsonmergepatch_test", + srcs = ["k8s.io/apimachinery/pkg/util/jsonmergepatch/patch_test.go"], + library = ":k8s.io/apimachinery/pkg/util/jsonmergepatch", + tags = ["automanaged"], + deps = [ + "//vendor:github.com/davecgh/go-spew/spew", + "//vendor:github.com/evanphx/json-patch", + "//vendor:github.com/ghodss/yaml", + "//vendor:k8s.io/apimachinery/pkg/util/json", + ], +) + +go_library( + name = "k8s.io/apimachinery/pkg/util/jsonmergepatch", + srcs = ["k8s.io/apimachinery/pkg/util/jsonmergepatch/patch.go"], + tags = ["automanaged"], + deps = [ + "//vendor:github.com/evanphx/json-patch", + "//vendor:k8s.io/apimachinery/pkg/util/json", + "//vendor:k8s.io/apimachinery/pkg/util/strategicpatch", + ], +)