Skip to content

Commit 11ae3e4

Browse files
amisevskamisevsk
committed
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 <[email protected]>
1 parent e9e42a9 commit 11ae3e4

File tree

5 files changed

+69
-34
lines changed

5 files changed

+69
-34
lines changed

‎pkg/cache/cache.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ package cache
1616
import (
1717
"fmt"
1818

19-
"github.com/devfile/devworkspace-operator/pkg/common"
2019
"github.com/devfile/devworkspace-operator/pkg/constants"
2120
"github.com/devfile/devworkspace-operator/pkg/infrastructure"
2221
routev1 "github.com/openshift/api/route/v1"
@@ -25,7 +24,6 @@ import (
2524
corev1 "k8s.io/api/core/v1"
2625
networkingv1 "k8s.io/api/networking/v1"
2726
rbacv1 "k8s.io/api/rbac/v1"
28-
"k8s.io/apimachinery/pkg/fields"
2927
"k8s.io/apimachinery/pkg/labels"
3028
"sigs.k8s.io/controller-runtime/pkg/cache"
3129
)
@@ -48,6 +46,10 @@ func GetCacheFunc() (cache.NewCacheFunc, error) {
4846
if err != nil {
4947
return nil, err
5048
}
49+
rbacObjectSelector, err := labels.Parse("controller.devfile.io/workspace-rbac=true")
50+
if err != nil {
51+
return nil, err
52+
}
5153

5254
selectors := cache.SelectorsByObject{
5355
&appsv1.Deployment{}: {
@@ -75,10 +77,10 @@ func GetCacheFunc() (cache.NewCacheFunc, error) {
7577
Label: secretObjectSelector,
7678
},
7779
&rbacv1.Role{}: {
78-
Field: fields.SelectorFromSet(fields.Set{"metadata.name": common.OldWorkspaceRoleName()}),
80+
Label: rbacObjectSelector,
7981
},
8082
&rbacv1.RoleBinding{}: {
81-
Field: fields.SelectorFromSet(fields.Set{"metadata.name": common.OldWorkspaceRolebindingName()}),
83+
Label: rbacObjectSelector,
8284
},
8385
}
8486

‎pkg/provision/workspace/rbac/common.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ import (
1818
"github.com/devfile/devworkspace-operator/pkg/provision/sync"
1919
)
2020

21+
var rbacLabels = map[string]string{
22+
"app.kubernetes.io/name": "devworkspace-workspaces",
23+
"app.kubernetes.io/part-of": "devworkspace-operator",
24+
"controller.devfile.io/workspace-rbac": "true",
25+
}
26+
2127
type RetryError struct {
2228
Err error
2329
}

‎pkg/provision/workspace/rbac/migrate.go

Lines changed: 54 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,47 +20,83 @@ import (
2020
"github.com/devfile/devworkspace-operator/pkg/provision/sync"
2121
rbacv1 "k8s.io/api/rbac/v1"
2222
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2324
"k8s.io/apimachinery/pkg/types"
2425
)
2526

2627
// cleanupDeprecatedRBAC removes old Roles and RoleBindings created by an earlier version
2728
// of the DevWorkspace Operator. These earlier roles and rolebindings are no longer used
2829
// and need to be removed directly as there is no usual mechanism for their removal.
30+
//
31+
// Since the cache filters used for the operator are label-based and the old roles/bindings
32+
// do not have the appropriate labels, the old role/binding are "invisible" to the controller
33+
// This means we have to delete the object without reading it first. To avoid submitting many
34+
// delete requests to the API, we only do this if the new role/binding are not present.
2935
// TODO: Remove this functionality for DevWorkspace Operator v0.19
3036
func cleanupDeprecatedRBAC(namespace string, api sync.ClusterAPI) error {
31-
role := &rbacv1.Role{}
32-
roleNN := types.NamespacedName{
33-
Name: common.OldWorkspaceRoleName(),
37+
newRole := &rbacv1.Role{}
38+
newRoleNN := types.NamespacedName{
39+
Name: common.WorkspaceRoleName(),
3440
Namespace: namespace,
3541
}
36-
err := api.Client.Get(api.Ctx, roleNN, role)
42+
oldRole := &rbacv1.Role{
43+
ObjectMeta: metav1.ObjectMeta{
44+
Name: common.OldWorkspaceRoleName(),
45+
Namespace: namespace,
46+
},
47+
}
48+
err := api.Client.Get(api.Ctx, newRoleNN, newRole)
3749
switch {
3850
case err == nil:
39-
if err := api.Client.Delete(api.Ctx, role); err != nil {
40-
return err
41-
}
42-
return &RetryError{fmt.Errorf("deleted deprecated DevWorkspace Role")}
43-
case k8sErrors.IsNotFound(err):
51+
// New role exists, don't try to delete old role
4452
break
53+
case k8sErrors.IsNotFound(err):
54+
// Try to delete old role
55+
deleteErr := api.Client.Delete(api.Ctx, oldRole)
56+
switch {
57+
case deleteErr == nil:
58+
return &RetryError{fmt.Errorf("deleted deprecated DevWorkspace Role")}
59+
case k8sErrors.IsNotFound(err):
60+
// Already deleted
61+
break
62+
default:
63+
return deleteErr
64+
}
4565
default:
4666
return err
4767
}
48-
rolebinding := &rbacv1.RoleBinding{}
49-
rolebindingNN := types.NamespacedName{
50-
Name: common.OldWorkspaceRolebindingName(),
68+
69+
newRolebinding := &rbacv1.RoleBinding{}
70+
newRolebindingNN := types.NamespacedName{
71+
Name: common.WorkspaceRolebindingName(),
5172
Namespace: namespace,
5273
}
53-
err = api.Client.Get(api.Ctx, rolebindingNN, rolebinding)
74+
oldRolebinding := &rbacv1.RoleBinding{
75+
ObjectMeta: metav1.ObjectMeta{
76+
Name: common.OldWorkspaceRolebindingName(),
77+
Namespace: namespace,
78+
},
79+
}
80+
err = api.Client.Get(api.Ctx, newRolebindingNN, newRolebinding)
5481
switch {
5582
case err == nil:
56-
if err := api.Client.Delete(api.Ctx, rolebinding); err != nil {
57-
return err
58-
}
59-
return &RetryError{fmt.Errorf("deleted deprecated DevWorkspace RoleBinding")}
60-
case k8sErrors.IsNotFound(err):
83+
// New role exists, don't try to delete old role
6184
break
85+
case k8sErrors.IsNotFound(err):
86+
// Try to delete old role
87+
deleteErr := api.Client.Delete(api.Ctx, oldRolebinding)
88+
switch {
89+
case deleteErr == nil:
90+
return &RetryError{fmt.Errorf("deleted deprecated DevWorkspace RoleBinding")}
91+
case k8sErrors.IsNotFound(err):
92+
// Already deleted
93+
break
94+
default:
95+
return deleteErr
96+
}
6297
default:
6398
return err
6499
}
100+
65101
return nil
66102
}

‎pkg/provision/workspace/rbac/role.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,7 @@ func generateDefaultRole(namespace string) *rbacv1.Role {
6868
ObjectMeta: metav1.ObjectMeta{
6969
Name: common.WorkspaceRoleName(),
7070
Namespace: namespace,
71-
Labels: map[string]string{
72-
"app.kubernetes.io/name": "devworkspace-workspaces",
73-
"app.kubernetes.io/part-of": "devworkspace-operator",
74-
},
71+
Labels: rbacLabels,
7572
},
7673
Rules: []rbacv1.PolicyRule{
7774
{
@@ -130,10 +127,7 @@ func generateUseRoleForSCC(namespace, sccName string) *rbacv1.Role {
130127
ObjectMeta: metav1.ObjectMeta{
131128
Name: common.WorkspaceSCCRoleName(sccName),
132129
Namespace: namespace,
133-
Labels: map[string]string{
134-
"app.kubernetes.io/name": "devworkspace-workspaces",
135-
"app.kubernetes.io/part-of": "devworkspace-operator",
136-
},
130+
Labels: rbacLabels,
137131
},
138132
Rules: []rbacv1.PolicyRule{
139133
{

‎pkg/provision/workspace/rbac/rolebinding.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,7 @@ func generateDefaultRolebinding(name, namespace, roleName string) *rbacv1.RoleBi
136136
ObjectMeta: metav1.ObjectMeta{
137137
Name: name,
138138
Namespace: namespace,
139-
Labels: map[string]string{
140-
"app.kubernetes.io/name": "devworkspace-workspaces",
141-
"app.kubernetes.io/part-of": "devworkspace-operator",
142-
},
139+
Labels: rbacLabels,
143140
},
144141
RoleRef: rbacv1.RoleRef{
145142
Kind: "Role",

0 commit comments

Comments
 (0)