split the securityContext in two: pod and container securityContext #259

2021-12-08T20:59:51Z

nmasse-itix commented

2021-12-08 20:59:51 +00:00

(Migrated from gitea.com)

Hello !

I'm using the new Helm chart (5.x) and I really like the new configuration mechanism. ?

I would like to contribute the following enhancement.

The problem I want to solve

I'm trying to deploy Gitea in a Kubernetes shared platform and I need to make sure each instance is running as a different user so that in case of container escape, the risk of data leak is minimized.

Additionally, on my platform (OpenShift), arbitrary users (such as uid 1000 for Gitea) are not allowed.

The current helm chart does not allow me to achieve this because:

the container security context is configurable only for the main container. The security context of init containers cannot be specified.
a fixed uid is hard coded
a fixed fs group is hard coded

Also, the securityContext of a pod and the securityContext of a container do not accept the same options.

How I'm solving the problem

I split the securityContext (values.yaml) in two: containerSecurityContext and podSecurityContext. The containerSecurityContext applies to all containers (init and main) in order to be consistent with file permissions.

The behavior for existing deployments is unchanged:

fsGroup 1000 is the default value for the podSecurityContext variable
the "configure-gitea" init container uses the uid 1000 unless otherwise stated in the containerSecurityContext
the main container is using the existing securityContext variable when defined in order not to break existing deployments and uses the new containerSecurityContext variable if not.

This approach is well tested: it is used consistently on bitnami's Helm charts.

How I tested

I tested both root and rootless variants on a Kubernetes 1.22, as well as rootless variant on OpenShift 4.7.

rootless variant on Kubernetes:

podSecurityContext:
  fsGroup: 10001

containerSecurityContext:
  allowPrivilegeEscalation: false
  capabilities:
    drop:
      - ALL
    add:
      - SYS_CHROOT
  privileged: false
  runAsGroup: 10001
  runAsNonRoot: true
  runAsUser: 10001

extraVolumes:
- name: var-lib-gitea
  emptyDir: {}

extraVolumeMounts:
- name: var-lib-gitea
  readOnly: false
  mountPath: "/var/lib/gitea"

rootless variant on OpenShift:

podSecurityContext:
  fsGroup: null

containerSecurityContext:
  allowPrivilegeEscalation: false
  privileged: false
  runAsNonRoot: true
  runAsUser: 1000790000

extraVolumes:
- name: var-lib-gitea
  emptyDir: {}

extraVolumeMounts:
- name: var-lib-gitea
  readOnly: false
  mountPath: "/var/lib/gitea"

Let me know if something is unclear.

Hello ! I'm using the new Helm chart (5.x) and I really like the new configuration mechanism. ? I would like to contribute the following enhancement. ## The problem I want to solve I'm trying to deploy Gitea in a Kubernetes shared platform and I need to make sure each instance is running as a different user so that in case of container escape, the risk of data leak is minimized. Additionally, on my platform (OpenShift), arbitrary users (such as uid 1000 for Gitea) are not allowed. The current helm chart does not allow me to achieve this because: - the container security context is configurable only for the main container. The security context of init containers cannot be specified. - a fixed uid is hard coded - a fixed fs group is hard coded Also, the securityContext of a pod and the securityContext of a container do not accept the same options. - https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#podsecuritycontext-v1-core - https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#securitycontext-v1-core ## How I'm solving the problem I split the `securityContext` (values.yaml) in two: `containerSecurityContext` and `podSecurityContext`. The containerSecurityContext applies to all containers (init and main) in order to be consistent with file permissions. The behavior for existing deployments is unchanged: - fsGroup 1000 is the default value for the podSecurityContext variable - the "configure-gitea" init container uses the uid 1000 unless otherwise stated in the containerSecurityContext - the main container is using the existing securityContext variable when defined in order not to break existing deployments and uses the new containerSecurityContext variable if not. This approach is well tested: it is used consistently on bitnami's Helm charts. ## How I tested I tested both root and rootless variants on a Kubernetes 1.22, as well as rootless variant on OpenShift 4.7. **rootless variant on Kubernetes**: ```yaml podSecurityContext: fsGroup: 10001 containerSecurityContext: allowPrivilegeEscalation: false capabilities: drop: - ALL add: - SYS_CHROOT privileged: false runAsGroup: 10001 runAsNonRoot: true runAsUser: 10001 extraVolumes: - name: var-lib-gitea emptyDir: {} extraVolumeMounts: - name: var-lib-gitea readOnly: false mountPath: "/var/lib/gitea" ``` **rootless variant on OpenShift**: ```yaml podSecurityContext: fsGroup: null containerSecurityContext: allowPrivilegeEscalation: false privileged: false runAsNonRoot: true runAsUser: 1000790000 extraVolumes: - name: var-lib-gitea emptyDir: {} extraVolumeMounts: - name: var-lib-gitea readOnly: false mountPath: "/var/lib/gitea" ``` Let me know if something is unclear.

❤️ 2

justusbunsi commented

2021-12-09 04:58:48 +00:00

(Migrated from gitea.com)

Thank your for your contribution. I just had a quick glance at your changes and it looks really promising. The way you prevent a breaking change is neat. What's missing is the modification of README.md. There are all values defined and shortly described.

nmasse-itix commented

2021-12-09 09:30:11 +00:00

(Migrated from gitea.com)

Thanks for your review and feedback. I completed the documentation with the two new items.

luhahn commented

2021-12-13 08:48:19 +00:00

(Migrated from gitea.com)

Will check and test this PR today :)

👍 1

luhahn (Migrated from gitea.com) approved these changes 2021-12-13 10:31:39 +00:00

luhahn commented

2021-12-13 10:31:46 +00:00