From 17dfedb4dfb5dcd8f622816612c8f94d0165da80 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Thu, 20 Oct 2022 10:28:07 +0300 Subject: [PATCH] feat: Automatically add the container build SCC to users if containerBuildCapability is enabled (#1543) * feat: Automatically add the container build SCC to users if containerBuildCapability is enabled Signed-off-by: Anatolii Bazko --- .../che-operator.clusterserviceversion.yaml | 5 +- config/rbac/cluster_role.yaml | 3 +- controllers/usernamespace/controller.go | 58 ++++++ controllers/usernamespace/controller_test.go | 72 +++++++ controllers/usernamespace/namespacecache.go | 9 + deploy/deployment/kubernetes/combined.yaml | 1 + .../objects/che-operator.ClusterRole.yaml | 1 + deploy/deployment/openshift/combined.yaml | 1 + .../objects/che-operator.ClusterRole.yaml | 1 + .../templates/che-operator.ClusterRole.yaml | 1 + pkg/common/constants/constants.go | 1 + pkg/deploy/clusterrole.go | 4 +- pkg/deploy/clusterrolebinding.go | 17 +- pkg/deploy/container-build/container_build.go | 184 +++++++++++------- .../container-build/container_build_test.go | 20 +- pkg/deploy/rolebinding.go | 4 +- 16 files changed, 285 insertions(+), 97 deletions(-) diff --git a/bundle/next/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml b/bundle/next/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml index 971910f58..2cb9e9177 100644 --- a/bundle/next/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml +++ b/bundle/next/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml @@ -77,7 +77,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: https://github.com/eclipse-che/che-operator support: Eclipse Foundation - name: eclipse-che-preview-openshift.v7.56.0-707.next + name: eclipse-che-preview-openshift.v7.56.0-708.next namespace: placeholder spec: apiservicedefinitions: {} @@ -872,6 +872,7 @@ spec: - create - delete - update + - use serviceAccountName: che-operator deployments: - name: che-operator @@ -1239,7 +1240,7 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.56.0-707.next + version: 7.56.0-708.next webhookdefinitions: - admissionReviewVersions: - v1 diff --git a/config/rbac/cluster_role.yaml b/config/rbac/cluster_role.yaml index 264ba16bf..22c044a5c 100644 --- a/config/rbac/cluster_role.yaml +++ b/config/rbac/cluster_role.yaml @@ -400,4 +400,5 @@ rules: - get - create - delete - - update \ No newline at end of file + - update + - use \ No newline at end of file diff --git a/controllers/usernamespace/controller.go b/controllers/usernamespace/controller.go index e5f2d3fb3..ad68fe0c5 100644 --- a/controllers/usernamespace/controller.go +++ b/controllers/usernamespace/controller.go @@ -15,8 +15,12 @@ package usernamespace import ( "context" "encoding/json" + "fmt" "strconv" + containerbuild "github.com/eclipse-che/che-operator/pkg/deploy/container-build" + rbacv1 "k8s.io/api/rbac/v1" + "github.com/eclipse-che/che-operator/pkg/common/chetypes" "github.com/eclipse-che/che-operator/pkg/common/constants" "github.com/eclipse-che/che-operator/pkg/deploy/tls" @@ -249,6 +253,11 @@ func (r *CheUserNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, err } + if err = r.reconcileSCCPrivileges(info.Username, req.Name, checluster, deployContext); err != nil { + logrus.Errorf("Failed to reconcile the SCC privileges in namespace '%s': %v", req.Name, err) + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -600,6 +609,55 @@ func (r *CheUserNamespaceReconciler) reconcileNodeSelectorAndTolerations(ctx con return r.client.Update(ctx, ns) } +func (r *CheUserNamespaceReconciler) reconcileSCCPrivileges(username string, targetNs string, checluster *chev2.CheCluster, deployContext *chetypes.DeployContext) error { + delRoleBinding := func() error { + _, err := deploy.Delete( + deployContext, + types.NamespacedName{Name: containerbuild.GetUserSccRbacResourcesName(), Namespace: targetNs}, + &rbacv1.RoleBinding{}) + return err + } + + if !checluster.IsContainerBuildCapabilitiesEnabled() { + return delRoleBinding() + } + + if username == "" { + _ = delRoleBinding() + return fmt.Errorf("unknown user for %s namespace", targetNs) + } + + rb := &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: containerbuild.GetUserSccRbacResourcesName(), + Namespace: targetNs, + Labels: map[string]string{constants.KubernetesPartOfLabelKey: constants.CheEclipseOrg}, + }, + RoleRef: rbacv1.RoleRef{ + Name: containerbuild.GetUserSccRbacResourcesName(), + Kind: "ClusterRole", + APIGroup: "rbac.authorization.k8s.io", + }, + Subjects: []rbacv1.Subject{ + { + Kind: rbacv1.UserKind, + APIGroup: "rbac.authorization.k8s.io", + Name: username, + }, + }, + } + + if _, err := deploy.Sync(deployContext, rb, deploy.RollBindingDiffOpts); err != nil { + return err + } + + return nil +} + func prefixedName(name string) string { return "che-" + name } diff --git a/controllers/usernamespace/controller_test.go b/controllers/usernamespace/controller_test.go index 5fcb4e32c..ed78e6644 100644 --- a/controllers/usernamespace/controller_test.go +++ b/controllers/usernamespace/controller_test.go @@ -18,6 +18,9 @@ import ( "sync" "testing" + containerbuild "github.com/eclipse-che/che-operator/pkg/deploy/container-build" + rbacv1 "k8s.io/api/rbac/v1" + dwconstants "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/infrastructure" devworkspaceinfra "github.com/devfile/devworkspace-operator/pkg/infrastructure" @@ -431,6 +434,75 @@ func TestCreatesDataInNamespace(t *testing.T) { }) } +func TestUpdateSccClusterRoleBinding(t *testing.T) { + infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) + + pr1 := &projectv1.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1", + Labels: map[string]string{ + workspaceNamespaceOwnerUidLabel: "uid_1", + }, + Annotations: map[string]string{ + cheUsernameAnnotation: "user_1", + }, + }, + } + + ns1 := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1", + Labels: map[string]string{ + workspaceNamespaceOwnerUidLabel: "uid_1", + }, + Annotations: map[string]string{ + cheUsernameAnnotation: "user_1", + }, + }, + } + + cheCluster := &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eclipse-che", + Namespace: "eclipse-che", + }, + Spec: chev2.CheClusterSpec{ + DevEnvironments: chev2.CheClusterDevEnvironments{ + DisableContainerBuildCapabilities: pointer.BoolPtr(false), + ContainerBuildConfiguration: &chev2.ContainerBuildConfiguration{ + OpenShiftSecurityContextConstraint: "container-build", + }, + }, + Networking: chev2.CheClusterSpecNetworking{ + Domain: "root-domain", + }, + }, + Status: chev2.CheClusterStatus{ + CheURL: "https://che-host", + }, + } + + allObjs := []runtime.Object{ns1, pr1, cheCluster} + scheme, cl, usernamespaceReconciler := setup(infrastructure.OpenShiftv4, allObjs...) + + // the reconciliation needs to run twice for it to be truly finished - we're setting up finalizers etc... + devworkspaceReconciler := devworkspace.New(cl, scheme) + if _, err := devworkspaceReconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "eclipse-che", Namespace: "eclipse-che"}}); err != nil { + t.Fatal(err) + } + if _, err := devworkspaceReconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "eclipse-che", Namespace: "eclipse-che"}}); err != nil { + t.Fatal(err) + } + + _, err := usernamespaceReconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: ns1.GetName()}}) + assert.Nil(t, err) + + rb := &rbacv1.RoleBinding{} + err = cl.Get(context.TODO(), types.NamespacedName{Name: containerbuild.GetUserSccRbacResourcesName(), Namespace: "ns1"}, rb) + assert.Nil(t, err) + assert.Equal(t, "user_1", rb.Subjects[0].Name) +} + func TestWatchRulesForSecretsInSameNamespace(t *testing.T) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/controllers/usernamespace/namespacecache.go b/controllers/usernamespace/namespacecache.go index 8672656c1..5eb9eafad 100644 --- a/controllers/usernamespace/namespacecache.go +++ b/controllers/usernamespace/namespacecache.go @@ -33,6 +33,7 @@ const ( chePartOfLabelValue string = "che.eclipse.org" cheComponentLabel string = "app.kubernetes.io/component" cheComponentLabelValue string = "workspaces-namespace" + cheUsernameAnnotation string = "che.eclipse.org/username" ) type namespaceCache struct { @@ -43,6 +44,7 @@ type namespaceCache struct { type namespaceInfo struct { IsWorkspaceNamespace bool + Username string CheCluster *types.NamespacedName } @@ -120,6 +122,11 @@ func (c *namespaceCache) examineNamespaceUnsafe(ctx context.Context, ns string) labels = map[string]string{} } + annotations := namespace.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + // ownerUid is the legacy label that we used to use. Let's not break the existing workspace namespaces and still // recognize it ownerUid := labels[workspaceNamespaceOwnerUidLabel] @@ -127,9 +134,11 @@ func (c *namespaceCache) examineNamespaceUnsafe(ctx context.Context, ns string) cheNamespace := labels[cheNamespaceLabel] partOfLabel := labels[chePartOfLabel] componentLabel := labels[cheComponentLabel] + username := annotations[cheUsernameAnnotation] ret := namespaceInfo{ IsWorkspaceNamespace: ownerUid != "" || (partOfLabel == chePartOfLabelValue && componentLabel == cheComponentLabelValue), + Username: username, CheCluster: &types.NamespacedName{ Name: cheName, Namespace: cheNamespace, diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 6f191a06d..d32532d07 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -5629,6 +5629,7 @@ rules: - create - delete - update + - use --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding diff --git a/deploy/deployment/kubernetes/objects/che-operator.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/che-operator.ClusterRole.yaml index 11e38211b..8579174d2 100644 --- a/deploy/deployment/kubernetes/objects/che-operator.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/che-operator.ClusterRole.yaml @@ -401,3 +401,4 @@ rules: - create - delete - update + - use diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index f299c3368..b2543f816 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -5629,6 +5629,7 @@ rules: - create - delete - update + - use --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding diff --git a/deploy/deployment/openshift/objects/che-operator.ClusterRole.yaml b/deploy/deployment/openshift/objects/che-operator.ClusterRole.yaml index 11e38211b..8579174d2 100644 --- a/deploy/deployment/openshift/objects/che-operator.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/che-operator.ClusterRole.yaml @@ -401,3 +401,4 @@ rules: - create - delete - update + - use diff --git a/helmcharts/next/templates/che-operator.ClusterRole.yaml b/helmcharts/next/templates/che-operator.ClusterRole.yaml index 11e38211b..8579174d2 100644 --- a/helmcharts/next/templates/che-operator.ClusterRole.yaml +++ b/helmcharts/next/templates/che-operator.ClusterRole.yaml @@ -401,3 +401,4 @@ rules: - create - delete - update + - use diff --git a/pkg/common/constants/constants.go b/pkg/common/constants/constants.go index 4d717c7b8..49c1bdbc0 100644 --- a/pkg/common/constants/constants.go +++ b/pkg/common/constants/constants.go @@ -128,6 +128,7 @@ const ( // DevWorkspace DevWorkspaceServiceAccountName = "devworkspace-controller-serviceaccount" + DefaultContainerBuildSccName = "container-build" ) var ( diff --git a/pkg/deploy/clusterrole.go b/pkg/deploy/clusterrole.go index 1ae41242c..1c749d744 100644 --- a/pkg/deploy/clusterrole.go +++ b/pkg/deploy/clusterrole.go @@ -21,7 +21,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -var crDiffOpts = cmp.Options{ +var ClusterRoleDiffOpts = cmp.Options{ cmpopts.IgnoreFields(rbac.ClusterRole{}, "TypeMeta", "ObjectMeta"), } @@ -31,7 +31,7 @@ func SyncClusterRoleToCluster( policyRule []rbac.PolicyRule) (bool, error) { crSpec := getClusterRoleSpec(deployContext, name, policyRule) - return Sync(deployContext, crSpec, crDiffOpts) + return Sync(deployContext, crSpec, ClusterRoleDiffOpts) } func getClusterRoleSpec(deployContext *chetypes.DeployContext, name string, policyRule []rbac.PolicyRule) *rbac.ClusterRole { diff --git a/pkg/deploy/clusterrolebinding.go b/pkg/deploy/clusterrolebinding.go index 0d886697c..74fbb0709 100644 --- a/pkg/deploy/clusterrolebinding.go +++ b/pkg/deploy/clusterrolebinding.go @@ -24,7 +24,7 @@ import ( "k8s.io/apimachinery/pkg/types" ) -var crbDiffOpts = cmp.Options{ +var ClusterRoleBindingDiffOpts = cmp.Options{ cmpopts.IgnoreFields(rbac.ClusterRoleBinding{}, "TypeMeta", "ObjectMeta"), } @@ -35,18 +35,7 @@ func SyncClusterRoleBindingToCluster( clusterRoleName string) (bool, error) { crbSpec := getClusterRoleBindingSpec(deployContext, name, serviceAccountName, deployContext.CheCluster.Namespace, clusterRoleName) - return Sync(deployContext, crbSpec, crbDiffOpts) -} - -func SyncClusterRoleBindingToClusterInGivenNamespace( - deployContext *chetypes.DeployContext, - name string, - serviceAccountName string, - serviceAccountNamespace string, - clusterRoleName string) (bool, error) { - - crbSpec := getClusterRoleBindingSpec(deployContext, name, serviceAccountName, serviceAccountNamespace, clusterRoleName) - return Sync(deployContext, crbSpec, crbDiffOpts) + return Sync(deployContext, crbSpec, ClusterRoleBindingDiffOpts) } func SyncClusterRoleBindingAndAddFinalizerToCluster( @@ -57,7 +46,7 @@ func SyncClusterRoleBindingAndAddFinalizerToCluster( finalizer := GetFinalizerName(strings.ToLower(name) + ".crb") crbSpec := getClusterRoleBindingSpec(deployContext, name, serviceAccountName, deployContext.CheCluster.Namespace, clusterRoleName) - return SyncAndAddFinalizer(deployContext, crbSpec, crbDiffOpts, finalizer) + return SyncAndAddFinalizer(deployContext, crbSpec, ClusterRoleBindingDiffOpts, finalizer) } func ReconcileClusterRoleBindingFinalizer(deployContext *chetypes.DeployContext, name string) error { diff --git a/pkg/deploy/container-build/container_build.go b/pkg/deploy/container-build/container_build.go index f462f1025..27145c738 100644 --- a/pkg/deploy/container-build/container_build.go +++ b/pkg/deploy/container-build/container_build.go @@ -39,6 +39,9 @@ func NewContainerBuildReconciler() *ContainerBuildReconciler { func (cb *ContainerBuildReconciler) Reconcile(ctx *chetypes.DeployContext) (reconcile.Result, bool, error) { if ctx.CheCluster.IsContainerBuildCapabilitiesEnabled() { + // If container build capabilities are enabled, then container build configuration is supposed to be set + // with default values, see `api/v2/checluster_webhook.go` and `api/v2/checluster_types.go` (for default values). + // The check below to avoid NPE while CheCluster is not updated with defaults. if ctx.CheCluster.IsOpenShiftSecurityContextConstraintSet() { if done, err := cb.syncSCC(ctx); !done { return reconcile.Result{}, false, err @@ -91,40 +94,121 @@ func (cb *ContainerBuildReconciler) Finalize(ctx *chetypes.DeployContext) bool { } func (cb *ContainerBuildReconciler) syncSCC(ctx *chetypes.DeployContext) (bool, error) { - if exists, err := deploy.GetClusterObject( - ctx, - ctx.CheCluster.Spec.DevEnvironments.ContainerBuildConfiguration.OpenShiftSecurityContextConstraint, - &securityv1.SecurityContextConstraints{}, - ); err != nil { - return false, nil - } else if exists { - // Don't override existed SCC - return true, nil - } - - return deploy.Sync(ctx, cb.getSCCSpec(ctx)) + _, err := deploy.CreateIfNotExists(ctx, cb.getSccSpec(ctx)) + return err == nil, err } func (cb *ContainerBuildReconciler) syncRBAC(ctx *chetypes.DeployContext) (bool, error) { - if done, err := deploy.SyncClusterRoleToCluster(ctx, cb.getClusterRoleName(), cb.getPolicyRules(ctx)); !done { + if done, err := deploy.SyncClusterRoleToCluster( + ctx, + GetDevWorkspaceSccRbacResourcesName(), + cb.getDevWorkspaceSccPolicyRules(ctx), + ); !done { return false, err } - if devWorkspaceServiceAccountNamespace, err := cb.getDevWorkspaceServiceAccountNamespace(ctx); devWorkspaceServiceAccountNamespace == "" { + if crb, err := cb.getDevWorkspaceSccClusterRoleBindingSpec(ctx); err != nil { return false, err } else { - return deploy.SyncClusterRoleBindingToClusterInGivenNamespace( - ctx, - cb.getClusterRoleBindingName(), - constants.DevWorkspaceServiceAccountName, - devWorkspaceServiceAccountNamespace, - cb.getClusterRoleName()) + if done, err := deploy.Sync(ctx, crb, deploy.ClusterRoleBindingDiffOpts); !done { + return false, err + } } + + if done, err := deploy.SyncClusterRoleToCluster( + ctx, + GetUserSccRbacResourcesName(), + cb.getUserSccPolicyRules(ctx), + ); !done { + return false, err + } + + return true, nil +} + +func (cb *ContainerBuildReconciler) removeSCC(ctx *chetypes.DeployContext) (bool, error) { + sccName := constants.DefaultContainerBuildSccName + if ctx.CheCluster.IsOpenShiftSecurityContextConstraintSet() { + sccName = ctx.CheCluster.Spec.DevEnvironments.ContainerBuildConfiguration.OpenShiftSecurityContextConstraint + } + + scc := &securityv1.SecurityContextConstraints{} + if exists, err := deploy.GetClusterObject(ctx, sccName, scc); !exists { + return err == nil, err + } + + if scc.Labels[constants.KubernetesManagedByLabelKey] == deploy.GetManagedByLabel() { + // Removes only if it is managed by operator + return deploy.DeleteClusterObject(ctx, sccName, &securityv1.SecurityContextConstraints{}) + } + + return true, nil +} + +func (cb *ContainerBuildReconciler) removeRBAC(ctx *chetypes.DeployContext) (bool, error) { + if done, err := deploy.DeleteClusterObject(ctx, GetDevWorkspaceSccRbacResourcesName(), &rbacv1.ClusterRole{}); !done { + return false, err + } + + if done, err := deploy.DeleteClusterObject(ctx, GetDevWorkspaceSccRbacResourcesName(), &rbacv1.ClusterRoleBinding{}); !done { + return false, err + } + + if done, err := deploy.DeleteClusterObject(ctx, GetUserSccRbacResourcesName(), &rbacv1.ClusterRole{}); !done { + return false, err + } + + return true, nil +} + +func (cb *ContainerBuildReconciler) getFinalizerName() string { + return "container-build.finalizers.che.eclipse.org" +} + +func (cb *ContainerBuildReconciler) getDevWorkspaceSccPolicyRules(ctx *chetypes.DeployContext) []rbacv1.PolicyRule { + return []rbacv1.PolicyRule{ + { + APIGroups: []string{"security.openshift.io"}, + Resources: []string{"securitycontextconstraints"}, + Verbs: []string{"get", "update"}, + ResourceNames: []string{ctx.CheCluster.Spec.DevEnvironments.ContainerBuildConfiguration.OpenShiftSecurityContextConstraint}, + }, + } +} + +func (cb *ContainerBuildReconciler) getDevWorkspaceSccClusterRoleBindingSpec(ctx *chetypes.DeployContext) (*rbacv1.ClusterRoleBinding, error) { + devWorkspaceServiceAccountNamespace, err := cb.getDevWorkspaceServiceAccountNamespace(ctx) + if devWorkspaceServiceAccountNamespace == "" { + return nil, err + } + + return &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: GetDevWorkspaceSccRbacResourcesName(), + Labels: deploy.GetLabels(defaults.GetCheFlavor()), + }, + Subjects: []rbacv1.Subject{ + { + Kind: rbacv1.ServiceAccountKind, + Name: constants.DevWorkspaceServiceAccountName, + Namespace: devWorkspaceServiceAccountNamespace, + }, + }, + RoleRef: rbacv1.RoleRef{ + Name: GetDevWorkspaceSccRbacResourcesName(), + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + }, + }, nil } func (cb *ContainerBuildReconciler) getDevWorkspaceServiceAccountNamespace(ctx *chetypes.DeployContext) (string, error) { crb := &rbacv1.ClusterRoleBinding{} - if exists, err := deploy.GetClusterObject(ctx, cb.getClusterRoleBindingName(), crb); err != nil { + if exists, err := deploy.GetClusterObject(ctx, GetDevWorkspaceSccRbacResourcesName(), crb); err != nil { return "", err } else if exists { return crb.Subjects[0].Namespace, nil @@ -144,62 +228,18 @@ func (cb *ContainerBuildReconciler) getDevWorkspaceServiceAccountNamespace(ctx * return "", fmt.Errorf("ServiceAccount %s not found", constants.DevWorkspaceServiceAccountName) } -func (cb *ContainerBuildReconciler) removeSCC(ctx *chetypes.DeployContext) (bool, error) { - if ctx.CheCluster.Spec.DevEnvironments.ContainerBuildConfiguration == nil { - return true, nil - } - - sccName := ctx.CheCluster.Spec.DevEnvironments.ContainerBuildConfiguration.OpenShiftSecurityContextConstraint - if sccName != "" { - scc := &securityv1.SecurityContextConstraints{} - if exists, err := deploy.GetClusterObject(ctx, sccName, scc); !exists { - return err == nil, err - } - - if scc.Labels[constants.KubernetesManagedByLabelKey] == deploy.GetManagedByLabel() { - // Removes only if it is managed by operator - return deploy.DeleteClusterObject(ctx, sccName, &securityv1.SecurityContextConstraints{}) - } - } - - return true, nil -} - -func (cb *ContainerBuildReconciler) removeRBAC(ctx *chetypes.DeployContext) (bool, error) { - if done, err := deploy.DeleteClusterObject(ctx, cb.getClusterRoleName(), &rbacv1.ClusterRole{}); !done { - return false, err - } - - if done, err := deploy.DeleteClusterObject(ctx, cb.getClusterRoleBindingName(), &rbacv1.ClusterRoleBinding{}); !done { - return false, err - } - - return true, nil -} - -func (cb *ContainerBuildReconciler) getClusterRoleName() string { - return defaults.GetCheFlavor() + "-container-build-scc" -} -func (cb *ContainerBuildReconciler) getClusterRoleBindingName() string { - return defaults.GetCheFlavor() + "-container-build-scc" -} - -func (cb *ContainerBuildReconciler) getFinalizerName() string { - return "container-build.finalizers.che.eclipse.org" -} - -func (cb *ContainerBuildReconciler) getPolicyRules(ctx *chetypes.DeployContext) []rbacv1.PolicyRule { +func (cb *ContainerBuildReconciler) getUserSccPolicyRules(ctx *chetypes.DeployContext) []rbacv1.PolicyRule { return []rbacv1.PolicyRule{ { APIGroups: []string{"security.openshift.io"}, Resources: []string{"securitycontextconstraints"}, + Verbs: []string{"use"}, ResourceNames: []string{ctx.CheCluster.Spec.DevEnvironments.ContainerBuildConfiguration.OpenShiftSecurityContextConstraint}, - Verbs: []string{"get", "update"}, }, } } -func (cb *ContainerBuildReconciler) getSCCSpec(ctx *chetypes.DeployContext) *securityv1.SecurityContextConstraints { +func (cb *ContainerBuildReconciler) getSccSpec(ctx *chetypes.DeployContext) *securityv1.SecurityContextConstraints { return &securityv1.SecurityContextConstraints{ TypeMeta: metav1.TypeMeta{ Kind: "SecurityContextConstraints", @@ -238,3 +278,11 @@ func (cb *ContainerBuildReconciler) getSCCSpec(ctx *chetypes.DeployContext) *sec }, } } + +func GetUserSccRbacResourcesName() string { + return defaults.GetCheFlavor() + "-user-container-build" +} + +func GetDevWorkspaceSccRbacResourcesName() string { + return "dev-workspace-container-build" +} diff --git a/pkg/deploy/container-build/container_build_test.go b/pkg/deploy/container-build/container_build_test.go index c67d7e205..07c53dc62 100644 --- a/pkg/deploy/container-build/container_build_test.go +++ b/pkg/deploy/container-build/container_build_test.go @@ -57,8 +57,9 @@ func TestContainerBuildReconciler(t *testing.T) { assert.Nil(t, err) assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "scc"}, &securityv1.SecurityContextConstraints{})) - assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: containerBuildReconciler.getClusterRoleName()}, &rbacv1.ClusterRole{})) - assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: containerBuildReconciler.getClusterRoleBindingName()}, &rbacv1.ClusterRoleBinding{})) + assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetDevWorkspaceSccRbacResourcesName()}, &rbacv1.ClusterRole{})) + assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetDevWorkspaceSccRbacResourcesName()}, &rbacv1.ClusterRoleBinding{})) + assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetUserSccRbacResourcesName()}, &rbacv1.ClusterRole{})) assert.True(t, utils.Contains(ctx.CheCluster.Finalizers, containerBuildReconciler.getFinalizerName())) // Disable Container build capabilities @@ -69,8 +70,9 @@ func TestContainerBuildReconciler(t *testing.T) { assert.Nil(t, err) assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "scc"}, &securityv1.SecurityContextConstraints{})) - assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: containerBuildReconciler.getClusterRoleName()}, &rbacv1.ClusterRole{})) - assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: containerBuildReconciler.getClusterRoleBindingName()}, &rbacv1.ClusterRoleBinding{})) + assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetDevWorkspaceSccRbacResourcesName()}, &rbacv1.ClusterRole{})) + assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetDevWorkspaceSccRbacResourcesName()}, &rbacv1.ClusterRoleBinding{})) + assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetUserSccRbacResourcesName()}, &rbacv1.ClusterRole{})) assert.False(t, utils.Contains(ctx.CheCluster.Finalizers, containerBuildReconciler.getFinalizerName())) } @@ -95,15 +97,17 @@ func TestSyncAndRemoveRBAC(t *testing.T) { assert.True(t, done) assert.Nil(t, err) - assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: containerBuildReconciler.getClusterRoleName()}, &rbacv1.ClusterRole{})) - assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: containerBuildReconciler.getClusterRoleBindingName()}, &rbacv1.ClusterRoleBinding{})) + assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetDevWorkspaceSccRbacResourcesName()}, &rbacv1.ClusterRole{})) + assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetDevWorkspaceSccRbacResourcesName()}, &rbacv1.ClusterRoleBinding{})) + assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetUserSccRbacResourcesName()}, &rbacv1.ClusterRole{})) done, err = containerBuildReconciler.removeRBAC(ctx) assert.True(t, done) assert.Nil(t, err) - assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: containerBuildReconciler.getClusterRoleName()}, &rbacv1.ClusterRole{})) - assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: containerBuildReconciler.getClusterRoleBindingName()}, &rbacv1.ClusterRoleBinding{})) + assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetDevWorkspaceSccRbacResourcesName()}, &rbacv1.ClusterRole{})) + assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetDevWorkspaceSccRbacResourcesName()}, &rbacv1.ClusterRoleBinding{})) + assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: GetUserSccRbacResourcesName()}, &rbacv1.ClusterRole{})) } func TestSyncAndRemoveSCC(t *testing.T) { diff --git a/pkg/deploy/rolebinding.go b/pkg/deploy/rolebinding.go index b4a10641a..aca472fd1 100644 --- a/pkg/deploy/rolebinding.go +++ b/pkg/deploy/rolebinding.go @@ -20,7 +20,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -var rolebindingDiffOpts = cmp.Options{ +var RollBindingDiffOpts = cmp.Options{ cmpopts.IgnoreFields(rbac.RoleBinding{}, "TypeMeta", "ObjectMeta"), } @@ -32,7 +32,7 @@ func SyncRoleBindingToCluster( roleKind string) (bool, error) { rbSpec := getRoleBindingSpec(deployContext, name, serviceAccountName, roleName, roleKind) - return Sync(deployContext, rbSpec, rolebindingDiffOpts) + return Sync(deployContext, rbSpec, RollBindingDiffOpts) } func getRoleBindingSpec(