Merge pull request #1 from incident-io/ben/no-drb

incident-io · Aug 29, 2023 · 17097a4 · 17097a4
2 parents 82d6c21 + 87666ef
commit 17097a4
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 38 deletions.
diff --git a/cmd/workloads-manager/main.go b/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)
 	}

diff --git a/config/base/managers/workloads.yaml b/config/base/managers/workloads.yaml
@@ -30,6 +30,7 @@ rules:
       - rbac.authorization.k8s.io
     resources:
       - roles
+      - rolebindings
     verbs:
       - "*"
   - apiGroups:

diff --git a/controllers/workloads/console/controller.go b/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

diff --git a/controllers/workloads/console/integration/suite_test.go b/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())
 

diff --git a/pkg/recutil/reconcile.go b/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
+}