consoles: Make DirectoryRoleBinding optional

Currently, the workloads controller has a hard dependency on
`DirectoryRoleBinding`s, as provided by the RBAC controller, in order to
create consoles. If you try to do this, without having the RBAC
controller installed, then the reconcile loop gets stuck, as the
rolebinding to access the console can't be provisioned.

This is a bit presumptuous; it should be possible to run consoles
without DRBs, e.g. if you just want to reference plain `User` kinds in
`additionalAttachSubjects`.

This change adds a flag, which makes the usage of `DirectoryRoleBinding`
optional. The flag defaults to true, meaning that this isn't a breaking
change.

We intended to use this in conjunction with [Google Groups for RBAC][0]
as an alternative.

[0]: https://cloud.google.com/kubernetes-engine/docs/how-to/google-groups-rbac

cmd/workloads-manager/main.go

@@ -24,14 +24,15 @@ import (
 var (
 	scheme = runtime.NewScheme()
 
-	app                    = kingpin.New("workloads-manager", "Manages workloads.crd.gocardless.com resources").Version(cmd.VersionStanza())
-	contextName            = app.Flag("context-name", "Distinct name for the context this controller runs within. Usually the user-facing name of the kubernetes context for the cluster").Envar("CONTEXT_NAME").String()
-	pubsubProjectId        = app.Flag("pubsub-project-id", "ID for the project containing the Pub/Sub topic for console event publishing").Envar("PUBSUB_PROJECT_ID").String()
-	pubsubTopicId          = app.Flag("pubsub-topic-id", "ID of the topic to publish lifecycle event messages").Envar("PUBSUB_TOPIC_ID").String()
-	enableSessionRecording = app.Flag("session-recording", "Enable session recording features").Envar("ENABLE_SESSION_RECORDING").Default("false").Bool()
-	sessionSidecarImage    = app.Flag("session-sidecar-image", "Container image to use for the session recording sidecar container").Envar("SESSION_SIDECAR_IMAGE").Default("").String()
-	sessionPubsubProjectId = app.Flag("session-pubsub-project-id", "ID for the project containing the Pub/Sub topic for session recording").Envar("SESSION_PUBSUB_PROJECT_ID").Default("").String()
-	sessionPubsubTopicId   = app.Flag("session-pubsub-topic-id", "ID of the topic to publish session recording data to").Envar("SESSION_PUBSUB_TOPIC_ID").Default("").String()
+	app                        = kingpin.New("workloads-manager", "Manages workloads.crd.gocardless.com resources").Version(cmd.VersionStanza())
+	contextName                = app.Flag("context-name", "Distinct name for the context this controller runs within. Usually the user-facing name of the kubernetes context for the cluster").Envar("CONTEXT_NAME").String()
+	pubsubProjectId            = app.Flag("pubsub-project-id", "ID for the project containing the Pub/Sub topic for console event publishing").Envar("PUBSUB_PROJECT_ID").String()
+	pubsubTopicId              = app.Flag("pubsub-topic-id", "ID of the topic to publish lifecycle event messages").Envar("PUBSUB_TOPIC_ID").String()
+	enableDirectoryRoleBinding = app.Flag("directory-role-binding", "Use DirectoryRoleBinding for provisioning RBAC against console objects").Envar("ENABLE_DIRECTORY_ROLE_BINDING").Default("true").Bool()
+	enableSessionRecording     = app.Flag("session-recording", "Enable session recording features").Envar("ENABLE_SESSION_RECORDING").Default("false").Bool()
+	sessionSidecarImage        = app.Flag("session-sidecar-image", "Container image to use for the session recording sidecar container").Envar("SESSION_SIDECAR_IMAGE").Default("").String()
+	sessionPubsubProjectId     = app.Flag("session-pubsub-project-id", "ID for the project containing the Pub/Sub topic for session recording").Envar("SESSION_PUBSUB_PROJECT_ID").Default("").String()
+	sessionPubsubTopicId       = app.Flag("session-pubsub-topic-id", "ID of the topic to publish session recording data to").Envar("SESSION_PUBSUB_TOPIC_ID").Default("").String()
 
 	commonOpts = cmd.NewCommonOptions(app).WithMetrics(app)
 )
@@ -93,15 +94,16 @@ func main() {
 
 	// controller
 	if err = (&consolecontroller.ConsoleReconciler{
-		Client:                 mgr.GetClient(),
-		LifecycleRecorder:      lifecycleRecorder,
-		ConsoleIdBuilder:       idBuilder,
-		Log:                    ctrl.Log.WithName("controllers").WithName("console"),
-		Scheme:                 mgr.GetScheme(),
-		EnableSessionRecording: *enableSessionRecording,
-		SessionSidecarImage:    *sessionSidecarImage,
-		SessionPubsubProjectId: *sessionPubsubProjectId,
-		SessionPubsubTopicId:   *sessionPubsubTopicId,
+		Client:                     mgr.GetClient(),
+		LifecycleRecorder:          lifecycleRecorder,
+		ConsoleIdBuilder:           idBuilder,
+		Log:                        ctrl.Log.WithName("controllers").WithName("console"),
+		Scheme:                     mgr.GetScheme(),
+		EnableDirectoryRoleBinding: *enableDirectoryRoleBinding,
+		EnableSessionRecording:     *enableSessionRecording,
+		SessionSidecarImage:        *sessionSidecarImage,
+		SessionPubsubProjectId:     *sessionPubsubProjectId,
+		SessionPubsubTopicId:       *sessionPubsubTopicId,
 	}).SetupWithManager(ctx, mgr); err != nil {
 		app.Fatalf("failed to create controller: %v", err)
 	}

config/base/managers/workloads.yaml

@@ -30,6 +30,7 @@ rules:
       - rbac.authorization.k8s.io
     resources:
       - roles
+      - rolebindings
     verbs:
       - "*"
   - apiGroups:

controllers/workloads/console/controller.go

@@ -62,6 +62,7 @@ const (
 	ConsoleAuthorisation = "consoleauthorisation"
 	ConsoleTemplate      = "consoletemplate"
 	Role                 = "role"
+	RoleBinding          = "rolebinding"
 	DirectoryRoleBinding = "directoryrolebinding"
 
 	DefaultTTLBeforeRunning = 1 * time.Hour
@@ -88,6 +89,9 @@ type ConsoleReconciler struct {
 	ConsoleIdBuilder  workloadsv1alpha1.ConsoleIdBuilder
 	Log               logr.Logger
 	Scheme            *runtime.Scheme
+	// Use DRBs for RBAC on console objects
+	EnableDirectoryRoleBinding bool
+
 	// Enable injection of console session recording using tlog
 	EnableSessionRecording bool
 	// The image reference for the sidecar to inject to stream session
@@ -140,10 +144,10 @@ func (r *ConsoleReconciler) createOrUpdateUserRbac(logger logr.Logger, ctx conte
 	// Create or update the user role
 	role := buildUserRole(req.NamespacedName, csl.Status.PodName)
 	if err := r.createOrUpdate(ctx, logger, csl, role, Role, recutil.RoleDiff); err != nil {
-		return err
+		return errors.Wrap(err, "failed to create role for user")
 	}
 
-	// Create or update the directory role binding
+	// Create or update the role binding
 	subjects := append(
 		tpl.Spec.AdditionalAttachSubjects,
 		rbacv1.Subject{Kind: "User", Name: csl.Spec.User},
@@ -153,9 +157,16 @@ func (r *ConsoleReconciler) createOrUpdateUserRbac(logger logr.Logger, ctx conte
 		subjects = append(subjects, authorisation.Spec.Authorisations...)
 	}
 
-	drb := buildUserDirectoryRoleBinding(req.NamespacedName, role, subjects)
-	if err := r.createOrUpdate(ctx, logger, csl, drb, DirectoryRoleBinding, recutil.DirectoryRoleBindingDiff); err != nil {
-		return err
+	if r.EnableDirectoryRoleBinding {
+		drb := buildUserDirectoryRoleBinding(req.NamespacedName, role, subjects)
+		if err := r.createOrUpdate(ctx, logger, csl, drb, DirectoryRoleBinding, recutil.DirectoryRoleBindingDiff); err != nil {
+			return errors.Wrap(err, "failed to create directory rolebinding for user")
+		}
+	} else {
+		rb := buildUserRoleBinding(req.NamespacedName, role, subjects)
+		if err := r.createOrUpdate(ctx, logger, csl, rb, RoleBinding, recutil.RoleBindingDiff); err != nil {
+			return errors.Wrap(err, "failed to create rolebinding for user")
+		}
 	}
 
 	return nil
@@ -168,7 +179,7 @@ func (r *ConsoleReconciler) createOrUpdateServiceRbac(logger logr.Logger, ctx co
 	// result is to allow the console pod to read information about itself, which is required by our wrapper so that it can
 	// exit the sidecar when the main console container exits.
 
-	// We already create roles and directory rolebindings with the same name as
+	// We already create roles and rolebindings with the same name as
 	// the console to provide permissions on the job/pod. Therefore for the name
 	// of these objects, suffix the console name with '-svc'.
 	rbacName := types.NamespacedName{
@@ -179,13 +190,20 @@ func (r *ConsoleReconciler) createOrUpdateServiceRbac(logger logr.Logger, ctx co
 	// Create or update the service role
 	role := buildServiceRole(rbacName, csl.Status.PodName)
 	if err := r.createOrUpdate(ctx, logger, csl, role, Role, recutil.RoleDiff); err != nil {
-		return err
+		return errors.Wrap(err, "failed to create role for service")
 	}
 
 	// Create or update the service rolebinding
-	drb := buildServiceRoleBinding(rbacName, role, tpl.Spec.Template.Spec.ServiceAccountName)
-	if err := r.createOrUpdate(ctx, logger, csl, drb, DirectoryRoleBinding, recutil.DirectoryRoleBindingDiff); err != nil {
-		return err
+	if r.EnableDirectoryRoleBinding {
+		drb := buildServiceDirectoryRoleBinding(rbacName, role, tpl.Spec.Template.Spec.ServiceAccountName)
+		if err := r.createOrUpdate(ctx, logger, csl, drb, DirectoryRoleBinding, recutil.DirectoryRoleBindingDiff); err != nil {
+			return errors.Wrap(err, "failed to create directory rolebinding for service")
+		}
+	} else {
+		rb := buildServiceRoleBinding(rbacName, role, tpl.Spec.Template.Spec.ServiceAccountName)
+		if err := r.createOrUpdate(ctx, logger, csl, rb, RoleBinding, recutil.RoleBindingDiff); err != nil {
+			return errors.Wrap(err, "failed to create rolebinding for service")
+		}
 	}
 
 	return nil
@@ -969,7 +987,7 @@ func buildServiceRole(name types.NamespacedName, podName string) *rbacv1.Role {
 	}
 }
 
-func buildServiceRoleBinding(name types.NamespacedName, role *rbacv1.Role, serviceAccountName string) *rbacv1alpha1.DirectoryRoleBinding {
+func buildServiceDirectoryRoleBinding(name types.NamespacedName, role *rbacv1.Role, serviceAccountName string) *rbacv1alpha1.DirectoryRoleBinding {
 	// There are cases where the console template lacks a service account
 	// In these cases the pods and jobs will run under the "default" account
 	// for the namespace
@@ -997,6 +1015,32 @@ func buildServiceRoleBinding(name types.NamespacedName, role *rbacv1.Role, servi
 	}
 }
 
+func buildServiceRoleBinding(name types.NamespacedName, role *rbacv1.Role, serviceAccountName string) *rbacv1.RoleBinding {
+	// There are cases where the console template lacks a service account
+	// In these cases the pods and jobs will run under the "default" account
+	// for the namespace
+	if len(serviceAccountName) == 0 {
+		serviceAccountName = "default"
+	}
+	return &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name.Name,
+			Namespace: name.Namespace,
+		},
+		Subjects: []rbacv1.Subject{
+			{
+				Kind: "ServiceAccount",
+				Name: serviceAccountName,
+			},
+		},
+		RoleRef: rbacv1.RoleRef{
+			APIGroup: rbacv1.GroupName,
+			Kind:     "Role",
+			Name:     name.Name,
+		},
+	}
+}
+
 func buildUserRole(name types.NamespacedName, podName string) *rbacv1.Role {
 	return &rbacv1.Role{
 		ObjectMeta: metav1.ObjectMeta{
@@ -1043,6 +1087,21 @@ func buildUserDirectoryRoleBinding(name types.NamespacedName, role *rbacv1.Role,
 	}
 }
 
+func buildUserRoleBinding(name types.NamespacedName, role *rbacv1.Role, subjects []rbacv1.Subject) *rbacv1.RoleBinding {
+	return &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name.Name,
+			Namespace: name.Namespace,
+		},
+		Subjects: subjects,
+		RoleRef: rbacv1.RoleRef{
+			APIGroup: rbacv1.GroupName,
+			Kind:     "Role",
+			Name:     name.Name,
+		},
+	}
+}
+
 func (r *ConsoleReconciler) createAuthorisationObjects(ctx context.Context, logger logr.Logger, csl *workloadsv1alpha1.Console, name types.NamespacedName, subjects []rbacv1.Subject) error {
 	authorisation := &workloadsv1alpha1.ConsoleAuthorisation{
 		ObjectMeta: metav1.ObjectMeta{
@@ -1061,7 +1120,7 @@ func (r *ConsoleReconciler) createAuthorisationObjects(ctx context.Context, logg
 		return errors.Wrap(err, "failed to create consoleauthorisation")
 	}
 
-	// We already create roles and directory rolebindings with the same name as
+	// We already create roles and rolebindings with the same name as
 	// the console to provide permissions on the job/pod. Therefore for the name
 	// of these objects, suffix the console name with '-authorisation'.
 	rbacName := types.NamespacedName{
@@ -1089,10 +1148,17 @@ func (r *ConsoleReconciler) createAuthorisationObjects(ctx context.Context, logg
 		return errors.Wrap(err, "failed to create role for consoleauthorisation")
 	}
 
-	// Create or update the directory role binding
-	drb := buildUserDirectoryRoleBinding(rbacName, role, subjects)
-	if err := r.createOrUpdate(ctx, logger, csl, drb, DirectoryRoleBinding, recutil.DirectoryRoleBindingDiff); err != nil {
-		return errors.Wrap(err, "failed to create directory rolebinding for consoleauthorisation")
+	// Create or update the role binding
+	if r.EnableDirectoryRoleBinding {
+		drb := buildUserDirectoryRoleBinding(rbacName, role, subjects)
+		if err := r.createOrUpdate(ctx, logger, csl, drb, DirectoryRoleBinding, recutil.DirectoryRoleBindingDiff); err != nil {
+			return errors.Wrap(err, "failed to create directory rolebinding for consoleauthorisation")
+		}
+	} else {
+		rb := buildUserRoleBinding(rbacName, role, subjects)
+		if err := r.createOrUpdate(ctx, logger, csl, rb, RoleBinding, recutil.RoleBindingDiff); err != nil {
+			return errors.Wrap(err, "failed to create rolebinding for consoleauthorisation")
+		}
 	}
 
 	return nil

controllers/workloads/console/integration/suite_test.go

@@ -99,11 +99,12 @@ var _ = BeforeSuite(func() {
 	})
 
 	err = (&consolecontroller.ConsoleReconciler{
-		Client:            mgr.GetClient(),
-		LifecycleRecorder: lifecycleRecorder,
-		Log:               ctrl.Log.WithName("controllers").WithName("console"),
-		Scheme:            mgr.GetScheme(),
-		ConsoleIdBuilder:  workloadsv1alpha1.NewConsoleIdBuilder("test"),
+		Client:                     mgr.GetClient(),
+		LifecycleRecorder:          lifecycleRecorder,
+		Log:                        ctrl.Log.WithName("controllers").WithName("console"),
+		Scheme:                     mgr.GetScheme(),
+		ConsoleIdBuilder:           workloadsv1alpha1.NewConsoleIdBuilder("test"),
+		EnableDirectoryRoleBinding: true,
 	}).SetupWithManager(context.TODO(), mgr)
 	Expect(err).ToNot(HaveOccurred())
 

pkg/recutil/reconcile.go

@@ -212,3 +212,23 @@ func DirectoryRoleBindingDiff(expectedObj runtime.Object, existingObj runtime.Ob
 
 	return operation
 }
+
+// RoleBindingDiff is a DiffFunc for RoleBindings
+func RoleBindingDiff(expectedObj runtime.Object, existingObj runtime.Object) Outcome {
+	expected := expectedObj.(*rbacv1.RoleBinding)
+	existing := existingObj.(*rbacv1.RoleBinding)
+
+	operation := None
+
+	if !reflect.DeepEqual(expected.Subjects, existing.Subjects) {
+		existing.Subjects = expected.Subjects
+		operation = Update
+	}
+
+	if !reflect.DeepEqual(expected.RoleRef, existing.RoleRef) {
+		existing.RoleRef = expected.RoleRef
+		operation = Update
+	}
+
+	return operation
+}