From 7fc8ea30fc250a95d2f80211694262dbbe5b1611 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 29 Nov 2023 15:54:52 +0000 Subject: [PATCH] mount volumes for tmp directories, apply best practice to sc (#38) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * mount volumes for tmp directories, apply best practice to sc * bump version * use safe sc for bundled apps (postgres, memcached) too * use read-only filesystem with mounted tmp volumes unless in development mode * revert version bump to let this be handled by changeset flow * amend dev readme * Create bright-students-eat.md --------- Co-authored-by: Oliver Günther --- .changeset/bright-students-eat.md | 7 +++ charts/openproject/README.md | 29 +++++++-- charts/openproject/bin/debug | 46 ++++++++++++++ charts/openproject/bin/install-dev | 6 ++ charts/openproject/templates/_helpers.tpl | 40 ++++++++++++ charts/openproject/templates/secrets.yaml | 2 +- charts/openproject/templates/seeder-job.yaml | 41 +++++++----- .../openproject/templates/web-deployment.yaml | 63 ++++++++++++------- .../templates/worker-deployment.yaml | 54 +++++++++------- charts/openproject/values.yaml | 50 ++++++++++++++- 10 files changed, 268 insertions(+), 70 deletions(-) create mode 100644 .changeset/bright-students-eat.md create mode 100755 charts/openproject/bin/debug create mode 100755 charts/openproject/bin/install-dev create mode 100644 charts/openproject/templates/_helpers.tpl diff --git a/.changeset/bright-students-eat.md b/.changeset/bright-students-eat.md new file mode 100644 index 0000000..5ac0432 --- /dev/null +++ b/.changeset/bright-students-eat.md @@ -0,0 +1,7 @@ +--- +"@fake-scope/fake-pkg": major +--- + +* rename `securityContext` to `containerSecurityContext` in `values.yaml` +* mount volumes for tmp directories to make containers work in accordance with best practices, that is with read-only file systems +* use secure defaults for container security policy diff --git a/charts/openproject/README.md b/charts/openproject/README.md index 237c270..c084c4d 100644 --- a/charts/openproject/README.md +++ b/charts/openproject/README.md @@ -7,12 +7,31 @@ This is the chart for OpenProject itself. To install or update from this directory run the following command. ```bash -helm upgrade \ - --create-namespace --namespace openproject \ - --install --reuse-values openproject-dev . +bin/install-dev ``` -### TLS +This will install the chart with `--set develop=true` which is recommended +on local clusters such as **minikube** or **kind**. + +This will also set `OPENPROJECT_HTTPS` to false so no TLS certificate is required +to access it. + +You can set other options just like when installing via `--set` +(e.g. `bin/install-dev --set persistence.enabled=false`). + +### Debugging + +Changes to the chart can be debugged using the following. + +```bash +bin/debug +``` + +This will try to render the templates and show any errors. +You can set values just like when installing via `--set` +(e.g. `bin/debug --set persistence.enabled=false`). + +## TLS Create a TLS certificate, e.g. using [mkcert](https://github.com/FiloSottile/mkcert). @@ -34,7 +53,7 @@ Set the tls secret value during installation or an upgrade by adding the followi --set ingress.tls.enabled=true --set tls.secretName=openproject-tls ``` -#### Root CA +### Root CA If you want to add your own root CA for outgoing TLS connection, do the following. diff --git a/charts/openproject/bin/debug b/charts/openproject/bin/debug new file mode 100755 index 0000000..0190310 --- /dev/null +++ b/charts/openproject/bin/debug @@ -0,0 +1,46 @@ +#!/bin/bash + +# Outputs the generated helm configurations after templating. + +yaml_output=/tmp/op-hc-yaml-output.txt +error_output=/tmp/op-hc-error-output.txt +section_output=/tmp/op-hc-section-output.yml +vimrc=/tmp/op-hc-vim-rc + +rm $yaml_output $error_output $section_output $vimrc &>/dev/null + +helm template --debug "$@" . 1> $yaml_output 2> $error_output + +if [ $? -gt 0 ]; then + section=`cat $error_output | grep 'Error: YAML parse error on' | cut -d: -f2 | cut -d' ' -f6-` + + if [ -n "$section" ]; then + cat $yaml_output | sed -e "0,/\# Source: ${section//\//\\/}/d" | tail -n+2 | sed -e '/---/,$d' > $section_output + + line=`cat $error_output | grep line | head -n1 | perl -nle 'm/line (\d+)/; print $1'` + + if [ -n "$line" ]; then + echo "autocmd VimEnter * echo '`cat $error_output | grep line | head -n1`'" > $vimrc + vim +$line -u $vimrc $section_output + else + echo + echo "Template error: " + echo + echo --- + cat $section_output + cat $error_output + fi + else + echo + echo "Template error: " + echo + echo --- + cat $yaml_output + cat $error_output + fi +else + cat $yaml_output + + echo + echo "Syntax ok" +fi diff --git a/charts/openproject/bin/install-dev b/charts/openproject/bin/install-dev new file mode 100755 index 0000000..b38d542 --- /dev/null +++ b/charts/openproject/bin/install-dev @@ -0,0 +1,6 @@ +# !/bin/bash + +# Install OpenProject in development mode, that is without https and allowing writes +# to the container file system. + +helm upgrade --create-namespace --namespace openproject --install openproject --set develop=true "$@" . diff --git a/charts/openproject/templates/_helpers.tpl b/charts/openproject/templates/_helpers.tpl new file mode 100644 index 0000000..e3e6f47 --- /dev/null +++ b/charts/openproject/templates/_helpers.tpl @@ -0,0 +1,40 @@ +{{/* +Returns the OpenProject image to be used including the respective registry and image tag. +*/}} +{{- define "openproject.image" -}} +{{ .Values.image.registry }}/{{ .Values.image.repository }}{{ if .Values.image.sha256 }}@sha256:{{ .Values.image.sha256 }}{{ else }}:{{ .Values.image.tag }}{{ end }} +{{- end -}} + +{{/* +Yields the configured container security context if enabled. + +Allows writing to the container file system in development mode +This way the OpenProject container works without mounted tmp volumes +which may not work correctly in local development clusters. +*/}} +{{- define "openproject.containerSecurityContext" }} +{{- if .Values.containerSecurityContext.enabled }} +securityContext: + {{- + mergeOverwrite + (omit .Values.containerSecurityContext "enabled" | deepCopy) + (dict "readOnlyRootFilesystem" (not .Values.develop)) + | toYaml + | nindent 2 + }} +{{- end }} +{{- end }} + +{{/* Yields the configured pod security context if enabled. */}} +{{- define "openproject.podSecurityContext" }} +{{- if .Values.podSecurityContext.enabled }} +securityContext: + {{ omit .Values.podSecurityContext "enabled" | toYaml | nindent 2 | trim }} +{{- end }} +{{- end }} + +{{- define "openproject.useTmpVolumes" -}} +{{- if not .Values.develop -}} + {{- true -}} +{{- end -}} +{{- end -}} diff --git a/charts/openproject/templates/secrets.yaml b/charts/openproject/templates/secrets.yaml index 2c151b9..532a095 100644 --- a/charts/openproject/templates/secrets.yaml +++ b/charts/openproject/templates/secrets.yaml @@ -19,7 +19,7 @@ stringData: OPENPROJECT_SEED_ADMIN_USER_PASSWORD_RESET: {{ .Values.openproject.admin_user.password_reset | quote }} OPENPROJECT_SEED_ADMIN_USER_NAME: {{ .Values.openproject.admin_user.name | quote }} OPENPROJECT_SEED_ADMIN_USER_MAIL: {{ .Values.openproject.admin_user.mail | quote }} - OPENPROJECT_HTTPS: {{ .Values.openproject.https | quote }} + OPENPROJECT_HTTPS: {{ (.Values.develop | ternary "false" .Values.openproject.https) | quote }} OPENPROJECT_SEED_LOCALE: {{ .Values.openproject.seed_locale | quote }} OPENPROJECT_HOST__NAME: {{ .Values.openproject.host | default .Values.ingress.host | quote }} OPENPROJECT_HSTS: {{ .Values.openproject.hsts | quote }} diff --git a/charts/openproject/templates/seeder-job.yaml b/charts/openproject/templates/seeder-job.yaml index e652332..8d4b7e4 100644 --- a/charts/openproject/templates/seeder-job.yaml +++ b/charts/openproject/templates/seeder-job.yaml @@ -19,20 +19,29 @@ spec: {{- toYaml . | nindent 8 }} {{- end }} spec: - {{- if .Values.podSecurityContext.enabled }} - securityContext: - {{ omit .Values.podSecurityContext "enabled" | toYaml | nindent 8 | trim }} - {{- end }} + {{- include "openproject.podSecurityContext" . | indent 6 }} {{- with .Values.nodeSelector }} nodeSelector: {{ toYaml . | nindent 8 | trim }} {{- end }} - {{- if .Values.persistence.enabled }} volumes: + {{- if (include "openproject.useTmpVolumes" .) }} + - name: tmp + # we can't use emptyDir due to the sticky bit issue + # see: https://github.com/kubernetes/kubernetes/issues/110835 + ephemeral: + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + resources: + requests: + storage: 1Gi + {{- end }} + {{- if .Values.persistence.enabled }} - name: "data" persistentVolumeClaim: claimName: {{ if .Values.persistence.existingClaim }}{{ .Values.persistence.existingClaim }}{{- else }}{{ include "common.names.fullname" . }}{{- end }} - {{- end }} + {{- end }} initContainers: - name: check-db-ready image: "{{ .Values.initdb.image.registry }}/{{ .Values.initdb.image.repository }}:{{ .Values.initdb.image.tag }}" @@ -40,17 +49,14 @@ spec: command: [ 'sh', '-c', - 'until pg_isready -h $DATABASE_HOST -p $DATABASE_PORT; do echo "waiting for database $DATABASE_HOST:$DATABASE_PORT"; sleep 2; done;' + 'until pg_isready -h $DATABASE_HOST -p $DATABASE_PORT -U {{ .Values.postgresql.auth.username }}; do echo "waiting for database $DATABASE_HOST:$DATABASE_PORT"; sleep 2; done;' ] envFrom: - secretRef: name: {{ include "common.names.fullname" . }} resources: {{- toYaml .Values.initdb.resources | nindent 12 }} - {{- if .Values.securityContext.enabled }} - securityContext: - {{- omit .Values.securityContext "enabled" | toYaml | nindent 12 }} - {{- end }} + {{- include "openproject.containerSecurityContext" . | indent 10 }} containers: - name: seeder image: "{{ .Values.image.registry }}/{{ .Values.image.repository }}{{ if .Values.image.sha256 }}@sha256:{{ .Values.image.sha256 }}{{ else }}:{{ .Values.image.tag }}{{ end }}" @@ -61,13 +67,14 @@ spec: envFrom: - secretRef: name: {{ include "common.names.fullname" . }} - {{- if .Values.persistence.enabled }} volumeMounts: + {{- if (include "openproject.useTmpVolumes" .) }} + - mountPath: /tmp + name: tmp + {{- end }} + {{- if .Values.persistence.enabled }} - name: "data" mountPath: "/var/openproject/assets" - {{- end }} - {{- if .Values.securityContext.enabled }} - securityContext: - {{- omit .Values.securityContext "enabled" | toYaml | nindent 12 }} - {{- end }} + {{- end }} + {{- include "openproject.containerSecurityContext" . | indent 10 }} restartPolicy: OnFailure diff --git a/charts/openproject/templates/web-deployment.yaml b/charts/openproject/templates/web-deployment.yaml index 6f67c2b..b3aa14f 100644 --- a/charts/openproject/templates/web-deployment.yaml +++ b/charts/openproject/templates/web-deployment.yaml @@ -46,29 +46,45 @@ spec: nodeSelector: {{ toYaml . | nindent 8 | trim }} {{- end }} - {{- if .Values.podSecurityContext.enabled }} - securityContext: - {{ omit .Values.podSecurityContext "enabled" | toYaml | nindent 8 | trim }} - {{- end }} + {{- include "openproject.podSecurityContext" . | indent 6 }} serviceAccountName: {{ include "common.names.fullname" . }} volumes: - {{- if .Values.egress.tls.rootCA.fileName }} + {{- if (include "openproject.useTmpVolumes" .) }} + - name: tmp + # we can't use emptyDir due to the sticky bit issue + # see: https://github.com/kubernetes/kubernetes/issues/110835 + ephemeral: + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + resources: + requests: + storage: 1Gi + - name: app-tmp + # we can't use emptyDir due to the sticky bit / world writable issue + # see: https://github.com/kubernetes/kubernetes/issues/110835 + ephemeral: + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + resources: + requests: + storage: 1Gi + {{- end }} + {{- if .Values.egress.tls.rootCA.fileName }} - name: ca-pemstore configMap: name: "{{- .Values.egress.tls.rootCA.configMap }}" - {{- end }} - {{- if .Values.persistence.enabled }} + {{- end }} + {{- if .Values.persistence.enabled }} - name: "data" persistentVolumeClaim: claimName: {{ if .Values.persistence.existingClaim }}{{ .Values.persistence.existingClaim }}{{- else }}{{ include "common.names.fullname" . }}{{- end }} - {{- end }} + {{- end }} initContainers: - name: wait-for-db - {{- if .Values.securityContext.enabled }} - securityContext: - {{- omit .Values.securityContext "enabled" | toYaml | nindent 12 }} - {{- end }} - image: "{{ .Values.image.registry }}/{{ .Values.image.repository }}{{ if .Values.image.sha256 }}@sha256:{{ .Values.image.sha256 }}{{ else }}:{{ .Values.image.tag }}{{ end }}" + {{- include "openproject.containerSecurityContext" . | indent 10 }} + image: {{ include "openproject.image" . }} imagePullPolicy: {{ .Values.image.imagePullPolicy }} envFrom: - secretRef: @@ -78,11 +94,8 @@ spec: - /app/docker/prod/wait-for-db containers: - name: "openproject" - {{- if .Values.securityContext.enabled }} - securityContext: - {{- omit .Values.securityContext "enabled" | toYaml | nindent 12 }} - {{- end }} - image: "{{ .Values.image.registry }}/{{ .Values.image.repository }}{{ if .Values.image.sha256 }}@sha256:{{ .Values.image.sha256 }}{{ else }}:{{ .Values.image.tag }}{{ end }}" + {{- include "openproject.containerSecurityContext" . | indent 10 }} + image: {{ include "openproject.image" . }} imagePullPolicy: {{ .Values.image.imagePullPolicy }} envFrom: - secretRef: @@ -93,16 +106,22 @@ spec: value: "/etc/ssl/certs/custom-ca.pem" {{- end }} volumeMounts: - {{- if .Values.persistence.enabled }} + {{- if (include "openproject.useTmpVolumes" .) }} + - mountPath: /tmp + name: tmp + - mountPath: /app/tmp + name: app-tmp + {{- end }} + {{- if .Values.persistence.enabled }} - name: "data" mountPath: "/var/openproject/assets" - {{- end }} - {{- if .Values.egress.tls.rootCA.fileName }} + {{- end }} + {{- if .Values.egress.tls.rootCA.fileName }} - name: ca-pemstore mountPath: /etc/ssl/certs/custom-ca.pem subPath: {{ .Values.egress.tls.rootCA.fileName }} readOnly: false - {{- end }} + {{- end }} ports: {{- range $key, $value := .Values.service.ports }} - name: {{ $key }} diff --git a/charts/openproject/templates/worker-deployment.yaml b/charts/openproject/templates/worker-deployment.yaml index 35bb796..4b22347 100644 --- a/charts/openproject/templates/worker-deployment.yaml +++ b/charts/openproject/templates/worker-deployment.yaml @@ -46,29 +46,38 @@ spec: nodeSelector: {{ toYaml . | nindent 8 | trim }} {{- end }} - {{- if .Values.podSecurityContext.enabled }} - securityContext: - {{ omit .Values.podSecurityContext "enabled" | toYaml | nindent 8 | trim }} - {{- end }} + {{- include "openproject.podSecurityContext" . | indent 6 }} serviceAccountName: {{ include "common.names.fullname" . }} volumes: - {{- if .Values.egress.tls.rootCA.fileName }} + {{- if (include "openproject.useTmpVolumes" .) }} + - name: tmp + # we can't use emptyDir due to the sticky bit issue + # see: https://github.com/kubernetes/kubernetes/issues/110835 + ephemeral: + volumeClaimTemplate: + spec: + accessModes: ["ReadWriteOnce"] + resources: + requests: + # the worker may need a lot of memory in case of big exports + # or backups + # @todo put this into a separate PVC per replica + storage: 5Gi + {{- end }} + {{- if .Values.egress.tls.rootCA.fileName }} - name: ca-pemstore configMap: name: "{{- .Values.egress.tls.rootCA.configMap }}" - {{- end }} - {{- if .Values.persistence.enabled }} + {{- end }} + {{- if .Values.persistence.enabled }} - name: "data" persistentVolumeClaim: claimName: {{ include "common.names.fullname" . }} - {{- end }} + {{- end }} initContainers: - name: wait-for-db - {{- if .Values.securityContext.enabled }} - securityContext: - {{- omit .Values.securityContext "enabled" | toYaml | nindent 12 }} - {{- end }} - image: "{{ .Values.image.registry }}/{{ .Values.image.repository }}{{ if .Values.image.sha256 }}@sha256:{{ .Values.image.sha256 }}{{ else }}:{{ .Values.image.tag }}{{ end }}" + {{- include "openproject.containerSecurityContext" . | indent 10 }} + image: {{ include "openproject.image" . }} imagePullPolicy: {{ .Values.image.imagePullPolicy }} envFrom: - secretRef: @@ -78,11 +87,8 @@ spec: - /app/docker/prod/wait-for-db containers: - name: "openproject" - {{- if .Values.securityContext.enabled }} - securityContext: - {{- omit .Values.securityContext "enabled" | toYaml | nindent 12 }} - {{- end }} - image: "{{ .Values.image.registry }}/{{ .Values.image.repository }}{{ if .Values.image.sha256 }}@sha256:{{ .Values.image.sha256 }}{{ else }}:{{ .Values.image.tag }}{{ end }}" + {{- include "openproject.containerSecurityContext" . | indent 10 }} + image: {{ include "openproject.image" . }} imagePullPolicy: {{ .Values.image.imagePullPolicy }} envFrom: - secretRef: @@ -96,15 +102,19 @@ spec: - bash - /app/docker/prod/worker volumeMounts: - {{- if .Values.persistence.enabled }} + {{- if (include "openproject.useTmpVolumes" .) }} + - mountPath: /tmp + name: tmp + {{- end }} + {{- if .Values.persistence.enabled }} - name: "data" mountPath: "/var/openproject/assets" - {{- end }} - {{- if .Values.egress.tls.rootCA.fileName }} + {{- end }} + {{- if .Values.egress.tls.rootCA.fileName }} - name: ca-pemstore mountPath: /etc/ssl/certs/custom-ca.pem subPath: {{ .Values.egress.tls.rootCA.fileName }} readOnly: false - {{- end }} + {{- end }} resources: {{- toYaml .Values.resources | nindent 12 }} diff --git a/charts/openproject/values.yaml b/charts/openproject/values.yaml index f697745..916c615 100644 --- a/charts/openproject/values.yaml +++ b/charts/openproject/values.yaml @@ -2,6 +2,15 @@ # This is a YAML-formatted file. # Declare variables to be passed into your templates. +## Enable development mode. +## +## Set this to true if you want are working on the charts locally using +## local clusters such as minikube or kind. +## +## This will set `OPENPROJECT_HTTPS` to `false` and avoid using volumes for +## tmp folders as (permissions for) these don't work correctly in local clusters. +develop: false + global: ## Credentials to fetch images from private registry. ## @@ -186,6 +195,18 @@ memcached: # bundled: true + global: + containerSecurityContext: + enabled: true + allowPrivilegeEscalation: false + capabilities: + drop: + - "ALL" + seccompProfile: + type: "RuntimeDefault" + readOnlyRootFilesystem: true + runAsNonRoot: true + ## When "bundled" is set to false, you need to define the memcached connection details. # connection: @@ -358,12 +379,23 @@ podSecurityContext: enabled: true fsGroup: 1000 -## Security Context. +## Container security context using as a default best practice values +## granting minimum privileges. ## ## Ref: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ # -securityContext: - enabled: false +containerSecurityContext: + enabled: true + runAsUser: 1000 + runAsGroup: 1000 + allowPrivilegeEscalation: false + capabilities: + drop: + - "ALL" + seccompProfile: + type: "RuntimeDefault" + readOnlyRootFilesystem: true + runAsNonRoot: true ## Configure PostgreSQL settings. # @@ -373,6 +405,18 @@ postgresql: # bundled: true + global: + containerSecurityContext: + enabled: true + allowPrivilegeEscalation: false + capabilities: + drop: + - "ALL" + seccompProfile: + type: "RuntimeDefault" + readOnlyRootFilesystem: true + runAsNonRoot: true + ## When "bundled" is set to false, you need to define the database connection details. # connection: