Implement BillingEntity RBAC reconcile

Used a cron job since we never implemented watch for BEs.

Author: bastjan

apiserver/billing/rbac.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 
 	"go.uber.org/multierr"
-	rbacv1 "k8s.io/api/rbac/v1"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -15,6 +14,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/appuio/control-api/apiserver/billing/odoostorage"
+	"github.com/appuio/control-api/pkg/billingrbac"
 )
 
 // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=get;list;watch;create;delete;patch;update;edit
@@ -44,68 +44,12 @@ func (c *createRBACWrapper) Create(ctx context.Context, obj runtime.Object, crea
 		return createdObj, fmt.Errorf("could not get name of created object: %w", err)
 	}
 
-	viewRoleName := fmt.Sprintf("billingentities-%s-viewer", objName)
-	viewRole := &rbacv1.ClusterRole{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: viewRoleName,
-		},
-		Rules: []rbacv1.PolicyRule{
-			{
-				APIGroups:     []string{"rbac.appuio.io"},
-				Resources:     []string{"billingentities"},
-				Verbs:         []string{"get"},
-				ResourceNames: []string{objName},
-			},
-		},
-	}
-	viewRoleBinding := &rbacv1.ClusterRoleBinding{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: viewRoleName,
-		},
-		Subjects: []rbacv1.Subject{},
-		RoleRef: rbacv1.RoleRef{
-			Kind:     "ClusterRole",
-			APIGroup: "rbac.authorization.k8s.io",
-			Name:     viewRoleName,
-		},
-	}
-	adminRoleName := fmt.Sprintf("billingentities-%s-admin", objName)
-	adminRole := &rbacv1.ClusterRole{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: adminRoleName,
-		},
-		Rules: []rbacv1.PolicyRule{
-			{
-				APIGroups:     []string{"rbac.appuio.io", "billing.appuio.io"},
-				Resources:     []string{"billingentities"},
-				Verbs:         []string{"get", "patch", "update", "edit"},
-				ResourceNames: []string{objName},
-			},
-			{
-				APIGroups:     []string{"rbac.authorization.k8s.io"},
-				Resources:     []string{"clusterrolebindings"},
-				Verbs:         []string{"get", "edit", "update", "patch"},
-				ResourceNames: []string{viewRoleName, adminRoleName},
-			},
-		},
-	}
-	adminRoleBinding := &rbacv1.ClusterRoleBinding{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: adminRoleName,
-		},
-		Subjects: []rbacv1.Subject{
-			{
-				Kind:     "User",
-				APIGroup: "rbac.authorization.k8s.io",
-				Name:     user.GetName(),
-			},
-		},
-		RoleRef: rbacv1.RoleRef{
-			Kind:     "ClusterRole",
-			APIGroup: "rbac.authorization.k8s.io",
-			Name:     adminRoleName,
-		},
-	}
+	ar, arb, vr, vrb := billingrbac.ClusterRoles(objName, billingrbac.ClusterRolesParams{
+		AllowSubjectsToViewRole: true,
+
+		AdminUsers: []string{user.GetName()},
+	})
+	toCreate := []client.Object{ar, arb, vr, vrb}
 
 	rollback := func() error {
 		if deleter, canDelete := c.Storage.(rest.GracefulDeleter); canDelete {
@@ -116,7 +60,6 @@ func (c *createRBACWrapper) Create(ctx context.Context, obj runtime.Object, crea
 		return nil
 	}
 
-	toCreate := []client.Object{viewRole, viewRoleBinding, adminRole, adminRoleBinding}
 	created := make([]client.Object, 0, len(toCreate))
 	var createErr error
 	for _, obj := range toCreate {

config/rbac/controller/role.yaml

@@ -97,6 +97,13 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - billing.appuio.io
+  - rbac.appuio.io
+  resources:
+  - billingentities
+  verbs:
+  - '*'
 - apiGroups:
   - organization.appuio.io
   resources:
@@ -179,6 +186,8 @@ rules:
   - clusterroles
   verbs:
   - create
+  - delete
+  - edit
   - get
   - list
   - patch

controller.go

@@ -43,7 +43,7 @@ A user of APPUiO Cloud has invited you to join them. Follow https://portal.dev/i
 
 APPUiO Cloud is a shared Kubernetes offering based on OpenShift provided by https://vshn.ch.
 
-Unsure what to do next? Accept this invitation using the link above, login to one of the zones listed at https://portal.appuio.cloud/zones, deploy your application. A getting started guide on how to do so, is available at https://docs.appuio.cloud/user/tutorials/getting-started.html. To learn more about APPUiO Cloud in general, please visit https://appuio.cloud. 
+Unsure what to do next? Accept this invitation using the link above, login to one of the zones listed at https://portal.appuio.cloud/zones, deploy your application. A getting started guide on how to do so, is available at https://docs.appuio.cloud/user/tutorials/getting-started.html. To learn more about APPUiO Cloud in general, please visit https://appuio.cloud.
 
 If you have any problems or questions, please email us at [email protected].
 
@@ -102,7 +102,8 @@ func ControllerCommand() *cobra.Command {
 	billingEntityEmailBodyTemplate := cmd.Flags().String("billingentity-email-body-template", defaultBillingEntityEmailTemplate, "Body for billing entity modification update mails")
 	billingEntityEmailRecipient := cmd.Flags().String("billingentity-email-recipient", "", "Recipient e-mail address for billing entity modification update mails")
 	billingEntityEmailSubject := cmd.Flags().String("billingentity-email-subject", "An APPUiO Billing Entity has been updated", "Subject for billing entity modification update mails")
-	billingEntityCronInterval := cmd.Flags().String("billingentity-email-cron-interval", "@every 1h", "Cron interval for how frequently billing entity update e-mails are sent")
+	billingEntityEmailCronInterval := cmd.Flags().String("billingentity-email-cron-interval", "@every 1h", "Cron interval for how frequently billing entity update e-mails are sent")
+	billingEntityRBACCronInterval := cmd.Flags().String("billingentity-rbac-cron-interval", "@every 3m", "Cron interval for how frequently billing entity rbac is reconciled")
 
 	saleOrderCompatMode := cmd.Flags().Bool("sale-order-compatibility-mode", false, "Whether to enable compatibility mode for Sales Orders. If enabled, odoo8 billing entity IDs are used to create sales orders in odoo16.")
 	saleOrderStorage := cmd.Flags().String("sale-order-storage", "none", "Type of sale order storage to use. Valid values are `none` and `odoo16`")
@@ -212,9 +213,10 @@ func ControllerCommand() *cobra.Command {
 			os.Exit(1)
 		}
 
-		cron, err := setupCron(
+		setupLog.Info("setting up email cron")
+		emailCron, err := setupEmailCron(
 			ctx,
-			*billingEntityCronInterval,
+			*billingEntityEmailCronInterval,
 			mgr,
 			beMailSender,
 			*billingEntityEmailRecipient,
@@ -223,15 +225,30 @@ func ControllerCommand() *cobra.Command {
 			setupLog.Error(err, "unable to setup email cron")
 			os.Exit(1)
 		}
-		cron.Start()
+		emailCron.Start()
+
+		setupLog.Info("setting up billing entity rbac cron")
+		beRBACCron, err := setupBillingEntityRBACCron(
+			ctx,
+			*billingEntityRBACCronInterval,
+			mgr,
+		)
+		if err != nil {
+			setupLog.Error(err, "unable to setup rbac cron")
+			os.Exit(1)
+		}
+		beRBACCron.Start()
 
 		setupLog.Info("starting manager")
 		if err := mgr.Start(ctx); err != nil {
 			setupLog.Error(err, "problem running manager")
 			os.Exit(1)
 		}
 		setupLog.Info("Stopping...")
-		<-cron.Stop().Done()
+		ecs := emailCron.Stop()
+		becs := beRBACCron.Stop()
+		<-ecs.Done()
+		<-becs.Done()
 	}
 
 	return cmd
@@ -390,7 +407,7 @@ func setupManager(
 	return mgr, err
 }
 
-func setupCron(
+func setupEmailCron(
 	ctx context.Context,
 	crontab string,
 	mgr ctrl.Manager,
@@ -406,7 +423,7 @@ func setupCron(
 	)
 
 	metrics.Registry.MustRegister(bemail.GetMetrics())
-	syncLog := ctrl.Log.WithName("cron")
+	syncLog := ctrl.Log.WithName("email_cron")
 
 	c := cron.New()
 	_, err := c.AddFunc(crontab, func() {
@@ -423,3 +440,21 @@ func setupCron(
 	}
 	return c, nil
 }
+
+func setupBillingEntityRBACCron(
+	ctx context.Context,
+	crontab string,
+	mgr ctrl.Manager,
+) (*cron.Cron, error) {
+
+	rbac := &controllers.BillingEntityRBACCronJob{Client: mgr.GetClient()}
+	syncLog := ctrl.Log.WithName("be_rbac_cron")
+	c := cron.New()
+	_, err := c.AddFunc(crontab, func() {
+		err := rbac.Run(ctx)
+		if err != nil {
+			syncLog.Error(err, "Error during periodic job")
+		}
+	})
+	return c, err
+}

controllers/billingentity_rbac_cronjob.go

@@ -0,0 +1,66 @@
+package controllers
+
+import (
+	"context"
+	"fmt"
+
+	"go.uber.org/multierr"
+	"k8s.io/client-go/tools/record"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+
+	billingv1 "github.com/appuio/control-api/apis/billing/v1"
+	"github.com/appuio/control-api/pkg/billingrbac"
+)
+
+// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=get;list;watch;create;delete;patch;update;edit
+// +kubebuilder:rbac:groups=rbac.appuio.io;billing.appuio.io,resources=billingentities,verbs=*
+
+// BillingEntityRBACCronJob periodically checks billing entities and sends notification emails if appropriate
+type BillingEntityRBACCronJob struct {
+	client.Client
+}
+
+func NewBillingEntityRBACCronJob(client client.Client, eventRecorder record.EventRecorder) BillingEntityRBACCronJob {
+	return BillingEntityRBACCronJob{
+		Client: client,
+	}
+}
+
+// Run lists all BillingEntity resources and sends notification emails if needed.
+func (r *BillingEntityRBACCronJob) Run(ctx context.Context) error {
+	log := log.FromContext(ctx).WithName("BillingEntityRBACCronJob")
+	log.Info("Reconciling BillingEntity RBAC")
+
+	list := &billingv1.BillingEntityList{}
+	err := r.Client.List(ctx, list)
+	if err != nil {
+		return fmt.Errorf("could not list billing entities: %w", err)
+	}
+
+	var errors []error
+	for _, be := range list.Items {
+		log := log.WithValues("billingentity", be.Name)
+		err := r.reconcile(ctx, &be)
+		if err != nil {
+			log.Error(err, "could not reconcile billing entity")
+			errors = append(errors, err)
+		}
+	}
+	return multierr.Combine(errors...)
+}
+
+func (r *BillingEntityRBACCronJob) reconcile(ctx context.Context, be *billingv1.BillingEntity) error {
+	ar, arBinding, vr, vrBinding := billingrbac.ClusterRoles(be.Name, billingrbac.ClusterRolesParams{
+		AllowSubjectsToViewRole: true,
+	})
+
+	arErr := r.Client.Patch(ctx, ar, client.Apply, client.ForceOwnership, client.FieldOwner("control-api"))
+	arBinding.Subjects = nil // we don't want to manage the subjects
+	arBindingErr := r.Client.Patch(ctx, arBinding, client.Apply, client.ForceOwnership, client.FieldOwner("control-api"))
+	vrErr := r.Client.Patch(ctx, vr, client.Apply, client.ForceOwnership, client.FieldOwner("control-api"))
+	vrBinding.Subjects = nil // we don't want to manage the subjects
+	vrBindingErr := r.Client.Patch(ctx, vrBinding, client.Apply, client.ForceOwnership, client.FieldOwner("control-api"))
+
+	return multierr.Combine(arErr, arBindingErr, vrErr, vrBindingErr)
+}

controllers/billingentity_rbac_cronjob_test.go

@@ -0,0 +1,53 @@
+package controllers_test
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	. "github.com/appuio/control-api/controllers"
+)
+
+func Test_BillingEntityRBACCronJob_Run(t *testing.T) {
+	ctx := context.Background()
+
+	be := baseBillingEntity()
+	c := prepareTest(t, be)
+
+	subject := &BillingEntityRBACCronJob{
+		Client: c,
+	}
+
+	require.NoError(t, subject.Run(ctx))
+
+	var adminRole rbacv1.ClusterRole
+	var adminRoleBinding rbacv1.ClusterRoleBinding
+	adminRoleName := fmt.Sprintf("billingentities-%s-admin", be.Name)
+	require.NoError(t, c.Get(ctx, types.NamespacedName{Name: adminRoleName}, &adminRole), "admin role should be created")
+	require.NoError(t, c.Get(ctx, types.NamespacedName{Name: adminRoleName}, &adminRoleBinding), "admin role binding should be created")
+
+	var viewerRole rbacv1.ClusterRole
+	var viewerRoleBinding rbacv1.ClusterRoleBinding
+	viewerRoleName := fmt.Sprintf("billingentities-%s-viewer", be.Name)
+	require.NoError(t, c.Get(ctx, types.NamespacedName{Name: viewerRoleName}, &viewerRole), "viewer role should be created")
+	require.NoError(t, c.Get(ctx, types.NamespacedName{Name: viewerRoleName}, &viewerRoleBinding), "viewer role binding should be created")
+
+	testSubjects := []rbacv1.Subject{
+		{
+			APIGroup: "rbac.authorization.k8s.io",
+			Kind:     "User",
+			Name:     "testuser",
+		},
+	}
+	viewerRoleBinding.Subjects = testSubjects
+	require.NoError(t, c.Update(ctx, &viewerRoleBinding))
+
+	require.NoError(t, subject.Run(ctx))
+
+	require.NoError(t, c.Get(ctx, types.NamespacedName{Name: viewerRoleName}, &viewerRoleBinding))
+	require.Equal(t, testSubjects, viewerRoleBinding.Subjects, "role bindings should not be changed")
+}