Merge pull request #85 from appuio/fix-be-stor-not-allowed-method

Properly expose storage methods through createRBACWrapper
appuio · Jan 5, 2023 · e3b9c46 · e3b9c46
2 parents 3451197 + ef045ed
commit e3b9c46
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 29 deletions.
diff --git a/apiserver/billing/billing.go b/apiserver/billing/billing.go
@@ -11,6 +11,7 @@ import (
 
 	billingv1 "github.com/appuio/control-api/apis/billing/v1"
 	"github.com/appuio/control-api/apiserver/authwrapper"
+	"github.com/appuio/control-api/apiserver/billing/odoostorage"
 )
 
 // New returns a new storage provider with RBAC authentication for BillingEntities
@@ -31,8 +32,8 @@ func New(stor authwrapper.StorageScoper) restbuilder.ResourceHandlerProvider {
 		}
 
 		stor := &createRBACWrapper{
-			storageCreator: astor.(storageCreator),
-			client:         c,
+			Storage: astor.(odoostorage.Storage),
+			client:  c,
 		}
 
 		return stor, nil

diff --git a/apiserver/billing/odoostorage/create.go b/apiserver/billing/odoostorage/create.go
@@ -11,8 +11,6 @@ import (
 	billingv1 "github.com/appuio/control-api/apis/billing/v1"
 )
 
-var _ rest.Creater = &billingEntityStorage{}
-
 func (s *billingEntityStorage) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
 	be, ok := obj.(*billingv1.BillingEntity)
 	if !ok {

diff --git a/apiserver/billing/odoostorage/get.go b/apiserver/billing/odoostorage/get.go
@@ -7,14 +7,11 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apiserver/pkg/registry/rest"
 
 	billingv1 "github.com/appuio/control-api/apis/billing/v1"
 	"github.com/appuio/control-api/apiserver/billing/odoostorage/odoo"
 )
 
-var _ rest.Getter = &billingEntityStorage{}
-
 func (s *billingEntityStorage) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) {
 	be, err := s.storage.Get(ctx, name)
 	if err != nil {

diff --git a/apiserver/billing/odoostorage/list.go b/apiserver/billing/odoostorage/list.go
@@ -11,8 +11,6 @@ import (
 	billingv1 "github.com/appuio/control-api/apis/billing/v1"
 )
 
-var _ rest.Lister = &billingEntityStorage{}
-
 func (s billingEntityStorage) NewList() runtime.Object {
 	return &billingv1.BillingEntityList{}
 }

diff --git a/apiserver/billing/odoostorage/odoostorage.go b/apiserver/billing/odoostorage/odoostorage.go
@@ -10,7 +10,7 @@ import (
 )
 
 // New returns a new storage provider for Organizations
-func New() rest.Storage {
+func New() Storage {
 	return &billingEntityStorage{
 		storage: fake.NewFakeOdooStorage(false),
 	}
@@ -20,16 +20,23 @@ type billingEntityStorage struct {
 	storage odoo.OdooStorage
 }
 
-var _ rest.Storage = &billingEntityStorage{}
+type Storage interface {
+	rest.Storage
+	rest.Scoper
+
+	rest.CreaterUpdater
+	rest.Lister
+	rest.Getter
+}
+
+var _ Storage = &billingEntityStorage{}
 
 func (s billingEntityStorage) New() runtime.Object {
 	return &billingv1.BillingEntity{}
 }
 
 func (s billingEntityStorage) Destroy() {}
 
-var _ rest.Scoper = &billingEntityStorage{}
-
 func (s *billingEntityStorage) NamespaceScoped() bool {
 	return false
 }
diff --git a/apiserver/billing/odoostorage/update.go b/apiserver/billing/odoostorage/update.go
@@ -11,9 +11,6 @@ import (
 	billingv1 "github.com/appuio/control-api/apis/billing/v1"
 )
 
-var _ rest.Updater = &billingEntityStorage{}
-var _ rest.CreaterUpdater = &billingEntityStorage{}
-
 func (s *billingEntityStorage) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo,
 	createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc,
 	forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {

diff --git a/apiserver/billing/rbac.go b/apiserver/billing/rbac.go
@@ -13,17 +13,13 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/filters"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-)
 
-type storageCreator interface {
-	rest.Storage
-	rest.Creater
-	rest.Scoper
-}
+	"github.com/appuio/control-api/apiserver/billing/odoostorage"
+)
 
 // createRBACWrapper is a wrapper around the storage that creates a ClusterRole and ClusterRoleBinding for each BillingEntity on creation.
 type createRBACWrapper struct {
-	storageCreator
+	odoostorage.Storage
 	client client.Client
 }
 
@@ -34,7 +30,7 @@ func (c *createRBACWrapper) Create(ctx context.Context, obj runtime.Object, crea
 	}
 	user := attr.GetUser()
 
-	createdObj, err := c.storageCreator.Create(ctx, obj, createValidation, opts)
+	createdObj, err := c.Storage.Create(ctx, obj, createValidation, opts)
 	if err != nil {
 		return createdObj, err
 	}
@@ -80,7 +76,7 @@ func (c *createRBACWrapper) Create(ctx context.Context, obj runtime.Object, crea
 	}
 
 	rollback := func() error {
-		if deleter, canDelete := c.storageCreator.(rest.GracefulDeleter); canDelete {
+		if deleter, canDelete := c.Storage.(rest.GracefulDeleter); canDelete {
 			_, _, err := deleter.Delete(ctx, objName, nil, &metav1.DeleteOptions{DryRun: opts.DryRun})
 			return err
 		}

diff --git a/apiserver/billing/rbac_test.go b/apiserver/billing/rbac_test.go
@@ -33,8 +33,8 @@ func Test_createRBACWrapper(t *testing.T) {
 	defer ctrl.Finish()
 
 	subject := &createRBACWrapper{
-		storageCreator: clusterScopedStorage{store},
-		client:         c,
+		Storage: clusterScopedStorage{store},
+		client:  c,
 	}
 
 	store.EXPECT().
@@ -62,8 +62,8 @@ func Test_createRBACWrapper_rollback(t *testing.T) {
 	defer ctrl.Finish()
 
 	subject := &createRBACWrapper{
-		storageCreator: clusterScopedStorage{store},
-		client:         c,
+		Storage: clusterScopedStorage{store},
+		client:  c,
 	}
 
 	store.EXPECT().