diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index ae42aee5f..dbb65c3fa 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -39,6 +39,7 @@ import ( "github.com/devfile/devworkspace-operator/pkg/provision/storage" "github.com/devfile/devworkspace-operator/pkg/provision/sync" wsprovision "github.com/devfile/devworkspace-operator/pkg/provision/workspace" + "github.com/devfile/devworkspace-operator/pkg/provision/workspace/rbac" "github.com/devfile/devworkspace-operator/pkg/timing" "github.com/go-logr/logr" "github.com/google/uuid" @@ -85,7 +86,8 @@ type DevWorkspaceReconciler struct { // +kubebuilder:rbac:groups="batch",resources=jobs,verbs=get;create;list;watch;update;patch;delete // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=mutatingwebhookconfigurations;validatingwebhookconfigurations,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=authorization.k8s.io,resources=subjectaccessreviews;localsubjectaccessreviews,verbs=create -// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings;clusterroles;clusterrolebindings,verbs=get;list;watch;create;update +// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=get;list;watch;create;update +// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;watch;create;update;delete // +kubebuilder:rbac:groups=oauth.openshift.io,resources=oauthclients,verbs=get;list;watch;create;update;patch;delete;deletecollection // +kubebuilder:rbac:groups=monitoring.coreos.com,resources=servicemonitors,verbs=get;create // +kubebuilder:rbac:groups=config.openshift.io,resources=proxies,verbs=get,resourceNames=cluster @@ -285,7 +287,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request // Set finalizer on DevWorkspace if necessary // Note: we need to check the flattened workspace to see if a finalizer is needed, as plugins could require storage - if storageProvisioner.NeedsStorage(&workspace.Spec.Template) { + if storageProvisioner.NeedsStorage(&workspace.Spec.Template) && !coputil.HasFinalizer(clusterWorkspace, constants.StorageCleanupFinalizer) { coputil.AddFinalizer(clusterWorkspace, constants.StorageCleanupFinalizer) if err := r.Update(ctx, clusterWorkspace.DevWorkspace); err != nil { return reconcile.Result{}, err @@ -340,9 +342,24 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request timing.SetTime(timingInfo, timing.ComponentsReady) - rbacStatus := wsprovision.SyncRBAC(workspace, clusterAPI) - if rbacStatus.Err != nil || !rbacStatus.Continue { - return reconcile.Result{Requeue: true}, rbacStatus.Err + // Add finalizer to ensure workspace rolebinding gets cleaned up when workspace + // is deleted. + if !coputil.HasFinalizer(clusterWorkspace, constants.RBACCleanupFinalizer) { + coputil.AddFinalizer(clusterWorkspace, constants.RBACCleanupFinalizer) + if err := r.Update(ctx, clusterWorkspace.DevWorkspace); err != nil { + return reconcile.Result{}, err + } + } + if err := rbac.SyncRBAC(workspace, clusterAPI); err != nil { + switch rbacErr := err.(type) { + case *rbac.RetryError: + reqLogger.Info(rbacErr.Error()) + return reconcile.Result{Requeue: true}, nil + case *rbac.FailError: + return r.failWorkspace(workspace, fmt.Sprintf("Error provisioning rbac: %s", rbacErr), metrics.ReasonInfrastructureFailure, reqLogger, &reconcileStatus) + default: + return reconcile.Result{}, err + } } // Step two: Create routing, and wait for routing to be ready @@ -431,12 +448,6 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } return reconcile.Result{Requeue: serviceAcctStatus.Requeue}, serviceAcctStatus.Err } - if wsprovision.NeedsServiceAccountFinalizer(&workspace.Spec.Template) { - coputil.AddFinalizer(clusterWorkspace, constants.ServiceAccountCleanupFinalizer) - if err := r.Update(ctx, clusterWorkspace.DevWorkspace); err != nil { - return reconcile.Result{}, err - } - } serviceAcctName = serviceAcctStatus.ServiceAccountName reconcileStatus.setConditionTrue(dw.DevWorkspaceServiceAccountReady, "DevWorkspace serviceaccount ready") } diff --git a/controllers/workspace/finalize.go b/controllers/workspace/finalize.go index 2a3e3549a..1151858d1 100644 --- a/controllers/workspace/finalize.go +++ b/controllers/workspace/finalize.go @@ -24,6 +24,7 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/pkg/provision/sync" + "github.com/devfile/devworkspace-operator/pkg/provision/workspace/rbac" "github.com/go-logr/logr" coputil "github.com/redhat-cop/operator-utils/pkg/util" @@ -70,6 +71,8 @@ func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, return r.finalizeStorage(ctx, log, workspace, finalizeStatus) case constants.ServiceAccountCleanupFinalizer: return r.finalizeServiceAccount(ctx, log, workspace, finalizeStatus) + case constants.RBACCleanupFinalizer: + return r.finalizeRBAC(ctx, log, workspace, finalizeStatus) } } return reconcile.Result{}, nil @@ -130,6 +133,46 @@ func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.L return reconcile.Result{}, r.Update(ctx, workspace.DevWorkspace) } +func (r *DevWorkspaceReconciler) finalizeRBAC(ctx context.Context, log logr.Logger, workspace *common.DevWorkspaceWithConfig, finalizeStatus *currentStatus) (reconcile.Result, error) { + terminating, err := r.namespaceIsTerminating(ctx, workspace.Namespace) + if err != nil { + return reconcile.Result{}, err + } else if terminating { + // Namespace is terminating, it's redundant to update roles/rolebindings since they will be removed with the workspace + log.Info("Namespace is terminating; clearing storage finalizer") + coputil.RemoveFinalizer(workspace, constants.RBACCleanupFinalizer) + return reconcile.Result{}, r.Update(ctx, workspace.DevWorkspace) + } + + if err := rbac.FinalizeRBAC(workspace, sync.ClusterAPI{ + Ctx: ctx, + Client: r.Client, + Scheme: r.Scheme, + Logger: log, + }); err != nil { + switch rbacErr := err.(type) { + case *rbac.RetryError: + log.Info(rbacErr.Error()) + return reconcile.Result{Requeue: true}, nil + case *rbac.FailError: + if workspace.Status.Phase != dw.DevWorkspaceStatusError { + // Avoid repeatedly logging error unless it's relevant + log.Error(rbacErr, "Failed to finalize workspace RBAC") + } + finalizeStatus.phase = dw.DevWorkspaceStatusError + finalizeStatus.setConditionTrue(dw.DevWorkspaceError, err.Error()) + return reconcile.Result{}, nil + default: + return reconcile.Result{}, err + } + } + log.Info("RBAC cleanup successful; clearing finalizer") + coputil.RemoveFinalizer(workspace, constants.RBACCleanupFinalizer) + return reconcile.Result{}, r.Update(ctx, workspace.DevWorkspace) +} + +// Deprecated: Only required to support old workspaces that use the service account finalizer. The service account finalizer should +// not be added to new workspaces. func (r *DevWorkspaceReconciler) finalizeServiceAccount(ctx context.Context, log logr.Logger, workspace *common.DevWorkspaceWithConfig, finalizeStatus *currentStatus) (reconcile.Result, error) { retry, err := wsprovision.FinalizeServiceAccount(workspace, ctx, r.NonCachingClient) if err != nil { diff --git a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml index 91762c767..2392ab7f4 100644 --- a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml +++ b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml @@ -277,10 +277,20 @@ spec: resources: - clusterrolebindings - clusterroles + verbs: + - create + - get + - list + - update + - watch + - apiGroups: + - rbac.authorization.k8s.io + resources: - rolebindings - roles verbs: - create + - delete - get - list - update diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 1ec1e17ff..f0173e812 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -23534,10 +23534,20 @@ rules: resources: - clusterrolebindings - clusterroles + verbs: + - create + - get + - list + - update + - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: - rolebindings - roles verbs: - create + - delete - get - list - update diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml index a292a209e..67111535b 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml @@ -201,10 +201,20 @@ rules: resources: - clusterrolebindings - clusterroles + verbs: + - create + - get + - list + - update + - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: - rolebindings - roles verbs: - create + - delete - get - list - update diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index ac1a43f4c..d2f669b59 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -23534,10 +23534,20 @@ rules: resources: - clusterrolebindings - clusterroles + verbs: + - create + - get + - list + - update + - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: - rolebindings - roles verbs: - create + - delete - get - list - update diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml index a292a209e..67111535b 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml @@ -201,10 +201,20 @@ rules: resources: - clusterrolebindings - clusterroles + verbs: + - create + - get + - list + - update + - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: - rolebindings - roles verbs: - create + - delete - get - list - update diff --git a/deploy/templates/components/rbac/role.yaml b/deploy/templates/components/rbac/role.yaml index c9b8621a0..ed33239a4 100644 --- a/deploy/templates/components/rbac/role.yaml +++ b/deploy/templates/components/rbac/role.yaml @@ -200,10 +200,20 @@ rules: resources: - clusterrolebindings - clusterroles + verbs: + - create + - get + - list + - update + - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: - rolebindings - roles verbs: - create + - delete - get - list - update diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index baaeec5fa..4af5236fc 100644 --- a/pkg/cache/cache.go +++ b/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.WorkspaceRoleName()}), + Label: rbacObjectSelector, }, &rbacv1.RoleBinding{}: { - Field: fields.SelectorFromSet(fields.Set{"metadata.name": common.WorkspaceRolebindingName()}), + Label: rbacObjectSelector, }, } diff --git a/pkg/common/naming.go b/pkg/common/naming.go index 853e7a347..a692cbf5d 100644 --- a/pkg/common/naming.go +++ b/pkg/common/naming.go @@ -119,21 +119,45 @@ func MetadataConfigMapName(workspaceId string) string { // can potentially push the name over the 63 character limit (if the original // object has a long name) func AutoMountConfigMapVolumeName(volumeName string) string { - return fmt.Sprintf("%s", volumeName) + return volumeName } func AutoMountSecretVolumeName(volumeName string) string { - return fmt.Sprintf("%s", volumeName) + return volumeName } func AutoMountPVCVolumeName(pvcName string) string { - return fmt.Sprintf("%s", pvcName) + return pvcName } func WorkspaceRoleName() string { - return "workspace" + return "devworkspace-default-role" } func WorkspaceRolebindingName() string { + return "devworkspace-default-rolebinding" +} + +func WorkspaceSCCRoleName(sccName string) string { + return fmt.Sprintf("devworkspace-use-%s", sccName) +} + +func WorkspaceSCCRolebindingName(sccName string) string { + return fmt.Sprintf("devworkspace-use-%s", sccName) +} + +// OldWorkspaceRoleName returns the name used for the workspace serviceaccount role +// +// Deprecated: use WorkspaceRoleName() instead. +// TODO: remove for DevWorkspace Operator v0.19 +func OldWorkspaceRoleName() string { + return "workspace" +} + +// OldWorkspaceRolebindingName returns the name used for the workspace serviceaccount rolebinding +// +// Deprecated: use WorkspaceRoleBindingName() instead. +// TODO: remove for DevWorkspace Operator v0.19 +func OldWorkspaceRolebindingName() string { return constants.ServiceAccount + "dw" } diff --git a/pkg/constants/finalizers.go b/pkg/constants/finalizers.go index 487cab7f2..d098b7267 100644 --- a/pkg/constants/finalizers.go +++ b/pkg/constants/finalizers.go @@ -20,5 +20,13 @@ const ( // ServiceAccountCleanupFinalizer is used to block DevWorkspace deletion when it is // necessary to clean up additional non-workspace roles added to the workspace // serviceaccount + // + // Deprecated: Will not be added to new workspaces but needs to be tracked for + // removal to ensure workspaces that used it previously will be cleaned up. ServiceAccountCleanupFinalizer = "serviceaccount.controller.devfile.io" + // RBACCleanupFinalizer is used to block DevWorkspace deletion in order to ensure + // the workspace role and rolebinding are cleaned up correctly. Since each workspace + // serviceaccount is added to the workspace rolebinding, it is necessary to remove it + // when a workspace is deleted + RBACCleanupFinalizer = "rbac.controller.devfile.io" ) diff --git a/pkg/provision/workspace/rbac.go b/pkg/provision/workspace/rbac.go deleted file mode 100644 index 096015e51..000000000 --- a/pkg/provision/workspace/rbac.go +++ /dev/null @@ -1,124 +0,0 @@ -// -// Copyright (c) 2019-2022 Red Hat, Inc. -// 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 workspace - -import ( - "github.com/devfile/devworkspace-operator/pkg/common" - "github.com/devfile/devworkspace-operator/pkg/provision/sync" - - rbacv1 "k8s.io/api/rbac/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// SyncRBAC generates RBAC and synchronizes the runtime objects -func SyncRBAC(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) ProvisioningStatus { - rbacs := generateRBAC(workspace.Namespace) - requeue := false - for _, rbac := range rbacs { - _, err := sync.SyncObjectWithCluster(rbac, clusterAPI) - switch t := err.(type) { - case nil: - break - case *sync.NotInSyncError: - requeue = true - case *sync.UnrecoverableSyncError: - return ProvisioningStatus{FailStartup: true, Err: t.Cause} - default: - return ProvisioningStatus{Err: err} - } - } - - return ProvisioningStatus{Continue: !requeue} -} - -func generateRBAC(namespace string) []client.Object { - // TODO: The rolebindings here are created namespace-wide; find a way to limit this, given that each workspace is only - // interested in a specific set of resources (e.g. the workspace shouldn't need to watch other pods, etc.) - return []client.Object{ - &rbacv1.Role{ - ObjectMeta: metav1.ObjectMeta{ - Name: common.WorkspaceRoleName(), - Namespace: namespace, - }, - Rules: []rbacv1.PolicyRule{ - { - Resources: []string{"pods/exec"}, - APIGroups: []string{""}, - Verbs: []string{"create"}, - }, - { - Resources: []string{"pods"}, - APIGroups: []string{""}, - Verbs: []string{"get", "list", "watch"}, - }, - { - Resources: []string{"pods"}, - APIGroups: []string{"metrics.k8s.io"}, - Verbs: []string{"get", "list", "watch"}, - }, - { - Resources: []string{"deployments", "replicasets"}, - APIGroups: []string{"apps", "extensions"}, - Verbs: []string{"get", "list", "watch"}, - }, - { - Resources: []string{"secrets"}, - APIGroups: []string{""}, - Verbs: []string{"get", "create", "patch", "delete"}, - ResourceNames: []string{"workspace-credentials-secret"}, - }, - { - Resources: []string{"configmaps"}, - APIGroups: []string{""}, - Verbs: []string{"get", "create", "patch", "delete"}, - ResourceNames: []string{"workspace-preferences-configmap"}, - }, - { - Resources: []string{"devworkspaces"}, - APIGroups: []string{"workspace.devfile.io"}, - Verbs: []string{"get", "watch", "list", "patch", "update"}, - }, - { - Resources: []string{"devworkspaceroutings"}, - APIGroups: []string{"controller.devfile.io"}, - Verbs: []string{"get", "list", "watch"}, - }, - { - Resources: []string{"devworkspacetemplates"}, - APIGroups: []string{"workspace.devfile.io"}, - Verbs: []string{"get", "create", "patch", "update", "delete", "list", "watch"}, - }, - }, - }, - &rbacv1.RoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: common.WorkspaceRolebindingName(), - Namespace: namespace, - }, - RoleRef: rbacv1.RoleRef{ - Kind: "Role", - Name: "workspace", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "Group", - Name: "system:serviceaccounts:" + namespace, - }, - }, - }, - } -} diff --git a/pkg/provision/workspace/rbac/common.go b/pkg/provision/workspace/rbac/common.go new file mode 100644 index 000000000..58fa9925e --- /dev/null +++ b/pkg/provision/workspace/rbac/common.go @@ -0,0 +1,65 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "github.com/devfile/devworkspace-operator/pkg/common" + "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 +} + +func (e *RetryError) Error() string { + return e.Err.Error() +} + +type FailError struct { + Err error +} + +func (e *FailError) Error() string { + return e.Err.Error() +} + +func wrapSyncError(err error) error { + switch syncErr := err.(type) { + case *sync.NotInSyncError: + return &RetryError{syncErr} + case *sync.UnrecoverableSyncError: + return &FailError{syncErr} + default: + return err + } +} + +func SyncRBAC(workspace *common.DevWorkspaceWithConfig, api sync.ClusterAPI) error { + if err := cleanupDeprecatedRBAC(workspace.Namespace, api); err != nil { + return err + } + if err := syncRoles(workspace, api); err != nil { + return err + } + if err := syncRolebindings(workspace, api); err != nil { + return err + } + return nil +} diff --git a/pkg/provision/workspace/rbac/common_test.go b/pkg/provision/workspace/rbac/common_test.go new file mode 100644 index 000000000..affa543fa --- /dev/null +++ b/pkg/provision/workspace/rbac/common_test.go @@ -0,0 +1,225 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "context" + "fmt" + "testing" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/api/v2/pkg/attributes" + "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/provision/sync" + testlog "github.com/go-logr/logr/testing" + "github.com/stretchr/testify/assert" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const ( + testNamespace = "test-namespace" + testSCCName = "test-scc" +) + +var ( + scheme = runtime.NewScheme() +) + +func init() { + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(v1alpha1.AddToScheme(scheme)) + utilruntime.Must(dw.AddToScheme(scheme)) +} + +var ( + oldRole = &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.OldWorkspaceRoleName(), + Namespace: testNamespace, + }, + Rules: []rbacv1.PolicyRule{}, + } + oldRolebinding = &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.OldWorkspaceRolebindingName(), + Namespace: testNamespace, + }, + } + newRole = &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.WorkspaceRoleName(), + Namespace: testNamespace, + }, + Rules: []rbacv1.PolicyRule{}, + } + newRolebinding = &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.WorkspaceRolebindingName(), + Namespace: testNamespace, + }, + RoleRef: rbacv1.RoleRef{ + Kind: "Role", + Name: common.WorkspaceRoleName(), + }, + } + newSCCRole = &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.WorkspaceSCCRoleName(testSCCName), + Namespace: testNamespace, + }, + Rules: []rbacv1.PolicyRule{}, + } + newSCCRolebinding = &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.WorkspaceSCCRolebindingName(testSCCName), + Namespace: testNamespace, + }, + RoleRef: rbacv1.RoleRef{ + Kind: "Role", + Name: common.WorkspaceSCCRoleName(testSCCName), + }, + } +) + +func TestSyncRBAC(t *testing.T) { + testdw1 := getTestDevWorkspaceWithAttributes(t, "test-devworkspace", constants.WorkspaceSCCAttribute, testSCCName) + testdw2 := getTestDevWorkspaceWithAttributes(t, "test-devworkspace2", constants.WorkspaceSCCAttribute, testSCCName) + testdw1SAName := common.ServiceAccountName(testdw1) + testdw2SAName := common.ServiceAccountName(testdw2) + api := getTestClusterAPI(t, testdw1.DevWorkspace, testdw2.DevWorkspace, oldRole, oldRolebinding) + // Keep calling SyncRBAC until error returned is nil, to account for multiple steps + iterCount := 0 + maxIters := 30 + retryErr := &RetryError{} + for err := SyncRBAC(testdw1, api); err != nil; err = SyncRBAC(testdw1, api) { + iterCount += 1 + if err == nil { + break + } + if !assert.ErrorAs(t, err, &retryErr, "Unexpected error from SyncRBAC: %s", err) { + return + } + if !assert.LessOrEqual(t, iterCount, maxIters, fmt.Sprintf("SyncRBAC did not sync everything within %d iterations", maxIters)) { + return + } + } + for err := SyncRBAC(testdw2, api); err != nil; err = SyncRBAC(testdw2, api) { + iterCount += 1 + if err == nil { + break + } + if !assert.ErrorAs(t, err, &retryErr, "Unexpected error from SyncRBAC: %s", err) { + return + } + if !assert.LessOrEqual(t, iterCount, maxIters, fmt.Sprintf("SyncRBAC did not sync everything within %d iterations", maxIters)) { + return + } + } + actualRole := &rbacv1.Role{} + err := api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceRoleName(), + Namespace: testNamespace, + }, actualRole) + assert.NoError(t, err, "Role should be created") + + actualSCCRole := &rbacv1.Role{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceSCCRoleName(testSCCName), + Namespace: testNamespace, + }, actualSCCRole) + assert.NoError(t, err, "SCC Role should be created") + + actualRolebinding := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceRolebindingName(), + Namespace: testNamespace, + }, actualRolebinding) + assert.NoError(t, err, "Role should be created") + assert.True(t, testHasSubject(testdw1SAName, testNamespace, actualRolebinding), "Should have testdw1 SA as subject") + assert.True(t, testHasSubject(testdw2SAName, testNamespace, actualRolebinding), "Should have testdw2 SA as subject") + + actualSCCRolebinding := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceSCCRolebindingName(testSCCName), + Namespace: testNamespace, + }, actualSCCRolebinding) + assert.NoError(t, err, "SCC Rolebindind should be created") + assert.True(t, testHasSubject(testdw1SAName, testNamespace, actualSCCRolebinding), "Should have testdw1 SA as subject") + assert.True(t, testHasSubject(testdw2SAName, testNamespace, actualSCCRolebinding), "Should have testdw2 SA as subject") +} + +func getTestClusterAPI(t *testing.T, initialObjects ...client.Object) sync.ClusterAPI { + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initialObjects...).Build() + return sync.ClusterAPI{ + Ctx: context.Background(), + Client: fakeClient, + Scheme: scheme, + Logger: testlog.TestLogger{T: t}, + } +} + +func getTestDevWorkspace(id string) *common.DevWorkspaceWithConfig { + return &common.DevWorkspaceWithConfig{ + DevWorkspace: &dw.DevWorkspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: id, + Namespace: testNamespace, + }, + + Status: dw.DevWorkspaceStatus{ + DevWorkspaceId: id, + }, + }, + Config: config.GetConfigForTesting(nil), + } +} + +func getTestDevWorkspaceWithAttributes(t *testing.T, id string, keysAndValues ...string) *common.DevWorkspaceWithConfig { + attr := attributes.Attributes{} + if len(keysAndValues)%2 != 0 { + t.Fatalf("Invalid keysAndValues for getTestDevWorkspaceWithAttributes") + } + for i := 0; i < len(keysAndValues); i += 2 { + attr.PutString(keysAndValues[i], keysAndValues[i+1]) + } + return &common.DevWorkspaceWithConfig{ + Config: config.GetConfigForTesting(nil), + DevWorkspace: &dw.DevWorkspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: id, + Namespace: testNamespace, + }, + Spec: dw.DevWorkspaceSpec{ + Template: dw.DevWorkspaceTemplateSpec{ + DevWorkspaceTemplateSpecContent: dw.DevWorkspaceTemplateSpecContent{ + Attributes: attr, + }, + }, + }, + Status: dw.DevWorkspaceStatus{ + DevWorkspaceId: id, + }, + }, + } +} diff --git a/pkg/provision/workspace/rbac/finalize.go b/pkg/provision/workspace/rbac/finalize.go new file mode 100644 index 000000000..1e94d92d2 --- /dev/null +++ b/pkg/provision/workspace/rbac/finalize.go @@ -0,0 +1,116 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/provision/sync" + "sigs.k8s.io/controller-runtime/pkg/client" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" +) + +func FinalizeRBAC(workspace *common.DevWorkspaceWithConfig, api sync.ClusterAPI) error { + if workspace.Spec.Template.Attributes.Exists(constants.WorkspaceSCCAttribute) { + if err := finalizeSCCRBAC(workspace, api); err != nil { + return err + } + } + saName := common.ServiceAccountName(workspace) + roleName := common.WorkspaceRoleName() + rolebindingName := common.WorkspaceRolebindingName() + numWorkspaces, err := countNonDeletedWorkspaces(workspace.Namespace, api) + if err != nil { + return err + } + if numWorkspaces == 0 { + if err := deleteRole(roleName, workspace.Namespace, api); err != nil { + return err + } + if err := deleteRolebinding(rolebindingName, workspace.Namespace, api); err != nil { + return err + } + return nil + } + if err := removeServiceAccountFromRolebinding(saName, workspace.Namespace, rolebindingName, api); err != nil { + return err + } + return nil +} + +func finalizeSCCRBAC(workspace *common.DevWorkspaceWithConfig, api sync.ClusterAPI) error { + sccName := workspace.Spec.Template.Attributes.GetString(constants.WorkspaceSCCAttribute, nil) + saName := common.ServiceAccountName(workspace) + roleName := common.WorkspaceSCCRoleName(sccName) + rolebindingName := common.WorkspaceSCCRolebindingName(sccName) + numWorkspaces, err := countNonDeletedWorkspacesUsingSCC(sccName, workspace.Namespace, api) + if err != nil { + return err + } + if numWorkspaces == 0 { + if err := deleteRole(roleName, workspace.Namespace, api); err != nil { + return err + } + if err := deleteRolebinding(rolebindingName, workspace.Namespace, api); err != nil { + return err + } + return nil + } + if err := removeServiceAccountFromRolebinding(saName, workspace.Namespace, rolebindingName, api); err != nil { + return err + } + return nil +} + +func countNonDeletedWorkspaces(namespace string, api sync.ClusterAPI) (int, error) { + count := 0 + allWorkspaces := &dw.DevWorkspaceList{} + allWorkspacesListOptions := &client.ListOptions{ + Namespace: namespace, + } + if err := api.Client.List(api.Ctx, allWorkspaces, allWorkspacesListOptions); err != nil { + return -1, err + } + for _, workspace := range allWorkspaces.Items { + if workspace.DeletionTimestamp != nil { + // ignore workspaces that are being deleted + continue + } + count = count + 1 + } + return count, nil +} + +func countNonDeletedWorkspacesUsingSCC(sccName, namespace string, api sync.ClusterAPI) (int, error) { + count := 0 + allWorkspaces := &dw.DevWorkspaceList{} + allWorkspacesListOptions := &client.ListOptions{ + Namespace: namespace, + } + if err := api.Client.List(api.Ctx, allWorkspaces, allWorkspacesListOptions); err != nil { + return -1, err + } + for _, workspace := range allWorkspaces.Items { + if workspace.DeletionTimestamp != nil { + // ignore workspaces that are being deleted + continue + } + attrs := workspace.Spec.Template.Attributes + if attrs.Exists(constants.WorkspaceSCCAttribute) && attrs.GetString(constants.WorkspaceSCCAttribute, nil) == sccName { + count = count + 1 + } + } + return count, nil +} diff --git a/pkg/provision/workspace/rbac/finalize_test.go b/pkg/provision/workspace/rbac/finalize_test.go new file mode 100644 index 000000000..446791dd5 --- /dev/null +++ b/pkg/provision/workspace/rbac/finalize_test.go @@ -0,0 +1,228 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "fmt" + "testing" + "time" + + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" + "github.com/stretchr/testify/assert" + 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" +) + +func TestDeletesRoleAndRolebindingWhenLastWorkspaceIsDeleted(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspace("test-devworkspace") + testdw.DeletionTimestamp = &metav1.Time{Time: time.Now()} + api := getTestClusterAPI(t, testdw.DevWorkspace, newRole, newRolebinding) + retryErr := &RetryError{} + err := FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate role deleted") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + assert.Regexp(t, fmt.Sprintf("deleted role .* in namespace %s", testNamespace), err.Error()) + } + err = FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate rolebinding deleted") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + assert.Regexp(t, fmt.Sprintf("deleted rolebinding .* in namespace %s", testNamespace), err.Error()) + } + err = FinalizeRBAC(testdw, api) + assert.NoError(t, err, "Should not return error once role and rolebinding deleted") +} + +func TestDeletesRoleAndRolebindingWhenAllWorkspacesAreDeleted(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspace("test-devworkspace") + testdw2 := getTestDevWorkspace("test-devworkspace2") + testdw.DeletionTimestamp = &metav1.Time{Time: time.Now()} + testdw2.DeletionTimestamp = &metav1.Time{Time: time.Now()} + api := getTestClusterAPI(t, testdw.DevWorkspace, testdw2.DevWorkspace, newRole, newRolebinding) + + retryErr := &RetryError{} + err := FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate role deleted") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + assert.Regexp(t, fmt.Sprintf("deleted role .* in namespace %s", testNamespace), err.Error()) + } + err = FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate rolebinding deleted") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + assert.Regexp(t, fmt.Sprintf("deleted rolebinding .* in namespace %s", testNamespace), err.Error()) + } + err = FinalizeRBAC(testdw, api) + assert.NoError(t, err, "Should not return error once role and rolebinding deleted") +} + +func TestShouldRemoveWorkspaceSAFromRolebindingWhenDeleted(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspace("test-devworkspace") + testdw2 := getTestDevWorkspace("test-devworkspace2") + testdw.DeletionTimestamp = &metav1.Time{Time: time.Now()} + testdwSAName := common.ServiceAccountName(testdw) + testdw2SAName := common.ServiceAccountName(testdw2) + testrb := newRolebinding.DeepCopy() + testrb.Subjects = append(testrb.Subjects, + rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: testdwSAName, + Namespace: testNamespace, + }, rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: testdw2SAName, + Namespace: testNamespace, + }) + api := getTestClusterAPI(t, testdw.DevWorkspace, testdw2.DevWorkspace, testrb, newRole) + retryErr := &RetryError{} + err := FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate rolebinding updated") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + } + err = FinalizeRBAC(testdw, api) + assert.NoError(t, err, "Should not return error once rolebinding is in sync") + + actualRolebinding := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceRolebindingName(), + Namespace: testNamespace, + }, actualRolebinding) + assert.NoError(t, err, "Unexpected test error getting rolebinding") + assert.False(t, testHasSubject(testdwSAName, testNamespace, actualRolebinding), "Should remove delete workspace SA from rolebinding subjects") + assert.True(t, testHasSubject(testdw2SAName, testNamespace, actualRolebinding), "Should leave workspace SA in rolebinding subjects") +} + +func TestFinalizeDoesNothingWhenRolebindingDoesNotExist(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspace("test-devworkspace") + testdw2 := getTestDevWorkspace("test-devworkspace2") + testdw.DeletionTimestamp = &metav1.Time{Time: time.Now()} + api := getTestClusterAPI(t, testdw.DevWorkspace, testdw2.DevWorkspace, newRole) + err := FinalizeRBAC(testdw, api) + assert.NoError(t, err, "Should not return error once rolebinding is in sync") + + actualRolebinding := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceRolebindingName(), + Namespace: testNamespace, + }, actualRolebinding) + if assert.Error(t, err, "Expect error when getting non-existent rolebinding") { + assert.True(t, k8sErrors.IsNotFound(err), "Error should have IsNotFound type") + } +} + +func TestDeletesSCCRoleAndRolebindingWhenLastWorkspaceIsDeleted(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspaceWithAttributes(t, "test-devworkspace", constants.WorkspaceSCCAttribute, testSCCName) + testdw2 := getTestDevWorkspace("test-devworkspace2") + testdw.DeletionTimestamp = &metav1.Time{Time: time.Now()} + api := getTestClusterAPI(t, testdw.DevWorkspace, testdw2.DevWorkspace, newSCCRole, newSCCRolebinding) + retryErr := &RetryError{} + err := FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate role deleted") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + assert.Regexp(t, fmt.Sprintf("deleted role .* in namespace %s", testNamespace), err.Error()) + } + err = FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate rolebinding deleted") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + assert.Regexp(t, fmt.Sprintf("deleted rolebinding .* in namespace %s", testNamespace), err.Error()) + } + err = FinalizeRBAC(testdw, api) + assert.NoError(t, err, "Should not return error once role and rolebinding deleted") +} + +func TestDeletesSCCRoleAndRolebindingWhenAllWorkspacesAreDeleted(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspaceWithAttributes(t, "test-devworkspace", constants.WorkspaceSCCAttribute, testSCCName) + testdw2 := getTestDevWorkspaceWithAttributes(t, "test-devworkspace2", constants.WorkspaceSCCAttribute, testSCCName) + testdw.DeletionTimestamp = &metav1.Time{Time: time.Now()} + testdw2.DeletionTimestamp = &metav1.Time{Time: time.Now()} + api := getTestClusterAPI(t, testdw.DevWorkspace, testdw2.DevWorkspace, newSCCRole, newSCCRolebinding) + + retryErr := &RetryError{} + err := FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate role deleted") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + assert.Regexp(t, fmt.Sprintf("deleted role .* in namespace %s", testNamespace), err.Error()) + } + err = FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate rolebinding deleted") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + assert.Regexp(t, fmt.Sprintf("deleted rolebinding .* in namespace %s", testNamespace), err.Error()) + } + err = FinalizeRBAC(testdw, api) + assert.NoError(t, err, "Should not return error once role and rolebinding deleted") +} + +func TestShouldRemoveWorkspaceSAFromSCCRolebindingWhenDeleted(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspaceWithAttributes(t, "test-devworkspace", constants.WorkspaceSCCAttribute, testSCCName) + testdw2 := getTestDevWorkspaceWithAttributes(t, "test-devworkspace2", constants.WorkspaceSCCAttribute, testSCCName) + testdw.DeletionTimestamp = &metav1.Time{Time: time.Now()} + testdwSAName := common.ServiceAccountName(testdw) + testdw2SAName := common.ServiceAccountName(testdw2) + testrb := newSCCRolebinding.DeepCopy() + testrb.Subjects = append(testrb.Subjects, + rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: testdwSAName, + Namespace: testNamespace, + }, rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: testdw2SAName, + Namespace: testNamespace, + }) + api := getTestClusterAPI(t, testdw.DevWorkspace, testdw2.DevWorkspace, testrb, newSCCRole) + retryErr := &RetryError{} + err := FinalizeRBAC(testdw, api) + if assert.Error(t, err, "Should return error to indicate rolebinding updated") { + assert.ErrorAs(t, err, &retryErr, "Error should be RetryError") + } + err = FinalizeRBAC(testdw, api) + assert.NoError(t, err, "Should not return error once rolebinding is in sync") + + actualRolebinding := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceSCCRolebindingName(testSCCName), + Namespace: testNamespace, + }, actualRolebinding) + assert.NoError(t, err, "Unexpected test error getting rolebinding") + assert.False(t, testHasSubject(testdwSAName, testNamespace, actualRolebinding), "Should remove delete workspace SA from rolebinding subjects") + assert.True(t, testHasSubject(testdw2SAName, testNamespace, actualRolebinding), "Should leave workspace SA in rolebinding subjects") +} + +func TestFinalizeDoesNothingWhenSCCRolebindingDoesNotExist(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspaceWithAttributes(t, "test-devworkspace", constants.WorkspaceSCCAttribute, testSCCName) + testdw2 := getTestDevWorkspaceWithAttributes(t, "test-devworkspace2", constants.WorkspaceSCCAttribute, testSCCName) + testdw.DeletionTimestamp = &metav1.Time{Time: time.Now()} + api := getTestClusterAPI(t, testdw.DevWorkspace, testdw2.DevWorkspace, newSCCRole) + err := FinalizeRBAC(testdw, api) + assert.NoError(t, err, "Should not return error once rolebinding is in sync") + + actualRolebinding := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceSCCRolebindingName(testSCCName), + Namespace: testNamespace, + }, actualRolebinding) + if assert.Error(t, err, "Expect error when getting non-existent rolebinding") { + assert.True(t, k8sErrors.IsNotFound(err), "Error should have IsNotFound type") + } +} diff --git a/pkg/provision/workspace/rbac/migrate.go b/pkg/provision/workspace/rbac/migrate.go new file mode 100644 index 000000000..85128bab8 --- /dev/null +++ b/pkg/provision/workspace/rbac/migrate.go @@ -0,0 +1,102 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "fmt" + + "github.com/devfile/devworkspace-operator/pkg/common" + "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 { + newRole := &rbacv1.Role{} + newRoleNN := types.NamespacedName{ + Name: common.WorkspaceRoleName(), + Namespace: namespace, + } + oldRole := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.OldWorkspaceRoleName(), + Namespace: namespace, + }, + } + err := api.Client.Get(api.Ctx, newRoleNN, newRole) + switch { + case err == nil: + // 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 + } + + newRolebinding := &rbacv1.RoleBinding{} + newRolebindingNN := types.NamespacedName{ + Name: common.WorkspaceRolebindingName(), + Namespace: namespace, + } + oldRolebinding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.OldWorkspaceRolebindingName(), + Namespace: namespace, + }, + } + err = api.Client.Get(api.Ctx, newRolebindingNN, newRolebinding) + switch { + case err == nil: + // 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 +} diff --git a/pkg/provision/workspace/rbac/migrate_test.go b/pkg/provision/workspace/rbac/migrate_test.go new file mode 100644 index 000000000..de4e724ba --- /dev/null +++ b/pkg/provision/workspace/rbac/migrate_test.go @@ -0,0 +1,130 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "testing" + + "github.com/devfile/devworkspace-operator/pkg/infrastructure" + "github.com/stretchr/testify/assert" + rbacv1 "k8s.io/api/rbac/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" +) + +func TestRemovesOldRBACWhenNewRBACNotPresent(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + api := getTestClusterAPI(t, oldRole, oldRolebinding) + // Expect three calls to be required: 1. delete role, 2. delete rolebinding, 3. return nil + err := cleanupDeprecatedRBAC(testNamespace, api) + retryErr := &RetryError{} + if assert.ErrorAs(t, err, &retryErr, "Error should be of type RetryErr") { + assert.Contains(t, err.Error(), "deleted deprecated DevWorkspace Role") + } + err = cleanupDeprecatedRBAC(testNamespace, api) + if assert.ErrorAs(t, err, &retryErr, "Error should be of type RetryErr") { + assert.Contains(t, err.Error(), "deleted deprecated DevWorkspace RoleBinding") + } + err = cleanupDeprecatedRBAC(testNamespace, api) + assert.NoError(t, err, "Should not return error if old rbac does not exist") + + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: oldRole.Name, + Namespace: testNamespace, + }, &rbacv1.Role{}) + if assert.Error(t, err, "Expect get old role to return IsNotFound error") { + assert.True(t, k8sErrors.IsNotFound(err), "Expect error to have IsNotFound type") + } + + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: oldRolebinding.Name, + Namespace: testNamespace, + }, &rbacv1.RoleBinding{}) + if assert.Error(t, err, "Expect get old role to return IsNotFound error") { + assert.True(t, k8sErrors.IsNotFound(err), "Expect error to have IsNotFound type") + } +} + +func TestDoesNotRemoveOldRBACWhenNewRBACPresent(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + api := getTestClusterAPI(t, oldRole, oldRolebinding, newRole, newRolebinding) + err := cleanupDeprecatedRBAC(testNamespace, api) + assert.NoError(t, err, "Should do nothing if new RBAC role/rolebinding are present") + + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: oldRole.Name, + Namespace: testNamespace, + }, &rbacv1.Role{}) + assert.NoError(t, err, "Old role should still exist if present initially") + + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: oldRolebinding.Name, + Namespace: testNamespace, + }, &rbacv1.RoleBinding{}) + assert.NoError(t, err, "Old rolebinding should still exist if present initially") +} + +func TestRemovesOldRolebindingWhenNewRolebindingNotPresent(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + api := getTestClusterAPI(t, oldRole, oldRolebinding, newRole) + // Expect two calls to be required: 1. delete rolebinding, 2. return nil + err := cleanupDeprecatedRBAC(testNamespace, api) + retryErr := &RetryError{} + if assert.ErrorAs(t, err, &retryErr, "Error should be of type RetryErr") { + assert.Contains(t, err.Error(), "deleted deprecated DevWorkspace RoleBinding") + } + err = cleanupDeprecatedRBAC(testNamespace, api) + assert.NoError(t, err, "Should not return error if old rbac does not exist") + + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: oldRole.Name, + Namespace: testNamespace, + }, &rbacv1.Role{}) + assert.NoError(t, err, "Old role should still exist if present initially") + + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: oldRolebinding.Name, + Namespace: testNamespace, + }, &rbacv1.RoleBinding{}) + if assert.Error(t, err, "Expect get old role to return IsNotFound error") { + assert.True(t, k8sErrors.IsNotFound(err), "Expect error to have IsNotFound type") + } +} + +func TestRemovesOldRoleWhenNewRoleNotPresent(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + api := getTestClusterAPI(t, oldRole, oldRolebinding, newRolebinding) + // Expect two calls to be required: 1. delete role, 2. return nil + err := cleanupDeprecatedRBAC(testNamespace, api) + retryErr := &RetryError{} + if assert.ErrorAs(t, err, &retryErr, "Error should be of type RetryErr") { + assert.Contains(t, err.Error(), "deleted deprecated DevWorkspace Role") + } + err = cleanupDeprecatedRBAC(testNamespace, api) + assert.NoError(t, err, "Should not return error if old rbac does not exist") + + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: oldRole.Name, + Namespace: testNamespace, + }, &rbacv1.Role{}) + if assert.Error(t, err, "Expect get old role to return IsNotFound error") { + assert.True(t, k8sErrors.IsNotFound(err), "Expect error to have IsNotFound type") + } + + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: oldRolebinding.Name, + Namespace: testNamespace, + }, &rbacv1.RoleBinding{}) + assert.NoError(t, err, "Old rolebinding should still exist if present initially") +} diff --git a/pkg/provision/workspace/rbac/role.go b/pkg/provision/workspace/rbac/role.go new file mode 100644 index 000000000..033e4bf4d --- /dev/null +++ b/pkg/provision/workspace/rbac/role.go @@ -0,0 +1,141 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "fmt" + + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/constants" + "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" +) + +func syncRoles(workspace *common.DevWorkspaceWithConfig, api sync.ClusterAPI) error { + defaultRole := generateDefaultRole(workspace.Namespace) + if _, err := sync.SyncObjectWithCluster(defaultRole, api); err != nil { + return wrapSyncError(err) + } + if !workspace.Spec.Template.Attributes.Exists(constants.WorkspaceSCCAttribute) { + return nil + } + sccName := workspace.Spec.Template.Attributes.GetString(constants.WorkspaceSCCAttribute, nil) + sccRole := generateUseRoleForSCC(workspace.Namespace, sccName) + if _, err := sync.SyncObjectWithCluster(sccRole, api); err != nil { + return wrapSyncError(err) + } + return nil +} + +func deleteRole(name, namespace string, api sync.ClusterAPI) error { + role := &rbacv1.Role{} + namespacedName := types.NamespacedName{ + Name: name, + Namespace: namespace, + } + err := api.Client.Get(api.Ctx, namespacedName, role) + switch { + case err == nil: + if err := api.Client.Delete(api.Ctx, role); err != nil { + return &RetryError{fmt.Errorf("failed to delete role %s in namespace %s: %w", name, namespace, err)} + } + return &RetryError{fmt.Errorf("deleted role %s in namespace %s", name, namespace)} + case k8sErrors.IsNotFound(err): + // Already deleted + return nil + default: + return err + } +} + +func generateDefaultRole(namespace string) *rbacv1.Role { + return &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.WorkspaceRoleName(), + Namespace: namespace, + Labels: rbacLabels, + }, + Rules: []rbacv1.PolicyRule{ + { + Resources: []string{"pods/exec"}, + APIGroups: []string{""}, + Verbs: []string{"create"}, + }, + { + Resources: []string{"pods"}, + APIGroups: []string{""}, + Verbs: []string{"get", "list", "watch"}, + }, + { + Resources: []string{"pods"}, + APIGroups: []string{"metrics.k8s.io"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + Resources: []string{"deployments", "replicasets"}, + APIGroups: []string{"apps", "extensions"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + Resources: []string{"secrets"}, + APIGroups: []string{""}, + Verbs: []string{"get", "create", "patch", "delete"}, + ResourceNames: []string{"workspace-credentials-secret"}, + }, + { + Resources: []string{"configmaps"}, + APIGroups: []string{""}, + Verbs: []string{"get", "create", "patch", "delete"}, + ResourceNames: []string{"workspace-preferences-configmap"}, + }, + { + Resources: []string{"devworkspaces"}, + APIGroups: []string{"workspace.devfile.io"}, + Verbs: []string{"get", "watch", "list", "patch", "update"}, + }, + { + Resources: []string{"devworkspaceroutings"}, + APIGroups: []string{"controller.devfile.io"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + Resources: []string{"devworkspacetemplates"}, + APIGroups: []string{"workspace.devfile.io"}, + Verbs: []string{"get", "create", "patch", "update", "delete", "list", "watch"}, + }, + }, + } +} + +func generateUseRoleForSCC(namespace, sccName string) *rbacv1.Role { + return &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.WorkspaceSCCRoleName(sccName), + Namespace: namespace, + Labels: rbacLabels, + }, + Rules: []rbacv1.PolicyRule{ + { + Resources: []string{"securitycontextconstraints"}, + APIGroups: []string{"security.openshift.io"}, + Verbs: []string{"use"}, + ResourceNames: []string{sccName}, + }, + }, + } +} diff --git a/pkg/provision/workspace/rbac/role_test.go b/pkg/provision/workspace/rbac/role_test.go new file mode 100644 index 000000000..389e98f1a --- /dev/null +++ b/pkg/provision/workspace/rbac/role_test.go @@ -0,0 +1,113 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "testing" + + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" + "github.com/stretchr/testify/assert" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/types" +) + +func TestCreatesRoleIfNotExists(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspace("test-devworkspace") + api := getTestClusterAPI(t, testdw.DevWorkspace) + err := syncRoles(testdw, api) + retryErr := &RetryError{} + if assert.Error(t, err, "Should return RetryError to indicate that role was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRoles(testdw, api) + assert.NoError(t, err, "Should not return error if role is in sync") + actualRole := &rbacv1.Role{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceRoleName(), + Namespace: testNamespace, + }, actualRole) + assert.NoError(t, err, "Role should be created") +} + +func TestDoesNothingIfRoleAlreadyInSync(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspace("test-devworkspace") + api := getTestClusterAPI(t, testdw.DevWorkspace) + err := syncRoles(testdw, api) + retryErr := &RetryError{} + if assert.Error(t, err, "Should return RetryError to indicate that role was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRoles(testdw, api) + assert.NoError(t, err, "Should not return error if role is in sync") + actualRole := &rbacv1.Role{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceRoleName(), + Namespace: testNamespace, + }, actualRole) + assert.NoError(t, err, "Role should be created") + err = syncRoles(testdw, api) + assert.NoError(t, err, "Should not return error if role is in sync") +} + +func TestCreatesSCCRoleIfNotExists(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) + testdw := getTestDevWorkspaceWithAttributes(t, "test-devworkspace", constants.WorkspaceSCCAttribute, testSCCName) + api := getTestClusterAPI(t, testdw.DevWorkspace) + retryErr := &RetryError{} + err := syncRoles(testdw, api) + if assert.Error(t, err, "Should return RetryError to indicate that role was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRoles(testdw, api) + if assert.Error(t, err, "Should return RetryError to indicate that SCC role was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRoles(testdw, api) + assert.NoError(t, err, "Should not return error if roles are in sync") + actualRole := &rbacv1.Role{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceSCCRoleName(testSCCName), + Namespace: testNamespace, + }, actualRole) + assert.NoError(t, err, "Role should be created") +} + +func TestDoesNothingIfSCCRoleAlreadyInSync(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) + testdw := getTestDevWorkspaceWithAttributes(t, "test-devworkspace", constants.WorkspaceSCCAttribute, testSCCName) + api := getTestClusterAPI(t, testdw.DevWorkspace) + retryErr := &RetryError{} + err := syncRoles(testdw, api) + if assert.Error(t, err, "Should return RetryError to indicate that role was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRoles(testdw, api) + if assert.Error(t, err, "Should return RetryError to indicate that SCC role was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRoles(testdw, api) + assert.NoError(t, err, "Should not return error if roles are in sync") + actualRole := &rbacv1.Role{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceSCCRoleName(testSCCName), + Namespace: testNamespace, + }, actualRole) + assert.NoError(t, err, "Role should be created") + err = syncRoles(testdw, api) + assert.NoError(t, err, "Should not return error if role is in sync") +} diff --git a/pkg/provision/workspace/rbac/rolebinding.go b/pkg/provision/workspace/rbac/rolebinding.go new file mode 100644 index 000000000..f4e0ab0e8 --- /dev/null +++ b/pkg/provision/workspace/rbac/rolebinding.go @@ -0,0 +1,157 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "fmt" + + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/constants" + "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" +) + +func syncRolebindings(workspace *common.DevWorkspaceWithConfig, api sync.ClusterAPI) error { + saName := common.ServiceAccountName(workspace) + defaultRoleName := common.WorkspaceRoleName() + defaultRolebindingName := common.WorkspaceRolebindingName() + if err := addServiceAccountToRolebinding(saName, workspace.Namespace, defaultRoleName, defaultRolebindingName, api); err != nil { + return err + } + if !workspace.Spec.Template.Attributes.Exists(constants.WorkspaceSCCAttribute) { + return nil + } + sccName := workspace.Spec.Template.Attributes.GetString(constants.WorkspaceSCCAttribute, nil) + sccRoleName := common.WorkspaceSCCRoleName(sccName) + sccRolebindingName := common.WorkspaceSCCRolebindingName(sccName) + if err := addServiceAccountToRolebinding(saName, workspace.Namespace, sccRoleName, sccRolebindingName, api); err != nil { + return err + } + return nil +} + +func deleteRolebinding(name, namespace string, api sync.ClusterAPI) error { + rolebinding := &rbacv1.RoleBinding{} + namespacedName := types.NamespacedName{ + Name: name, + Namespace: namespace, + } + err := api.Client.Get(api.Ctx, namespacedName, rolebinding) + switch { + case err == nil: + if err := api.Client.Delete(api.Ctx, rolebinding); err != nil { + return &RetryError{fmt.Errorf("failed to delete rolebinding %s in namespace %s: %w", name, namespace, err)} + } + return &RetryError{fmt.Errorf("deleted rolebinding %s in namespace %s", name, namespace)} + case k8sErrors.IsNotFound(err): + // Already deleted + return nil + default: + return err + } +} + +func addServiceAccountToRolebinding(saName, namespace, roleName, rolebindingName string, api sync.ClusterAPI) error { + rolebinding := &rbacv1.RoleBinding{} + namespacedName := types.NamespacedName{ + Name: rolebindingName, + Namespace: namespace, + } + err := api.Client.Get(api.Ctx, namespacedName, rolebinding) + switch { + case err == nil: + // Got existing rolebinding, need to make sure SA is part of it + break + case k8sErrors.IsNotFound(err): + // Rolebinding not created yet, initiailize default rolebinding and add SA to it + rolebinding = generateDefaultRolebinding(rolebindingName, namespace, roleName) + default: + return err + } + if !rolebindingHasSubject(rolebinding, saName, namespace) { + rolebinding.Subjects = append(rolebinding.Subjects, rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: saName, + Namespace: namespace, + }) + } + + if _, err = sync.SyncObjectWithCluster(rolebinding, api); err != nil { + return wrapSyncError(err) + } + return nil +} + +func removeServiceAccountFromRolebinding(saName, namespace, roleBindingName string, api sync.ClusterAPI) error { + rolebinding := &rbacv1.RoleBinding{} + namespacedName := types.NamespacedName{ + Name: roleBindingName, + Namespace: namespace, + } + err := api.Client.Get(api.Ctx, namespacedName, rolebinding) + switch { + case err == nil: + // Found rolebinding, ensure saName is not a subject + break + case k8sErrors.IsNotFound(err): + // Rolebinding does not exist; nothing to do + return nil + default: + return err + } + if !rolebindingHasSubject(rolebinding, saName, namespace) { + return nil + } + var newSubjects []rbacv1.Subject + for _, subject := range rolebinding.Subjects { + if subject.Kind == rbacv1.ServiceAccountKind && subject.Name == saName && subject.Namespace == namespace { + continue + } + newSubjects = append(newSubjects, subject) + } + rolebinding.Subjects = newSubjects + if _, err := sync.SyncObjectWithCluster(rolebinding, api); err != nil { + return wrapSyncError(err) + } + return nil +} + +func generateDefaultRolebinding(name, namespace, roleName string) *rbacv1.RoleBinding { + return &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: rbacLabels, + }, + RoleRef: rbacv1.RoleRef{ + Kind: "Role", + Name: roleName, + }, + // Subjects added for each workspace ServiceAccount + Subjects: []rbacv1.Subject{}, + } +} + +func rolebindingHasSubject(rolebinding *rbacv1.RoleBinding, saName, namespace string) bool { + for _, subject := range rolebinding.Subjects { + if subject.Kind == rbacv1.ServiceAccountKind && subject.Name == saName && subject.Namespace == namespace { + return true + } + } + return false +} diff --git a/pkg/provision/workspace/rbac/rolebinding_test.go b/pkg/provision/workspace/rbac/rolebinding_test.go new file mode 100644 index 000000000..0a19c9a91 --- /dev/null +++ b/pkg/provision/workspace/rbac/rolebinding_test.go @@ -0,0 +1,154 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// 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 rbac + +import ( + "testing" + + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" + "github.com/stretchr/testify/assert" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/types" +) + +func TestCreatesRolebindingIfNotExists(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspace("test-devworkspace") + api := getTestClusterAPI(t, testdw.DevWorkspace) + err := syncRolebindings(testdw, api) + retryErr := &RetryError{} + if assert.Error(t, err, "Should return RetryError to indicate that rolebinding was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRolebindings(testdw, api) + assert.NoError(t, err, "Should not return error if rolebinding is in sync") + actualRB := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceRolebindingName(), + Namespace: testNamespace, + }, actualRB) + assert.NoError(t, err, "Rolebinding should be created") + assert.Equal(t, common.WorkspaceRoleName(), actualRB.RoleRef.Name, "Rolebinding shold reference default role") + expectedSAName := common.ServiceAccountName(testdw) + assert.True(t, testHasSubject(expectedSAName, testNamespace, actualRB), "Created rolebinding should have workspace SA as subject") +} + +func TestAddsMultipleSubjectsToRolebinding(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + testdw := getTestDevWorkspace("test-devworkspace") + testdw2 := getTestDevWorkspace("test-devworkspace-2") + api := getTestClusterAPI(t, testdw.DevWorkspace) + err := syncRolebindings(testdw, api) + retryErr := &RetryError{} + if assert.Error(t, err, "Should return RetryError to indicate that rolebinding was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRolebindings(testdw, api) + assert.NoError(t, err, "Should not return error if rolebinding is in sync") + err = syncRolebindings(testdw2, api) + if assert.Error(t, err, "Should return RetryError to indicate that rolebinding was updated") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRolebindings(testdw2, api) + assert.NoError(t, err, "Should not return error if rolebinding is in sync") + + actualRB := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceRolebindingName(), + Namespace: testNamespace, + }, actualRB) + assert.NoError(t, err, "Rolebinding should be created") + assert.Equal(t, common.WorkspaceRoleName(), actualRB.RoleRef.Name, "Rolebinding shold reference default role") + expectedSAName := common.ServiceAccountName(testdw) + assert.True(t, testHasSubject(expectedSAName, testNamespace, actualRB), "Created rolebinding should have both workspace SAs as subjects") + expectedSAName2 := common.ServiceAccountName(testdw2) + assert.True(t, testHasSubject(expectedSAName2, testNamespace, actualRB), "Created rolebinding should have both workspace SAs as subjects") +} + +func TestCreatesSCCRolebindingIfNotExists(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) + testdw := getTestDevWorkspaceWithAttributes(t, "test-devworkspace", constants.WorkspaceSCCAttribute, testSCCName) + api := getTestClusterAPI(t, testdw.DevWorkspace) + retryErr := &RetryError{} + err := syncRolebindings(testdw, api) + if assert.Error(t, err, "Should return RetryError to indicate that default rolebinding was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRolebindings(testdw, api) + if assert.Error(t, err, "Should return RetryError to indicate that SCC rolebinding was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRolebindings(testdw, api) + assert.NoError(t, err, "Should not return error if rolebindings are in sync") + actualRB := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceSCCRolebindingName(testSCCName), + Namespace: testNamespace, + }, actualRB) + assert.NoError(t, err, "Rolebinding should be created") + assert.Equal(t, common.WorkspaceSCCRoleName(testSCCName), actualRB.RoleRef.Name, "Rolebinding shold reference default role") + expectedSAName := common.ServiceAccountName(testdw) + assert.True(t, testHasSubject(expectedSAName, testNamespace, actualRB), "Created rolebinding should have workspace SA as subject") +} + +func TestAddsMultipleSubjectsToSCCRolebinding(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) + testdw := getTestDevWorkspaceWithAttributes(t, "test-devworkspace", constants.WorkspaceSCCAttribute, testSCCName) + testdw2 := getTestDevWorkspaceWithAttributes(t, "test-devworkspace-2", constants.WorkspaceSCCAttribute, testSCCName) + api := getTestClusterAPI(t, testdw.DevWorkspace) + retryErr := &RetryError{} + err := syncRolebindings(testdw, api) + if assert.Error(t, err, "Should return RetryError to indicate that default rolebinding was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRolebindings(testdw, api) + if assert.Error(t, err, "Should return RetryError to indicate that SCC rolebinding was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRolebindings(testdw, api) + assert.NoError(t, err, "Should not return error if rolebindings are in sync") + err = syncRolebindings(testdw2, api) + if assert.Error(t, err, "Should return RetryError to indicate that default rolebinding was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRolebindings(testdw2, api) + if assert.Error(t, err, "Should return RetryError to indicate that SCC rolebinding was created") { + assert.ErrorAs(t, err, &retryErr, "Error should have RetryError type") + } + err = syncRolebindings(testdw2, api) + assert.NoError(t, err, "Should not return error if rolebindings are in sync") + + actualRB := &rbacv1.RoleBinding{} + err = api.Client.Get(api.Ctx, types.NamespacedName{ + Name: common.WorkspaceSCCRolebindingName(testSCCName), + Namespace: testNamespace, + }, actualRB) + assert.NoError(t, err, "Rolebinding should be created") + assert.Equal(t, common.WorkspaceSCCRoleName(testSCCName), actualRB.RoleRef.Name, "Rolebinding shold reference default role") + expectedSAName := common.ServiceAccountName(testdw) + assert.True(t, testHasSubject(expectedSAName, testNamespace, actualRB), "Created SCC rolebinding should have both workspace SAs as subjects") + expectedSAName2 := common.ServiceAccountName(testdw2) + assert.True(t, testHasSubject(expectedSAName2, testNamespace, actualRB), "Created SCC rolebinding should have both workspace SAs as subjects") +} + +func testHasSubject(subjName, namespace string, rolebinding *rbacv1.RoleBinding) bool { + for _, subject := range rolebinding.Subjects { + if subject.Name == subjName && subject.Namespace == namespace && subject.Kind == rbacv1.ServiceAccountKind { + return true + } + } + return false +} diff --git a/pkg/provision/workspace/serviceaccount.go b/pkg/provision/workspace/serviceaccount.go index 637961aa2..fa0793136 100644 --- a/pkg/provision/workspace/serviceaccount.go +++ b/pkg/provision/workspace/serviceaccount.go @@ -19,7 +19,6 @@ import ( "context" "fmt" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/provision/sync" securityv1 "github.com/openshift/api/security/v1" @@ -94,17 +93,6 @@ func SyncServiceAccount( return ServiceAcctProvisioningStatus{ProvisioningStatus: ProvisioningStatus{Err: err}} } - if workspace.Spec.Template.Attributes.Exists(constants.WorkspaceSCCAttribute) { - sccName := workspace.Spec.Template.Attributes.GetString(constants.WorkspaceSCCAttribute, nil) - retry, err := addSCCToServiceAccount(specSA.Name, specSA.Namespace, sccName, clusterAPI) - if err != nil { - return ServiceAcctProvisioningStatus{ProvisioningStatus: ProvisioningStatus{FailStartup: true, Message: err.Error()}} - } - if retry { - return ServiceAcctProvisioningStatus{ProvisioningStatus: ProvisioningStatus{Requeue: true}} - } - } - return ServiceAcctProvisioningStatus{ ProvisioningStatus: ProvisioningStatus{ Continue: true, @@ -113,16 +101,10 @@ func SyncServiceAccount( } } -func NeedsServiceAccountFinalizer(workspace *dw.DevWorkspaceTemplateSpec) bool { - if !workspace.Attributes.Exists(constants.WorkspaceSCCAttribute) { - return false - } - if workspace.Attributes.GetString(constants.WorkspaceSCCAttribute, nil) == "" { - return false - } - return true -} - +// FinalizeServiceAccount removes the workspace service account from the SCC specified by the controller.devfile.io/scc attribute. +// +// Deprecated: This should no longer be needed as the serviceaccount finalizer is no longer added to workspaces (and workspaces +// do not update SCCs) but is kept here in order to clear finalizers from existing workspaces on deletion. func FinalizeServiceAccount(workspace *common.DevWorkspaceWithConfig, ctx context.Context, nonCachingClient crclient.Client) (retry bool, err error) { saName := common.ServiceAccountName(workspace) namespace := workspace.Namespace @@ -134,43 +116,8 @@ func FinalizeServiceAccount(workspace *common.DevWorkspaceWithConfig, ctx contex return removeSCCFromServiceAccount(saName, namespace, sccName, ctx, nonCachingClient) } -func addSCCToServiceAccount(saName, namespace, sccName string, clusterAPI sync.ClusterAPI) (retry bool, err error) { - serviceaccount := fmt.Sprintf("system:serviceaccount:%s:%s", namespace, saName) - - scc := &securityv1.SecurityContextConstraints{} - if err := clusterAPI.NonCachingClient.Get(clusterAPI.Ctx, types.NamespacedName{Name: sccName}, scc); err != nil { - switch { - case k8sErrors.IsForbidden(err): - return false, fmt.Errorf("operator does not have permissions to get the '%s' SecurityContextConstraints", sccName) - case k8sErrors.IsNotFound(err): - return false, fmt.Errorf("requested SecurityContextConstraints '%s' not found on cluster", sccName) - default: - return false, err - } - } - - for _, user := range scc.Users { - if user == serviceaccount { - // This serviceaccount is already added to the SCC - return false, nil - } - } - - scc.Users = append(scc.Users, serviceaccount) - if err := clusterAPI.NonCachingClient.Update(clusterAPI.Ctx, scc); err != nil { - switch { - case k8sErrors.IsForbidden(err): - return false, fmt.Errorf("operator does not have permissions to update the '%s' SecurityContextConstraints", sccName) - case k8sErrors.IsConflict(err): - return true, nil - default: - return false, err - } - } - - return false, nil -} - +// Deprecated: This function is left in place ot ensure changes to SCCs can be undone when a workspace is deleted. However, +// the DevWorkspace Operator no longer updates SCCs, so this functionality is not required for new workspaces. func removeSCCFromServiceAccount(saName, namespace, sccName string, ctx context.Context, nonCachingClient crclient.Client) (retry bool, err error) { serviceaccount := fmt.Sprintf("system:serviceaccount:%s:%s", namespace, saName)