From 99200040ed8d35db317924cac5354dd56d78fc18 Mon Sep 17 00:00:00 2001 From: Andrea Nodari Date: Tue, 14 Jul 2020 17:42:54 +0200 Subject: [PATCH] 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. Kubernetes-commit: c522ee08a3d248ec1097e3673119ffa7a4e1ef7b --- .../handlers/fieldmanager/capmanagers_test.go | 2 + .../handlers/fieldmanager/fieldmanager.go | 84 ++++++++++++------- .../fieldmanager/fieldmanager_test.go | 63 +++++++++++++- .../fieldmanager/lastappliedupdater_test.go | 1 + .../fieldmanager/skipnonapplied_test.go | 4 +- pkg/endpoints/installer.go | 1 + 6 files changed, 120 insertions(+), 35 deletions(-) diff --git a/pkg/endpoints/handlers/fieldmanager/capmanagers_test.go b/pkg/endpoints/handlers/fieldmanager/capmanagers_test.go index 7ab319c49..aaf2c96a0 100644 --- a/pkg/endpoints/handlers/fieldmanager/capmanagers_test.go +++ b/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) }) diff --git a/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/pkg/endpoints/handlers/fieldmanager/fieldmanager.go index 2f6e5bd08..5008260ec 100644 --- a/pkg/endpoints/handlers/fieldmanager/fieldmanager.go +++ b/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 { - return &FieldManager{f} +func NewFieldManager(f Manager, ignoreManagedFieldsFromRequestObject bool) *FieldManager { + 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 - if isResetManagedFields(newAccessor.GetManagedFields()) { - managed = internal.NewEmptyManaged() - } 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() - } + managed, err := decodeManagedFields(liveObj, newObj, f.ignoreManagedFieldsFromRequestObject) + if err != nil { + return newObj, nil } internal.RemoveObjectManagedFields(liveObj) diff --git a/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go b/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go index 447b9e32d..b95f2938d 100644 --- a/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go +++ b/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()) @@ -118,7 +122,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(), } @@ -1233,3 +1237,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) + } +} diff --git a/pkg/endpoints/handlers/fieldmanager/lastappliedupdater_test.go b/pkg/endpoints/handlers/fieldmanager/lastappliedupdater_test.go index c93b7ef2c..7c3d2362d 100644 --- a/pkg/endpoints/handlers/fieldmanager/lastappliedupdater_test.go +++ b/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) }) diff --git a/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go b/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go index ba7049792..8c7ac7019 100644 --- a/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go +++ b/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"}}, diff --git a/pkg/endpoints/installer.go b/pkg/endpoints/installer.go index b406c91b1..9f790b727 100644 --- a/pkg/endpoints/installer.go +++ b/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)