From 9c0320f1bfd8edfb11c283b3fd422dc6e49360c5 Mon Sep 17 00:00:00 2001 From: "Julian V. Modesto" Date: Thu, 2 Apr 2020 17:14:17 -0400 Subject: [PATCH] Ensure diff doesn't persist patches --- .../src/k8s.io/kubectl/pkg/cmd/apply/patcher.go | 15 ++++----------- staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go | 10 +++++----- test/cmd/diff.sh | 13 +++++++++++++ 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go index 9d00c982a98..5c57aa5bce2 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/patcher.go @@ -80,16 +80,9 @@ func newPatcher(o *ApplyOptions, info *resource.Info) (*Patcher, error) { openapiSchema = o.OpenAPISchema } - helper := resource.NewHelper(info.Client, info.Mapping) - if o.DryRunStrategy == cmdutil.DryRunServer { - if err := o.DryRunVerifier.HasSupport(info.Mapping.GroupVersionKind); err != nil { - return nil, err - } - helper.DryRun(true) - } return &Patcher{ Mapping: info.Mapping, - Helper: helper, + Helper: resource.NewHelper(info.Client, info.Mapping), DynamicClient: o.DynamicClient, Overwrite: o.Overwrite, BackOff: clockwork.NewRealClock(), @@ -185,7 +178,7 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, source, names } } - patchedObj, err := p.Helper.Patch(namespace, name, patchType, patch, nil) + patchedObj, err := p.Helper.DryRun(p.ServerDryRun).Patch(namespace, name, patchType, patch, nil) return patch, patchedObj, err } @@ -230,11 +223,11 @@ func (p *Patcher) deleteAndCreate(original runtime.Object, modified []byte, name if err != nil { return modified, nil, err } - createdObject, err := p.Helper.Create(namespace, true, versionedObject) + createdObject, err := p.Helper.DryRun(p.ServerDryRun).Create(namespace, true, versionedObject) if err != nil { // restore the original object if we fail to create the new one // but still propagate and advertise error to user - recreated, recreateErr := p.Helper.Create(namespace, true, original) + recreated, recreateErr := p.Helper.DryRun(p.ServerDryRun).Create(namespace, true, original) if recreateErr != nil { err = fmt.Errorf("An error occurred force-replacing the existing object with the newly provided one:\n\n%v.\n\nAdditionally, an error occurred attempting to restore the original object:\n\n%v", err, recreateErr) } else { diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go index 196d599b6fb..a90e2222c43 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go @@ -312,6 +312,7 @@ func (obj InfoObject) Live() runtime.Object { // Returns the "merged" object, as it would look like if applied or // created. func (obj InfoObject) Merged() (runtime.Object, error) { + helper := resource.NewHelper(obj.Info.Client, obj.Info.Mapping).DryRun(true) if obj.ServerSideApply { data, err := runtime.Encode(unstructured.UnstructuredJSONScheme, obj.LocalObj) if err != nil { @@ -320,9 +321,8 @@ func (obj InfoObject) Merged() (runtime.Object, error) { options := metav1.PatchOptions{ Force: &obj.ForceConflicts, FieldManager: obj.FieldManager, - DryRun: []string{metav1.DryRunAll}, } - return resource.NewHelper(obj.Info.Client, obj.Info.Mapping).Patch( + return helper.Patch( obj.Info.Namespace, obj.Info.Name, types.ApplyPatchType, @@ -334,11 +334,11 @@ func (obj InfoObject) Merged() (runtime.Object, error) { // Build the patcher, and then apply the patch with dry-run, unless the object doesn't exist, in which case we need to create it. if obj.Live() == nil { // Dry-run create if the object doesn't exist. - return resource.NewHelper(obj.Info.Client, obj.Info.Mapping).CreateWithOptions( + return helper.CreateWithOptions( obj.Info.Namespace, true, obj.LocalObj, - &metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}, + &metav1.CreateOptions{}, ) } @@ -361,7 +361,7 @@ func (obj InfoObject) Merged() (runtime.Object, error) { // We plan on replacing this with server-side apply when it becomes available. patcher := &apply.Patcher{ Mapping: obj.Info.Mapping, - Helper: resource.NewHelper(obj.Info.Client, obj.Info.Mapping), + Helper: helper, Overwrite: true, BackOff: clockwork.NewRealClock(), ServerDryRun: true, diff --git a/test/cmd/diff.sh b/test/cmd/diff.sh index c1eaaea46bb..f0bf6f7e3d3 100755 --- a/test/cmd/diff.sh +++ b/test/cmd/diff.sh @@ -34,21 +34,34 @@ run_kubectl_diff_tests() { kubectl apply -f hack/testdata/pod.yaml kube::test::get_object_assert 'pod' "{{range.items}}{{ if eq ${id_field:?} \\\"test-pod\\\" }}found{{end}}{{end}}:" 'found:' + initialResourceVersion=$(kubectl get "${kube_flags[@]:?}" -f hack/testdata/pod.yaml -o go-template='{{ .metadata.resourceVersion }}') # Make sure that diffing the resource right after returns nothing (0 exit code). kubectl diff -f hack/testdata/pod.yaml + # Ensure diff only dry-runs and doesn't persist change + resourceVersion=$(kubectl get "${kube_flags[@]:?}" -f hack/testdata/pod.yaml -o go-template='{{ .metadata.resourceVersion }}') + kube::test::if_has_string "${resourceVersion}" "${initialResourceVersion}" + # Make sure that: # 1. the exit code for diff is 1 because it found a difference # 2. the difference contains the changed image output_message=$(kubectl diff -f hack/testdata/pod-changed.yaml || test $? -eq 1) kube::test::if_has_string "${output_message}" 'k8s.gcr.io/pause:3.0' + # Ensure diff only dry-runs and doesn't persist change + resourceVersion=$(kubectl get "${kube_flags[@]:?}" -f hack/testdata/pod.yaml -o go-template='{{ .metadata.resourceVersion }}') + kube::test::if_has_string "${resourceVersion}" "${initialResourceVersion}" + # Test found diff with server-side apply kubectl apply -f hack/testdata/pod.yaml output_message=$(kubectl diff -f hack/testdata/pod-changed.yaml --server-side --force-conflicts || test $? -eq 1) kube::test::if_has_string "${output_message}" 'k8s.gcr.io/pause:3.0' + # Ensure diff --server-side only dry-runs and doesn't persist change + resourceVersion=$(kubectl get "${kube_flags[@]:?}" -f hack/testdata/pod.yaml -o go-template='{{ .metadata.resourceVersion }}') + kube::test::if_has_string "${resourceVersion}" "${initialResourceVersion}" + # Test that we have a return code bigger than 1 if there is an error when diffing kubectl diff -f hack/testdata/invalid-pod.yaml || test $? -gt 1