Do not allow manual changes to manageFields via subresources

If a request tries to change managedFields, the response returns the
managedField of the live object.

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -849,6 +849,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 				reqScope.Kind,
 				reqScope.HubGroupVersion,
 				crd.Spec.PreserveUnknownFields,
+				false,
 			)
 			if err != nil {
 				return nil, err
@@ -876,6 +877,21 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 		// override status subresource values
 		// shallow copy
 		statusScope := *requestScopes[v.Name]
+		if utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) {
+			statusScope.FieldManager, err = fieldmanager.NewDefaultCRDFieldManager(
+				openAPIModels,
+				statusScope.Convertor,
+				statusScope.Defaulter,
+				statusScope.Creater,
+				statusScope.Kind,
+				statusScope.HubGroupVersion,
+				crd.Spec.PreserveUnknownFields,
+				true,
+			)
+			if err != nil {
+				return nil, err
+			}
+		}
 		statusScope.Subresource = "status"
 		statusScope.Namer = handlers.ContextBasedNaming{
 			SelfLinker:         meta.NewAccessor(),

staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/resources.go

@@ -205,6 +205,9 @@ func NewMultipleVersionNoxuCRD(scope apiextensionsv1beta1.ResourceScope) *apiext
 					Storage: false,
 				},
 			},
+			Subresources: &apiextensionsv1beta1.CustomResourceSubresources{
+				Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{},
+			},
 		},
 	}
 }

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

@@ -48,6 +48,7 @@ func (*fakeManager) Apply(_, _ runtime.Object, _ fieldmanager.Managed, _ string,
 
 func TestCapManagersManagerMergesEntries(t *testing.T) {
 	f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"),
+		false,
 		func(m fieldmanager.Manager) fieldmanager.Manager {
 			return fieldmanager.NewCapManagersManager(m, 3)
 		})
@@ -113,6 +114,7 @@ func TestCapManagersManagerMergesEntries(t *testing.T) {
 
 func TestCapUpdateManagers(t *testing.T) {
 	f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"),
+		false,
 		func(m fieldmanager.Manager) fieldmanager.Manager {
 			return fieldmanager.NewCapManagersManager(m, 3)
 		})

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

@@ -67,18 +67,19 @@ type Manager interface {
 // FieldManager updates the managed fields and merge applied
 // configurations.
 type FieldManager struct {
-	fieldManager Manager
+	fieldManager                         Manager
+	ignoreManagedFieldsFromRequestObject bool
 }
 
 // NewFieldManager creates a new FieldManager that decodes, manages, then re-encodes managedFields
 // on update and apply requests.
-func NewFieldManager(f Manager) *FieldManager {
+func NewFieldManager(f Manager, ignoreManagedFieldsFromRequestObject bool) *FieldManager {
-	return &FieldManager{f}
+	return &FieldManager{fieldManager: f, ignoreManagedFieldsFromRequestObject: ignoreManagedFieldsFromRequestObject}
 }
 
 // NewDefaultFieldManager creates a new FieldManager that merges apply requests
 // and update managed fields for other types of requests.
-func NewDefaultFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion) (*FieldManager, error) {
+func NewDefaultFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, ignoreManagedFieldsFromRequestObject bool) (*FieldManager, error) {
 	typeConverter, err := internal.NewTypeConverter(models, false)
 	if err != nil {
 		return nil, err
@@ -88,13 +89,13 @@ func NewDefaultFieldManager(models openapiproto.Models, objectConverter runtime.
 	if err != nil {
 		return nil, fmt.Errorf("failed to create field manager: %v", err)
 	}
-	return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind), nil
+	return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind, ignoreManagedFieldsFromRequestObject), nil
 }
 
 // NewDefaultCRDFieldManager creates a new FieldManager specifically for
 // CRDs. This allows for the possibility of fields which are not defined
 // in models, as well as having no models defined at all.
-func NewDefaultCRDFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, preserveUnknownFields bool) (_ *FieldManager, err error) {
+func NewDefaultCRDFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, preserveUnknownFields, ignoreManagedFieldsFromRequestObject bool) (_ *FieldManager, err error) {
 	var typeConverter internal.TypeConverter = internal.DeducedTypeConverter{}
 	if models != nil {
 		typeConverter, err = internal.NewTypeConverter(models, preserveUnknownFields)
@@ -106,11 +107,11 @@ func NewDefaultCRDFieldManager(models openapiproto.Models, objectConverter runti
 	if err != nil {
 		return nil, fmt.Errorf("failed to create field manager: %v", err)
 	}
-	return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind), nil
+	return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind, ignoreManagedFieldsFromRequestObject), nil
 }
 
 // newDefaultFieldManager is a helper function which wraps a Manager with certain default logic.
-func newDefaultFieldManager(f Manager, typeConverter internal.TypeConverter, objectConverter runtime.ObjectConvertor, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind) *FieldManager {
+func newDefaultFieldManager(f Manager, typeConverter internal.TypeConverter, objectConverter runtime.ObjectConvertor, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, ignoreManagedFieldsFromRequestObject bool) *FieldManager {
 	f = NewStripMetaManager(f)
 	f = NewManagedFieldsUpdater(f)
 	f = NewBuildManagerInfoManager(f, kind.GroupVersion())
@@ -119,36 +120,59 @@ func newDefaultFieldManager(f Manager, typeConverter internal.TypeConverter, obj
 	f = NewLastAppliedManager(f, typeConverter, objectConverter, kind.GroupVersion())
 	f = NewLastAppliedUpdater(f)
 
-	return NewFieldManager(f)
+	return NewFieldManager(f, ignoreManagedFieldsFromRequestObject)
+}
+
+func decodeLiveManagedFields(liveObj runtime.Object) (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)
+	}
+
+	// If the object doesn't have metadata, we should just return without trying to
+	// set the managedFields at all, so creates/updates/patches will work normally.
+	newAccessor, err := meta.Accessor(newObj)
+	if err != nil {
+		return nil, err
+	}
+
+	if isResetManagedFields(newAccessor.GetManagedFields()) {
+		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.
+	if err != nil || len(managed.Fields()) == 0 {
+		return decodeLiveManagedFields(liveObj)
+	}
+
+	return managed, nil
 }
 
 // Update is used when the object has already been merged (non-apply
 // use-case), and simply updates the managed fields in the output
 // object.
 func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (object runtime.Object, err error) {
-	// If the object doesn't have metadata, we should just return without trying to
-	// set the managedFields at all, so creates/updates/patches will work normally.
-	newAccessor, err := meta.Accessor(newObj)
-	if err != nil {
-		return newObj, nil
-	}
-
 	// First try to decode the managed fields provided in the update,
 	// This is necessary to allow directly updating managed fields.
-	var managed Managed
+	managed, err := decodeManagedFields(liveObj, newObj, f.ignoreManagedFieldsFromRequestObject)
-	if isResetManagedFields(newAccessor.GetManagedFields()) {
+	if err != nil {
-		managed = internal.NewEmptyManaged()
+		return newObj, nil
-	} else if managed, err = internal.DecodeObjectManagedFields(newAccessor.GetManagedFields()); err != nil || len(managed.Fields()) == 0 {
-		liveAccessor, err := meta.Accessor(liveObj)
-		if err != nil {
-			return newObj, nil
-		}
-		// 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.
-		if managed, err = internal.DecodeObjectManagedFields(liveAccessor.GetManagedFields()); err != nil {
-			managed = internal.NewEmptyManaged()
-		}
 	}
 
 	internal.RemoveObjectManagedFields(liveObj)

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

@@ -86,10 +86,14 @@ type TestFieldManager struct {
 }
 
 func NewDefaultTestFieldManager(gvk schema.GroupVersionKind) TestFieldManager {
-	return NewTestFieldManager(gvk, nil)
+	return NewTestFieldManager(gvk, false, nil)
 }
 
-func NewTestFieldManager(gvk schema.GroupVersionKind, chainFieldManager func(fieldmanager.Manager) fieldmanager.Manager) TestFieldManager {
+func NewSubresourceTestFieldManager(gvk schema.GroupVersionKind) TestFieldManager {
+	return NewTestFieldManager(gvk, true, nil)
+}
+
+func NewTestFieldManager(gvk schema.GroupVersionKind, ignoreManagedFieldsFromRequestObject bool, chainFieldManager func(fieldmanager.Manager) fieldmanager.Manager) TestFieldManager {
 	m := NewFakeOpenAPIModels()
 	typeConverter := NewFakeTypeConverter(m)
 	converter := internal.NewVersionConverter(typeConverter, &fakeObjectConvertor{}, gvk.GroupVersion())
@@ -117,7 +121,7 @@ func NewTestFieldManager(gvk schema.GroupVersionKind, chainFieldManager func(fie
 		f = chainFieldManager(f)
 	}
 	return TestFieldManager{
-		fieldManager: fieldmanager.NewFieldManager(f),
+		fieldManager: fieldmanager.NewFieldManager(f, ignoreManagedFieldsFromRequestObject),
 		emptyObj:     live,
 		liveObj:      live.DeepCopyObject(),
 	}
@@ -1093,3 +1097,56 @@ func getLastApplied(obj runtime.Object) (string, error) {
 	}
 	return lastApplied, nil
 }
+
+func TestUpdateViaSubresources(t *testing.T) {
+	f := NewSubresourceTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"))
+
+	obj := &unstructured.Unstructured{Object: map[string]interface{}{}}
+	if err := yaml.Unmarshal([]byte(`{
+		"apiVersion": "v1",
+		"kind": "Pod",
+		"metadata": {
+			"labels": {
+				"a":"b"
+			},
+		}
+	}`), &obj.Object); err != nil {
+		t.Fatalf("error decoding YAML: %v", err)
+	}
+	obj.SetManagedFields([]metav1.ManagedFieldsEntry{
+		{
+			Manager:    "test",
+			Operation:  metav1.ManagedFieldsOperationApply,
+			APIVersion: "apps/v1",
+			FieldsType: "FieldsV1",
+			FieldsV1: &metav1.FieldsV1{
+				[]byte(`{"f:metadata":{"f:labels":{"f:another_field":{}}}}`),
+			},
+		},
+	})
+
+	// Check that managed fields cannot be changed via subresources
+	expectedManager := "fieldmanager_test_subresource"
+	if err := f.Update(obj, expectedManager); err != nil {
+		t.Fatalf("failed to apply object: %v", err)
+	}
+
+	managedFields := f.ManagedFields()
+	if len(managedFields) != 1 {
+		t.Fatalf("Expected new managed fields to have one entry. Got:\n%#v", managedFields)
+	}
+	if managedFields[0].Manager != expectedManager {
+		t.Fatalf("Expected first item to have manager set to: %s. Got: %s", expectedManager, managedFields[0].Manager)
+	}
+
+	// Check that managed fields cannot be reset via subresources
+	newObj := obj.DeepCopy()
+	newObj.SetManagedFields([]metav1.ManagedFieldsEntry{})
+	if err := f.Update(newObj, expectedManager); err != nil {
+		t.Fatalf("failed to apply object: %v", err)
+	}
+	newManagedFields := f.ManagedFields()
+	if len(newManagedFields) != 1 {
+		t.Fatalf("Expected new managed fields to have one entry. Got:\n%#v", newManagedFields)
+	}
+}

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

@@ -28,6 +28,7 @@ import (
 
 func TestLastAppliedUpdater(t *testing.T) {
 	f := NewTestFieldManager(schema.FromAPIVersionAndKind("apps/v1", "Deployment"),
+		false,
 		func(m fieldmanager.Manager) fieldmanager.Manager {
 			return fieldmanager.NewLastAppliedUpdater(m)
 		})

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

@@ -43,7 +43,7 @@ func (f *fakeObjectCreater) New(_ schema.GroupVersionKind) (runtime.Object, erro
 }
 
 func TestNoUpdateBeforeFirstApply(t *testing.T) {
-	f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), func(m fieldmanager.Manager) fieldmanager.Manager {
+	f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), false, func(m fieldmanager.Manager) fieldmanager.Manager {
 		return fieldmanager.NewSkipNonAppliedManager(
 			m,
 			&fakeObjectCreater{gvk: schema.GroupVersionKind{Version: "v1", Kind: "Pod"}},
@@ -83,7 +83,7 @@ func TestNoUpdateBeforeFirstApply(t *testing.T) {
 }
 
 func TestUpdateBeforeFirstApply(t *testing.T) {
-	f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), func(m fieldmanager.Manager) fieldmanager.Manager {
+	f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), false, func(m fieldmanager.Manager) fieldmanager.Manager {
 		return fieldmanager.NewSkipNonAppliedManager(
 			m,
 			&fakeObjectCreater{gvk: schema.GroupVersionKind{Version: "v1", Kind: "Pod"}},

staging/src/k8s.io/apiserver/pkg/endpoints/installer.go

@@ -570,6 +570,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 			a.group.Creater,
 			fqKindToRegister,
 			reqScope.HubGroupVersion,
+			isSubresource,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create field manager: %v", err)

test/integration/apiserver/apply/apply_crd_test.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	"k8s.io/apiextensions-apiserver/test/integration/fixtures"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	genericfeatures "k8s.io/apiserver/pkg/features"
@@ -129,6 +130,69 @@ spec:
 		t.Fatalf("failed to apply object with force after updating replicas: %v:\n%v", err, string(result))
 	}
 	verifyReplicas(t, result, 1)
+
+	// Try to set managed fields using a subresource and verify that it has no effect
+	existingManagedFields, err := getManagedFields(result)
+	if err != nil {
+		t.Fatalf("failed to get managedFields from response: %v", err)
+	}
+	updateBytes := []byte(`{
+		"metadata": {
+			"managedFields": [{
+				"manager":"testing",
+				"operation":"Update",
+				"apiVersion":"v1",
+				"fieldsType":"FieldsV1",
+				"fieldsV1":{
+					"f:spec":{
+						"f:containers":{
+							"k:{\"name\":\"testing\"}":{
+								".":{},
+								"f:image":{},
+								"f:name":{}
+							}
+						}
+					}
+				}
+			}]
+		}
+	}`)
+	result, err = rest.Patch(types.MergePatchType).
+		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
+		SubResource("status").
+		Name(name).
+		Param("fieldManager", "subresource_test").
+		Body(updateBytes).
+		DoRaw(context.TODO())
+	if err != nil {
+		t.Fatalf("Error updating subresource: %v ", err)
+	}
+	newManagedFields, err := getManagedFields(result)
+	if err != nil {
+		t.Fatalf("failed to get managedFields from response: %v", err)
+	}
+	if !reflect.DeepEqual(existingManagedFields, newManagedFields) {
+		t.Fatalf("Expected managed fields to not have changed when trying manually settting them via subresoures.\n\nExpected: %#v\n\nGot: %#v", existingManagedFields, newManagedFields)
+	}
+
+	// However, it is possible to modify managed fields using the main resource
+	result, err = rest.Patch(types.MergePatchType).
+		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
+		Name(name).
+		Param("fieldManager", "subresource_test").
+		Body([]byte(`{"metadata":{"managedFields":[{}]}}`)).
+		DoRaw(context.TODO())
+	if err != nil {
+		t.Fatalf("Error updating managed fields of the main resource: %v ", err)
+	}
+	newManagedFields, err = getManagedFields(result)
+	if err != nil {
+		t.Fatalf("failed to get managedFields from response: %v", err)
+	}
+
+	if len(newManagedFields) != 0 {
+		t.Fatalf("Expected managed fields to have been reset, but got: %v", newManagedFields)
+	}
 }
 
 // TestApplyCRDStructuralSchema tests that when a CRD has a structural schema in its validation field,
@@ -753,3 +817,11 @@ spec:
 	}
 	verifyReplicas(t, result, 1)
 }
+
+func getManagedFields(rawResponse []byte) ([]metav1.ManagedFieldsEntry, error) {
+	obj := unstructured.Unstructured{}
+	if err := obj.UnmarshalJSON(rawResponse); err != nil {
+		return nil, err
+	}
+	return obj.GetManagedFields(), nil
+}

test/integration/apiserver/apply/apply_test.go

@@ -1577,7 +1577,108 @@ func TestErrorsDontFailPatch(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to patch object with empty FieldsType: %v", err)
 	}
+}
 
+func TestApplyDoesNotChangeManagedFieldsViaSubresources(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
+
+	_, client, closeFn := setup(t)
+	defer closeFn()
+
+	podBytes := []byte(`{
+		"apiVersion": "v1",
+		"kind": "Pod",
+		"metadata": {
+			"name": "just-a-pod"
+		},
+		"spec": {
+			"containers": [{
+				"name":  "test-container-a",
+				"image": "test-image-one"
+			}]
+		}
+	}`)
+
+	liveObj, err := client.CoreV1().RESTClient().
+		Patch(types.ApplyPatchType).
+		Namespace("default").
+		Param("fieldManager", "apply_test").
+		Resource("pods").
+		Name("just-a-pod").
+		Body(podBytes).
+		Do(context.TODO()).
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object: %v", err)
+	}
+
+	updateBytes := []byte(`{
+		"metadata": {
+			"managedFields": [{
+				"manager":"testing",
+				"operation":"Update",
+				"apiVersion":"v1",
+				"fieldsType":"FieldsV1",
+				"fieldsV1":{
+					"f:spec":{
+						"f:containers":{
+							"k:{\"name\":\"testing\"}":{
+								".":{},
+								"f:image":{},
+								"f:name":{}
+							}
+						}
+					}
+				}
+			}]
+		},
+		"status": {
+			"conditions": [{"type": "MyStatus", "status":"true"}]
+		}
+	}`)
+
+	updateActor := "update_managedfields_test"
+	newObj, err := client.CoreV1().RESTClient().
+		Patch(types.MergePatchType).
+		Namespace("default").
+		Param("fieldManager", updateActor).
+		Name("just-a-pod").
+		Resource("pods").
+		SubResource("status").
+		Body(updateBytes).
+		Do(context.TODO()).
+		Get()
+
+	if err != nil {
+		t.Fatalf("Error updating subresource: %v ", err)
+	}
+
+	liveAccessor, err := meta.Accessor(liveObj)
+	if err != nil {
+		t.Fatalf("Failed to get meta accessor for live object: %v", err)
+	}
+	newAccessor, err := meta.Accessor(newObj)
+	if err != nil {
+		t.Fatalf("Failed to get meta accessor for new object: %v", err)
+	}
+
+	liveManagedFields := liveAccessor.GetManagedFields()
+	if len(liveManagedFields) != 1 {
+		t.Fatalf("Expected managedFields in the live object to have exactly one entry, got %d: %v", len(liveManagedFields), liveManagedFields)
+	}
+
+	newManagedFields := newAccessor.GetManagedFields()
+	if len(newManagedFields) != 2 {
+		t.Fatalf("Expected managedFields in the new object to have exactly two entries, got %d: %v", len(newManagedFields), newManagedFields)
+	}
+
+	if !reflect.DeepEqual(liveManagedFields[0], newManagedFields[0]) {
+		t.Fatalf("managedFields updated via subresource:\n\nlive managedFields: %v\nnew managedFields: %v\n\n", liveManagedFields, newManagedFields)
+	}
+
+	if newManagedFields[1].Manager != updateActor {
+		t.Fatalf(`Expected managerFields to have an entry with manager set to %q`, updateActor)
+	}
 }
 
 // TestClearManagedFieldsWithUpdateEmptyList verifies it's possible to clear the managedFields by sending an empty list.