feat(chart): support existingSecret for externalDatabase, fail fast on misconfiguration - #198
Open
danielqb wants to merge 1 commit into
Open
feat(chart): support existingSecret for externalDatabase, fail fast on misconfiguration#198danielqb wants to merge 1 commit into
danielqb wants to merge 1 commit into
Conversation
…n misconfiguration
## Problem
externalDatabase.password (used when mariadb.enabled: false, e.g. RDS /
Cloud SQL / Azure Database) could only be set as a plaintext value in
values.yaml - no existingSecret alternative, unlike the internal
mariadb.auth.existingSecret the helmforge/mariadb subchart already
supports. This is exactly the audience (managed external databases)
most likely to run GitOps/Vault/Doppler/External Secrets workflows and
want to avoid plaintext passwords in values.yaml/release history.
Separately, if mariadb.enabled is set to false without configuring
externalDatabase.host, the chart silently rendered MARIADB_HOST: "" -
the wait-for-mariadb initContainers (`nc -z "" 3306`) then hang
indefinitely on every install/upgrade instead of failing with a clear
error.
## Design
Mirrors the pattern used by mainstream charts for this exact scenario
(verified against bitnami/moodle, which has the same
mariadb.enabled-or-externalDatabase shape): externalDatabase.existingSecret,
falling back to a chart-generated Secret from the plaintext password
when unset, both read via a fixed, documented key ("password") rather
than a second customizable "which key" field.
- Added externalDatabase.existingSecret to values.yaml.
- Added glpi.externalDatabase.secretName helper (existingSecret, or
this chart's own generated {fullname}-externaldb Secret).
- Added glpi.database.secretName / glpi.database.secretPasswordKey
helpers that resolve to either the internal mariadb subchart's secret
or the external database's secret depending on mariadb.enabled - lets
every consumer (php-fpm, the 4 DB-touching init Jobs, cronjob) use a
single unconditional secretKeyRef instead of duplicating the
mariadb.enabled branch in each template.
- Added glpi.externalDatabase.validate: fails fast with a clear message
if mariadb.enabled is false and host/database/username are missing,
or neither existingSecret nor password is set. Invoked from
glpi-configmap.yaml since that template always renders regardless of
which specific resource Helm processes first.
- New glpi-externaldb-secret.yaml: generates the chart-owned Secret,
skipped entirely when existingSecret is set.
- glpi-secret.yaml no longer inlines MARIADB_PASSWORD for the
externalDatabase case (same reasoning as the mariadb.enabled fix in
eftechcombr#191: doing so would silently render an empty password whenever
existingSecret is used).
## Testing
- helm lint --strict: 0 failures
- helm template (default, mariadb.enabled=true): unchanged, still reads
MARIADB_PASSWORD from the mariadb subchart's own secret
- helm template with externalDatabase + plaintext password: chart
generates its own Secret correctly (verified the base64 value decodes
back to the plaintext password)
- helm template with externalDatabase.existingSecret set: secretKeyRef
points directly at it, no chart-generated Secret rendered at all
- helm template with mariadb.enabled=false and progressively missing
fields: 3 separate fail-fast errors, one per missing
host/database/(password-or-existingSecret)
- Full live kind cluster test: deployed a standalone MariaDB Deployment
simulating an external/managed database, plus a manually-created
Secret simulating a Vault/Doppler-injected one
- With externalDatabase.existingSecret pointing to it: `helm install`
completed (STATUS: deployed, meaning the dbInstall/dbConfigure hooks
genuinely connected and configured the external DB), confirmed no
internal mariadb subchart pod was created, php-fpm's MARIADB_PASSWORD
env var matches the external secret exactly, no chart-generated
Secret exists, and GLPI served its real login page (HTTP 200,
"Authentication - GLPI")
- Re-ran with externalDatabase.password (plaintext) instead: confirmed
the chart-generated Secret exists this time, HTTP 200 again
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
externalDatabase.password (used when mariadb.enabled: false, e.g. RDS /
Cloud SQL / Azure Database) could only be set as a plaintext value in
values.yaml - no existingSecret alternative, unlike the internal
mariadb.auth.existingSecret the helmforge/mariadb subchart already
supports. This is exactly the audience (managed external databases)
most likely to run GitOps/Vault/Doppler/External Secrets workflows and
want to avoid plaintext passwords in values.yaml/release history.
Separately, if mariadb.enabled is set to false without configuring
externalDatabase.host, the chart silently rendered MARIADB_HOST: "" -
the wait-for-mariadb initContainers (
nc -z "" 3306) then hangindefinitely on every install/upgrade instead of failing with a clear
error.
Design
Mirrors the pattern used by mainstream charts for this exact scenario
(verified against bitnami/moodle, which has the same
mariadb.enabled-or-externalDatabase shape): externalDatabase.existingSecret,
falling back to a chart-generated Secret from the plaintext password
when unset, both read via a fixed, documented key ("password") rather
than a second customizable "which key" field.
this chart's own generated {fullname}-externaldb Secret).
helpers that resolve to either the internal mariadb subchart's secret
or the external database's secret depending on mariadb.enabled - lets
every consumer (php-fpm, the 4 DB-touching init Jobs, cronjob) use a
single unconditional secretKeyRef instead of duplicating the
mariadb.enabled branch in each template.
if mariadb.enabled is false and host/database/username are missing,
or neither existingSecret nor password is set. Invoked from
glpi-configmap.yaml since that template always renders regardless of
which specific resource Helm processes first.
skipped entirely when existingSecret is set.
externalDatabase case (same reasoning as the mariadb.enabled fix in
fix(chart): read MARIADB_PASSWORD from the mariadb subchart's own Secret #191: doing so would silently render an empty password whenever
existingSecret is used).
Testing
MARIADB_PASSWORD from the mariadb subchart's own secret
generates its own Secret correctly (verified the base64 value decodes
back to the plaintext password)
points directly at it, no chart-generated Secret rendered at all
fields: 3 separate fail-fast errors, one per missing
host/database/(password-or-existingSecret)
simulating an external/managed database, plus a manually-created
Secret simulating a Vault/Doppler-injected one
helm installcompleted (STATUS: deployed, meaning the dbInstall/dbConfigure hooks
genuinely connected and configured the external DB), confirmed no
internal mariadb subchart pod was created, php-fpm's MARIADB_PASSWORD
env var matches the external secret exactly, no chart-generated
Secret exists, and GLPI served its real login page (HTTP 200,
"Authentication - GLPI")
the chart-generated Secret exists this time, HTTP 200 again