Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to use SCCs in DevWorkspaces on OpenShift #679

Merged
merged 7 commits into from
Dec 3, 2021
28 changes: 20 additions & 8 deletions controllers/workspace/devworkspace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ const (
// DevWorkspaceReconciler reconciles a DevWorkspace object
type DevWorkspaceReconciler struct {
client.Client
Log logr.Logger
Scheme *runtime.Scheme
NonCachingClient client.Client
Log logr.Logger
Scheme *runtime.Scheme
}

/////// CRD-related RBAC roles
Expand All @@ -84,6 +85,7 @@ type DevWorkspaceReconciler struct {
// +kubebuilder:rbac:groups="",resources=namespaces;events,verbs=get;list;watch
// +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=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
Expand All @@ -98,10 +100,11 @@ type DevWorkspaceReconciler struct {
func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (reconcileResult ctrl.Result, err error) {
reqLogger := r.Log.WithValues("Request.Namespace", req.Namespace, "Request.Name", req.Name)
clusterAPI := sync.ClusterAPI{
Client: r.Client,
Scheme: r.Scheme,
Logger: reqLogger,
Ctx: ctx,
Client: r.Client,
NonCachingClient: r.NonCachingClient,
Scheme: r.Scheme,
Logger: reqLogger,
Ctx: ctx,
}

// Fetch the Workspace instance
Expand Down Expand Up @@ -257,7 +260,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 isFinalizerNecessary(workspace, storageProvisioner) {
if storageProvisioner.NeedsStorage(&workspace.Spec.Template) {
coputil.AddFinalizer(clusterWorkspace, storageCleanupFinalizer)
if err := r.Update(ctx, clusterWorkspace); err != nil {
return reconcile.Result{}, err
Expand Down Expand Up @@ -354,14 +357,23 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
}
serviceAcctStatus := wsprovision.SyncServiceAccount(workspace, saAnnotations, clusterAPI)
if !serviceAcctStatus.Continue {
// FailStartup is not possible for generating the serviceaccount
if serviceAcctStatus.FailStartup {
return r.failWorkspace(workspace, serviceAcctStatus.Message, metrics.ReasonBadRequest, reqLogger, &reconcileStatus)
}
reqLogger.Info("Waiting for workspace ServiceAccount")
reconcileStatus.setConditionFalse(dw.DevWorkspaceServiceAccountReady, "Waiting for DevWorkspace ServiceAccount")
if !serviceAcctStatus.Requeue && serviceAcctStatus.Err == nil {
return reconcile.Result{RequeueAfter: startingWorkspaceRequeueInterval}, nil
}
return reconcile.Result{Requeue: serviceAcctStatus.Requeue}, serviceAcctStatus.Err
}
if wsprovision.NeedsServiceAccountFinalizer(&workspace.Spec.Template) {
coputil.AddFinalizer(clusterWorkspace, serviceAccountCleanupFinalizer)
if err := r.Update(ctx, clusterWorkspace); err != nil {
return reconcile.Result{}, err
}
}

serviceAcctName := serviceAcctStatus.ServiceAccountName
reconcileStatus.setConditionTrue(dw.DevWorkspaceServiceAccountReady, "DevWorkspace serviceaccount ready")

Expand Down
46 changes: 40 additions & 6 deletions controllers/workspace/finalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,42 @@ import (
)

const (
storageCleanupFinalizer = "storage.controller.devfile.io"
storageCleanupFinalizer = "storage.controller.devfile.io"
serviceAccountCleanupFinalizer = "serviceaccount.controller.devfile.io"
)

func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace) (reconcile.Result, error) {
if !coputil.HasFinalizer(workspace, storageCleanupFinalizer) {
return reconcile.Result{}, nil
func (r *DevWorkspaceReconciler) workspaceNeedsFinalize(workspace *dw.DevWorkspace) bool {
for _, finalizer := range workspace.Finalizers {
if finalizer == storageCleanupFinalizer {
return true
}
if finalizer == serviceAccountCleanupFinalizer {
return true
}
}
return false
}

func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace) (reconcile.Result, error) {
workspace.Status.Message = "Cleaning up resources for deletion"
workspace.Status.Phase = devworkspacePhaseTerminating
err := r.Client.Status().Update(ctx, workspace)
if err != nil && !k8sErrors.IsConflict(err) {
return reconcile.Result{}, err
}

for _, finalizer := range workspace.Finalizers {
switch finalizer {
case storageCleanupFinalizer:
return r.finalizeStorage(ctx, log, workspace)
case serviceAccountCleanupFinalizer:
return r.finalizeServiceAccount(ctx, log, workspace)
}
}
return reconcile.Result{}, nil
}

func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace) (reconcile.Result, error) {
// Need to make sure Deployment is cleaned up before starting job to avoid mounting issues for RWO PVCs
wait, err := wsprovision.DeleteWorkspaceDeployment(ctx, workspace, r.Client)
if err != nil {
Expand Down Expand Up @@ -98,8 +120,20 @@ func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger,
return reconcile.Result{}, r.Update(ctx, workspace)
}

func isFinalizerNecessary(workspace *dw.DevWorkspace, provisioner storage.Provisioner) bool {
return provisioner.NeedsStorage(&workspace.Spec.Template)
func (r *DevWorkspaceReconciler) finalizeServiceAccount(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace) (reconcile.Result, error) {
retry, err := wsprovision.FinalizeServiceAccount(workspace, ctx, r.NonCachingClient)
if err != nil {
log.Error(err, "Failed to finalize workspace ServiceAccount")
failedStatus := currentStatus{phase: dw.DevWorkspaceStatusError}
failedStatus.setConditionTrue(dw.DevWorkspaceError, err.Error())
return r.updateWorkspaceStatus(workspace, r.Log, &failedStatus, reconcile.Result{}, nil)
}
if retry {
return reconcile.Result{Requeue: true}, nil
}
log.Info("ServiceAccount clean up successful; clearing finalizer")
coputil.RemoveFinalizer(workspace, serviceAccountCleanupFinalizer)
return reconcile.Result{}, r.Update(ctx, workspace)
}

func (r *DevWorkspaceReconciler) namespaceIsTerminating(ctx context.Context, namespace string) (bool, error) {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions deploy/deployment/kubernetes/combined.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions deploy/deployment/openshift/combined.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions deploy/templates/components/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@ rules:
- get
- list
- watch
- apiGroups:
- authorization.k8s.io
resources:
- localsubjectaccessreviews
- subjectaccessreviews
verbs:
- create
- apiGroups:
- batch
resources:
Expand Down
17 changes: 14 additions & 3 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (

oauthv1 "github.com/openshift/api/oauth/v1"
routev1 "github.com/openshift/api/route/v1"
securityv1 "github.com/openshift/api/security/v1"
templatev1 "github.com/openshift/api/template/v1"
corev1 "k8s.io/api/core/v1"
k8sruntime "k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -75,6 +76,9 @@ func init() {
utilruntime.Must(routev1.Install(scheme))
utilruntime.Must(templatev1.Install(scheme))
utilruntime.Must(oauthv1.Install(scheme))
// Enable controller to manage SCCs in OpenShift; permissions to do this are not requested
// by default and must be added by a cluster-admin.
utilruntime.Must(securityv1.Install(scheme))
}

// +kubebuilder:scaffold:scheme
Expand Down Expand Up @@ -122,6 +126,12 @@ func main() {
os.Exit(1)
}

nonCachingClient, err := client.New(mgr.GetConfig(), client.Options{Scheme: scheme})
if err != nil {
setupLog.Error(err, "unable to initialize non-caching client")
os.Exit(1)
}

// Index Events on involvedObject.name to allow us to get events involving a DevWorkspace's pod(s). This is used to
// check for issues that prevent the pod from starting, so that DevWorkspaces aren't just hanging indefinitely.
if err := mgr.GetFieldIndexer().IndexField(context.Background(), &corev1.Event{}, "involvedObject.name", func(obj client.Object) []string {
Expand All @@ -143,9 +153,10 @@ func main() {
os.Exit(1)
}
if err = (&workspacecontroller.DevWorkspaceReconciler{
Client: mgr.GetClient(),
Log: ctrl.Log.WithName("controllers").WithName("DevWorkspace"),
Scheme: mgr.GetScheme(),
Client: mgr.GetClient(),
NonCachingClient: nonCachingClient,
Log: ctrl.Log.WithName("controllers").WithName("DevWorkspace"),
Scheme: mgr.GetScheme(),
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "DevWorkspace")
os.Exit(1)
Expand Down
14 changes: 14 additions & 0 deletions pkg/constants/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ const (
// value: VAL_2
WorkspaceEnvAttribute = "workspaceEnv"

// WorkspaceSCCAttribute defines additional SCCs that should be added to the DevWorkspace. The user adding
// this attribute to a workspace must have the RBAC permissions to "use" the SCC with the given name. For example,
// to add the 'anyuid' SCC to the workspace Pod, the DevWorkspace should contain
//
// spec:
// template:
// attributes:
// controller.devfile.io/scc: "anyuid"
//
// Creating a workspace with this attribute, or updating an existing workspace to include this attribute will fail
// if the user making the request does not have the "use" permission for the "anyuid" SCC.
// Only supported on OpenShift.
WorkspaceSCCAttribute = "controller.devfile.io/scc"

// ProjectCloneAttribute configures how the DevWorkspace will treat project cloning. By default, an init container
// will be added to the workspace deployment to clone projects to the workspace before it starts. This attribute
// must be applied to top-level attributes field in the DevWorkspace.
Expand Down
9 changes: 5 additions & 4 deletions pkg/provision/sync/cluster_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ import (
)

type ClusterAPI struct {
Client crclient.Client
Scheme *runtime.Scheme
Logger logr.Logger
Ctx context.Context
Client crclient.Client
NonCachingClient crclient.Client
Scheme *runtime.Scheme
Logger logr.Logger
Ctx context.Context
}

// NotInSyncError is returned when a spec object is out-of-sync with its cluster counterpart
Expand Down
Loading