cleanup managedFields admission and test

Kubernetes-commit: 2d1ba0c35829a2f146a712d49cb21f382c9894cb

pkg/endpoints/handlers/fieldmanager/admission.go

@@ -46,12 +46,12 @@ var _ admission.Interface = &managedFieldsValidatingAdmissionController{}
 var _ admission.MutationInterface = &managedFieldsValidatingAdmissionController{}
 var _ admission.ValidationInterface = &managedFieldsValidatingAdmissionController{}
 
-// Handles calls the wrapped admission.Interface if aplicable
+// Handles calls the wrapped admission.Interface if applicable
 func (admit *managedFieldsValidatingAdmissionController) Handles(operation admission.Operation) bool {
 	return admit.wrap.Handles(operation)
 }
 
-// Admit calls the wrapped admission.Interface if aplicable and resets the managedFields to their state before admission if they
+// Admit calls the wrapped admission.Interface if applicable and resets the managedFields to their state before admission if they
 // got modified in an invalid way
 func (admit *managedFieldsValidatingAdmissionController) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) {
 	mutationInterface, isMutationInterface := admit.wrap.(admission.MutationInterface)

pkg/endpoints/handlers/fieldmanager/admission_test.go

@@ -21,89 +21,80 @@ import (
 	"reflect"
 	"testing"
 
+	v1 "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/schema"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager"
+	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal"
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
 func TestAdmission(t *testing.T) {
 	wrap := &mockAdmissionController{}
 	ac := fieldmanager.NewManagedFieldsValidatingAdmissionController(wrap)
+	now := metav1.Now()
 
-	tests := []struct {
+	validFieldsV1, err := internal.SetToFields(*fieldpath.NewSet(fieldpath.MakePathOrDie("metadata", "labels", "test-label")))
-		beforeAdmission []metav1.ManagedFieldsEntry
+	if err != nil {
-		afterAdmission  []metav1.ManagedFieldsEntry
+		t.Fatal(err)
-		expected        []metav1.ManagedFieldsEntry
+	}
-	}{
+	validManagedFieldsEntry := metav1.ManagedFieldsEntry{
-		{
+		APIVersion: "v1",
-			beforeAdmission: []metav1.ManagedFieldsEntry{
+		Operation:  metav1.ManagedFieldsOperationApply,
-				{
+		Time:       &now,
-					Manager: "test",
+		Manager:    "test",
-				},
+		FieldsType: "FieldsV1",
-			},
+		FieldsV1:   &validFieldsV1,
-			afterAdmission: []metav1.ManagedFieldsEntry{
+	}
-				{
+
-					Manager: "",
+	managedFieldsMutators := map[string]func(in metav1.ManagedFieldsEntry) (out metav1.ManagedFieldsEntry, shouldReset bool){
-				},
+		"invalid APIVersion": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
-			},
+			managedFields.APIVersion = ""
-			expected: []metav1.ManagedFieldsEntry{
+			return managedFields, true
-				{
-					Manager: "test",
-				},
-			},
 		},
-		{
+		"invalid Operation": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
-			beforeAdmission: []metav1.ManagedFieldsEntry{
+			managedFields.Operation = "invalid operation"
-				{
+			return managedFields, true
-					APIVersion: "test",
-				},
-			},
-			afterAdmission: []metav1.ManagedFieldsEntry{
-				{
-					APIVersion: "",
-				},
-			},
-			expected: []metav1.ManagedFieldsEntry{
-				{
-					APIVersion: "test",
-				},
-			},
 		},
-		{
+		"invalid fieldsType": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
-			beforeAdmission: []metav1.ManagedFieldsEntry{
+			managedFields.FieldsType = "invalid fieldsType"
-				{
+			return managedFields, true
-					FieldsType: "FieldsV1",
+		},
-				},
+		"invalid fieldsV1": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
-			},
+			managedFields.FieldsV1 = &metav1.FieldsV1{Raw: []byte("{invalid}")}
-			afterAdmission: []metav1.ManagedFieldsEntry{
+			return managedFields, true
-				{
+		},
-					FieldsType: "test",
+		"invalid manager": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
-				},
+			managedFields.Manager = ""
-			},
+			return managedFields, false
-			expected: []metav1.ManagedFieldsEntry{
-				{
-					FieldsType: "FieldsV1",
-				},
-			},
 		},
 	}
 
-	for _, test := range tests {
+	for name, mutate := range managedFieldsMutators {
-		obj := &unstructured.Unstructured{}
+		t.Run(name, func(t *testing.T) {
-		obj.SetManagedFields(test.beforeAdmission)
+			mutated, shouldReset := mutate(validManagedFieldsEntry)
-		wrap.admit = replaceManagedFields(test.afterAdmission)
+			validEntries := []metav1.ManagedFieldsEntry{validManagedFieldsEntry}
+			mutatedEntries := []metav1.ManagedFieldsEntry{mutated}
 
-		attrs := admission.NewAttributesRecord(obj, obj, schema.GroupVersionKind{}, "default", "", schema.GroupVersionResource{}, "", admission.Update, nil, false, nil)
+			obj := &v1.ConfigMap{}
-		if err := ac.(admission.MutationInterface).Admit(context.TODO(), attrs, nil); err != nil {
+			obj.SetManagedFields(validEntries)
-			t.Fatal(err)
-		}
 
-		if !reflect.DeepEqual(obj.GetManagedFields(), test.expected) {
+			wrap.admit = replaceManagedFields(mutatedEntries)
-			t.Fatalf("expected: \n%v\ngot:\n%v", test.expected, obj.GetManagedFields())
+
-		}
+			attrs := admission.NewAttributesRecord(obj, obj, schema.GroupVersionKind{}, "default", "", schema.GroupVersionResource{}, "", admission.Update, nil, false, nil)
+			if err := ac.(admission.MutationInterface).Admit(context.TODO(), attrs, nil); err != nil {
+				t.Fatal(err)
+			}
+
+			if shouldReset && !reflect.DeepEqual(obj.GetManagedFields(), validEntries) {
+				t.Fatalf("expected: \n%v\ngot:\n%v", validEntries, obj.GetManagedFields())
+			}
+			if !shouldReset && reflect.DeepEqual(obj.GetManagedFields(), validEntries) {
+				t.Fatalf("expected: \n%v\ngot:\n%v", mutatedEntries, obj.GetManagedFields())
+			}
+		})
 	}
 }