Reset managedFields corrupted by admission controllers

staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go

@@ -180,9 +180,7 @@ func ValidateObjectMetaAccessor(meta metav1.Object, requiresNamespace bool, name
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("clusterName"), meta.GetClusterName(), msg))
 		}
 	}
-	for _, entry := range meta.GetManagedFields() {
-		allErrs = append(allErrs, v1validation.ValidateFieldManager(entry.Manager, fldPath.Child("fieldManager"))...)
-	}
+
 	allErrs = append(allErrs, ValidateNonnegativeField(meta.GetGeneration(), fldPath.Child("generation"))...)
 	allErrs = append(allErrs, v1validation.ValidateLabels(meta.GetLabels(), fldPath.Child("labels"))...)
 	allErrs = append(allErrs, ValidateAnnotations(meta.GetAnnotations(), fldPath.Child("annotations"))...)
@@ -248,9 +246,6 @@ func ValidateObjectMetaAccessorUpdate(newMeta, oldMeta metav1.Object, fldPath *f
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("generation"), newMeta.GetGeneration(), "must not be decremented"))
 	}
 
-	for _, entry := range newMeta.GetManagedFields() {
-		allErrs = append(allErrs, v1validation.ValidateFieldManager(entry.Manager, fldPath.Child("fieldManager"))...)
-	}
 	allErrs = append(allErrs, ValidateImmutableField(newMeta.GetName(), oldMeta.GetName(), fldPath.Child("name"))...)
 	allErrs = append(allErrs, ValidateImmutableField(newMeta.GetNamespace(), oldMeta.GetNamespace(), fldPath.Child("namespace"))...)
 	allErrs = append(allErrs, ValidateImmutableField(newMeta.GetUID(), oldMeta.GetUID(), fldPath.Child("uid"))...)

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go

@@ -173,7 +173,8 @@ func ValidateTableOptions(opts *metav1.TableOptions) field.ErrorList {
 
 func ValidateManagedFields(fieldsList []metav1.ManagedFieldsEntry, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
-	for _, fields := range fieldsList {
+	for i, fields := range fieldsList {
+		fldPath := fldPath.Index(i)
 		switch fields.Operation {
 		case metav1.ManagedFieldsOperationApply, metav1.ManagedFieldsOperationUpdate:
 		default:
@@ -182,6 +183,7 @@ func ValidateManagedFields(fieldsList []metav1.ManagedFieldsEntry, fldPath *fiel
 		if len(fields.FieldsType) > 0 && fields.FieldsType != "FieldsV1" {
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("fieldsType"), fields.FieldsType, "must be `FieldsV1`"))
 		}
+		allErrs = append(allErrs, ValidateFieldManager(fields.Manager, fldPath.Child("manager"))...)
 	}
 	return allErrs
 }

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go

@@ -247,14 +247,24 @@ func TestValidateManagedFieldsInvalid(t *testing.T) {
 		{
 			Operation:  metav1.ManagedFieldsOperationUpdate,
 			FieldsType: "RandomVersion",
+			APIVersion: "v1",
 		},
 		{
 			Operation:  "RandomOperation",
 			FieldsType: "FieldsV1",
+			APIVersion: "v1",
 		},
 		{
 			// Operation is missing
 			FieldsType: "FieldsV1",
+			APIVersion: "v1",
+		},
+		{
+			Operation:  metav1.ManagedFieldsOperationUpdate,
+			FieldsType: "FieldsV1",
+			// Invalid fieldManager
+			Manager:    "field\nmanager",
+			APIVersion: "v1",
 		},
 	}
 
@@ -271,16 +281,25 @@ func TestValidateManagedFieldsInvalid(t *testing.T) {
 func TestValidateMangedFieldsValid(t *testing.T) {
 	tests := []metav1.ManagedFieldsEntry{
 		{
-			Operation: metav1.ManagedFieldsOperationUpdate,
+			Operation:  metav1.ManagedFieldsOperationUpdate,
+			APIVersion: "v1",
 			// FieldsType is missing
 		},
 		{
 			Operation:  metav1.ManagedFieldsOperationUpdate,
 			FieldsType: "FieldsV1",
+			APIVersion: "v1",
+		},
+		{
+			Operation:  metav1.ManagedFieldsOperationApply,
+			FieldsType: "FieldsV1",
+			APIVersion: "v1",
 		},
 		{
 			Operation:  metav1.ManagedFieldsOperationApply,
 			FieldsType: "FieldsV1",
+			APIVersion: "v1",
+			Manager:    "🍔",
 		},
 	}
 

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/audit"
+	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager"
 	"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
 	"k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/features"
@@ -163,6 +164,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
 					return nil, fmt.Errorf("failed to create new object (Create for %v): %v", scope.Kind, err)
 				}
 				obj = scope.FieldManager.UpdateNoErrors(liveObj, obj, managerOrUserAgent(options.FieldManager, req.UserAgent()))
+				admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit)
 			}
 			if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Create) {
 				if err := mutatingAdmission.Admit(ctx, admissionAttributes, scope); err != nil {

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/admission.go

@@ -0,0 +1,86 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fieldmanager
+
+import (
+	"context"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/apiserver/pkg/warning"
+)
+
+// InvalidManagedFieldsAfterMutatingAdmissionWarningFormat is the warning that a client receives
+// when a create/update/patch request results in invalid managedFields after going through the admission chain.
+const InvalidManagedFieldsAfterMutatingAdmissionWarningFormat = ".metadata.managedFields was in an invalid state after admission; this could be caused by an outdated mutating admission controller; please fix your requests: %v"
+
+// NewManagedFieldsValidatingAdmissionController validates the managedFields after calling
+// the provided admission and resets them to their original state if they got changed to an invalid value
+func NewManagedFieldsValidatingAdmissionController(wrap admission.Interface) admission.Interface {
+	if wrap == nil {
+		return nil
+	}
+	return &managedFieldsValidatingAdmissionController{wrap: wrap}
+}
+
+type managedFieldsValidatingAdmissionController struct {
+	wrap admission.Interface
+}
+
+var _ admission.Interface = &managedFieldsValidatingAdmissionController{}
+var _ admission.MutationInterface = &managedFieldsValidatingAdmissionController{}
+var _ admission.ValidationInterface = &managedFieldsValidatingAdmissionController{}
+
+// 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 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)
+	if !isMutationInterface {
+		return nil
+	}
+	objectMeta, err := meta.Accessor(a.GetObject())
+	if err != nil {
+		return err
+	}
+	managedFieldsBeforeAdmission := objectMeta.GetManagedFields()
+	if err := mutationInterface.Admit(ctx, a, o); err != nil {
+		return err
+	}
+	managedFieldsAfterAdmission := objectMeta.GetManagedFields()
+	if _, err := DecodeManagedFields(managedFieldsAfterAdmission); err != nil {
+		objectMeta.SetManagedFields(managedFieldsBeforeAdmission)
+		warning.AddWarning(ctx, "",
+			fmt.Sprintf(InvalidManagedFieldsAfterMutatingAdmissionWarningFormat,
+				err.Error()),
+		)
+	}
+	return nil
+}
+
+// Validate calls the wrapped admission.Interface if aplicable
+func (admit *managedFieldsValidatingAdmissionController) Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) {
+	if validationInterface, isValidationInterface := admit.wrap.(admission.ValidationInterface); isValidationInterface {
+		return validationInterface.Validate(ctx, a, o)
+	}
+	return nil
+}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/admission_test.go

@@ -0,0 +1,122 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fieldmanager_test
+
+import (
+	"context"
+	"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/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()
+
+	validFieldsV1, err := internal.SetToFields(*fieldpath.NewSet(fieldpath.MakePathOrDie("metadata", "labels", "test-label")))
+	if err != nil {
+		t.Fatal(err)
+	}
+	validManagedFieldsEntry := metav1.ManagedFieldsEntry{
+		APIVersion: "v1",
+		Operation:  metav1.ManagedFieldsOperationApply,
+		Time:       &now,
+		Manager:    "test",
+		FieldsType: "FieldsV1",
+		FieldsV1:   &validFieldsV1,
+	}
+
+	managedFieldsMutators := map[string]func(in metav1.ManagedFieldsEntry) (out metav1.ManagedFieldsEntry, shouldReset bool){
+		"invalid APIVersion": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
+			managedFields.APIVersion = ""
+			return managedFields, true
+		},
+		"invalid Operation": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
+			managedFields.Operation = "invalid operation"
+			return managedFields, true
+		},
+		"invalid fieldsType": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
+			managedFields.FieldsType = "invalid fieldsType"
+			return managedFields, true
+		},
+		"invalid fieldsV1": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
+			managedFields.FieldsV1 = &metav1.FieldsV1{Raw: []byte("{invalid}")}
+			return managedFields, true
+		},
+		"invalid manager": func(managedFields metav1.ManagedFieldsEntry) (metav1.ManagedFieldsEntry, bool) {
+			managedFields.Manager = ""
+			return managedFields, false
+		},
+	}
+
+	for name, mutate := range managedFieldsMutators {
+		t.Run(name, func(t *testing.T) {
+			mutated, shouldReset := mutate(validManagedFieldsEntry)
+			validEntries := []metav1.ManagedFieldsEntry{validManagedFieldsEntry}
+			mutatedEntries := []metav1.ManagedFieldsEntry{mutated}
+
+			obj := &v1.ConfigMap{}
+			obj.SetManagedFields(validEntries)
+
+			wrap.admit = replaceManagedFields(mutatedEntries)
+
+			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())
+			}
+		})
+	}
+}
+
+func replaceManagedFields(with []metav1.ManagedFieldsEntry) func(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error {
+	return func(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error {
+		objectMeta, err := meta.Accessor(a.GetObject())
+		if err != nil {
+			return err
+		}
+		objectMeta.SetManagedFields(with)
+		return nil
+	}
+}
+
+type mockAdmissionController struct {
+	admit func(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error
+}
+
+func (c *mockAdmissionController) Handles(operation admission.Operation) bool {
+	return true
+}
+
+func (c *mockAdmissionController) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error {
+	return c.admit(ctx, a, o)
+}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go

@@ -110,23 +110,22 @@ func newDefaultFieldManager(f Manager, typeConverter TypeConverter, objectConver
 	return NewFieldManager(f, ignoreManagedFieldsFromRequestObject)
 }
 
-func decodeLiveManagedFields(liveObj runtime.Object) (Managed, error) {
+// DecodeManagedFields converts ManagedFields from the wire format (api format)
+// to the format used by sigs.k8s.io/structured-merge-diff
+func DecodeManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) (Managed, error) {
+	return internal.DecodeManagedFields(encodedManagedFields)
+}
+
+func decodeLiveOrNew(liveObj, newObj runtime.Object, ignoreManagedFieldsFromRequestObject bool) (Managed, error) {
 	liveAccessor, err := meta.Accessor(liveObj)
 	if err != nil {
 		return nil, err
 	}
-	managed, err := internal.DecodeObjectManagedFields(liveAccessor.GetManagedFields())
-	if err != nil {
-		return internal.NewEmptyManaged(), nil
-	}
-	return managed, nil
-}
 
-func decodeManagedFields(liveObj, newObj runtime.Object, ignoreManagedFieldsFromRequestObject bool) (Managed, error) {
 	// We take the managedFields of the live object in case the request tries to
 	// manually set managedFields via a subresource.
 	if ignoreManagedFieldsFromRequestObject {
-		return decodeLiveManagedFields(liveObj)
+		return emptyManagedFieldsOnErr(DecodeManagedFields(liveAccessor.GetManagedFields()))
 	}
 
 	// If the object doesn't have metadata, we should just return without trying to
@@ -140,14 +139,20 @@ func decodeManagedFields(liveObj, newObj runtime.Object, ignoreManagedFieldsFrom
 		return internal.NewEmptyManaged(), nil
 	}
 
-	managed, err := internal.DecodeObjectManagedFields(newAccessor.GetManagedFields())
 	// If the managed field is empty or we failed to decode it,
 	// let's try the live object. This is to prevent clients who
 	// don't understand managedFields from deleting it accidentally.
+	managed, err := DecodeManagedFields(newAccessor.GetManagedFields())
 	if err != nil || len(managed.Fields()) == 0 {
-		return decodeLiveManagedFields(liveObj)
+		return emptyManagedFieldsOnErr(DecodeManagedFields(liveAccessor.GetManagedFields()))
 	}
+	return managed, nil
+}
 
+func emptyManagedFieldsOnErr(managed Managed, err error) (Managed, error) {
+	if err != nil {
+		return internal.NewEmptyManaged(), nil
+	}
 	return managed, nil
 }
 
@@ -157,7 +162,7 @@ func decodeManagedFields(liveObj, newObj runtime.Object, ignoreManagedFieldsFrom
 func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (object runtime.Object, err error) {
 	// First try to decode the managed fields provided in the update,
 	// This is necessary to allow directly updating managed fields.
-	managed, err := decodeManagedFields(liveObj, newObj, f.ignoreManagedFieldsFromRequestObject)
+	managed, err := decodeLiveOrNew(liveObj, newObj, f.ignoreManagedFieldsFromRequestObject)
 	if err != nil {
 		return newObj, nil
 	}
@@ -219,7 +224,7 @@ func (f *FieldManager) Apply(liveObj, appliedObj runtime.Object, manager string,
 	}
 
 	// Decode the managed fields in the live object, since it isn't allowed in the patch.
-	managed, err := internal.DecodeObjectManagedFields(accessor.GetManagedFields())
+	managed, err := DecodeManagedFields(accessor.GetManagedFields())
 	if err != nil {
 		return nil, fmt.Errorf("failed to decode managed fields: %v", err)
 	}