Add container security context field to DevWorkspace Operator Config

[amisevsk]

What does this PR do?

• Add .config.workspace.containerSecurityContext field to DevWorkspaceOperatorConfig to allow configuring container-level security context for workspace pods
• Update how podSecurityContext is handled to allow specifying a configured value in OpenShift as well (previously it was ignored)
• Merge podSecurityContext values into default configuration rather than overwriting it entirely
• Minor fixes:
  • Add config package test that fuzzes config to ensure all fields are considered by merging
  • Update CRD documentation
  • Allow specifying different defaults for OpenShift and Kubernetes for podSecurityContext
  • Log when a non-default container/pod security context is used

What issues does this PR fix or reference?

Closes #951
Closes #952

Is it tested? How?

To test:

• Verify that functionality is unchanged on OpenShift when no values are configured
• Verify that it's possible to configure both container and pod security context
• Verify that pod security context does not overwrite unset fields

On Kubernetes, the following DevWorkspaceOperatorConfig fields can be used to test:

config:
  workspace:
    containerSecurityContext:
      capabilities:
        add:
        - SETUID
        - SETGID
    podSecurityContext:
      runAsUser: 2048

This should result in the workspace pod having runAsUser: 2048 and pod containers having the SETUID and SETGID capabilities.

On OpenShift, the above DWOC will likely cause the workspace to fail to start. One permissible configuration is to specify a runAsUser within the UID range assigned the current project, e.g.

config:
  workspace:
    containerSecurityContext:
      runAsUser: 1000670002
    podSecurityContext:
      runAsUser: 1000670001

(where 1000670000 is the default UID used for running workspace pods).

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

[codecov]

Codecov Report

Base: 49.52% // Head: 50.18% // Increases project coverage by +0.65% 🎉

Coverage data is based on head (561e097) compared to base (f64d213).
Patch coverage: 57.14% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #953      +/-   ##
==========================================
+ Coverage   49.52%   50.18%   +0.65%     
==========================================
  Files          38       39       +1     
  Lines        2417     2467      +50     
==========================================
+ Hits         1197     1238      +41     
- Misses       1095     1103       +8     
- Partials      125      126       +1     

Impacted Files	Coverage Δ
pkg/config/sync.go	70.80% <34.88%> (+2.53%)	⬆️
pkg/config/defaults.go	71.42% <71.42%> (ø)
pkg/library/container/container.go	48.38% <100.00%> (ø)
pkg/library/container/conversion.go	91.11% <100.00%> (+0.09%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[AObuchow]

Code looks good to me.

I still have a bit of testing to do (had some install issues on my cluster & forgot to check the workspace deployment YAML instead of workspace pod YAML 🤦‍♂️) but here are my findings so far (tested on OpenShift 4.11 and Minikube):

Verify that functionality is unchanged on OpenShift when no values are configured

Things seem fine regarding this point, but I will post an update regarding this once I get a hold of another cluster.
When installed through OperatorHub (i.e previous functionality on OpenShift prior to this patch), the workspace deployment's securityContext should be empty, e.g
securityContext: {}

I still have to double-check this stays the same when the patch is applied, though it should.

For the workspace pod, without the patch applied, the pod SecurityContext was the following:

 securityContext:
   seLinuxOptions:
     level: 's0:c20,c10'
   fsGroup: 1000400000
   seccompProfile:
     type: RuntimeDefault

And the container SecurityContext:

      securityContext:
        capabilities:
          drop:
            - ALL
        runAsUser: 1000400000
        runAsNonRoot: true
        allowPrivilegeEscalation: false

For the workspace pod, with the patch applied, the pod SecurityContext was the following:

  securityContext:
    seLinuxOptions:
      level: 's0:c26,c10'
    fsGroup: 1000670000
    seccompProfile:
      type: RuntimeDefault

And the container SecurityContext:

      securityContext:
        capabilities:
          drop:
            - ALL
        runAsUser: 1000670000
        runAsNonRoot: true
        allowPrivilegeEscalation: false

The differences between the patch applied vs. without the patch are the seLinuxOptions level, the fsGroup and the runAsUser UID. I imagine these differences were caused by the cluster and not by the patch, but I am not sure.

Verify that it's possible to configure both container and pod security context

This worked on both OpenShift and Minikube.

On OpenShift, I used the example pod and container security contexts provided in the PR.
The workspace pod's SecurityContext was:

  securityContext:
    seLinuxOptions:
      level: 's0:c26,c10'
    runAsUser: 1000670001
    fsGroup: 1000670000
    seccompProfile:
      type: RuntimeDefault

And the container security contexts were:

      securityContext:
        capabilities:
          drop:
            - ALL
        runAsUser: 1000670002
        runAsNonRoot: true
        allowPrivilegeEscalation: false

Note that the values for runAsUser are correctly set.

On Minikube, I used the example pod and container security contexts provided in the PR, but also added the KILL capability.
The workspace pod's SecurityContext was:

   securityContext:
    runAsUser: 2048
    runAsGroup: 0
    runAsNonRoot: true
    fsGroup: 1234

And the container security contexts were:

      securityContext:
        capabilities:
          add:
            - SETUID
            - SETGID
            - KILL

Note that the correct capabilities were added (including KILL) and the value of runAsUser was correct.

Verify that pod security context does not overwrite unset fields

This seems to work.

On Minikube, any unset fields for the pod security context in the DWOC will result in the default values being used.

On OpenShift, fields that were not set in the pod security context (such as fsGroup) did not change.

With pod SecurityContext set in DWOC:

  securityContext:
    seLinuxOptions:
      level: 's0:c26,c10'
    runAsUser: 1000670001
    fsGroup: 1000670000
    seccompProfile:
      type: RuntimeDefault

Without pod SecurityContext set in DWOC:

  securityContext:
    seLinuxOptions:
      level: 's0:c26,c10'
    fsGroup: 1000670000
    seccompProfile:
      type: RuntimeDefault

[AObuchow]

Verify that functionality is unchanged on OpenShift when no values are configured

Things seem fine regarding this point, but I will post an update regarding this once I get a hold of another cluster. When installed through OperatorHub (i.e previous functionality on OpenShift prior to this patch), the workspace deployment's securityContext should be empty, e.g securityContext: {}

I still have to double-check this stays the same when the patch is applied, though it should.

Just confirmed that the workspace deployment's security context remains empty (securityContext: {}) when the patch is applied and no values are configured in the DWOC 👍

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

• OWNERS [amisevsk]

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

[openshift-ci]

New changes are detected. LGTM label has been removed.

[AObuchow]

Not really necessary, but I just found out you can use "k8s.io/utils/pointer" and do pointer.BoolPtr(true), as well as pointer.Int64Ptr(1234).

However, this still wouldn't eliminate the need for the commonStorageSize and perWorkspaceStorageSize variables.

[amisevsk]

That's a great idea -- we use things like &boolTrue extensively enough that we should likely fix it repo-wide in a separate PR

[amisevsk]

/retest