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

Reset managedFields corrupted by admission controllers

Kubernetes-commit: 4cf8823fbaedaeeb8a5c094b574d489b5ca5f6fe

Godeps/Godeps.json

@@ -700,15 +700,15 @@
 		},
 		{
 			"ImportPath": "k8s.io/api",
-			"Rev": "d04530e36602"
+			"Rev": "8e35c5077a13"
 		},
 		{
 			"ImportPath": "k8s.io/apimachinery",
-			"Rev": "283a3268598b"
+			"Rev": "a5a2acc259d2"
 		},
 		{
 			"ImportPath": "k8s.io/client-go",
-			"Rev": "75a1e7fffe07"
+			"Rev": "b4932b529fd2"
 		},
 		{
 			"ImportPath": "k8s.io/component-base",

go.mod

@@ -41,9 +41,9 @@ require (
 	gopkg.in/natefinch/lumberjack.v2 v2.0.0
 	gopkg.in/square/go-jose.v2 v2.2.2
 	gopkg.in/yaml.v2 v2.2.8
-	k8s.io/api v0.0.0-20210306132701-d04530e36602
+	k8s.io/api v0.0.0-20210307052319-8e35c5077a13
-	k8s.io/apimachinery v0.0.0-20210306132128-283a3268598b
+	k8s.io/apimachinery v0.0.0-20210307052016-a5a2acc259d2
-	k8s.io/client-go v0.0.0-20210306133322-75a1e7fffe07
+	k8s.io/client-go v0.0.0-20210307052716-b4932b529fd2
 	k8s.io/component-base v0.0.0-20210306134456-247e7b3ba560
 	k8s.io/klog/v2 v2.5.0
 	k8s.io/kube-openapi v0.0.0-20210305001622-591a79e4bda7
@@ -54,8 +54,8 @@ require (
 )
 
 replace (
-	k8s.io/api => k8s.io/api v0.0.0-20210306132701-d04530e36602
+	k8s.io/api => k8s.io/api v0.0.0-20210307052319-8e35c5077a13
-	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210306132128-283a3268598b
+	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210307052016-a5a2acc259d2
-	k8s.io/client-go => k8s.io/client-go v0.0.0-20210306133322-75a1e7fffe07
+	k8s.io/client-go => k8s.io/client-go v0.0.0-20210307052716-b4932b529fd2
 	k8s.io/component-base => k8s.io/component-base v0.0.0-20210306134456-247e7b3ba560
 )

go.sum

@@ -583,9 +583,9 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
-k8s.io/api v0.0.0-20210306132701-d04530e36602/go.mod h1:oXIlxI7P0Fm+wAl92K966U84hJ8jYH+vvhPMIFggXJQ=
+k8s.io/api v0.0.0-20210307052319-8e35c5077a13/go.mod h1:oXIlxI7P0Fm+wAl92K966U84hJ8jYH+vvhPMIFggXJQ=
-k8s.io/apimachinery v0.0.0-20210306132128-283a3268598b/go.mod h1:2LERmYT9PI3b4uQt87vnb2UVkblBDzZhucIf8PxvJ2o=
+k8s.io/apimachinery v0.0.0-20210307052016-a5a2acc259d2/go.mod h1:2LERmYT9PI3b4uQt87vnb2UVkblBDzZhucIf8PxvJ2o=
-k8s.io/client-go v0.0.0-20210306133322-75a1e7fffe07/go.mod h1:hcawgCmh6c53kdHiIH1dk9ZLukLcM6NPVE8rs3YLqdU=
+k8s.io/client-go v0.0.0-20210307052716-b4932b529fd2/go.mod h1:qv0Qrnu6UEKMfXsRgVBD0X/s4dNXOmHHWD79odx0C38=
 k8s.io/component-base v0.0.0-20210306134456-247e7b3ba560/go.mod h1:I1nomQYfl44UYAQFJJL/eA28WOBZNwpTUseGxPE/jpE=
 k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
 k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=

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 {

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

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

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

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

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

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,

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