From 2bc17d1cf03f2f2bcd683e7e79f01c929951cca3 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 29 Oct 2024 12:03:32 -0400 Subject: [PATCH] Add ResetFieldsFilterStrategy --- .../pkg/apiserver/customresource_handler.go | 2 +- .../pkg/util/managedfields/fieldmanager.go | 7 ++--- .../managedfields/internal/structuredmerge.go | 19 +++++++------- .../apiserver/pkg/endpoints/installer.go | 26 ++++++++++++++++--- .../apiserver/pkg/registry/rest/rest.go | 9 ++++++- 5 files changed, 45 insertions(+), 18 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index 0d190dd3b38..12a77a204e9 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -1072,7 +1072,7 @@ func scopeWithFieldManager(typeConverter managedfields.TypeConverter, reqScope h reqScope.Kind, reqScope.HubGroupVersion, subresource, - resetFields, + fieldpath.NewExcludeFilterSetMap(resetFields), ) if err != nil { return handlers.RequestScope{}, err diff --git a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/fieldmanager.go b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/fieldmanager.go index 978ffb3c3e6..de540c82ffb 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/fieldmanager.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/fieldmanager.go @@ -19,11 +19,12 @@ package managedfields import ( "fmt" + "sigs.k8s.io/structured-merge-diff/v4/fieldpath" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/managedfields/internal" - "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) // FieldManager updates the managed fields and merges applied @@ -32,7 +33,7 @@ type FieldManager = internal.FieldManager // NewDefaultFieldManager creates a new FieldManager that merges apply requests // and update managed fields for other types of requests. -func NewDefaultFieldManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, subresource string, resetFields map[fieldpath.APIVersion]*fieldpath.Set) (*FieldManager, error) { +func NewDefaultFieldManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, subresource string, resetFields map[fieldpath.APIVersion]fieldpath.Filter) (*FieldManager, error) { f, err := internal.NewStructuredMergeManager(typeConverter, objectConverter, objectDefaulter, kind.GroupVersion(), hub, resetFields) if err != nil { return nil, fmt.Errorf("failed to create field manager: %v", err) @@ -43,7 +44,7 @@ func NewDefaultFieldManager(typeConverter TypeConverter, objectConverter runtime // 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(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, subresource string, resetFields map[fieldpath.APIVersion]*fieldpath.Set) (_ *FieldManager, err error) { +func NewDefaultCRDFieldManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, subresource string, resetFields map[fieldpath.APIVersion]fieldpath.Filter) (_ *FieldManager, err error) { f, err := internal.NewCRDStructuredMergeManager(typeConverter, objectConverter, objectDefaulter, kind.GroupVersion(), hub, resetFields) if err != nil { return nil, fmt.Errorf("failed to create field manager: %v", err) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/structuredmerge.go b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/structuredmerge.go index 786ad991c23..3fe36edc9d7 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/structuredmerge.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/structuredmerge.go @@ -19,13 +19,14 @@ package internal import ( "fmt" + "sigs.k8s.io/structured-merge-diff/v4/fieldpath" + "sigs.k8s.io/structured-merge-diff/v4/merge" + "sigs.k8s.io/structured-merge-diff/v4/typed" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/structured-merge-diff/v4/fieldpath" - "sigs.k8s.io/structured-merge-diff/v4/merge" - "sigs.k8s.io/structured-merge-diff/v4/typed" ) type structuredMergeManager struct { @@ -41,7 +42,7 @@ var _ Manager = &structuredMergeManager{} // NewStructuredMergeManager creates a new Manager that merges apply requests // and update managed fields for other types of requests. -func NewStructuredMergeManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, gv schema.GroupVersion, hub schema.GroupVersion, resetFields map[fieldpath.APIVersion]*fieldpath.Set) (Manager, error) { +func NewStructuredMergeManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, gv schema.GroupVersion, hub schema.GroupVersion, resetFields map[fieldpath.APIVersion]fieldpath.Filter) (Manager, error) { if typeConverter == nil { return nil, fmt.Errorf("typeconverter must not be nil") } @@ -52,8 +53,8 @@ func NewStructuredMergeManager(typeConverter TypeConverter, objectConverter runt groupVersion: gv, hubVersion: hub, updater: merge.Updater{ - Converter: newVersionConverter(typeConverter, objectConverter, hub), // This is the converter provided to SMD from k8s - IgnoredFields: resetFields, + Converter: newVersionConverter(typeConverter, objectConverter, hub), // This is the converter provided to SMD from k8s + IgnoreFilter: resetFields, }, }, nil } @@ -61,7 +62,7 @@ func NewStructuredMergeManager(typeConverter TypeConverter, objectConverter runt // NewCRDStructuredMergeManager creates a new Manager 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 NewCRDStructuredMergeManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, gv schema.GroupVersion, hub schema.GroupVersion, resetFields map[fieldpath.APIVersion]*fieldpath.Set) (_ Manager, err error) { +func NewCRDStructuredMergeManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, gv schema.GroupVersion, hub schema.GroupVersion, resetFields map[fieldpath.APIVersion]fieldpath.Filter) (_ Manager, err error) { return &structuredMergeManager{ typeConverter: typeConverter, objectConverter: objectConverter, @@ -69,8 +70,8 @@ func NewCRDStructuredMergeManager(typeConverter TypeConverter, objectConverter r groupVersion: gv, hubVersion: hub, updater: merge.Updater{ - Converter: newCRDVersionConverter(typeConverter, objectConverter, hub), - IgnoredFields: resetFields, + Converter: newCRDVersionConverter(typeConverter, objectConverter, hub), + IgnoreFilter: resetFields, }, }, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index 707eb4a503b..6a1826b4e33 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -685,9 +685,27 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag reqScope.MetaGroupVersion = *a.group.MetaGroupVersion } - var resetFields map[fieldpath.APIVersion]*fieldpath.Set - if resetFieldsStrategy, isResetFieldsStrategy := storage.(rest.ResetFieldsStrategy); isResetFieldsStrategy { - resetFields = resetFieldsStrategy.GetResetFields() + // Strategies may ignore changes to some fields by resetting the field values. + // + // For instance, spec resource strategies should reset the status, and status subresource + // strategies should reset the spec. + // + // Strategies that reset fields must report to the field manager which fields are + // reset by implementing either the ResetFieldsStrategy or the ResetFieldsFilterStrategy + // interface. + // + // For subresources that provide write access to only specific nested fields + // fieldpath.NewPatternFilter can help create a filter to reset all other fields. + var resetFieldsFilter map[fieldpath.APIVersion]fieldpath.Filter + resetFieldsStrategy, isResetFieldsStrategy := storage.(rest.ResetFieldsStrategy) + if isResetFieldsStrategy { + resetFieldsFilter = fieldpath.NewExcludeFilterSetMap(resetFieldsStrategy.GetResetFields()) + } + if resetFieldsStrategy, isResetFieldsFilterStrategy := storage.(rest.ResetFieldsFilterStrategy); isResetFieldsFilterStrategy { + if isResetFieldsStrategy { + return nil, nil, fmt.Errorf("may not implement both ResetFieldsStrategy and ResetFieldsFilterStrategy") + } + resetFieldsFilter = resetFieldsStrategy.GetResetFieldsFilter() } reqScope.FieldManager, err = managedfields.NewDefaultFieldManager( @@ -698,7 +716,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag fqKindToRegister, reqScope.HubGroupVersion, subresource, - resetFields, + resetFieldsFilter, ) if err != nil { return nil, nil, fmt.Errorf("failed to create field manager: %v", err) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go index 03cea7bb70b..fa023917058 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go @@ -22,12 +22,13 @@ import ( "net/http" "net/url" + "sigs.k8s.io/structured-merge-diff/v4/fieldpath" + metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/watch" - "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) //TODO: @@ -387,6 +388,12 @@ type ResetFieldsStrategy interface { GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set } +// ResetFieldsFilterStrategy is an optional interface that a storage object can +// implement if it wishes to provide a fields filter reset by its strategies. +type ResetFieldsFilterStrategy interface { + GetResetFieldsFilter() map[fieldpath.APIVersion]fieldpath.Filter +} + // CreateUpdateResetFieldsStrategy is a union of RESTCreateUpdateStrategy // and ResetFieldsStrategy. type CreateUpdateResetFieldsStrategy interface {