Skip to content

Resolve external secrets by convention #26

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.

Sign up for GitHub

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

Merged
maor-rozenfeld merged 10 commits into from
Mar 6, 2026
Merged

Resolve external secrets by convention #26

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
83bf554
Resolve external secrets by convention
maor-rozenfeld Mar 6, 2026
fe1e80e
Address PR review: sort keys, remove unused root param, fix whitespace
maor-rozenfeld Mar 6, 2026
5fa2795
Set SUPERSET_SECRET_KEY default in base values
maor-rozenfeld Mar 6, 2026
1c2076a
Replace keyword-based secret detection with explicit openopsEnvSecret…
maor-rozenfeld Mar 6, 2026
030b538
Add all known secret keys with empty defaults to base values
maor-rozenfeld Mar 6, 2026
35e32fb
Set valid bcrypt salt default for OPS_OPENOPS_ADMIN_PASSWORD_SALT
maor-rozenfeld Mar 6, 2026
496a8c3
Remove OPS_OPENOPS_ADMIN_PASSWORD_SALT from base values
maor-rozenfeld Mar 6, 2026
d6084da
Move secrets to openopsEnvSecrets in override example file
maor-rozenfeld Mar 6, 2026
2b8d4d0
Fix openssl rand -hex guidance: -hex 16 for 32-char output
maor-rozenfeld Mar 6, 2026
6c23453
feat: add optional: true to secretKeyRef to prevent pod crashes
maor-rozenfeld Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion chart/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,42 @@ Expected dict: { "root": $, "env": dict }
{{- end }}

{{/*
Render deployment strategy
Resolve the AWS Secrets Manager property name for a secret key.
For standalone keys (in openopsEnv), the property is the key itself.
For derived keys (in tables/analytics/etc), the value is a template ref like
"{{ .Values.openopsEnv.OPS_POSTGRES_PASSWORD }}" - extract the referenced key name.
Expected dict: { "key": "DATABASE_PASSWORD", "value": "{{ .Values.openopsEnv.OPS_POSTGRES_PASSWORD }}" }
*/}}
{{- define "openops.secretPropertyName" -}}
{{- $key := .key -}}
{{- $value := .value | toString -}}
{{- if contains ".Values.openopsEnv." $value -}}
{{- $value | trimPrefix "{{" | trimPrefix " " | trimSuffix "}}" | trimSuffix " " | trimPrefix ".Values.openopsEnv." -}}
{{- else -}}
{{- $key -}}
{{- end -}}
{{- end }}
Comment on lines +207 to +212
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openops.secretPropertyName is not robust for the actual value strings used in values.yaml (they are typically quoted, e.g. "{{ .Values.openopsEnvSecrets.X }}"). Because the function only trims {{/}} and spaces, it will often fail to strip leading quotes/whitespace and may emit an invalid remoteRef.property like the full template string. Consider extracting the referenced key with a regex (capture group on \.Values\.openopsEnvSecrets\.([A-Z0-9_]+)) and falling back to $key when there is no match.

Suggested change
{{- if contains ".Values.openopsEnvSecrets." $value -}}
{{- $value | trimPrefix "{{" | trimPrefix " " | trimSuffix "}}" | trimSuffix " " | trimPrefix ".Values.openopsEnvSecrets." -}}
{{- else -}}
{{- $key -}}
{{- end -}}
{{- end }}
{{- $extracted := regexReplaceAll ".*\\.Values\\.openopsEnvSecrets\\.([A-Z0-9_]+).*" $value "$1" -}}
{{- if ne $extracted "" -}}
{{- $extracted -}}
{{- else -}}
{{- $key -}}
{{- end }}
{{- end }}

Copilot uses AI. Check for mistakes.

{{/*
Collect ExternalSecret data entries for all secret keys in an env map.
Emits YAML list items for keys detected by isSecretKey.
Expected dict: { "env": dict, "secretName": "my-secret" }
*/}}
{{- define "openops.collectSecretEntries" -}}
{{- $env := .env -}}
{{- $secretName := .secretName -}}
{{- range $k := keys $env | sortAlpha -}}
{{- $v := index $env $k -}}
{{- if eq (include "openops.isSecretKey" $k) "true" }}
- secretKey: {{ $k }}
remoteRef:
key: {{ $secretName }}
property: {{ include "openops.secretPropertyName" (dict "key" $k "value" ($v | toString)) }}
{{- end -}}
{{- end -}}
{{- end }}

{{/*
*/}}
{{- define "openops.deploymentStrategy" -}}
{{- if .Values.global.strategy }}
Expand Down
96 changes: 9 additions & 87 deletions chart/templates/external-secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,91 +41,13 @@ spec:
name: {{ .Values.secretEnv.existingSecret | default "openops-env" }}
creationPolicy: Owner
data:
- secretKey: OPS_ENCRYPTION_KEY
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_ENCRYPTION_KEY
- secretKey: OPS_JWT_SECRET
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_JWT_SECRET
- secretKey: OPS_POSTGRES_PASSWORD
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_POSTGRES_PASSWORD
- secretKey: OPS_OPENOPS_ADMIN_PASSWORD
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_OPENOPS_ADMIN_PASSWORD
- secretKey: OPS_ANALYTICS_ADMIN_PASSWORD
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_ANALYTICS_ADMIN_PASSWORD
- secretKey: ANALYTICS_POWERUSER_PASSWORD
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: ANALYTICS_POWERUSER_PASSWORD
- secretKey: OPS_SLACK_APP_SIGNING_SECRET
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_SLACK_APP_SIGNING_SECRET
- secretKey: OPS_LOGZIO_TOKEN
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_LOGZIO_TOKEN
- secretKey: OPS_OPENOPS_ADMIN_PASSWORD_SALT
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_OPENOPS_ADMIN_PASSWORD_SALT
- secretKey: OPS_LANGFUSE_PUBLIC_KEY
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_LANGFUSE_PUBLIC_KEY
- secretKey: OPS_LANGFUSE_SECRET_KEY
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_LANGFUSE_SECRET_KEY
- secretKey: OPS_SSO_FRONTEGG_PUBLIC_KEY
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_SSO_FRONTEGG_PUBLIC_KEY
# Tables derived keys
- secretKey: LOGZIO_TOKEN
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_LOGZIO_TOKEN
- secretKey: OPENOPS_ADMIN_PASSWORD_SALT
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_OPENOPS_ADMIN_PASSWORD_SALT
# Tables (Baserow) derived keys
- secretKey: BASEROW_ADMIN_PASSWORD
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_OPENOPS_ADMIN_PASSWORD
- secretKey: BASEROW_JWT_SIGNING_KEY
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_JWT_SECRET
- secretKey: SECRET_KEY
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_ENCRYPTION_KEY
- secretKey: DATABASE_PASSWORD
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_POSTGRES_PASSWORD
# Analytics (Superset) derived keys
- secretKey: ADMIN_PASSWORD
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: OPS_ANALYTICS_ADMIN_PASSWORD
- secretKey: POWERUSER_PASSWORD
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: ANALYTICS_POWERUSER_PASSWORD
- secretKey: SUPERSET_SECRET_KEY
remoteRef:
key: {{ .Values.externalSecrets.secretName }}
property: SUPERSET_SECRET_KEY
{{- $allEnv := dict -}}
{{- range $k, $v := .Values.openopsEnv }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}
{{- range $k, $v := .Values.tables.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
Comment on lines 43 to +47
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The templating whitespace trimming here is likely to generate invalid YAML under spec.data:. In particular, {{- $allEnv := dict -}} trims both the preceding and following newlines, which can collapse data: and the first list item onto the same line. Remove the trailing - (and consider using include ... | nindent 2/nindent 4 for the emitted list) to ensure data: is followed by a newline and properly indented - secretKey entries.

Copilot uses AI. Check for mistakes.
{{- range $k, $v := .Values.analytics.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- range $k, $v := .Values.postgres.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- if .Values.engine }}{{- if .Values.engine.env }}
{{- range $k, $v := .Values.engine.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
Comment on lines +47 to +51
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The env merge into $allEnv currently uses if not (hasKey $allEnv $k) which prevents later env maps (e.g. analytics.env) from overriding keys defined earlier (e.g. tables.env). This differs from templates/secret-env.yaml, which overwrites on collisions, and it can block legitimate per-component overrides (notably DATABASE_PASSWORD exists in both tables and analytics env by default). Consider using overwrite semantics (later sources win) to match existing behavior.

Suggested change
{{- range $k, $v := .Values.tables.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- range $k, $v := .Values.analytics.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- range $k, $v := .Values.postgres.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- if .Values.engine }}{{- if .Values.engine.env }}
{{- range $k, $v := .Values.engine.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- range $k, $v := .Values.tables.env }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}
{{- range $k, $v := .Values.analytics.env }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}
{{- range $k, $v := .Values.postgres.env }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}
{{- if .Values.engine }}{{- if .Values.engine.env }}
{{- range $k, $v := .Values.engine.env }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}

Copilot uses AI. Check for mistakes.
{{- end }}{{- end }}
{{- include "openops.collectSecretEntries" (dict "env" $allEnv "secretName" .Values.externalSecrets.secretName) }}
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data: is followed by template actions starting at column 0 using {{- ... -}}. The leading - trims the newline/whitespace before the action, which can cause data: and the first rendered list item to end up on the same line (invalid YAML) or otherwise mis-indented. Keep the newline after data: and indent the rendered list items under it (e.g., avoid left-trimming here and/or nindent the included output).

Suggested change
{{- $allEnv := dict -}}
{{- range $k, $v := .Values.openopsEnv }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}
{{- range $k, $v := .Values.tables.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- range $k, $v := .Values.analytics.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- range $k, $v := .Values.postgres.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- if .Values.engine }}{{- if .Values.engine.env }}
{{- range $k, $v := .Values.engine.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{- end }}{{- end }}
{{- include "openops.collectSecretEntries" (dict "env" $allEnv "secretName" .Values.externalSecrets.secretName) }}
{{ $allEnv := dict -}}
{{ range $k, $v := .Values.openopsEnv }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}
{{ range $k, $v := .Values.tables.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{ range $k, $v := .Values.analytics.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{ range $k, $v := .Values.postgres.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{ if .Values.engine }}{{ if .Values.engine.env }}
{{ range $k, $v := .Values.engine.env }}{{ if not (hasKey $allEnv $k) }}{{ $_ := set $allEnv $k ($v | toString) }}{{ end }}{{ end }}
{{ end }}{{ end }}
{{ include "openops.collectSecretEntries" (dict "env" $allEnv "secretName" .Values.externalSecrets.secretName) }}

Copilot uses AI. Check for mistakes.
{{- end }}
4 changes: 3 additions & 1 deletion chart/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ openopsEnv:
OPS_CODE_BLOCK_MEMORY_LIMIT_IN_MB: "256"
OPS_SLACK_APP_SIGNING_SECRET: ""
OPS_SLACK_ENABLE_INTERACTIONS: "true"
# Superset secret key (analytics) - separate from OPS_ENCRYPTION_KEY
SUPERSET_SECRET_KEY: ""
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUPERSET_SECRET_KEY is introduced with an empty default but it isn’t validated as required anywhere (unlike OPS_ENCRYPTION_KEY, OPS_JWT_SECRET, etc). Since analytics.env.SUPERSET_SECRET_KEY now reads from this value, non-external-secret installs can render/deploy with an empty Superset secret key and fail at runtime. Consider adding a chart-level validation for openopsEnv.SUPERSET_SECRET_KEY when analytics is deployed (and validation isn’t being skipped).

Suggested change
SUPERSET_SECRET_KEY: ""
# NOTE: Override this with a strong, unique value in your own values file for production.
SUPERSET_SECRET_KEY: "CHANGE_ME_SUPERSET_SECRET_KEY"

Copilot uses AI. Check for mistakes.

secretEnv:
create: true
Expand Down Expand Up @@ -346,7 +348,7 @@ analytics:
DATABASE_USER: "{{ .Values.openopsEnv.OPS_POSTGRES_USERNAME }}"
DATABASE_PASSWORD: "{{ .Values.openopsEnv.OPS_POSTGRES_PASSWORD }}"
DATABASE_HOST_ALT: "{{ .Values.openopsEnv.OPS_OPENOPS_TABLES_DB_HOST }}"
SUPERSET_SECRET_KEY: "{{ .Values.openopsEnv.OPS_ENCRYPTION_KEY }}"
SUPERSET_SECRET_KEY: "{{ .Values.openopsEnv.SUPERSET_SECRET_KEY }}"
Comment thread
maor-rozenfeld marked this conversation as resolved.
Outdated
SUPERSET_FEATURE_ALLOW_ADHOC_SUBQUERY: '"{{ .Values.openopsEnv.ANALYTICS_ALLOW_ADHOC_SUBQUERY }}"'
REDIS_HOST: "{{ .Values.openopsEnv.OPS_REDIS_HOST }}"
REDIS_PORT: "{{ .Values.openopsEnv.OPS_REDIS_PORT }}"
Expand Down
Loading