Update cache filter for roles and rolebindings, update migrate process

Update the cache filter for roles and rolebindings to check for
appropriate label. As a result, the old workspace role/rolebinding are
invisible to the controller, so the process for cleaning up old
resources needs to be updated as well.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

pkg/cache/cache.go

@@ -16,7 +16,6 @@ package cache
 import (
 	"fmt"
 
-	"github.com/devfile/devworkspace-operator/pkg/common"
 	"github.com/devfile/devworkspace-operator/pkg/constants"
 	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
 	routev1 "github.com/openshift/api/route/v1"
@@ -25,7 +24,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 )
@@ -48,6 +46,10 @@ func GetCacheFunc() (cache.NewCacheFunc, error) {
 	if err != nil {
 		return nil, err
 	}
+	rbacObjectSelector, err := labels.Parse("controller.devfile.io/workspace-rbac=true")
+	if err != nil {
+		return nil, err
+	}
 
 	selectors := cache.SelectorsByObject{
 		&appsv1.Deployment{}: {
@@ -75,10 +77,10 @@ func GetCacheFunc() (cache.NewCacheFunc, error) {
 			Label: secretObjectSelector,
 		},
 		&rbacv1.Role{}: {
-			Field: fields.SelectorFromSet(fields.Set{"metadata.name": common.OldWorkspaceRoleName()}),
+			Label: rbacObjectSelector,
 		},
 		&rbacv1.RoleBinding{}: {
-			Field: fields.SelectorFromSet(fields.Set{"metadata.name": common.OldWorkspaceRolebindingName()}),
+			Label: rbacObjectSelector,
 		},
 	}
 

pkg/provision/workspace/rbac/common.go

@@ -18,6 +18,12 @@ import (
 	"github.com/devfile/devworkspace-operator/pkg/provision/sync"
 )
 
+var rbacLabels = map[string]string{
+	"app.kubernetes.io/name":               "devworkspace-workspaces",
+	"app.kubernetes.io/part-of":            "devworkspace-operator",
+	"controller.devfile.io/workspace-rbac": "true",
+}
+
 type RetryError struct {
 	Err error
 }

pkg/provision/workspace/rbac/migrate.go

@@ -20,47 +20,83 @@ import (
 	"github.com/devfile/devworkspace-operator/pkg/provision/sync"
 	rbacv1 "k8s.io/api/rbac/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
 
 // cleanupDeprecatedRBAC removes old Roles and RoleBindings created by an earlier version
 // of the DevWorkspace Operator. These earlier roles and rolebindings are no longer used
 // and need to be removed directly as there is no usual mechanism for their removal.
+//
+// Since the cache filters used for the operator are label-based and the old roles/bindings
+// do not have the appropriate labels, the old role/binding are "invisible" to the controller
+// This means we have to delete the object without reading it first. To avoid submitting many
+// delete requests to the API, we only do this if the new role/binding are not present.
 // TODO: Remove this functionality for DevWorkspace Operator v0.19
 func cleanupDeprecatedRBAC(namespace string, api sync.ClusterAPI) error {
-	role := &rbacv1.Role{}
-	roleNN := types.NamespacedName{
-		Name:      common.OldWorkspaceRoleName(),
+	newRole := &rbacv1.Role{}
+	newRoleNN := types.NamespacedName{
+		Name:      common.WorkspaceRoleName(),
 		Namespace: namespace,
 	}
-	err := api.Client.Get(api.Ctx, roleNN, role)
+	oldRole := &rbacv1.Role{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      common.OldWorkspaceRoleName(),
+			Namespace: namespace,
+		},
+	}
+	err := api.Client.Get(api.Ctx, newRoleNN, newRole)
 	switch {
 	case err == nil:
-		if err := api.Client.Delete(api.Ctx, role); err != nil {
-			return err
-		}
-		return &RetryError{fmt.Errorf("deleted deprecated DevWorkspace Role")}
-	case k8sErrors.IsNotFound(err):
+		// New role exists, don't try to delete old role
 		break
+	case k8sErrors.IsNotFound(err):
+		// Try to delete old role
+		deleteErr := api.Client.Delete(api.Ctx, oldRole)
+		switch {
+		case deleteErr == nil:
+			return &RetryError{fmt.Errorf("deleted deprecated DevWorkspace Role")}
+		case k8sErrors.IsNotFound(err):
+			// Already deleted
+			break
+		default:
+			return deleteErr
+		}
 	default:
 		return err
 	}
-	rolebinding := &rbacv1.RoleBinding{}
-	rolebindingNN := types.NamespacedName{
-		Name:      common.OldWorkspaceRolebindingName(),
+
+	newRolebinding := &rbacv1.RoleBinding{}
+	newRolebindingNN := types.NamespacedName{
+		Name:      common.WorkspaceRolebindingName(),
 		Namespace: namespace,
 	}
-	err = api.Client.Get(api.Ctx, rolebindingNN, rolebinding)
+	oldRolebinding := &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      common.OldWorkspaceRolebindingName(),
+			Namespace: namespace,
+		},
+	}
+	err = api.Client.Get(api.Ctx, newRolebindingNN, newRolebinding)
 	switch {
 	case err == nil:
-		if err := api.Client.Delete(api.Ctx, rolebinding); err != nil {
-			return err
-		}
-		return &RetryError{fmt.Errorf("deleted deprecated DevWorkspace RoleBinding")}
-	case k8sErrors.IsNotFound(err):
+		// New role exists, don't try to delete old role
 		break
+	case k8sErrors.IsNotFound(err):
+		// Try to delete old role
+		deleteErr := api.Client.Delete(api.Ctx, oldRolebinding)
+		switch {
+		case deleteErr == nil:
+			return &RetryError{fmt.Errorf("deleted deprecated DevWorkspace RoleBinding")}
+		case k8sErrors.IsNotFound(err):
+			// Already deleted
+			break
+		default:
+			return deleteErr
+		}
 	default:
 		return err
 	}
+
 	return nil
 }

pkg/provision/workspace/rbac/role.go

@@ -68,10 +68,7 @@ func generateDefaultRole(namespace string) *rbacv1.Role {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      common.WorkspaceRoleName(),
 			Namespace: namespace,
-			Labels: map[string]string{
-				"app.kubernetes.io/name":    "devworkspace-workspaces",
-				"app.kubernetes.io/part-of": "devworkspace-operator",
-			},
+			Labels:    rbacLabels,
 		},
 		Rules: []rbacv1.PolicyRule{
 			{
@@ -130,10 +127,7 @@ func generateUseRoleForSCC(namespace, sccName string) *rbacv1.Role {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      common.WorkspaceSCCRoleName(sccName),
 			Namespace: namespace,
-			Labels: map[string]string{
-				"app.kubernetes.io/name":    "devworkspace-workspaces",
-				"app.kubernetes.io/part-of": "devworkspace-operator",
-			},
+			Labels:    rbacLabels,
 		},
 		Rules: []rbacv1.PolicyRule{
 			{

pkg/provision/workspace/rbac/rolebinding.go

@@ -136,10 +136,7 @@ func generateDefaultRolebinding(name, namespace, roleName string) *rbacv1.RoleBi
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
 			Namespace: namespace,
-			Labels: map[string]string{
-				"app.kubernetes.io/name":    "devworkspace-workspaces",
-				"app.kubernetes.io/part-of": "devworkspace-operator",
-			},
+			Labels:    rbacLabels,
 		},
 		RoleRef: rbacv1.RoleRef{
 			Kind: "Role",