From 9e5c533cb61b0afe50f926dea0e5562fd2cf7ea0 Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Tue, 9 Jun 2020 14:56:57 +0200 Subject: [PATCH] Strip .meta.managedFields for kubectl edit command --- .../src/k8s.io/kubectl/pkg/cmd/edit/edit.go | 8 +- .../k8s.io/kubectl/pkg/cmd/util/editor/BUILD | 11 + .../pkg/cmd/util/editor/editoptions.go | 61 +++++ .../pkg/cmd/util/editor/editoptions_test.go | 228 ++++++++++++++++++ 4 files changed, 302 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/edit/edit.go b/staging/src/k8s.io/kubectl/pkg/cmd/edit/edit.go index f9a6015280c..d466fbc1efd 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/edit/edit.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/edit/edit.go @@ -77,12 +77,8 @@ func NewCmdEdit(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra Long: editLong, Example: editExample, Run: func(cmd *cobra.Command, args []string) { - if err := o.Complete(f, args, cmd); err != nil { - cmdutil.CheckErr(err) - } - if err := o.Run(); err != nil { - cmdutil.CheckErr(err) - } + cmdutil.CheckErr(o.Complete(f, args, cmd)) + cmdutil.CheckErr(o.Run()) }, } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/BUILD b/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/BUILD index 3c6623b4c5b..58625401390 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/BUILD +++ b/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/BUILD @@ -13,6 +13,7 @@ go_library( "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", @@ -41,6 +42,16 @@ go_test( "editor_test.go", ], embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library", + "//staging/src/k8s.io/cli-runtime/pkg/resource:go_default_library", + ], ) filegroup( diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions.go b/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions.go index 3552211b80a..b63f1d3212e 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions.go @@ -35,6 +35,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -72,6 +73,8 @@ type EditOptions struct { ApplyAnnotation bool ChangeCause string + managedFields map[types.UID][]metav1.ManagedFieldsEntry + genericclioptions.IOStreams Recorder genericclioptions.Recorder @@ -262,6 +265,10 @@ func (o *EditOptions) Run() error { } if !containsError { + if err := o.extractManagedFields(originalObj); err != nil { + return preservedFile(err, results.file, o.ErrOut) + } + if err := o.editPrinterOptions.PrintObj(originalObj, w); err != nil { return preservedFile(err, results.file, o.ErrOut) } @@ -279,6 +286,7 @@ func (o *EditOptions) Run() error { if err != nil { return preservedFile(err, results.file, o.ErrOut) } + // If we're retrying the loop because of an error, and no change was made in the file, short-circuit if containsError && bytes.Equal(cmdutil.StripComments(editedDiff), cmdutil.StripComments(edited)) { return preservedFile(fmt.Errorf("%s", "Edit cancelled, no valid changes were saved."), file, o.ErrOut) @@ -334,10 +342,19 @@ func (o *EditOptions) Run() error { results.header.reasons = append(results.header.reasons, editReason{head: fmt.Sprintf("The edited file had a syntax error: %v", err)}) continue } + // not a syntax error as it turns out... containsError = false updatedVisitor := resource.InfoListVisitor(updatedInfos) + // we need to add back managedFields to both updated and original object + if err := o.restoreManagedFields(updatedInfos); err != nil { + return preservedFile(err, file, o.ErrOut) + } + if err := o.restoreManagedFields(infos); err != nil { + return preservedFile(err, file, o.ErrOut) + } + // need to make sure the original namespace wasn't changed while editing if err := updatedVisitor.Visit(resource.RequireNamespace(o.CmdNamespace)); err != nil { return preservedFile(err, file, o.ErrOut) @@ -437,6 +454,49 @@ func (o *EditOptions) Run() error { } } +func (o *EditOptions) extractManagedFields(obj runtime.Object) error { + o.managedFields = make(map[types.UID][]metav1.ManagedFieldsEntry) + if meta.IsListType(obj) { + err := meta.EachListItem(obj, func(obj runtime.Object) error { + uid, mf, err := clearManagedFields(obj) + if err != nil { + return err + } + o.managedFields[uid] = mf + return nil + }) + return err + } + uid, mf, err := clearManagedFields(obj) + if err != nil { + return err + } + o.managedFields[uid] = mf + return nil +} + +func clearManagedFields(obj runtime.Object) (types.UID, []metav1.ManagedFieldsEntry, error) { + metaObjs, err := meta.Accessor(obj) + if err != nil { + return "", nil, err + } + mf := metaObjs.GetManagedFields() + metaObjs.SetManagedFields(nil) + return metaObjs.GetUID(), mf, nil +} + +func (o *EditOptions) restoreManagedFields(infos []*resource.Info) error { + for _, info := range infos { + metaObjs, err := meta.Accessor(info.Object) + if err != nil { + return err + } + mf := o.managedFields[metaObjs.GetUID()] + metaObjs.SetManagedFields(mf) + } + return nil +} + func (o *EditOptions) visitToApplyEditPatch(originalInfos []*resource.Info, patchVisitor resource.Visitor) error { err := patchVisitor.Visit(func(info *resource.Info, incomingErr error) error { editObjUID, err := meta.NewAccessor().UID(info.Object) @@ -590,6 +650,7 @@ func (o *EditOptions) visitToPatch(originalInfos []*resource.Info, patchVisitor mergepatch.RequireKeyUnchanged("apiVersion"), mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name"), + mergepatch.RequireKeyUnchanged("managedFields"), } // Create the versioned struct from the type defined in the mapping diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions_test.go index 69a30583c0c..3142a8c7019 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/util/editor/editoptions_test.go @@ -17,7 +17,17 @@ limitations under the License. package editor import ( + "reflect" "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/cli-runtime/pkg/resource" ) func TestHashOnLineBreak(t *testing.T) { @@ -49,3 +59,221 @@ func TestHashOnLineBreak(t *testing.T) { } } } + +func TestManagedFieldsExtractAndRestore(t *testing.T) { + tests := map[string]struct { + object runtime.Object + managedFields map[types.UID][]metav1.ManagedFieldsEntry + }{ + "single object, empty managedFields": { + object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{UID: types.UID("12345")}}, + managedFields: map[types.UID][]metav1.ManagedFieldsEntry{ + types.UID("12345"): nil, + }, + }, + "multiple objects, empty managedFields": { + object: &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "kind": "List", + "apiVersion": "v1", + "metadata": map[string]interface{}{}, + }, + Items: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": map[string]interface{}{ + "uid": "12345", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, + }, + }, + { + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": map[string]interface{}{ + "uid": "98765", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, + }, + }, + }, + }, + managedFields: map[types.UID][]metav1.ManagedFieldsEntry{ + types.UID("12345"): nil, + types.UID("98765"): nil, + }, + }, + "single object, all managedFields": { + object: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + UID: types.UID("12345"), + ManagedFields: []metav1.ManagedFieldsEntry{ + { + Manager: "test", + Operation: metav1.ManagedFieldsOperationApply, + }, + }, + }}, + managedFields: map[types.UID][]metav1.ManagedFieldsEntry{ + types.UID("12345"): { + { + Manager: "test", + Operation: metav1.ManagedFieldsOperationApply, + }, + }, + }, + }, + "multiple objects, all managedFields": { + object: &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "kind": "List", + "apiVersion": "v1", + "metadata": map[string]interface{}{}, + }, + Items: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": map[string]interface{}{ + "uid": "12345", + "managedFields": []interface{}{ + map[string]interface{}{ + "manager": "test", + "operation": "Apply", + }, + }, + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, + }, + }, + { + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": map[string]interface{}{ + "uid": "98765", + "managedFields": []interface{}{ + map[string]interface{}{ + "manager": "test", + "operation": "Update", + }, + }, + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, + }, + }, + }, + }, + managedFields: map[types.UID][]metav1.ManagedFieldsEntry{ + types.UID("12345"): { + { + Manager: "test", + Operation: metav1.ManagedFieldsOperationApply, + }, + }, + types.UID("98765"): { + { + Manager: "test", + Operation: metav1.ManagedFieldsOperationUpdate, + }, + }, + }, + }, + "multiple objects, some managedFields": { + object: &unstructured.UnstructuredList{ + Object: map[string]interface{}{ + "kind": "List", + "apiVersion": "v1", + "metadata": map[string]interface{}{}, + }, + Items: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": map[string]interface{}{ + "uid": "12345", + "managedFields": []interface{}{ + map[string]interface{}{ + "manager": "test", + "operation": "Apply", + }, + }, + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, + }, + }, + { + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": map[string]interface{}{ + "uid": "98765", + }, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, + }, + }, + }, + }, + managedFields: map[types.UID][]metav1.ManagedFieldsEntry{ + types.UID("12345"): { + { + Manager: "test", + Operation: metav1.ManagedFieldsOperationApply, + }, + }, + types.UID("98765"): nil, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + // operate on a copy, so we can compare the original and the modified object + objCopy := test.object.DeepCopyObject() + var infos []*resource.Info + o := NewEditOptions(NormalEditMode, genericclioptions.NewTestIOStreamsDiscard()) + err := o.extractManagedFields(objCopy) + if err != nil { + t.Errorf("unexpected extraction error %v", err) + } + if meta.IsListType(objCopy) { + infos = []*resource.Info{} + meta.EachListItem(objCopy, func(obj runtime.Object) error { + metaObjs, _ := meta.Accessor(obj) + if metaObjs.GetManagedFields() != nil { + t.Errorf("unexpected managedFileds after extraction") + } + infos = append(infos, &resource.Info{Object: obj}) + return nil + }) + } else { + metaObjs, _ := meta.Accessor(objCopy) + if metaObjs.GetManagedFields() != nil { + t.Errorf("unexpected managedFileds after extraction") + } + infos = []*resource.Info{{Object: objCopy}} + } + + err = o.restoreManagedFields(infos) + if err != nil { + t.Errorf("unexpected restore error %v", err) + } + if !reflect.DeepEqual(test.object, objCopy) { + t.Errorf("mismatched object after extract and restore managedFields: %#v", objCopy) + } + if test.managedFields != nil && !reflect.DeepEqual(test.managedFields, o.managedFields) { + t.Errorf("mismatched managedFields %#v vs %#v", test.managedFields, o.managedFields) + } + }) + } +}