From 9082cac48240ebc316015dabb466e5b24a113dc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kevin=20Wiesm=C3=BCller?= <kwiesmueller@seibert-media.net>
Date: Sun, 3 Feb 2019 22:22:10 +0100
Subject: [PATCH] strip selected fields from managedFields

refactor fieldstrip and update tests

add checks and remove empty fields

shorten test and check for nil manager

fix gofmt

panic on nil manager
---
 .../handlers/fieldmanager/fieldmanager.go     | 31 +++++++++
 .../integration/apiserver/apply/apply_test.go | 66 +++++++++++++++++--
 2 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go
index 773063890cc..b42c35dde29 100644
--- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go
+++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go
@@ -51,6 +51,7 @@ func NewFieldManager(models openapiproto.Models, objectConverter runtime.ObjectC
 	if err != nil {
 		return nil, err
 	}
+
 	return &FieldManager{
 		typeConverter:   typeConverter,
 		objectConverter: objectConverter,
@@ -130,6 +131,7 @@ func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (r
 	if err != nil {
 		return nil, fmt.Errorf("failed to update ManagedFields: %v", err)
 	}
+	managed = f.stripFields(managed, manager)
 
 	if err := internal.EncodeObjectManagedFields(newObj, managed); err != nil {
 		return nil, fmt.Errorf("failed to encode managed fields: %v", err)
@@ -180,6 +182,7 @@ func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, force bool) (
 		}
 		return nil, err
 	}
+	managed = f.stripFields(managed, manager)
 
 	newObj, err := f.typeConverter.TypedToObject(newObjTyped)
 	if err != nil {
@@ -223,3 +226,31 @@ func (f *FieldManager) buildManagerInfo(prefix string, operation metav1.ManagedF
 	}
 	return internal.BuildManagerIdentifier(&managerInfo)
 }
+
+// stripSet is the list of fields that should never be part of a mangedFields.
+var stripSet = fieldpath.NewSet(
+	fieldpath.MakePathOrDie("apiVersion"),
+	fieldpath.MakePathOrDie("kind"),
+	fieldpath.MakePathOrDie("metadata", "name"),
+	fieldpath.MakePathOrDie("metadata", "namespace"),
+	fieldpath.MakePathOrDie("metadata", "creationTimestamp"),
+	fieldpath.MakePathOrDie("metadata", "selfLink"),
+	fieldpath.MakePathOrDie("metadata", "uid"),
+	fieldpath.MakePathOrDie("metadata", "resourceVersion"),
+)
+
+// stripFields removes a predefined set of paths found in typed from managed and returns the updated ManagedFields
+func (f *FieldManager) stripFields(managed fieldpath.ManagedFields, manager string) fieldpath.ManagedFields {
+	vs, ok := managed[manager]
+	if ok {
+		if vs == nil {
+			panic(fmt.Sprintf("Found unexpected nil manager which should never happen: %s", manager))
+		}
+		vs.Set = vs.Set.Difference(stripSet)
+		if vs.Set.Empty() {
+			delete(managed, manager)
+		}
+	}
+
+	return managed
+}
diff --git a/test/integration/apiserver/apply/apply_test.go b/test/integration/apiserver/apply/apply_test.go
index f66f6a171a8..1b48dad72f4 100644
--- a/test/integration/apiserver/apply/apply_test.go
+++ b/test/integration/apiserver/apply/apply_test.go
@@ -223,7 +223,11 @@ func TestApplyManagedFields(t *testing.T) {
 			"apiVersion": "v1",
 			"kind": "ConfigMap",
 			"metadata": {
-				"name": "test-cm"
+				"name": "test-cm",
+				"namespace": "default",
+				"labels": {
+					"test-label": "test"
+				}
 			},
 			"data": {
 				"key": "value"
@@ -267,16 +271,19 @@ func TestApplyManagedFields(t *testing.T) {
 			"uid": "` + string(accessor.GetUID()) + `",
 			"resourceVersion": "` + accessor.GetResourceVersion() + `",
 			"creationTimestamp": "` + accessor.GetCreationTimestamp().UTC().Format(time.RFC3339) + `",
+			"labels": {
+				"test-label": "test"
+			},
 			"managedFields": [
 				{
 					"manager": "apply",
 					"operation": "Apply",
 					"apiVersion": "v1",
 					"fields": {
-						"f:apiVersion": {},
-						"f:kind": {},
 						"f:metadata": {
-							"f:name": {}
+							"f:labels": {
+								"f:test-label": {}
+							}
 						}
 					}
 				},
@@ -302,3 +309,54 @@ func TestApplyManagedFields(t *testing.T) {
 		t.Fatalf("Expected:\n%v\nGot:\n%v", string(expected), string(actual))
 	}
 }
+
+// TestApplyRemovesEmptyManagedFields there are no empty managers in managedFields
+func TestApplyRemovesEmptyManagedFields(t *testing.T) {
+	defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
+
+	_, client, closeFn := setup(t)
+	defer closeFn()
+
+	obj := []byte(`{
+		"apiVersion": "v1",
+		"kind": "ConfigMap",
+		"metadata": {
+			"name": "test-cm",
+			"namespace": "default"
+		}
+	}`)
+
+	_, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
+		Namespace("default").
+		Resource("configmaps").
+		Name("test-cm").
+		Body(obj).
+		Do().
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object using Apply patch: %v", err)
+	}
+
+	_, err = client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
+		Namespace("default").
+		Resource("configmaps").
+		Name("test-cm").
+		Body(obj).Do().Get()
+	if err != nil {
+		t.Fatalf("Failed to patch object: %v", err)
+	}
+
+	object, err := client.CoreV1().RESTClient().Get().Namespace("default").Resource("configmaps").Name("test-cm").Do().Get()
+	if err != nil {
+		t.Fatalf("Failed to retrieve object: %v", err)
+	}
+
+	accessor, err := meta.Accessor(object)
+	if err != nil {
+		t.Fatalf("Failed to get meta accessor: %v", err)
+	}
+
+	if managed := accessor.GetManagedFields(); managed != nil {
+		t.Fatalf("Object contains unexpected managedFields: %v", managed)
+	}
+}