Repository: crossplaneio/crossplane (go)

Issue:
Stack Manager should watch namespaces for persona enabling label
### What problem are you facing?

Part of #1036

The [Stacks Security and Security Isolation One-Pager](https://github.com/crossplaneio/crossplane/blob/master/design/one-pager-stacks-security-isolation.md) was updated and expanded requiring new Stack behaviors.

https://github.com/crossplaneio/crossplane/blob/5b48149d758ceeb73c0b5f2e1d83d0af6e7a1776/design/one-pager-stacks-security-isolation.md#clusterroles-for-a-given-app-namespace

> Each namespace provides a unit of isolation with namespaced stacks, and thus a unique set of resources, based on the stacks that have been installed into that namespace. If someone were allowed to install stacks, it can't be assumed that they are cluster-admin, so the stack-manager will install and manage a set of roles that will aggregate the roles for the installed stacks into namespace specific roles for self-service. These roles should be bound to subjects to grant access to a particular namespace for end-user integration. For any namespace that is annotated with `rbac.crossplane.io/managed-roles: true` the stack-manager will ensure that ClusterRoles in the following format are created for the annotated namespace.
> ```
> crossplane:ns:{ns-name}:admin
> crossplane:ns:{ns-name}:edit
> crossplane:ns:{ns-name}:view
> ```

### How could Crossplane help solve your problem?

"StackManager should create workspace specific clusterroles for any properly labelled namespace."

Building on #1113, Stack Manager should begin to watch namespaces for the `managed-roles` label.

The stack-manager currently creates `crossplane:ns:{name}:{role}` clusterroles upon installation of a Stack.  This was a first step towards introducing managed roles for namespaces.

* Stop creating `crossplane:ns:{name}:{role}` roles on stack install
* Watch namespaces with the known label
  * When the label is added or removed, create or delete the `crossplane:ns:{name}:{role}` clusterroles
* Inventory `crossplane:ns:{name}:{role}` clusterroles and role-managed namespaces. Sync these periodically.


The clusterroles currently use the namespace records as owner references.  This ensures that the clusterroles will be deleted when the related namespaces are deleted.



Pull request description:
stacks: only labeled namespaces enroll in persona managment
### Description of your changes

Persona ClusterRoles are the ClusterRoles Crossplane creates when a Stack is installed in a namespace.  Given any namespace-scoped stack installed in namespace `foo`, Crossplane's Stack Manager will create the following clusterroles:

* `crossplane:ns:foo:view`
* `crossplane:ns:foo:edit`
* `crossplane:ns:foo:admin`

These persona roles allow for admins to administer `RoleBindings` granting their users the ability to work with these Stacks in various ways, including the ability to install more stacks within that namespace.  The problem with creating these roles at Stack install time is that the initial stack that led to the creation of these roles must be installed by the admin, not a user with `crossplane:ns:foo:admin`, since that clusterrole does not yet exist.

This PR moves the ClusterRole creation away from Stack creation time.  With this PR, an admin can now label a namespace `rbac.crossplane.io/managed-roles:"true"` to have these clusterroles created.  A user with a binding granting the crossplane-namespace-admin persona can then install the first stack in that namespace.

* Removes Persona ClusterRole (`crossplane:ns:foo:{view|edit|admin}`) creation from namespaced Stacks reconciliation
  * removes Stacks controller tests that looked for these clusterroles to be created
* Watches namespaces
  * determines if namespace is `rbac.crossplane.io/managed-roles:"true"` labeled
  * creates Persona ClusterRoles with the namespace as an OwnerReference (for GC cleanup)
  * deletes Persona ClusterRoles if the label is not present (or the resource has deletion timestamps)
  * adds tests to verify these changes

I determined that this PR does not need to be host-aware. 
 [RBAC rules are created with the tennant](https://github.com/crossplaneio/crossplane/pull/1138#discussion_r364281011) (not host) client that `h.kube.client` refers to. The code for the new persona controller was taken from the Stack controller where the same client was used.

Fixes #1124



### How has this code been tested?
<!--
Before reviewers can be confident in the correctness of a pull request,
it needs to tested and shown to be correct. In this section, briefly
describe the testing that has already been done or which is planned.
-->
1. `kubectl scale -n crossplane-system deployment --replicas=0 crossplane-stack-manager`
1. Run this branch `go run cmd/main.go stack manage --debug`
1. `kubectl create namespace foo`
  a. Observe there are no clusterroles for foo `kubectl get clusterrole | grep foo`
1. `kubectl label namespace foo rbac.crossplane.io/managed-roles=true`
  a. Observe there are now clusterroles for foo `kubectl get clusterrole | grep foo`
1. Delete label `kubectl label namespace foo rbac.crossplane.io/managed-roles-`
  a. Observe there are no clusterroles for foo `kubectl get clusterrole | grep foo`
1. Repeat the create+delete process by setting a non-true value for the label
1. Repeat the create+delete process by deleting the namespace
1. Break the `go run` (ctrl+c)
1. `kubectl scale -n crossplane-system deployment --replicas=1 crossplane-stack-manager`

### Checklist
<!--
Please run through the below readiness checklist. The first two items are
relevant to every Crossplane pull request.
-->
I have:
- [x] Run `make reviewable` to ensure this PR is ready for review.
- [x] Ensured this PR contains a neat, self documenting set of commits.
- [x] Updated any relevant [documentation] and [examples].
- [x] Reported all new error conditions into the log or as an event, as
  appropriate.

For more about what we believe makes a pull request complete, see our
[definition of done].

[documentation]: https://github.com/crossplaneio/crossplane/tree/master/docs
[examples]: https://github.com/crossplaneio/crossplane/tree/master/cluster/examples
[definition of done]: https://github.com/crossplaneio/crossplane/tree/master/design/one-pager-definition-of-done.md


Patch:
diff --git a/design/one-pager-stacks-security-isolation.md b/design/one-pager-stacks-security-isolation.md
index 4c6b8e70f..b602d42fb 100644
--- a/design/one-pager-stacks-security-isolation.md
+++ b/design/one-pager-stacks-security-isolation.md
@@ -375,8 +375,8 @@ Each namespace provides a unit of isolation with namespaced stacks, and thus a u
  for the installed stacks into namespace specific roles for self-service. These `ClusterRoles` should be bound to subjects
  using a `RoleBinding` (not `ClusterRoleBinding`) to grant access to a particular namespace for end-user integration.
 
-This design calls for the stack-manager to act on any namespace that is annotated with `rbac.crossplane.io/managed-roles: true`,
-ensuring that ClusterRoles in the following format are created for the annotated namespace.
+This design calls for the stack-manager to act on any namespace that is labeled with `rbac.crossplane.io/managed-roles: "true"`,
+ensuring that ClusterRoles in the following format are created for the labeled namespace.
 
  * `crossplane:ns:{ns-name}:admin`
  * `crossplane:ns:{ns-name}:edit`
@@ -384,11 +384,11 @@ ensuring that ClusterRoles in the following format are created for the annotated
 
 For discoverability of the roles for a given namespace we will add the following labels:
  * `crossplane.io/scope: "namespace"`
- * `namespace.crossplane.io/{namespace-name}: true`
+ * `namespace.crossplane.io/{namespace-name}: "true"`
 
 Match Roles:
  * `rbac.crossplane.io/aggregate-to-namespace-{role}: "true"`
- * `namespace.crossplane.io/{namespace-name}: true // to match specific namespace.`
+ * `namespace.crossplane.io/{namespace-name}: "true" // to match specific namespace.`
 
 |ClusterRole Name | Kubernetes Counterpart | Permissions |
 |:---------------------|:----------|:----------|
@@ -421,7 +421,7 @@ Labels for these roles will be for aggregation purposes:
  * `rbac.crossplane.io/aggregate-to-{install-scope}-{role}: "true"`
  * `namespace.crossplane.io/{namespace-name}: "true"` // Only included on namespaced stacks install
 
-The format for the namespace label `namespace.crossplane.io/{namespace-name}: true`, which may seem unintuitive
+The format for the namespace label `namespace.crossplane.io/{namespace-name}: "true"`, which may seem unintuitive
  is required to support ClusterRole aggregation. Since we will have some ClusterRoles which aggregate to more than
  multiple namespaces and label keys must be unique, we need to encode the key/value in the key.
 
@@ -502,7 +502,7 @@ kind: ClusterRole
 metadata:
   name: crossplane:ns:foo:view
   labels:
-    crossplane.io/scope: "namespace" # For discoverability for tools
+    crossplane.io/scope: "namespace" # For discoverability by tools
     namespace.crossplane.io/foo: "true"
 aggregationRule:
   clusterRoleSelectors:
@@ -562,8 +562,8 @@ kind: Namespace
 metadata:
   creationTimestamp: 2019-10-16T18:25:10Z
   name: app-team-1
-  annotations:
-    rbac.crossplane.io/managed-roles: true # Stack manager creates and will manage namespace specific ClusterRoles when it see's this.
+  labels:
+    rbac.crossplane.io/managed-roles: "true" # Stack manager creates and will manage namespace specific ClusterRoles when it see's this.
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 # This role binding allows "jane" to admin the Crossplane app-team-1 namespace.
diff --git a/pkg/controller/stacks/persona/persona.go b/pkg/controller/stacks/persona/persona.go
new file mode 100644
index 000000000..9ae004224
--- /dev/null
+++ b/pkg/controller/stacks/persona/persona.go
@@ -0,0 +1,260 @@
+/*
+Copyright 2020 The Crossplane Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package persona
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	"github.com/crossplaneio/crossplane-runtime/pkg/logging"
+	"github.com/crossplaneio/crossplane-runtime/pkg/meta"
+	"github.com/crossplaneio/crossplane/pkg/stacks"
+)
+
+const (
+	managedRolesLabel   = "rbac.crossplane.io/managed-roles"
+	managedRolesEnabled = "true"
+
+	reconcileTimeout = 1 * time.Minute
+
+	loggerName = "stacks/namespace-personas"
+
+	adminPersona = "admin"
+	editPersona  = "edit"
+	viewPersona  = "view"
+
+	errFailedToCreateClusterRole  = "failed to create clusterrole"
+	errFailedToDeleteClusterRoles = "failed to delete clusterroles"
+	errFailedToGetNamespace       = "failed to get namespace"
+
+	logFailedToCreateDuringSync = "failed to create during sync"
+	logFailedToDeleteDuringSync = "failed to delete during sync"
+)
+
+var (
+	personas = []string{adminPersona, editPersona, viewPersona}
+
+	resultRequeue = reconcile.Result{Requeue: true}
+)
+
+// Reconciler reconciles Namespaces
+type Reconciler struct {
+	kube client.Client
+	log  logging.Logger
+	factory
+}
+
+// Setup adds a controller that reconciles Namespaces.
+func Setup(mgr ctrl.Manager, l logging.Logger) error {
+	r := &Reconciler{
+		kube:    mgr.GetClient(),
+		factory: &nsPersonaHandlerFactory{},
+		log:     l.WithValues("controller", loggerName),
+	}
+
+	// TODO(displague) Should we own the ClusterRole and watch the Namespace?
+	// Not doing so means that changes to the ClusterRole won't be addressed by
+	// this controller. OTOH, Permitting such changes keeps this controller
+	// simple and gives users the flexibility to modify the clusterrole
+
+	return ctrl.NewControllerManagedBy(mgr).
+		Named(loggerName).
+		For(&corev1.Namespace{}).
+		Complete(r)
+}
+
+// Reconcile changes on Namespaces that may or may not have Stacks managed RBAC
+// labels.
+//
+// Reconcile gets the Namespace with the requested namespace name with the
+// management enabling label. When the label is found, we create matching
+// clusterroles.  When the label is not found, possibly removed, the
+// namespace-persona clusterroles are deleted.
+//
+// When a namespace is deleted, the clusterroles will be removed through
+// garbage-collection using OwnerReferences.
+func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) {
+	r.log.Debug("Reconciling", "request", req)
+
+	ctx, cancel := context.WithTimeout(context.Background(), reconcileTimeout)
+	defer cancel()
+	ns := &corev1.Namespace{}
+
+	if err := r.kube.Get(ctx, req.NamespacedName, ns); err != nil {
+		if kerrors.IsNotFound(err) {
+			return reconcile.Result{}, nil
+		}
+		r.log.Debug(errFailedToGetNamespace, "request", req, "error", err)
+
+		return reconcile.Result{}, err
+	}
+
+	handler := r.factory.newHandler(r.log, ns, r.kube)
+	return handler.sync(ctx)
+}
+
+type handler interface {
+	sync(context.Context) (reconcile.Result, error)
+	create(context.Context) error
+	delete(context.Context) error
+}
+
+type nsPersonaHandler struct {
+	kube client.Client
+	ns   *corev1.Namespace
+	log  logging.Logger
+}
+
+type factory interface {
+	newHandler(logging.Logger, *corev1.Namespace, client.Client) handler
+}
+
+type nsPersonaHandlerFactory struct{}
+
+func (f *nsPersonaHandlerFactory) newHandler(log logging.Logger, ns *corev1.Namespace, kube client.Client) handler {
+	return &nsPersonaHandler{
+		kube: kube,
+		ns:   ns,
+		log:  log,
+	}
+}
+
+// sync compares the namespace being handled to the desired labels
+// Matches warrant Clusterrole creation
+// Non-matches warrant Clusterrole deletion
+func (h *nsPersonaHandler) sync(ctx context.Context) (reconcile.Result, error) {
+	if nsHasPersonaManagement(h.ns) && !meta.WasDeleted(h.ns) {
+		if err := h.create(ctx); err != nil {
+			h.log.Debug(logFailedToCreateDuringSync, "namespace", h.ns.GetName(), "error", err)
+			return resultRequeue, err
+		}
+	} else {
+		if err := h.delete(ctx); err != nil {
+			h.log.Debug(logFailedToDeleteDuringSync, "namespace", h.ns.GetName(), "error", err)
+			return resultRequeue, err
+		}
+	}
+
+	return reconcile.Result{}, nil
+}
+
+// generateNamespaceClusterRoles generates roles for a given namespace
+// These clusterroles are named crossplane:ns:{nsName}:{persona}
+func generateNamespaceClusterRoles(ns *corev1.Namespace) (roles []*rbacv1.ClusterRole) {
+	nsName := ns.GetName()
+
+	for _, persona := range personas {
+		name := fmt.Sprintf(stacks.NamespaceClusterRoleNameFmt, nsName, persona)
+
+		labels := map[string]string{
+			fmt.Sprintf(stacks.LabelNamespaceFmt, nsName): "true",
+			stacks.LabelScope: stacks.NamespaceScoped,
+		}
+
+		if persona == adminPersona {
+			labels[fmt.Sprintf(stacks.LabelAggregateFmt, "crossplane", persona)] = "true"
+		}
+
+		// By specifying MatchLabels, ClusterRole Aggregation will pass
+		// along the rules from other ClusterRoles with the desired labels.
+		// This is why we don't define any Rules here.
+		role := &rbacv1.ClusterRole{
+			AggregationRule: &rbacv1.AggregationRule{
+				ClusterRoleSelectors: []metav1.LabelSelector{
+					{
+						MatchLabels: map[string]string{
+							fmt.Sprintf(stacks.LabelAggregateFmt, stacks.NamespaceScoped, persona): "true",
+							fmt.Sprintf(stacks.LabelNamespaceFmt, nsName):                          "true",
+						},
+					},
+					{
+						MatchLabels: map[string]string{
+							fmt.Sprintf(stacks.LabelAggregateFmt, "namespace-default", persona): "true",
+						},
+					},
+				},
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   name,
+				Labels: labels,
+			},
+		}
+
+		// Parent labels enable more precise DeleteAllOf calls than matching
+		// on clusterrole name
+		meta.AddLabels(role, stacks.ParentLabels(ns))
+
+		roles = append(roles, role)
+	}
+
+	return roles
+}
+
+func nsHasPersonaManagement(ns *corev1.Namespace) bool {
+	v, ok := ns.GetLabels()[managedRolesLabel]
+	return ok && v == managedRolesEnabled
+}
+
+// create ClusterRoles for namespace personas
+// example: crossplane:ns:{name}:{persona}
+func (h *nsPersonaHandler) create(ctx context.Context) error {
+	roles := generateNamespaceClusterRoles(h.ns)
+
+	for _, role := range roles {
+		// When the namespace is deleted, clusterroles are no longer needed.
+		// Set the owner to the Namespace for garbage collection.
+		role.SetOwnerReferences([]metav1.OwnerReference{
+			meta.AsOwner(meta.ReferenceTo(h.ns, corev1.SchemeGroupVersion.WithKind("Namespace"))),
+		})
+
+		// Creating the clusterroles. Rules in these clusterroles are populated
+		// through aggregation from the stacks installed in the namespaces, we
+		// won't need to update them.
+		//
+		// We are not patching existing clusterroles. This permits user
+		// modification.
+		h.log.Debug("Creating ClusterRole", "name", role.GetName())
+		if err := h.kube.Create(ctx, role); err != nil && !kerrors.IsAlreadyExists(err) {
+			return errors.Wrapf(err, errFailedToCreateClusterRole)
+		}
+	}
+	return nil
+}
+
+// delete ClusterRoles for namespace personas
+func (h *nsPersonaHandler) delete(ctx context.Context) error {
+	// Logging that clusterroles are attempting to be deleted would
+	// either be noisy (logged on unmanaged namespaces) or cost a
+	// lookup for existing clusterroles
+
+	labels := stacks.ParentLabels(h.ns)
+	if err := h.kube.DeleteAllOf(ctx, &rbacv1.ClusterRole{}, client.MatchingLabels(labels)); err != nil {
+		return errors.Wrapf(err, errFailedToDeleteClusterRoles)
+	}
+
+	return nil
+}
diff --git a/pkg/controller/stacks/stack/stack.go b/pkg/controller/stacks/stack/stack.go
index 0b1827ca3..46b873031 100644
--- a/pkg/controller/stacks/stack/stack.go
+++ b/pkg/controller/stacks/stack/stack.go
@@ -31,7 +31,6 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -338,99 +337,6 @@ func (h *stackHandler) createPersonaClusterRoles(ctx context.Context, labels map
 	return nil
 }
 
-// generateNamespaceClusterRoles generates the crossplane:ns:{name}:{persona}
-// roles for a given stack's namespace.
-func generateNamespaceClusterRoles(stack *v1alpha1.Stack) (roles []*rbacv1.ClusterRole) {
-	personas := []string{"admin", "edit", "view"}
-
-	nsName := stack.GetNamespace()
-
-	for _, persona := range personas {
-		name := fmt.Sprintf(stacks.NamespaceClusterRoleNameFmt, nsName, persona)
-
-		labels := map[string]string{
-			fmt.Sprintf(stacks.LabelNamespaceFmt, nsName): "true",
-			stacks.LabelScope: stacks.NamespaceScoped,
-		}
-
-		if persona == "admin" {
-			labels[fmt.Sprintf(stacks.LabelAggregateFmt, "crossplane", persona)] = "true"
-		}
-
-		// By specifying MatchLabels, ClusterRole Aggregation will pass
-		// along the rules from other ClusterRoles with the desired labels.
-		// This is why we don't define any Rules here.
-		role := &rbacv1.ClusterRole{
-			AggregationRule: &rbacv1.AggregationRule{
-				ClusterRoleSelectors: []metav1.LabelSelector{
-					{
-						MatchLabels: map[string]string{
-							fmt.Sprintf(stacks.LabelAggregateFmt, stacks.NamespaceScoped, persona): "true",
-							fmt.Sprintf(stacks.LabelNamespaceFmt, nsName):                          "true",
-						},
-					},
-					{
-						MatchLabels: map[string]string{
-							fmt.Sprintf(stacks.LabelAggregateFmt, "namespace-default", persona): "true",
-						},
-					},
-				},
-			},
-
-			// TODO(displague) set parent labels?
-			ObjectMeta: metav1.ObjectMeta{
-				Name:   name,
-				Labels: labels,
-			},
-		}
-
-		roles = append(roles, role)
-	}
-
-	return roles
-}
-
-// createNamespaceClusterRoles creates the crossplane:ns:{name}:{persona}
-// roles for a given stack's namespace
-func (h *stackHandler) createNamespaceClusterRoles(ctx context.Context) error {
-	if !h.isNamespaced() {
-		return nil
-	}
-
-	// Get the Namepsace because we need the UID for OwnerReference
-	ns := &corev1.Namespace{}
-	nsName := h.ext.GetNamespace()
-
-	if err := h.kube.Get(ctx, types.NamespacedName{Name: nsName}, ns); err != nil {
-		return errors.Wrapf(err, "failed to get namespace %q for stack %q", nsName, h.ext.GetName())
-	}
-
-	// generate namespace + stack specific clusterroles for all personas
-	roles := generateNamespaceClusterRoles(h.ext)
-
-	for _, role := range roles {
-		// When the namespace is deleted, we no longer need these clusterroles.
-		// Set the ClusterRole owner to the Namespace so they are cleaned up
-		// automatically
-		role.SetOwnerReferences([]metav1.OwnerReference{
-			{
-				APIVersion: "v1",
-				Kind:       "Namespace",
-				Name:       nsName,
-				UID:        ns.GetUID(),
-			},
-		})
-
-		// Creating the clusterroles. Since these Rules in these clusterroles
-		// are populated through aggregation from the stacks installed in the
-		// namespaces, we won't need to update them.
-		if err := h.kube.Create(ctx, role); err != nil && !kerrors.IsAlreadyExists(err) {
-			return errors.Wrapf(err, "failed to create clusterrole %s for stack %s", role.GetName(), h.ext.GetName())
-		}
-	}
-	return nil
-}
-
 func (h *stackHandler) createDeploymentClusterRole(ctx context.Context, labels map[string]string) (string, error) {
 	name := stacks.PersonaRoleName(h.ext, "system")
 	cr := &rbacv1.ClusterRole{
@@ -519,9 +425,6 @@ func (h *stackHandler) processRBAC(ctx context.Context) error {
 	case apiextensions.ClusterScoped:
 		roleBindingErr = h.createClusterRoleBinding(ctx, clusterRoleName, labels)
 	case "", apiextensions.NamespaceScoped:
-		if nsClusterRoleErr := h.createNamespaceClusterRoles(ctx); nsClusterRoleErr != nil {
-			return nsClusterRoleErr
-		}
 		roleBindingErr = h.createNamespacedRoleBinding(ctx, clusterRoleName, owner)
 
 	default:
diff --git a/pkg/controller/stacks/stacks.go b/pkg/controller/stacks/stacks.go
index 27fa551b0..b86c25773 100644
--- a/pkg/controller/stacks/stacks.go
+++ b/pkg/controller/stacks/stacks.go
@@ -22,6 +22,7 @@ import (
 	"github.com/crossplaneio/crossplane-runtime/pkg/logging"
 
 	"github.com/crossplaneio/crossplane/pkg/controller/stacks/install"
+	"github.com/crossplaneio/crossplane/pkg/controller/stacks/persona"
 	"github.com/crossplaneio/crossplane/pkg/controller/stacks/stack"
 )
 
@@ -35,6 +36,9 @@ func Setup(mgr ctrl.Manager, l logging.Logger, hostControllerNamespace, tsContro
 		return err
 	}
 
+	if err := persona.Setup(mgr, l); err != nil {
+		return nil
+	}
 	if err := stack.Setup(mgr, l, hostControllerNamespace); err != nil {
 		return err
 	}


Test patch:
diff --git a/pkg/controller/stacks/persona/persona_test.go b/pkg/controller/stacks/persona/persona_test.go
new file mode 100644
index 000000000..34562c859
--- /dev/null
+++ b/pkg/controller/stacks/persona/persona_test.go
@@ -0,0 +1,437 @@
+/*
+Copyright 2020 The Crossplane Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package persona
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/google/go-cmp/cmp"
+	. "github.com/onsi/gomega"
+	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
+	rbac "k8s.io/api/rbac/v1"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	"github.com/crossplaneio/crossplane-runtime/pkg/logging"
+	"github.com/crossplaneio/crossplane-runtime/pkg/meta"
+	"github.com/crossplaneio/crossplane-runtime/pkg/test"
+	"github.com/crossplaneio/crossplane/apis/stacks/v1alpha1"
+	"github.com/crossplaneio/crossplane/pkg/stacks"
+)
+
+const (
+	namespace = "cool-namespace"
+	uid       = types.UID("definitely-a-uuid")
+)
+
+var (
+	ctx     = context.Background()
+	errBoom = errors.New("boom")
+
+	expectedViewClusterRole = &rbac.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:            "crossplane:ns:" + namespace + ":view",
+			OwnerReferences: []metav1.OwnerReference{{APIVersion: "v1", Kind: "Namespace", Name: namespace, UID: uid}},
+			Labels: map[string]string{
+				"namespace.crossplane.io/" + namespace: "true",
+				"crossplane.io/scope":                  "namespace",
+				stacks.LabelParentGroup:                "",
+				stacks.LabelParentKind:                 "",
+				stacks.LabelParentName:                 namespace,
+				stacks.LabelParentNamespace:            "",
+				stacks.LabelParentUID:                  string(uid),
+				stacks.LabelParentVersion:              "",
+			},
+		},
+		Rules: nil,
+		AggregationRule: &rbac.AggregationRule{
+			ClusterRoleSelectors: []metav1.LabelSelector{
+				{
+					MatchLabels: map[string]string{
+						"namespace.crossplane.io/cool-namespace":         "true",
+						"rbac.crossplane.io/aggregate-to-namespace-view": "true",
+					},
+				},
+				{
+					MatchLabels: map[string]string{
+						"rbac.crossplane.io/aggregate-to-namespace-default-view": "true",
+					},
+				},
+			},
+		},
+	}
+)
+
+func init() {
+	_ = corev1.AddToScheme(scheme.Scheme)
+	_ = rbac.AddToScheme(scheme.Scheme)
+}
+
+var _ reconcile.Reconciler = &Reconciler{}
+
+type resourceModifier func(*corev1.Namespace)
+
+func withDeletionTimestamp(t time.Time) resourceModifier {
+	return func(ns *corev1.Namespace) {
+		ns.SetDeletionTimestamp(&metav1.Time{Time: t})
+	}
+}
+
+func withPersonaManagement() resourceModifier {
+	return func(ns *corev1.Namespace) {
+		meta.AddLabels(ns, personaEnablingLabels())
+	}
+}
+
+func personaEnablingLabels() map[string]string {
+	return map[string]string{managedRolesLabel: managedRolesEnabled}
+}
+
+func resource(rm ...resourceModifier) *corev1.Namespace {
+	r := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: namespace,
+			UID:  uid,
+		},
+	}
+
+	for _, m := range rm {
+		m(r)
+	}
+
+	return r
+}
+
+type mockFactory struct {
+	MockNewHandler func(logging.Logger, *corev1.Namespace, client.Client) handler
+}
+
+func (f *mockFactory) newHandler(log logging.Logger, ns *corev1.Namespace, c client.Client) handler {
+	return f.MockNewHandler(log, ns, c)
+}
+
+type mockHandler struct {
+	MockSync   func(context.Context) (reconcile.Result, error)
+	MockCreate func(context.Context) error
+	MockDelete func(context.Context) error
+}
+
+func (m *mockHandler) sync(ctx context.Context) (reconcile.Result, error) {
+	return m.MockSync(ctx)
+}
+
+func (m *mockHandler) create(ctx context.Context) error {
+	return m.MockCreate(ctx)
+}
+
+func (m *mockHandler) delete(ctx context.Context) error {
+	return m.MockDelete(ctx)
+}
+
+func TestReconcile(t *testing.T) {
+	type want struct {
+		result reconcile.Result
+		err    error
+	}
+
+	tests := []struct {
+		name string
+		req  reconcile.Request
+		rec  *Reconciler
+		want want
+	}{
+		{
+			name: "SuccessfulSync",
+			req:  reconcile.Request{NamespacedName: types.NamespacedName{Name: namespace}},
+			rec: &Reconciler{
+				kube: &test.MockClient{
+					MockGet: func(ctx context.Context, key client.ObjectKey, obj runtime.Object) error {
+						*obj.(*corev1.Namespace) = *(resource())
+						return nil
+					},
+				},
+				factory: &mockFactory{
+					MockNewHandler: func(logging.Logger, *corev1.Namespace, client.Client) handler {
+						return &mockHandler{
+							MockSync: func(context.Context) (reconcile.Result, error) {
+								return reconcile.Result{}, nil
+							},
+						}
+					},
+				},
+				log: logging.NewNopLogger(),
+			},
+			want: want{result: reconcile.Result{}, err: nil},
+		},
+		{
+			name: "ResourceNotFound",
+			req:  reconcile.Request{NamespacedName: types.NamespacedName{Name: namespace}},
+			rec: &Reconciler{
+				kube: &test.MockClient{
+					MockGet: func(ctx context.Context, key client.ObjectKey, obj runtime.Object) error {
+						return kerrors.NewNotFound(schema.GroupResource{Group: v1alpha1.Group}, key.Name)
+					},
+				},
+				factory: nil,
+				log:     logging.NewNopLogger(),
+			},
+			want: want{result: reconcile.Result{}, err: nil},
+		},
+		{
+			name: "ResourceGetError",
+			req:  reconcile.Request{NamespacedName: types.NamespacedName{Name: namespace}},
+			rec: &Reconciler{
+				kube: &test.MockClient{
+					MockGet: func(ctx context.Context, key client.ObjectKey, obj runtime.Object) error {
+						return fmt.Errorf("test-get-error")
+					},
+				},
+				factory: nil,
+				log:     logging.NewNopLogger(),
+			},
+			want: want{result: reconcile.Result{}, err: fmt.Errorf("test-get-error")},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotResult, gotErr := tt.rec.Reconcile(tt.req)
+
+			if diff := cmp.Diff(tt.want.err, gotErr, test.EquateErrors()); diff != "" {
+				t.Errorf("Reconcile() -want error, +got error:\n%s", diff)
+			}
+
+			if diff := cmp.Diff(tt.want.result, gotResult); diff != "" {
+				t.Errorf("Reconcile() -want, +got:\n%s", diff)
+			}
+		})
+	}
+}
+
+func TestNSPersonaCreate(t *testing.T) {
+	errBoom := errors.New("boom")
+
+	type want struct {
+		err    error
+		cr     []*rbac.ClusterRole
+		result reconcile.Result
+	}
+
+	tests := []struct {
+		name       string
+		ns         *corev1.Namespace
+		clientFunc func(*corev1.Namespace) client.Client
+		want       want
+	}{
+		{
+			name:       "NoManagementRequested",
+			ns:         resource(),
+			clientFunc: func(ns *corev1.Namespace) client.Client { return fake.NewFakeClient(ns) },
+			want: want{
+				err:    nil,
+				cr:     nil,
+				result: reconcile.Result{},
+			},
+		},
+		{
+			name: "CreateClusterRoleError",
+			ns:   resource(withPersonaManagement()),
+			clientFunc: func(ns *corev1.Namespace) client.Client {
+				return &test.MockClient{
+					MockCreate: func(ctx context.Context, obj runtime.Object, _ ...client.CreateOption) error {
+						if _, ok := obj.(*rbac.ClusterRole); ok {
+							return errBoom
+						}
+						return nil
+					},
+				}
+			},
+			want: want{
+				err:    errors.Wrap(errBoom, errFailedToCreateClusterRole),
+				cr:     nil,
+				result: resultRequeue,
+			},
+		},
+		{
+			name: "Success",
+			ns:   resource(withPersonaManagement()),
+			clientFunc: func(ns *corev1.Namespace) client.Client {
+				return fake.NewFakeClient(ns)
+			},
+			want: want{
+				err:    nil,
+				cr:     []*rbac.ClusterRole{expectedViewClusterRole},
+				result: reconcile.Result{},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewGomegaWithT(t)
+			handler := &nsPersonaHandler{
+				kube: tt.clientFunc(tt.ns),
+				ns:   tt.ns,
+				log:  logging.NewNopLogger(),
+			}
+
+			gotResult, gotErr := handler.sync(ctx)
+
+			if diff := cmp.Diff(tt.want.err, gotErr, test.EquateErrors()); diff != "" {
+				t.Errorf("create(): -want error, +got error:\n%s", diff)
+			}
+
+			if diff := cmp.Diff(tt.want.result, gotResult); diff != "" {
+				t.Errorf("delete() -want result, +got result:\n%v", diff)
+			}
+
+			if tt.want.cr != nil {
+				for _, wanted := range tt.want.cr {
+					got := &rbac.ClusterRole{}
+					assertKubernetesObject(t, g, got, wanted, handler.kube)
+				}
+			}
+		})
+	}
+}
+
+// TestNamespaceDelete tests the delete function of the Namespace handler
+func TestNSPersonaDelete(t *testing.T) {
+	tn := time.Now()
+
+	type want struct {
+		result  reconcile.Result
+		err     error
+		ns      *corev1.Namespace
+		present []*rbac.ClusterRole
+		gone    []*rbac.ClusterRole
+	}
+
+	tests := []struct {
+		name     string
+		initObjs []runtime.Object
+		clientFn func(initObjs ...runtime.Object) client.Client
+		ns       *corev1.Namespace
+		want     want
+	}{
+		{
+			name:     "FailDeleteAllOf",
+			ns:       resource(),
+			initObjs: []runtime.Object{expectedViewClusterRole},
+			clientFn: func(initObjs ...runtime.Object) client.Client {
+				return &test.MockClient{
+					MockDeleteAllOf: func(ctx context.Context, obj runtime.Object, _ ...client.DeleteAllOfOption) error { return errBoom },
+				}
+			},
+			want: want{
+				result:  resultRequeue,
+				err:     errors.Wrap(errBoom, errFailedToDeleteClusterRoles),
+				ns:      resource(),
+				present: []*rbac.ClusterRole{expectedViewClusterRole},
+			},
+		},
+		{
+			name:     "SuccessfulDelete",
+			ns:       resource(withDeletionTimestamp(tn)),
+			initObjs: []runtime.Object{expectedViewClusterRole},
+			clientFn: fake.NewFakeClient,
+			want: want{
+				result: reconcile.Result{},
+				err:    nil,
+				ns:     resource(withDeletionTimestamp(tn)),
+				gone:   []*rbac.ClusterRole{expectedViewClusterRole},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewGomegaWithT(t)
+
+			handler := &nsPersonaHandler{
+				kube: tt.clientFn(append(tt.initObjs, tt.ns)...),
+				log:  logging.NewNopLogger(),
+				ns:   tt.ns,
+			}
+			gotResult, gotErr := handler.sync(ctx)
+
+			if diff := cmp.Diff(tt.want.err, gotErr, test.EquateErrors()); diff != "" {
+				t.Errorf("delete() -want error, +got error:\n%s", diff)
+			}
+
+			if diff := cmp.Diff(tt.want.result, gotResult); diff != "" {
+				t.Errorf("delete() -want result, +got result:\n%v", diff)
+			}
+
+			if diff := cmp.Diff(tt.want.ns, handler.ns, test.EquateConditions()); diff != "" {
+				t.Errorf("delete() -want Namespace, +got Namespace:\n%v", diff)
+			}
+
+			if tt.want.present == nil {
+				for _, wanted := range tt.want.present {
+					got := &rbac.ClusterRole{}
+					assertKubernetesObject(t, g, got, wanted, handler.kube)
+				}
+			}
+
+			if tt.want.gone == nil {
+				for _, unwanted := range tt.want.gone {
+					got := &rbac.ClusterRole{}
+					assertNoKubernetesObject(t, g, got, unwanted, handler.kube)
+				}
+			}
+
+		})
+	}
+}
+
+type objectWithGVK interface {
+	runtime.Object
+	metav1.Object
+}
+
+func assertKubernetesObject(t *testing.T, g *GomegaWithT, got objectWithGVK, want metav1.Object, kube client.Client) {
+	n := types.NamespacedName{Name: want.GetName(), Namespace: want.GetNamespace()}
+	g.Expect(kube.Get(ctx, n, got)).NotTo(HaveOccurred())
+
+	// NOTE(muvaf): retrieved objects have TypeMeta and
+	// ObjectMeta.ResourceVersion filled but since we work on strong-typed
+	// objects, we don't need to check them.
+	got.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{})
+	got.SetResourceVersion(want.GetResourceVersion())
+
+	if diff := cmp.Diff(want, got, test.EquateConditions()); diff != "" {
+		t.Errorf("-want, +got:\n%s", diff)
+	}
+}
+
+func assertNoKubernetesObject(t *testing.T, g *GomegaWithT, got runtime.Object, unwanted metav1.Object, kube client.Client) {
+	n := types.NamespacedName{Name: unwanted.GetName(), Namespace: unwanted.GetNamespace()}
+	g.Expect(kube.Get(ctx, n, got)).To(HaveOccurred())
+}
diff --git a/pkg/controller/stacks/stack/stack_test.go b/pkg/controller/stacks/stack/stack_test.go
index 420b73f26..62ba573bd 100644
--- a/pkg/controller/stacks/stack/stack_test.go
+++ b/pkg/controller/stacks/stack/stack_test.go
@@ -606,32 +606,6 @@ func TestProcessRBAC_Namespaced(t *testing.T) {
 						},
 						Rules: defaultPolicyRules(),
 					},
-					{
-						ObjectMeta: metav1.ObjectMeta{
-							Name:            "crossplane:ns:" + namespace + ":view",
-							OwnerReferences: []metav1.OwnerReference{{APIVersion: "v1", Kind: "Namespace", Name: namespace, UID: uid}},
-							Labels: map[string]string{
-								"namespace.crossplane.io/" + namespace: "true",
-								"crossplane.io/scope":                  "namespace",
-							},
-						},
-						Rules: nil,
-						AggregationRule: &rbac.AggregationRule{
-							ClusterRoleSelectors: []metav1.LabelSelector{
-								{
-									MatchLabels: map[string]string{
-										"namespace.crossplane.io/cool-namespace":         "true",
-										"rbac.crossplane.io/aggregate-to-namespace-view": "true",
-									},
-								},
-								{
-									MatchLabels: map[string]string{
-										"rbac.crossplane.io/aggregate-to-namespace-default-view": "true",
-									},
-								},
-							},
-						},
-					},
 				},
 				crb: &rbac.RoleBinding{
 					ObjectMeta: metav1.ObjectMeta{