Improve secret management (#46)

* add secret references * Update secrets.yaml exclude memcache and Postgres secret var * enter Postgres password secret * remove memcached secret conditions * remove memcached auth * remove global option * move env key * fix deployment * revert image version * ADD templates/secret_environment.yaml: move environment variables from .Values.environment to a separate secret * templates/deployment.yaml: mount environment secret, if .Values.environment is non-empty * templates/secrets.yaml: remove environment variables from .Values.environment, as those now live in a separate secret * ADD templates/secret_oidc.yaml: new secret for oidc variables * templates/secrets.yaml: remove OIDC variables, as those are now in a separate secret * templates/deployment.yaml: use oidc secret, if .Values.openproject.oidc.enabled is true * ADD templates/secret_memcached.yaml: move memcached variables to separate secret * templates/secrets.yaml: remove memcached settings, that are now stored in a separate secret * templates/deployment.yaml: use memcached secret, if memcache is to be used (.Values.openproject.cache.store equals memcache) * values.yaml: add keys and explanation for using an existing secret with the postgresql chart * values.yaml: comment out auth.password and auth.postgresPassword and add explanation * RENAME templates/secrets.yaml TO templates/secret_openproject.yaml * templates/secret_openproject.yaml: remove postgresql password from DATABASE_URL, needs to be set in another environment variable from e.g. the existing secret * FIXME: create new environment variable, either from postgresql secret or from .Values.postgresql.auth.password * adapt and unify secret changes * add missing key * set database password in test installation * Create thin-gorillas-impress.md --------- Co-authored-by: pitwegner <[email protected]> Co-authored-by: pitwegner <[email protected]> Co-authored-by: Johannes Kastl <[email protected]>
opf · Dec 13, 2023 · 5f4bce6 · 5f4bce6
1 parent c4564ff
commit 5f4bce6
Show file tree

Hide file tree

Showing 13 changed files with 224 additions and 119 deletions.
diff --git a/.changeset/thin-gorillas-impress.md b/.changeset/thin-gorillas-impress.md
@@ -0,0 +1,8 @@
+---
+"@openproject/helm-charts": major
+---
+
+Improve secret management.
+
+Add support for `existingSecret` for `postgresql` authentication.
+Move `s3.accessKeyId` and `s3.secretAccessKey` to `s3.auth.` and add an `existingSecret` option for S3.
diff --git a/.github/workflows/lint-test.yml b/.github/workflows/lint-test.yml
@@ -45,4 +45,4 @@ jobs:
         if: steps.list-changed.outputs.changed == 'true'
 
       - name: Run chart-testing (install)
-        run: ct install --target-branch ${{ github.event.repository.default_branch }} --chart-repos bitnami=https://charts.bitnami.com/bitnami --helm-extra-set-args "--set environment.OPENPROJECT_HTTPS=false --set persistence.accessModes=[ReadWriteOnce]" --helm-extra-args "--timeout 600s"
+        run: ct install --target-branch ${{ github.event.repository.default_branch }} --chart-repos bitnami=https://charts.bitnami.com/bitnami --helm-extra-set-args "--set environment.OPENPROJECT_HTTPS=false --set postgresql.auth.password=password --set persistence.accessModes=[ReadWriteOnce]" --helm-extra-args "--timeout 600s"
diff --git a/charts/openproject/templates/_helpers.tpl b/charts/openproject/templates/_helpers.tpl
@@ -41,3 +41,61 @@ securityContext:
   {{- true -}}
 {{- end -}}
 {{- end -}}
+
+{{- define "openproject.envFrom" -}}
+envFrom:
+  - secretRef:
+      name: {{ include "common.names.fullname" . }}-core
+  {{- if .Values.openproject.oidc.enabled }}
+  - secretRef:
+      name: {{ include "common.names.fullname" . }}-oidc
+  {{- end }}
+  {{- if .Values.s3.enabled }}
+  - secretRef:
+      name: {{ include "common.names.fullname" . }}-s3
+  {{- end }}
+  {{- if eq .Values.openproject.cache.store "memcache" }}
+  - secretRef:
+      name: {{ include "common.names.fullname" . }}-memcached
+  {{- end }}
+  {{- if .Values.environment }}
+  - secretRef:
+      name: {{ include "common.names.fullname" . }}-environment
+  {{- end }}
+  {{- if .Values.extraEnvVarsSecret }}
+  - secretRef:
+      name: {{ .Values.extraEnvVarsSecret }}
+  {{- end }}
+{{- end }}
+
+{{- define "openproject.env" -}}
+env:
+  {{- if .Values.egress.tls.rootCA.fileName }}
+  - name: SSL_CERT_FILE
+    value: "/etc/ssl/certs/custom-ca.pem"
+  {{- end }}
+  {{- if .Values.postgresql.auth.existingSecret }}
+  - name: OPENPROJECT_DB_PASSWORD
+    valueFrom:
+      secretKeyRef:
+        name: {{ .Values.postgresql.auth.existingSecret }}
+        key: {{ .Values.postgresql.auth.secretKeys.userPasswordKey }}
+  {{- else if .Values.postgresql.auth.password }}
+  - name: OPENPROJECT_DB_PASSWORD
+    value: {{ .Values.postgresql.auth.password }}
+  {{- else }}
+  # {{ required "Database password is required. Please set postgresql.auth.existingSecret (recommended) or postgresql.auth.password" "" }}
+  {{- end }}
+{{- end }}
+
+{{- define "openproject.envChecksums" }}
+# annotate pods with env value checksums so changes trigger re-deployments
+{{- $namespace := .Release.Namespace }}
+{{- $prefix := include "common.names.fullname" . }}
+{{/* If I knew how to map and reduce a range in helm I would do that and use a single checksum. But here we are. */}}
+{{- range $suffix := list "core" "memcached" "oidc" "s3" "environment" }}
+{{- $secretObj := (lookup "v1" "Secret" $namespace (printf "%s-%s" $prefix $suffix)) | default dict }}
+{{- $secretData := (get $secretObj "data") | default dict }}
+checksum/env-{{ $suffix }}: {{ values $secretData | sortAlpha | cat | sha256sum }}
+{{- end }}
+{{- end }}
diff --git a/charts/openproject/templates/secret_core.yaml b/charts/openproject/templates/secret_core.yaml
@@ -0,0 +1,29 @@
+---
+apiVersion: "v1"
+kind: "Secret"
+metadata:
+  name: "{{ include "common.names.fullname" . }}-core"
+  labels:
+    {{- include "common.labels.standard" . | nindent 4 }}
+stringData:
+  {{- if .Values.postgresql.bundled }}
+  DATABASE_HOST: {{ printf "%s-postgresql.%s.svc.%s" .Release.Name .Release.Namespace .Values.clusterDomain | quote }}
+  DATABASE_PORT: "{{ .Values.postgresql.primary.service.ports.postgresql }}"
+  DATABASE_URL: "postgresql://{{ .Values.postgresql.auth.username }}@{{ include "common.names.dependency.fullname" (dict "chartName" "postgresql" "chartValues" .Values.postgresql "context" $) }}:{{ .Values.postgresql.primary.service.ports.postgresql }}/{{ .Values.postgresql.auth.database }}"
+  {{- else }}
+  DATABASE_HOST: "{{ .Values.postgresql.connection.host }}"
+  DATABASE_PORT: "{{ .Values.postgresql.connection.port }}"
+  DATABASE_URL: "postgresql://{{ .Values.postgresql.auth.username }}@{{ .Values.postgresql.connection.host }}:{{ .Values.postgresql.connection.port }}/{{ .Values.postgresql.auth.database }}"
+  {{- end }}
+  OPENPROJECT_SEED_ADMIN_USER_PASSWORD: {{ .Values.openproject.admin_user.password | quote }}
+  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.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 }}
+  OPENPROJECT_RAILS__CACHE__STORE: {{ .Values.openproject.cache.store | quote }}
+  OPENPROJECT_RAILS__RELATIVE__URL__ROOT: {{ .Values.openproject.railsRelativeUrlRoot | default "" | quote }}
+  POSTGRES_STATEMENT_TIMEOUT: {{ .Values.openproject.postgresStatementTimeout | quote }}
+...
diff --git a/charts/openproject/templates/secret_environment.yaml b/charts/openproject/templates/secret_environment.yaml
@@ -0,0 +1,15 @@
+{{- if .Values.environment }}
+---
+apiVersion: "v1"
+kind: "Secret"
+metadata:
+  name: "{{ include "common.names.fullname" . }}-environment"
+  labels:
+    {{- include "common.labels.standard" . | nindent 4 }}
+stringData:
+  # Additional environment variables
+  {{- range $key, $value := .Values.environment }}
+  {{ $key }}: {{ $value | quote }}
+  {{- end }}
+...
+{{- end }}
diff --git a/charts/openproject/templates/secret_memcached.yaml b/charts/openproject/templates/secret_memcached.yaml
@@ -0,0 +1,16 @@
+{{- if eq .Values.openproject.cache.store "memcache" }}
+---
+apiVersion: "v1"
+kind: "Secret"
+metadata:
+  name: "{{ include "common.names.fullname" . }}-memcached"
+  labels:
+    {{- include "common.labels.standard" . | nindent 4 }}
+stringData:
+  {{- if .Values.memcached.bundled }}
+  OPENPROJECT_CACHE__MEMCACHE__SERVER: "{{ .Release.Name }}-memcached:11211"
+  {{- else }}
+  OPENPROJECT_CACHE__MEMCACHE__SERVER: "{{ .Values.memcached.connection.host }}:{{.Values.memcached.connection.port }}"
+  {{- end }}
+...
+{{- end }}
diff --git a/charts/openproject/templates/secret_oidc.yaml b/charts/openproject/templates/secret_oidc.yaml
@@ -0,0 +1,26 @@
+{{- if .Values.openproject.oidc.enabled }}
+---
+apiVersion: "v1"
+kind: "Secret"
+metadata:
+  name: "{{ include "common.names.fullname" . }}-oidc"
+  labels:
+    {{- include "common.labels.standard" . | nindent 4 }}
+stringData:
+  # OpenID Connect settings
+  {{ $oidc_prefix := printf "OPENPROJECT_OPENID__CONNECT_%s" (upper .Values.openproject.oidc.provider) }}
+  {{ $oidc_prefix }}_DISPLAY__NAME: {{ .Values.openproject.oidc.provider | quote }}
+  {{ $oidc_prefix }}_HOST: {{ .Values.openproject.oidc.host | quote }}
+  {{ $oidc_prefix }}_IDENTIFIER: {{ .Values.openproject.oidc.identifier | quote }}
+  {{ $oidc_prefix }}_SECRET: {{ .Values.openproject.oidc.secret | quote }}
+  {{ $oidc_prefix }}_AUTHORIZATION__ENDPOINT: {{ .Values.openproject.oidc.authorizationEndpoint | quote }}
+  {{ $oidc_prefix }}_TOKEN__ENDPOINT: {{ .Values.openproject.oidc.tokenEndpoint | quote }}
+  {{ $oidc_prefix }}_USERINFO__ENDPOINT: {{ .Values.openproject.oidc.userinfoEndpoint | quote }}
+  {{ $oidc_prefix }}_END__SESSION__ENDPOINT: {{ .Values.openproject.oidc.endSessionEndpoint | quote }}
+  {{ $oidc_prefix }}_SCOPE: {{ .Values.openproject.oidc.scope | quote }}
+  {{- range $key, $value := .Values.openproject.oidc.attribute_map }}
+  {{ $mapping_key := printf "%s_ATTRIBUTE_MAP_%s" $oidc_prefix (upper $key) }}
+  {{ $mapping_key }}: {{ $value | quote }}
+  {{- end }}
+...
+{{- end }}
diff --git a/charts/openproject/templates/secret_s3.yaml b/charts/openproject/templates/secret_s3.yaml
@@ -0,0 +1,29 @@
+{{- if .Values.s3.enabled }}
+---
+apiVersion: "v1"
+kind: "Secret"
+metadata:
+  name: "{{ include "common.names.fullname" . }}-s3"
+  labels:
+    {{- include "common.labels.standard" . | nindent 4 }}
+stringData:
+  OPENPROJECT_ATTACHMENTS__STORAGE: fog
+  OPENPROJECT_FOG_CREDENTIALS_PROVIDER: AWS
+  {{ $secret := (lookup "v1" "Secret" "openproject" .Values.s3.auth.existingSecret) | default (dict "data" dict) -}}
+  OPENPROJECT_FOG_CREDENTIALS_AWS__ACCESS__KEY__ID: {{
+    default .Values.s3.auth.accessKeyId (get $secret.data .Values.s3.auth.secretKeys.accessKeyId)
+  }}
+  OPENPROJECT_FOG_CREDENTIALS_AWS__SECRET__ACCESS__KEY: {{
+    default .Values.s3.auth.secretAccessKey (get $secret.data .Values.s3.auth.secretKeys.secretAccessKey)
+  }}
+  {{- if .Values.s3.endpoint -}}
+  OPENPROJECT_FOG_CREDENTIALS_ENDPOINT: {{ .Values.s3.endpoint }}
+  {{- end }}
+  OPENPROJECT_FOG_DIRECTORY: {{ .Values.s3.bucketName }}
+  OPENPROJECT_FOG_CREDENTIALS_REGION: {{ .Values.s3.region }}
+  OPENPROJECT_FOG_CREDENTIALS_PATH__STYLE: "{{ .Values.s3.pathStyle }}"
+  OPENPROJECT_FOG_CREDENTIALS_AWS__SIGNATURE__VERSION: "{{ .Values.s3.signatureVersion }}"
+  OPENPROJECT_FOG_CREDENTIALS_USE__IAM__PROFILE: "{{ .Values.s3.use_iam_profile }}"
+  OPENPROJECT_DIRECT__UPLOADS: "{{ .Values.s3.directUploads }}"
+...
+{{- end }}
diff --git a/charts/openproject/templates/secrets.yaml b/charts/openproject/templates/secrets.yaml
diff --git a/charts/openproject/templates/seeder-job.yaml b/charts/openproject/templates/seeder-job.yaml
@@ -14,6 +14,7 @@ spec:
     metadata:
       labels:
         {{- include "common.labels.standard" . | nindent 8 }}
+        openproject/process: seeder
       {{- with .Values.seederJob.annotations }}
       annotations:
         {{- toYaml . | nindent 8 }}
@@ -51,9 +52,8 @@ spec:
             '-c',
             '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" . }}
+          {{- include "openproject.envFrom" . | nindent 10 }}
+          {{- include "openproject.env" . | nindent 10 }}
           resources:
             {{- toYaml .Values.initdb.resources | nindent 12 }}
           {{- include "openproject.containerSecurityContext" . | indent 10 }}
@@ -64,9 +64,8 @@ spec:
           args:
             - bash
             - /app/docker/prod/seeder
-          envFrom:
-            - secretRef:
-                name: {{ include "common.names.fullname" . }}
+          {{- include "openproject.envFrom" . | nindent 10 }}
+          {{- include "openproject.env" . | nindent 10 }}
           volumeMounts:
             {{- if (include "openproject.useTmpVolumes" .) }}
             - mountPath: /tmp

diff --git a/charts/openproject/templates/web-deployment.yaml b/charts/openproject/templates/web-deployment.yaml
@@ -20,10 +20,7 @@ spec:
         {{- range $key, $val := .Values.podAnnotations }}
         {{ $key }}: {{ $val | quote }}
         {{- end }}
-        # annotate pods with env value checksum so changes trigger re-deployments
-        {{- $secretObj := (lookup "v1" "Secret" .Release.Namespace (include "common.names.fullname" .)) | default dict }}
-        {{- $secretData := (get $secretObj "data") | default dict }}
-        checksum/config: {{ values $secretData | sortAlpha | cat | sha256sum }}
+        {{- include "openproject.envChecksums" . | nindent 8 }}
       labels:
         {{- include "common.labels.standard" . | nindent 8 }}
         openproject/process: web
@@ -86,9 +83,8 @@ spec:
           {{- include "openproject.containerSecurityContext" . | indent 10 }}
           image: {{ include "openproject.image" . }}
           imagePullPolicy: {{ .Values.image.imagePullPolicy }}
-          envFrom:
-            - secretRef:
-                name: {{ include "common.names.fullname" . }}
+          {{- include "openproject.envFrom" . | nindent 10 }}
+          {{- include "openproject.env" . | nindent 10 }}
           command:
             - bash
             - /app/docker/prod/wait-for-db
@@ -97,14 +93,8 @@ spec:
           {{- include "openproject.containerSecurityContext" . | indent 10 }}
           image: {{ include "openproject.image" . }}
           imagePullPolicy: {{ .Values.image.imagePullPolicy }}
-          envFrom:
-            - secretRef:
-                name: {{ include "common.names.fullname" . }}
-          {{- if .Values.egress.tls.rootCA.fileName }}
-          env:
-            - name: SSL_CERT_FILE
-              value: "/etc/ssl/certs/custom-ca.pem"
-          {{- end }}
+          {{- include "openproject.envFrom" . | nindent 10 }}
+          {{- include "openproject.env" . | nindent 10 }}
           volumeMounts:
             {{- if (include "openproject.useTmpVolumes" .) }}
             - mountPath: /tmp

diff --git a/charts/openproject/templates/worker-deployment.yaml b/charts/openproject/templates/worker-deployment.yaml
@@ -20,10 +20,7 @@ spec:
         {{- range $key, $val := .Values.podAnnotations }}
         {{ $key }}: {{ $val | quote }}
         {{- end }}
-        # annotate pods with env value checksum so changes trigger re-deployments
-        {{- $secretObj := (lookup "v1" "Secret" .Release.Namespace (include "common.names.fullname" .)) | default dict }}
-        {{- $secretData := (get $secretObj "data") | default dict }}
-        checksum/config: {{ values $secretData | sortAlpha | cat | sha256sum }}
+        {{- include "openproject.envChecksums" . | nindent 8 }}
       labels:
         {{- include "common.labels.standard" . | nindent 8 }}
         openproject/process: worker
@@ -79,9 +76,8 @@ spec:
           {{- include "openproject.containerSecurityContext" . | indent 10 }}
           image: {{ include "openproject.image" . }}
           imagePullPolicy: {{ .Values.image.imagePullPolicy }}
-          envFrom:
-            - secretRef:
-                name: {{ include "common.names.fullname" . }}
+          {{- include "openproject.envFrom" . | nindent 10 }}
+          {{- include "openproject.env" . | nindent 10 }}
           command:
             - bash
             - /app/docker/prod/wait-for-db
@@ -90,14 +86,8 @@ spec:
           {{- include "openproject.containerSecurityContext" . | indent 10 }}
           image: {{ include "openproject.image" . }}
           imagePullPolicy: {{ .Values.image.imagePullPolicy }}
-          envFrom:
-            - secretRef:
-                name: {{ include "common.names.fullname" . }}
-          {{- if .Values.egress.tls.rootCA.fileName }}
-          env:
-            - name: SSL_CERT_FILE
-              value: "/etc/ssl/certs/custom-ca.pem"
-          {{- end }}
+          {{- include "openproject.envFrom" . | nindent 10 }}
+          {{- include "openproject.env" . | nindent 10 }}
           command:
             - bash
             - /app/docker/prod/worker