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

feat: Allow configuration of pod's security context for DevWorkspaces on Kubernetes #748

Merged
merged 4 commits into from
Jan 24, 2022

Conversation

amisevsk
Copy link
Collaborator

What does this PR do?

Adds field podSecurityContext to the DevWorkspaceOperatorConfig CR to allow overriding securityContext in all workspace-related pods when running on Kubernetes.

Also sets a default fsGroup = 1234 to avoid issues on some k8s clusters (see: eclipse-che/che#20965). Note that this is what is done in Eclipse Che as well as far as I can tell so should not cause issues.

Configuration value is ignored on OpenShift.

What issues does this PR fix or reference?

Closes #718
Related: eclipse-che/che#20965
Related: eclipse-che/che#20963

Is it tested? How?

  1. On Kubernetes, start a DevWorkspace with default config. Pods created for workspace (including cleanup job pods) should use fsGroup=1234
  2. Configure .config.workspace.podSecurityContext in the devworkspace-operator-config DevWorkspaceOperatorConfig and make sure change is propagated to DevWorkspaces (may need to trigger a reconcile for running workspaces)

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

In some Kubernetes clusters, fsGroup needs to be set in order for
workspace containers to read contents of mounted PVCs. This change
matches the fsGroup with the runAsUser field. Note this is the security
context that is used in Eclipse Che as well.

Only applies to Kubernetes; behavior in OpenShift is unchanged.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Add PodSecurityContext to DevWorkspaceOperatorConfig to allow
configuring PodSecurityContext when running on Kubernetes.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@nils-mosbach
Copy link

Thanks!
Kubernetes 20 supports a property that allows changing owners only once if the root node does not match: fsGroupChangePolicy: "OnRootMismatch"

I haven't used this before, but that could speed up things if projects contain a lot of npm dependencies. What do you think?

See: https://kubernetes.io/blog/2020/12/14/kubernetes-release-1.20-fsgroupchangepolicy-fsgrouppolicy/

@amisevsk amisevsk changed the title Allow configuration of pod's security context for DevWorkspaces on Kubernetes feat: Allow configuration of pod's security context for DevWorkspaces on Kubernetes Jan 20, 2022
@amisevsk
Copy link
Collaborator Author

@nils-mosbach Thanks for that link, I wasn't aware of this change! This is something I believe might solve a longterm complaint about workspace startup time for us (and Eclipse Che), as our use cases (especially with npm projects ;)) generally involve many files in the PVC.

The DevWorkspace Operator depends on the Kubernetes 1.21.3 API, which means that this PR implicitly supports setting fsGroupChangePolicy:

apiVersion: controller.devfile.io/v1alpha1
kind: DevWorkspaceOperatorConfig
metadata:
  name: devworkspace-operator-config
config:
  workspace:
    podSecurityContext:
      fsGroupChangePolicy: "OnRootMismatch" 
      # Other fields need to be set too

The impact of this on DevWorkspace startup time could be tested using the next version of DWO as soon as this PR is merged. However, I don't have access to non-toy Kubernetes clusters (only OpenShift, where I believe this option would be ignored by the cluster). If you have a chance to test it out on your cluster, I'd love to hear the result!

@amisevsk
Copy link
Collaborator Author

/test v8-devworkspace-operator-e2e

1 similar comment
@amisevsk
Copy link
Collaborator Author

/test v8-devworkspace-operator-e2e

@openshift-ci
Copy link

openshift-ci bot commented Jan 24, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, ibuziuk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@amisevsk amisevsk merged commit 5fef363 into devfile:main Jan 24, 2022
@amisevsk amisevsk deleted the configure-k8s-security-context branch January 24, 2022 15:21
@nils-mosbach
Copy link

Great, thanks a lot! I'll give it at go as soon as we manage to get devworkspaces running under Kubernetes. A version that I patched myself seemed to resolve the issue.

@nils-mosbach
Copy link

I've just updated devworkspace controller to quay.io/devfile/devworkspace-controller:next.
#20965 and #20963 are resolved in my case. I'll close the issues. Thanks, a lot!

@amisevsk
Copy link
Collaborator Author

I'm glad to hear it 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable configuration of pod SecurityContext on Operator level in Kubernetes
3 participants