From ee2f6a7b4911b5061d28609aad4947af59005c8e Mon Sep 17 00:00:00 2001 From: Leonor Oliveira <9090754+leonorfmartins@users.noreply.github.com> Date: Tue, 7 May 2024 14:02:30 +0100 Subject: [PATCH] Force interface implementation also on legacy storage (#87414) * Force interface implementation also on legacy storage * Add DeleteCollection to folders and dashboards * Fix integration tests * Fix tests --- pkg/apiserver/rest/dualwriter.go | 11 ++---- pkg/apiserver/rest/dualwriter_mode1.go | 37 +++---------------- pkg/apiserver/rest/dualwriter_mode2.go | 32 +++------------- pkg/apiserver/rest/dualwriter_mode3.go | 27 ++------------ pkg/registry/apis/dashboard/legacy_storage.go | 5 +++ pkg/registry/apis/folders/legacy_storage.go | 5 +++ pkg/registry/apis/playlist/legacy_storage.go | 5 +++ pkg/tests/apis/dashboard/dashboards_test.go | 1 + pkg/tests/apis/folder/folders_test.go | 1 + pkg/tests/apis/playlist/playlist_test.go | 1 + 10 files changed, 36 insertions(+), 89 deletions(-) diff --git a/pkg/apiserver/rest/dualwriter.go b/pkg/apiserver/rest/dualwriter.go index cafc7bb1fcf..3f20d34f973 100644 --- a/pkg/apiserver/rest/dualwriter.go +++ b/pkg/apiserver/rest/dualwriter.go @@ -2,7 +2,6 @@ package rest import ( "context" - "errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -38,6 +37,10 @@ type LegacyStorage interface { rest.Storage rest.Scoper rest.SingularNameProvider + rest.CreaterUpdater + rest.Lister + rest.GracefulDeleter + rest.CollectionDeleter rest.TableConvertor rest.Getter } @@ -70,12 +73,6 @@ type DualWriter interface { type DualWriterMode int -var errDualWriterCreaterMissing = errors.New("legacy storage rest.Creater is missing") -var errDualWriterListerMissing = errors.New("legacy storage rest.Lister is missing") -var errDualWriterDeleterMissing = errors.New("legacy storage rest.GracefulDeleter is missing") -var errDualWriterCollectionDeleterMissing = errors.New("legacy storage rest.CollectionDeleter is missing") -var errDualWriterUpdaterMissing = errors.New("legacy storage rest.Updater is missing") - const ( Mode1 DualWriterMode = iota Mode2 diff --git a/pkg/apiserver/rest/dualwriter_mode1.go b/pkg/apiserver/rest/dualwriter_mode1.go index 9f1c99de37a..75651998784 100644 --- a/pkg/apiserver/rest/dualwriter_mode1.go +++ b/pkg/apiserver/rest/dualwriter_mode1.go @@ -25,13 +25,7 @@ func NewDualWriterMode1(legacy LegacyStorage, storage Storage) *DualWriterMode1 // Create overrides the behavior of the generic DualWriter and writes only to LegacyStorage. func (d *DualWriterMode1) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { ctx = klog.NewContext(ctx, d.Log) - legacy, ok := d.Legacy.(rest.Creater) - if !ok { - d.Log.Error(errDualWriterCreaterMissing, "legacy storage rest.Creater is missing") - return nil, errDualWriterCreaterMissing - } - - return legacy.Create(ctx, obj, createValidation, options) + return d.Legacy.Create(ctx, obj, createValidation, options) } // Get overrides the behavior of the generic DualWriter and reads only from LegacyStorage. @@ -43,44 +37,23 @@ func (d *DualWriterMode1) Get(ctx context.Context, name string, options *metav1. // List overrides the behavior of the generic DualWriter and reads only from LegacyStorage. func (d *DualWriterMode1) List(ctx context.Context, options *metainternalversion.ListOptions) (runtime.Object, error) { ctx = klog.NewContext(ctx, d.Log) - legacy, ok := d.Legacy.(rest.Lister) - if !ok { - return nil, errDualWriterListerMissing - } - - return legacy.List(ctx, options) + return d.Legacy.List(ctx, options) } func (d *DualWriterMode1) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { ctx = klog.NewContext(ctx, d.Log) - legacy, ok := d.Legacy.(rest.GracefulDeleter) - if !ok { - return nil, false, errDualWriterDeleterMissing - } - - return legacy.Delete(ctx, name, deleteValidation, options) + return d.Legacy.Delete(ctx, name, deleteValidation, options) } // DeleteCollection overrides the behavior of the generic DualWriter and deletes only from LegacyStorage. func (d *DualWriterMode1) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { ctx = klog.NewContext(ctx, d.Log) - legacy, ok := d.Legacy.(rest.CollectionDeleter) - if !ok { - return nil, errDualWriterCollectionDeleterMissing - } - - return legacy.DeleteCollection(ctx, deleteValidation, options, listOptions) + return d.Legacy.DeleteCollection(ctx, deleteValidation, options, listOptions) } func (d *DualWriterMode1) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { ctx = klog.NewContext(ctx, d.Log) - legacy, ok := d.Legacy.(rest.Updater) - if !ok { - d.Log.Error(errDualWriterUpdaterMissing, "legacy storage rest.Updater is missing") - return nil, false, errDualWriterUpdaterMissing - } - - return legacy.Update(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options) + return d.Legacy.Update(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options) } func (d *DualWriterMode1) Destroy() { diff --git a/pkg/apiserver/rest/dualwriter_mode2.go b/pkg/apiserver/rest/dualwriter_mode2.go index 6aad5082deb..7fa8d2b0199 100644 --- a/pkg/apiserver/rest/dualwriter_mode2.go +++ b/pkg/apiserver/rest/dualwriter_mode2.go @@ -30,12 +30,8 @@ func NewDualWriterMode2(legacy LegacyStorage, storage Storage) *DualWriterMode2 // Create overrides the behavior of the generic DualWriter and writes to LegacyStorage and Storage. func (d *DualWriterMode2) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { ctx = klog.NewContext(ctx, d.Log) - legacy, ok := d.Legacy.(rest.Creater) - if !ok { - return nil, errDualWriterCreaterMissing - } - created, err := legacy.Create(ctx, obj, createValidation, options) + created, err := d.Legacy.Create(ctx, obj, createValidation, options) if err != nil { d.Log.Error(err, "unable to create object in legacy storage") return created, err @@ -86,12 +82,7 @@ func (d *DualWriterMode2) Get(ctx context.Context, name string, options *metav1. func (d *DualWriterMode2) List(ctx context.Context, options *metainternalversion.ListOptions) (runtime.Object, error) { log := d.Log.WithValues("kind", options.Kind, "resourceVersion", options.ResourceVersion) ctx = klog.NewContext(ctx, log) - legacy, ok := d.Legacy.(rest.Lister) - if !ok { - return nil, errDualWriterListerMissing - } - - ll, err := legacy.List(ctx, options) + ll, err := d.Legacy.List(ctx, options) if err != nil { log.Error(err, "unable to list objects from legacy storage") return nil, err @@ -144,12 +135,7 @@ func (d *DualWriterMode2) List(ctx context.Context, options *metainternalversion func (d *DualWriterMode2) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { log := d.Log.WithValues("kind", options.Kind, "resourceVersion", listOptions.ResourceVersion) ctx = klog.NewContext(ctx, log) - legacy, ok := d.Legacy.(rest.CollectionDeleter) - if !ok { - return nil, errDualWriterCollectionDeleterMissing - } - - deleted, err := legacy.DeleteCollection(ctx, deleteValidation, options, listOptions) + deleted, err := d.Legacy.DeleteCollection(ctx, deleteValidation, options, listOptions) if err != nil { log.WithValues("deleted", deleted).Error(err, "failed to delete collection successfully from legacy storage") return nil, err @@ -180,12 +166,8 @@ func (d *DualWriterMode2) DeleteCollection(ctx context.Context, deleteValidation func (d *DualWriterMode2) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { log := d.Log.WithValues("name", name) ctx = klog.NewContext(ctx, log) - legacy, ok := d.Legacy.(rest.GracefulDeleter) - if !ok { - return nil, false, errDualWriterDeleterMissing - } - deletedLS, async, err := legacy.Delete(ctx, name, deleteValidation, options) + deletedLS, async, err := d.Legacy.Delete(ctx, name, deleteValidation, options) if err != nil { if !apierrors.IsNotFound(err) { log.WithValues("objectList", deletedLS).Error(err, "could not delete from legacy store") @@ -208,10 +190,6 @@ func (d *DualWriterMode2) Update(ctx context.Context, name string, objInfo rest. var notFound bool log := d.Log.WithValues("name", name) ctx = klog.NewContext(ctx, log) - legacy, ok := d.Legacy.(rest.Updater) - if !ok { - return nil, false, errDualWriterUpdaterMissing - } // get old and new (updated) object so they can be stored in legacy store old, err := d.Storage.Get(ctx, name, &metav1.GetOptions{}) @@ -231,7 +209,7 @@ func (d *DualWriterMode2) Update(ctx context.Context, name string, objInfo rest. return nil, false, err } - obj, created, err := legacy.Update(ctx, name, &updateWrapper{upstream: objInfo, updated: updated}, createValidation, updateValidation, forceAllowCreate, options) + obj, created, err := d.Legacy.Update(ctx, name, &updateWrapper{upstream: objInfo, updated: updated}, createValidation, updateValidation, forceAllowCreate, options) if err != nil { log.WithValues("object", obj).Error(err, "could not update in legacy storage") return obj, created, err diff --git a/pkg/apiserver/rest/dualwriter_mode3.go b/pkg/apiserver/rest/dualwriter_mode3.go index 7f48894549b..3520d302f52 100644 --- a/pkg/apiserver/rest/dualwriter_mode3.go +++ b/pkg/apiserver/rest/dualwriter_mode3.go @@ -26,10 +26,6 @@ func NewDualWriterMode3(legacy LegacyStorage, storage Storage) *DualWriterMode3 // Create overrides the behavior of the generic DualWriter and writes to LegacyStorage and Storage. func (d *DualWriterMode3) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { log := klog.FromContext(ctx) - legacy, ok := d.Legacy.(rest.Creater) - if !ok { - return nil, errDualWriterCreaterMissing - } created, err := d.Storage.Create(ctx, obj, createValidation, options) if err != nil { @@ -37,7 +33,7 @@ func (d *DualWriterMode3) Create(ctx context.Context, obj runtime.Object, create return created, err } - if _, err := legacy.Create(ctx, obj, createValidation, options); err != nil { + if _, err := d.Legacy.Create(ctx, obj, createValidation, options); err != nil { log.WithValues("object", created).Error(err, "unable to create object in legacy storage") } return created, nil @@ -51,10 +47,6 @@ func (d *DualWriterMode3) Get(ctx context.Context, name string, options *metav1. func (d *DualWriterMode3) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { log := d.Log.WithValues("name", name) ctx = klog.NewContext(ctx, log) - legacy, ok := d.Legacy.(rest.GracefulDeleter) - if !ok { - return nil, false, errDualWriterDeleterMissing - } deleted, async, err := d.Storage.Delete(ctx, name, deleteValidation, options) if err != nil { @@ -64,7 +56,7 @@ func (d *DualWriterMode3) Delete(ctx context.Context, name string, deleteValidat } } - _, _, errLS := legacy.Delete(ctx, name, deleteValidation, options) + _, _, errLS := d.Legacy.Delete(ctx, name, deleteValidation, options) if errLS != nil { if !apierrors.IsNotFound(errLS) { log.WithValues("deleted", deleted).Error(errLS, "could not delete from legacy store") @@ -100,13 +92,7 @@ func (d *DualWriterMode3) Update(ctx context.Context, name string, objInfo rest. return obj, created, err } - legacy, ok := d.Legacy.(rest.Updater) - if !ok { - log.Error(errDualWriterUpdaterMissing, "legacy storage update not implemented") - return obj, created, err - } - - _, _, errLeg := legacy.Update(ctx, name, &updateWrapper{ + _, _, errLeg := d.Legacy.Update(ctx, name, &updateWrapper{ upstream: objInfo, updated: obj, }, createValidation, updateValidation, forceAllowCreate, options) @@ -120,18 +106,13 @@ func (d *DualWriterMode3) Update(ctx context.Context, name string, objInfo rest. func (d *DualWriterMode3) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { log := d.Log.WithValues("kind", options.Kind, "resourceVersion", listOptions.ResourceVersion) ctx = klog.NewContext(ctx, log) - legacy, ok := d.Legacy.(rest.CollectionDeleter) - if !ok { - return nil, errDualWriterCollectionDeleterMissing - } - // #TODO: figure out how to handle partial deletions deleted, err := d.Storage.DeleteCollection(ctx, deleteValidation, options, listOptions) if err != nil { log.Error(err, "failed to delete collection successfully from Storage") } - if deleted, err := legacy.DeleteCollection(ctx, deleteValidation, options, listOptions); err != nil { + if deleted, err := d.Legacy.DeleteCollection(ctx, deleteValidation, options, listOptions); err != nil { log.WithValues("deleted", deleted).Error(err, "failed to delete collection successfully from LegacyStorage") } diff --git a/pkg/registry/apis/dashboard/legacy_storage.go b/pkg/registry/apis/dashboard/legacy_storage.go index 20e5193b240..7e2a9881678 100644 --- a/pkg/registry/apis/dashboard/legacy_storage.go +++ b/pkg/registry/apis/dashboard/legacy_storage.go @@ -153,3 +153,8 @@ func (s *dashboardStorage) Get(ctx context.Context, name string, options *metav1 return s.access.GetDashboard(ctx, info.OrgID, name) } + +// GracefulDeleter +func (s *dashboardStorage) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) { + return nil, fmt.Errorf("DeleteCollection for dashboards not implemented") +} diff --git a/pkg/registry/apis/folders/legacy_storage.go b/pkg/registry/apis/folders/legacy_storage.go index 82a4f097a32..781aacd7526 100644 --- a/pkg/registry/apis/folders/legacy_storage.go +++ b/pkg/registry/apis/folders/legacy_storage.go @@ -289,3 +289,8 @@ func (s *legacyStorage) Delete(ctx context.Context, name string, deleteValidatio }) return p, true, err // true is instant delete } + +// GracefulDeleter +func (s *legacyStorage) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) { + return nil, fmt.Errorf("DeleteCollection for folders not implemented") +} diff --git a/pkg/registry/apis/playlist/legacy_storage.go b/pkg/registry/apis/playlist/legacy_storage.go index 316c0f52462..05626026c0f 100644 --- a/pkg/registry/apis/playlist/legacy_storage.go +++ b/pkg/registry/apis/playlist/legacy_storage.go @@ -186,3 +186,8 @@ func (s *legacyStorage) Delete(ctx context.Context, name string, deleteValidatio }) return p, true, err // true is instant delete } + +// CollectionDeleter +func (s *legacyStorage) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) { + return nil, fmt.Errorf("DeleteCollection for playlists not implemented") +} diff --git a/pkg/tests/apis/dashboard/dashboards_test.go b/pkg/tests/apis/dashboard/dashboards_test.go index 9771ef718e4..94c62a18988 100644 --- a/pkg/tests/apis/dashboard/dashboards_test.go +++ b/pkg/tests/apis/dashboard/dashboards_test.go @@ -89,6 +89,7 @@ func TestIntegrationDashboardsApp(t *testing.T) { "verbs": [ "create", "delete", + "deletecollection", "get", "list", "patch", diff --git a/pkg/tests/apis/folder/folders_test.go b/pkg/tests/apis/folder/folders_test.go index 1dbbddf3ff6..d6b4c765d2b 100644 --- a/pkg/tests/apis/folder/folders_test.go +++ b/pkg/tests/apis/folder/folders_test.go @@ -49,6 +49,7 @@ func TestIntegrationFoldersApp(t *testing.T) { "verbs": [ "create", "delete", + "deletecollection", "get", "list", "patch", diff --git a/pkg/tests/apis/playlist/playlist_test.go b/pkg/tests/apis/playlist/playlist_test.go index 91ec2f1d23c..42eae928440 100644 --- a/pkg/tests/apis/playlist/playlist_test.go +++ b/pkg/tests/apis/playlist/playlist_test.go @@ -63,6 +63,7 @@ func TestIntegrationPlaylist(t *testing.T) { "verbs": [ "create", "delete", + "deletecollection", "get", "list", "patch",