Merge pull request #98074 from kwiesmueller/admission-reset-managedFields

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

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

@@ -77,15 +77,6 @@ func RemoveObjectManagedFields(obj runtime.Object) {
 	accessor.SetManagedFields(nil)
 }
 
-// DecodeObjectManagedFields extracts and converts the objects ManagedFields into a fieldpath.ManagedFields.
-func DecodeObjectManagedFields(from []metav1.ManagedFieldsEntry) (ManagedInterface, error) {
-	managed, err := decodeManagedFields(from)
-	if err != nil {
-		return nil, fmt.Errorf("failed to convert managed fields from API: %v", err)
-	}
-	return &managed, nil
-}
-
 // EncodeObjectManagedFields converts and stores the fieldpathManagedFields into the objects ManagedFields
 func EncodeObjectManagedFields(obj runtime.Object, managed ManagedInterface) error {
 	accessor, err := meta.Accessor(obj)
@@ -102,32 +93,41 @@ func EncodeObjectManagedFields(obj runtime.Object, managed ManagedInterface) err
 	return nil
 }
 
-// decodeManagedFields converts ManagedFields from the wire format (api format)
+// 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 managedStruct, err error) {
+func DecodeManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) (ManagedInterface, error) {
+	managed := managedStruct{}
 	managed.fields = make(fieldpath.ManagedFields, len(encodedManagedFields))
 	managed.times = make(map[string]*metav1.Time, len(encodedManagedFields))
 
 	for i, encodedVersionedSet := range encodedManagedFields {
+		switch encodedVersionedSet.Operation {
+		case metav1.ManagedFieldsOperationApply, metav1.ManagedFieldsOperationUpdate:
+		default:
+			return nil, fmt.Errorf("operation must be `Apply` or `Update`")
+		}
+		if len(encodedVersionedSet.APIVersion) < 1 {
+			return nil, fmt.Errorf("apiVersion must not be empty")
+		}
 		switch encodedVersionedSet.FieldsType {
 		case "FieldsV1":
 			// Valid case.
 		case "":
-			return managedStruct{}, fmt.Errorf("missing fieldsType in managed fields entry %d", i)
+			return nil, fmt.Errorf("missing fieldsType in managed fields entry %d", i)
 		default:
-			return managedStruct{}, fmt.Errorf("invalid fieldsType %q in managed fields entry %d", encodedVersionedSet.FieldsType, i)
+			return nil, fmt.Errorf("invalid fieldsType %q in managed fields entry %d", encodedVersionedSet.FieldsType, i)
 		}
 		manager, err := BuildManagerIdentifier(&encodedVersionedSet)
 		if err != nil {
-			return managedStruct{}, fmt.Errorf("error decoding manager from %v: %v", encodedVersionedSet, err)
+			return nil, fmt.Errorf("error decoding manager from %v: %v", encodedVersionedSet, err)
 		}
 		managed.fields[manager], err = decodeVersionedSet(&encodedVersionedSet)
 		if err != nil {
-			return managedStruct{}, fmt.Errorf("error decoding versioned set from %v: %v", encodedVersionedSet, err)
+			return nil, fmt.Errorf("error decoding versioned set from %v: %v", encodedVersionedSet, err)
 		}
 		managed.times[manager] = encodedVersionedSet.Time
 	}
-	return managed, nil
+	return &managed, nil
 }
 
 // BuildManagerIdentifier creates a manager identifier string from a ManagedFieldsEntry

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

@@ -40,7 +40,7 @@ func TestHasFieldsType(t *testing.T) {
 `), &unmarshaled); err != nil {
 		t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
 	}
-	if _, err := decodeManagedFields(unmarshaled); err != nil {
+	if _, err := DecodeManagedFields(unmarshaled); err != nil {
 		t.Fatalf("did not expect decoding error but got: %v", err)
 	}
 
@@ -54,7 +54,7 @@ func TestHasFieldsType(t *testing.T) {
 `), &unmarshaled); err != nil {
 		t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
 	}
-	if _, err := decodeManagedFields(unmarshaled); err == nil {
+	if _, err := DecodeManagedFields(unmarshaled); err == nil {
 		t.Fatal("Expect decoding error but got none")
 	}
 
@@ -67,7 +67,85 @@ func TestHasFieldsType(t *testing.T) {
 `), &unmarshaled); err != nil {
 		t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
 	}
-	if _, err := decodeManagedFields(unmarshaled); err == nil {
+	if _, err := DecodeManagedFields(unmarshaled); err == nil {
+		t.Fatal("Expect decoding error but got none")
+	}
+}
+
+// TestHasAPIVersion makes sure that we fail if we don't have an
+// APIVersion set.
+func TestHasAPIVersion(t *testing.T) {
+	var unmarshaled []metav1.ManagedFieldsEntry
+	if err := yaml.Unmarshal([]byte(`- apiVersion: v1
+  fieldsType: FieldsV1
+  fieldsV1:
+    f:field: {}
+  manager: foo
+  operation: Apply
+`), &unmarshaled); err != nil {
+		t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
+	}
+	if _, err := DecodeManagedFields(unmarshaled); err != nil {
+		t.Fatalf("did not expect decoding error but got: %v", err)
+	}
+
+	// Missing apiVersion.
+	unmarshaled = nil
+	if err := yaml.Unmarshal([]byte(`- fieldsType: FieldsV1
+  fieldsV1:
+    f:field: {}
+  manager: foo
+  operation: Apply
+`), &unmarshaled); err != nil {
+		t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
+	}
+	if _, err := DecodeManagedFields(unmarshaled); err == nil {
+		t.Fatal("Expect decoding error but got none")
+	}
+}
+
+// TestHasOperation makes sure that we fail if we don't have an
+// Operation set properly.
+func TestHasOperation(t *testing.T) {
+	var unmarshaled []metav1.ManagedFieldsEntry
+	if err := yaml.Unmarshal([]byte(`- apiVersion: v1
+  fieldsType: FieldsV1
+  fieldsV1:
+    f:field: {}
+  manager: foo
+  operation: Apply
+`), &unmarshaled); err != nil {
+		t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
+	}
+	if _, err := DecodeManagedFields(unmarshaled); err != nil {
+		t.Fatalf("did not expect decoding error but got: %v", err)
+	}
+
+	// Invalid operation.
+	if err := yaml.Unmarshal([]byte(`- apiVersion: v1
+  fieldsType: FieldsV1
+  fieldsV1:
+    f:field: {}
+  manager: foo
+  operation: Invalid
+`), &unmarshaled); err != nil {
+		t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
+	}
+	if _, err := DecodeManagedFields(unmarshaled); err == nil {
+		t.Fatal("Expect decoding error but got none")
+	}
+
+	// Missing operation.
+	unmarshaled = nil
+	if err := yaml.Unmarshal([]byte(`- apiVersion: v1
+  fieldsType: FieldsV1
+  fieldsV1:
+    f:field: {}
+  manager: foo
+`), &unmarshaled); err != nil {
+		t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
+	}
+	if _, err := DecodeManagedFields(unmarshaled); err == nil {
 		t.Fatal("Expect decoding error but got none")
 	}
 }
@@ -189,11 +267,11 @@ func TestRoundTripManagedFields(t *testing.T) {
 			if err := yaml.Unmarshal([]byte(test), &unmarshaled); err != nil {
 				t.Fatalf("did not expect yaml unmarshalling error but got: %v", err)
 			}
-			decoded, err := decodeManagedFields(unmarshaled)
+			decoded, err := DecodeManagedFields(unmarshaled)
 			if err != nil {
 				t.Fatalf("did not expect decoding error but got: %v", err)
 			}
-			encoded, err := encodeManagedFields(&decoded)
+			encoded, err := encodeManagedFields(decoded)
 			if err != nil {
 				t.Fatalf("did not expect encoding error but got: %v", err)
 			}

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

@@ -171,6 +171,9 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac
 			userInfo,
 		)
 
+		if scope.FieldManager != nil {
+			admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit)
+		}
 		mutatingAdmission, _ := admit.(admission.MutationInterface)
 		createAuthorizerAttributes := authorizer.AttributesRecord{
 			User:            userInfo,

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

@@ -33,6 +33,7 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/audit"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
+	"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"
@@ -130,6 +131,7 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
 		// allows skipping managedFields update if the resulting object is too big
 		shouldUpdateManagedFields := true
 		if scope.FieldManager != nil {
+			admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit)
 			transformers = append(transformers, func(_ context.Context, newObj, liveObj runtime.Object) (runtime.Object, error) {
 				if shouldUpdateManagedFields {
 					return scope.FieldManager.UpdateNoErrors(liveObj, newObj, managerOrUserAgent(options.FieldManager, req.UserAgent())), nil

test/integration/apiserver/admissionwebhook/invalid_managedFields_test.go

@@ -0,0 +1,236 @@
+/*
+Copyright 2020 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 admissionwebhook
+
+import (
+	"bytes"
+	"context"
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	v1 "k8s.io/api/admission/v1"
+	admissionv1 "k8s.io/api/admissionregistration/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager"
+	clientset "k8s.io/client-go/kubernetes"
+	restclient "k8s.io/client-go/rest"
+	kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
+	"k8s.io/kubernetes/test/integration/framework"
+)
+
+// TestMutatingWebhookResetsInvalidManagedFields ensures that the API server
+// resets managedFields to their state before admission if a mutating webhook
+// patches create/update requests with invalid managedFields.
+func TestMutatingWebhookResetsInvalidManagedFields(t *testing.T) {
+	roots := x509.NewCertPool()
+	if !roots.AppendCertsFromPEM(localhostCert) {
+		t.Fatal("Failed to append Cert from PEM")
+	}
+	cert, err := tls.X509KeyPair(localhostCert, localhostKey)
+	if err != nil {
+		t.Fatalf("Failed to build cert with error: %+v", err)
+	}
+
+	webhookServer := httptest.NewUnstartedServer(newInvalidManagedFieldsWebhookHandler(t))
+	webhookServer.TLS = &tls.Config{
+		RootCAs:      roots,
+		Certificates: []tls.Certificate{cert},
+	}
+	webhookServer.StartTLS()
+	defer webhookServer.Close()
+
+	s := kubeapiservertesting.StartTestServerOrDie(t,
+		kubeapiservertesting.NewDefaultTestServerOptions(), []string{
+			"--disable-admission-plugins=ServiceAccount",
+		}, framework.SharedEtcd())
+	defer s.TearDownFn()
+
+	recordedWarnings := &bytes.Buffer{}
+	warningWriter := restclient.NewWarningWriter(recordedWarnings, restclient.WarningWriterOptions{})
+	s.ClientConfig.WarningHandler = warningWriter
+	client := clientset.NewForConfigOrDie(s.ClientConfig)
+
+	if _, err := client.CoreV1().Pods("default").Create(
+		context.TODO(), invalidManagedFieldsMarkerFixture, metav1.CreateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+	// make sure we delete the pod even on a failed test
+	defer func() {
+		if err := client.CoreV1().Pods("default").Delete(context.TODO(), invalidManagedFieldsMarkerFixture.Name, metav1.DeleteOptions{}); err != nil {
+			t.Fatalf("failed to delete marker pod: %v", err)
+		}
+	}()
+
+	fail := admissionv1.Fail
+	none := admissionv1.SideEffectClassNone
+	mutatingCfg, err := client.AdmissionregistrationV1().MutatingWebhookConfigurations().Create(context.TODO(), &admissionv1.MutatingWebhookConfiguration{
+		ObjectMeta: metav1.ObjectMeta{Name: "invalid-managedfields.admission.integration.test"},
+		Webhooks: []admissionv1.MutatingWebhook{{
+			Name: "invalid-managedfields.admission.integration.test",
+			ClientConfig: admissionv1.WebhookClientConfig{
+				URL:      &webhookServer.URL,
+				CABundle: localhostCert,
+			},
+			Rules: []admissionv1.RuleWithOperations{{
+				Operations: []admissionv1.OperationType{admissionv1.Create, admissionv1.Update},
+				Rule:       admissionv1.Rule{APIGroups: []string{""}, APIVersions: []string{"v1"}, Resources: []string{"pods"}},
+			}},
+			FailurePolicy:           &fail,
+			AdmissionReviewVersions: []string{"v1", "v1beta1"},
+			SideEffects:             &none,
+		}},
+	}, metav1.CreateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		err := client.AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(context.TODO(), mutatingCfg.GetName(), metav1.DeleteOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	var pod *corev1.Pod
+	var lastErr error
+	// We just expect the general warning text as the detail order might change
+	expectedWarning := fmt.Sprintf(fieldmanager.InvalidManagedFieldsAfterMutatingAdmissionWarningFormat, "")
+
+	// Make sure reset happens on patch requests
+	// wait until new webhook is called
+	if err := wait.PollImmediate(time.Millisecond*5, wait.ForeverTestTimeout, func() (bool, error) {
+		pod, err = client.CoreV1().Pods("default").Patch(context.TODO(), invalidManagedFieldsMarkerFixture.Name, types.JSONPatchType, []byte("[]"), metav1.PatchOptions{})
+		if err != nil {
+			return false, err
+		}
+		if err := validateManagedFieldsAndDecode(pod.ManagedFields); err != nil {
+			lastErr = err
+			return false, nil
+		}
+		return true, nil
+	}); err != nil {
+		t.Fatalf("failed to wait for apiserver handling webhook mutation: %v, last error: %v", err, lastErr)
+	}
+	if lastErr != nil {
+		t.Fatal(lastErr)
+	}
+	if warningWriter.WarningCount() != 1 {
+		t.Errorf("expected one warning, got: %v", warningWriter.WarningCount())
+	}
+	if !strings.Contains(recordedWarnings.String(), expectedWarning) {
+		t.Errorf("unexpected warning, expected: \n%v\n, got: \n%v",
+			expectedWarning, recordedWarnings.String())
+	}
+	recordedWarnings.Reset()
+
+	// Make sure reset happens in update requests
+	pod, err = client.CoreV1().Pods("default").Update(context.TODO(), pod, metav1.UpdateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := validateManagedFieldsAndDecode(pod.ManagedFields); err != nil {
+		t.Error(err)
+	}
+	if warningWriter.WarningCount() != 2 {
+		t.Errorf("expected two warnings, got: %v", warningWriter.WarningCount())
+	}
+	if !strings.Contains(recordedWarnings.String(), expectedWarning) {
+		t.Errorf("unexpected warning, expected: \n%v\n, got: \n%v",
+			expectedWarning, recordedWarnings.String())
+	}
+}
+
+// validate against both decoding and validation to make sure we use the hardest rule between the both to reset
+// with decoding being as strict as it gets, only using it should be enough in admission
+func validateManagedFieldsAndDecode(managedFields []metav1.ManagedFieldsEntry) error {
+	if _, err := fieldmanager.DecodeManagedFields(managedFields); err != nil {
+		return err
+	}
+	validationErrs := v1validation.ValidateManagedFields(managedFields, field.NewPath("metadata").Child("managedFields"))
+	return validationErrs.ToAggregate()
+}
+
+func newInvalidManagedFieldsWebhookHandler(t *testing.T) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		defer r.Body.Close()
+		data, err := ioutil.ReadAll(r.Body)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+		}
+		review := v1.AdmissionReview{}
+		if err := json.Unmarshal(data, &review); err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+		}
+
+		if len(review.Request.Object.Raw) == 0 {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+		pod := &corev1.Pod{}
+		if err := json.Unmarshal(review.Request.Object.Raw, pod); err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		review.Response = &v1.AdmissionResponse{
+			Allowed: true,
+			UID:     review.Request.UID,
+			Result:  &metav1.Status{Message: "admitted"},
+		}
+
+		if len(pod.ManagedFields) != 0 {
+			t.Logf("corrupting managedFields %v", pod.ManagedFields)
+			review.Response.Patch = []byte(`[
+				{"op":"remove","path":"/metadata/managedFields/0/apiVersion"},
+				{"op":"remove","path":"/metadata/managedFields/0/fieldsV1"},
+				{"op":"remove","path":"/metadata/managedFields/0/fieldsType"}
+			]`)
+			jsonPatch := v1.PatchTypeJSONPatch
+			review.Response.PatchType = &jsonPatch
+		}
+
+		w.Header().Set("Content-Type", "application/json")
+		if err := json.NewEncoder(w).Encode(review); err != nil {
+			t.Errorf("Marshal of response failed with error: %v", err)
+		}
+	})
+}
+
+var invalidManagedFieldsMarkerFixture = &corev1.Pod{
+	ObjectMeta: metav1.ObjectMeta{
+		Namespace: "default",
+		Name:      "invalid-managedfields-test-marker",
+	},
+	Spec: corev1.PodSpec{
+		Containers: []corev1.Container{{
+			Name:  "fake-name",
+			Image: "fakeimage",
+		}},
+	},
+}