Merge pull request #91946 from soltysh/strip_managed_fields

Strip .meta.managedFields for kubectl edit command

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(o.Complete(f, args, cmd))
-				cmdutil.CheckErr(err)
+			cmdutil.CheckErr(o.Run())
-			}
-			if err := o.Run(); err != nil {
-				cmdutil.CheckErr(err)
-			}
 		},
 	}
 

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(

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

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)
+			}
+		})
+	}
+}