From 559d12fcce0605fb0ca4b6193b4ece1e9a1a97c9 Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Tue, 28 Mar 2023 16:55:47 -0700 Subject: [PATCH] managedfields: Create NewFakeFieldManager And simplify how a lot of the fakes are created. Notably, the converter was never really used to do anything so this is simplified. --- .../internal/skipnonapplied_test.go | 4 +- .../internal/testing/testfieldmanager.go | 58 ++++++------------- .../managedfieldstest/testfieldmanager.go | 28 +++++++++ 3 files changed, 48 insertions(+), 42 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/skipnonapplied_test.go b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/skipnonapplied_test.go index 81179e2c8a5..d38e695ded8 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/skipnonapplied_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/skipnonapplied_test.go @@ -33,7 +33,7 @@ func TestNoUpdateBeforeFirstApply(t *testing.T) { f := internaltesting.NewTestFieldManagerImpl(fakeTypeConverter, schema.FromAPIVersionAndKind("v1", "Pod"), "", func(m internal.Manager) internal.Manager { return internal.NewSkipNonAppliedManager( m, - internaltesting.NewFakeObjectCreater(), + &internaltesting.FakeObjectCreater{}, schema.FromAPIVersionAndKind("v1", "Pod"), ) }) @@ -73,7 +73,7 @@ func TestUpdateBeforeFirstApply(t *testing.T) { f := internaltesting.NewTestFieldManagerImpl(fakeTypeConverter, schema.FromAPIVersionAndKind("v1", "Pod"), "", func(m internal.Manager) internal.Manager { return internal.NewSkipNonAppliedManager( m, - internaltesting.NewFakeObjectCreater(), + &internaltesting.FakeObjectCreater{}, schema.FromAPIVersionAndKind("v1", "Pod"), ) }) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/testing/testfieldmanager.go b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/testing/testfieldmanager.go index 1c6dec6a033..805359e3147 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/testing/testfieldmanager.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/testing/testfieldmanager.go @@ -27,62 +27,42 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/managedfields" "k8s.io/apimachinery/pkg/util/managedfields/internal" - "sigs.k8s.io/structured-merge-diff/v4/fieldpath" - "sigs.k8s.io/structured-merge-diff/v4/merge" - "sigs.k8s.io/structured-merge-diff/v4/typed" ) -// NewFakeObjectCreater implements ObjectCreater, it can create empty +// FakeObjectCreater implements ObjectCreater, it can create empty // objects (unstructured) of the given GVK. -func NewFakeObjectCreater() runtime.ObjectCreater { - return &fakeObjectCreater{} -} +type FakeObjectCreater struct{} -type fakeObjectCreater struct{} - -func (f *fakeObjectCreater) New(gvk schema.GroupVersionKind) (runtime.Object, error) { +func (f *FakeObjectCreater) New(gvk schema.GroupVersionKind) (runtime.Object, error) { u := unstructured.Unstructured{Object: map[string]interface{}{}} u.SetAPIVersion(gvk.GroupVersion().String()) u.SetKind(gvk.Kind) return &u, nil } -type fakeObjectConvertor struct { - converter merge.Converter - apiVersion fieldpath.APIVersion -} +// FakeObjectConvertor implements runtime.ObjectConvertor but it +// actually does nothing but return its input. +type FakeObjectConvertor struct{} //nolint:staticcheck,ineffassign // SA4009 backwards compatibility -func (c *fakeObjectConvertor) Convert(in, out, context interface{}) error { - if typedValue, ok := in.(*typed.TypedValue); ok { - var err error - out, err = c.converter.Convert(typedValue, c.apiVersion) - return err - } +func (c *FakeObjectConvertor) Convert(in, out, context interface{}) error { + out = in return nil } -func (c *fakeObjectConvertor) ConvertToVersion(in runtime.Object, _ runtime.GroupVersioner) (runtime.Object, error) { +func (c *FakeObjectConvertor) ConvertToVersion(in runtime.Object, _ runtime.GroupVersioner) (runtime.Object, error) { return in, nil } -func (c *fakeObjectConvertor) ConvertFieldLabel(_ schema.GroupVersionKind, _, _ string) (string, string, error) { +func (c *FakeObjectConvertor) ConvertFieldLabel(_ schema.GroupVersionKind, _, _ string) (string, string, error) { return "", "", errors.New("not implemented") } -type fakeObjectDefaulter struct{} +// FakeObjectDefaulter implements runtime.Defaulter, but it actually +// does nothing. +type FakeObjectDefaulter struct{} -func (d *fakeObjectDefaulter) Default(in runtime.Object) {} - -type sameVersionConverter struct{} - -func (sameVersionConverter) Convert(object *typed.TypedValue, version fieldpath.APIVersion) (*typed.TypedValue, error) { - return object, nil -} - -func (sameVersionConverter) IsMissingVersionError(error) bool { - return false -} +func (d *FakeObjectDefaulter) Default(in runtime.Object) {} type TestFieldManagerImpl struct { fieldManager *internal.FieldManager @@ -139,12 +119,10 @@ func (f *TestFieldManagerImpl) ManagedFields() []metav1.ManagedFieldsEntry { // NewTestFieldManager creates a new manager for the given GVK. func NewTestFieldManagerImpl(typeConverter managedfields.TypeConverter, gvk schema.GroupVersionKind, subresource string, chainFieldManager func(internal.Manager) internal.Manager) *TestFieldManagerImpl { - apiVersion := fieldpath.APIVersion(gvk.GroupVersion().String()) - objectConverter := &fakeObjectConvertor{sameVersionConverter{}, apiVersion} f, err := internal.NewStructuredMergeManager( typeConverter, - objectConverter, - &fakeObjectDefaulter{}, + &FakeObjectConvertor{}, + &FakeObjectDefaulter{}, gvk.GroupVersion(), gvk.GroupVersion(), nil, @@ -166,8 +144,8 @@ func NewTestFieldManagerImpl(typeConverter managedfields.TypeConverter, gvk sche internal.NewManagedFieldsUpdater( internal.NewStripMetaManager(f), ), gvk.GroupVersion(), subresource, - ), NewFakeObjectCreater(), gvk, internal.DefaultTrackOnCreateProbability, - ), typeConverter, objectConverter, gvk.GroupVersion(), + ), &FakeObjectCreater{}, gvk, internal.DefaultTrackOnCreateProbability, + ), typeConverter, &FakeObjectConvertor{}, gvk.GroupVersion(), ), ) if chainFieldManager != nil { diff --git a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/managedfieldstest/testfieldmanager.go b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/managedfieldstest/testfieldmanager.go index c6997f4d05b..afa7b06df99 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/managedfieldstest/testfieldmanager.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/managedfieldstest/testfieldmanager.go @@ -63,4 +63,32 @@ func NewTestFieldManager(typeConverter managedfields.TypeConverter, gvk schema.G // for the given gvk, on the given sub-resource. func NewTestFieldManagerSubresource(typeConverter managedfields.TypeConverter, gvk schema.GroupVersionKind, subresource string) TestFieldManager { return testing.NewTestFieldManagerImpl(typeConverter, gvk, subresource, nil) + +} + +// NewFakeFieldManager creates an actual FieldManager but that doesn't +// perform any conversion. This is just a convenience for tests to +// create an actual manager that they can use but in very restricted +// ways. +// +// This is different from the TestFieldManager because it's not meant to +// assert values, or hold the state, this acts like a normal +// FieldManager. +// +// Also, this only operates on the main-resource, and sub-resource can't +// be configured. +func NewFakeFieldManager(typeConverter managedfields.TypeConverter, gvk schema.GroupVersionKind) *managedfields.FieldManager { + ffm, err := managedfields.NewDefaultFieldManager( + typeConverter, + &testing.FakeObjectConvertor{}, + &testing.FakeObjectDefaulter{}, + &testing.FakeObjectCreater{}, + gvk, + gvk.GroupVersion(), + "", + nil) + if err != nil { + panic(err) + } + return ffm }