feat(home-assistant): render recorder_db_url from the ha-pg Secret #524
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: konairius/homelab#524
Reference in New Issue
Block a user
Refs #434. Phase 2a of the recorder move off SQLite — follows #521 (the empty ha-pg cluster).
What
An init container that writes
recorder_db_urlinto/config/secrets.yamlfrom the CNPG-generatedha-pg-appSecret.Still inert
It only adds a key to
secrets.yaml. Nothing reads it untildb_url: !secret recorder_db_urllands inconfiguration.yamlin the homeassistant repo.That ordering is required in this direction: HA refuses to start when a referenced secret is missing, so the config change must not go first. This PR is the prerequisite.
Why an init container, not a hand-added line
The password is generated by CNPG and can be rotated. A hand-written entry goes stale silently and HA then fails to start on a bad password. Re-rendering on every pod start is also what makes a rotation take effect.
The careful part
secrets.yamlis PVC runtime state that config-sync deliberately does not manage — it holdsinfluxdb_tokenand others. So this must edit the file, never replace it.The script filters any existing
recorder_db_urlline, appends the current one, then guards the result: if the rewrite would drop any non-recorder line it aborts rather than truncating the file that holds every other secret HA needs. Written to a temp path, moved atomically,chmod 600.Verified before commit
Run in HA own Alpine/busybox shell (matching the init container runtime) against a sample file:
Remaining for the cutover
SCHEMA_VERSION = 53, confirmed to match the live database) and copystatistics_meta->statistics->statistics_short_term. SQLite backup API is available, so the snapshot needs no HA downtime.configuration.yamlin the homeassistant repo — the one-linedb_urlchange. Single restart, landing in an already-populated database.Note: I cannot commit to the homeassistant repo from this session (worktree-isolated to homelab), so step 3 needs a hand.
🤖 Generated with Claude Code
🤖 Automated review — model
CLI default(complexity: complex), commit17f913ba2f4217415eb5cca95eed88c508f8e22e.Summary
Adds an
initContainer(render-db-url) to the Home AssistantDeploymentthat rendersrecorder_db_urlinto/config/secrets.yamlfrom the CNPG-generatedha-pg-appSecret, ahead of repointing the recorder off SQLite. The edit-in-place-with-guard approach forsecrets.yaml(filter old key, append new, abort on line-count shrink, atomicmv,chmod 600) is sound and well reasoned. One real defect: the script builds apostgresql://URL by directly interpolating the CNPG-generated username/password without percent-encoding, which this same repo has already identified as unsafe for this exact secret-generation mechanism.🔴 Blocking
kubernetes/apps/home-assistant/deployment.yaml:70—U="postgresql://${PGUSER}:${PGPASSWORD}@ha-pg-rw...interpolates the CNPG-generatedha-pg-appusername/password directly into a URL without percent-encoding. If the password (or username) contains any URL-reserved character (@,:,/,%,#,?, …), the resulting URI is malformed — SQLAlchemy's URL parser splits on the first@/:in the userinfo, so a reserved character in the password doesn't error loudly, it silently truncates or misparses the credential, and HA's recorder connects with the wrong password (or fails to start) oncedb_url: !secret recorder_db_urllands.This exact pitfall is already documented in this repo for the same CNPG password-generation mechanism:
kubernetes/apps/woodpecker/server.yaml:70-87deliberately uses alib/pqkeyword=value DSN instead of a URL DSN for thewp-pg-appsecret, with the comment "robust to any special chars in the generated password (unlike a URL DSN)". This PR does the thing that comment says to avoid, for the same class of secret (ha-pg-app, generated by the same CNPG default mechanism aswp-pg-app).Unlike woodpecker, HA's
recorder.db_urlis consumed as a SQLAlchemy URL, not a keyword DSN, so switching format isn't an option here — the fix has to percent-encodePGUSER/PGPASSWORDbefore building the URL (e.g., a smallsed/awkpercent-encoding pass over the reserved-character set, since thebusybox:1.36image has nopython3/jqto lean on). At minimum, if there's a reason to believe CNPG's default password charset is alnum-only and this can't actually fire, that assumption should be stated explicitly in the comment block, since the repo's own prior art argues against relying on it silently.Because this bug is inert today (nothing reads
recorder_db_urlyet) it won't be caught by anything before the cutover PR in thehomeassistantrepo — at which point it fails exactly the class of bug the guard logic in this same script was written to avoid (silent corruption of a value HA depends on), just one layer up (URL syntax instead of file syntax).🟡 Suggestions
kubernetes/apps/home-assistant/deployment.yaml:45— same root cause as above, one level down:printf 'recorder_db_url: "%s"\n' "$U"wraps the value in a double-quoted YAML scalar without escaping embedded"or\. IfPGPASSWORDever contained a", the emitted line would either break YAML parsing ofsecrets.yamlentirely (takinginfluxdb_token/mqtt_passworddown with it — the exact failure the line-count guard exists to prevent) or silently truncate the value at the quote. Worth a one-linesed 's/"/\\"/g'on$U(or on the raw password component) for defense in depth, even if percent-encoding for the URL fix above happens to sidestep this too (percent-encoding"removes it from the literal value).kubernetes/apps/home-assistant/MIGRATION-recorder-postgres.mdisn't updated by this PR. It still describes step 3 ("render it intosecrets.yamlthe same way ... zigbee2mqtt-deployment.yaml") as future work; since this PR does that (with a deliberately different, edit-in-place strategy — correctly, sincesecrets.yamlunlike Z2M'ssecret.yamlisn't fully machine-owned), a short note in the doc marking this step done and pointing at the actual approach would keep the migration doc trustworthy as a sequencing reference for the remaining steps.🟢 Nits
None beyond the above.
Tests
No automated coverage, and there realistically can't be much here — this is a GitOps manifest repo;
.woodpecker/ci-scripts-test.ymlonly coversscripts/ci/*.sh, andci-manifests-test.yml'sshellcheckstep explicitly does not reach into shell embedded in Kubernetes manifests (its own comment: "That embedded shell is where several real bugs lived this year, so it remains a gap worth closing separately") — this PR'srender-db-urlscript is exactly that gap, and it's where the blocking issue above lives. The PR body describes manual verification (guard behavior, idempotency, byte-identical re-run) in a matching Alpine/busybox shell, which is reasonable given no CI covers this, but that manual run wouldn't have exercised a password containing a URL-reserved or"character, so it wouldn't have caught the issue above.Recommendation
Request changes — 1 blocking issue.
🤖 Automated review — model
CLI default(complexity: complex), commit57bb39e7ee7ffd1a5181f94239658a1e35898494.Summary
Adds an
initContainersentry (render-db-url) tokubernetes/apps/home-assistant/deployment.yamlthat renders a percent-encodedrecorder_db_urlline into the runtimesecrets.yamlon the HA config PVC, sourced from the CNPG-generatedha-pg-appSecret, plus a documentation update toMIGRATION-recorder-postgres.mdexplaining the ordering requirement. The script itself is careful: line-wise edit (never a full YAML round-trip that would reorder/strip comments),json.dumpsfor a safely-escaped scalar, before/after key-set validation, atomic write via temp file +os.replace,chmod 600. I verified the referencedha-pg-appSecret name andha-pg-rwservice name againstha-pg-cluster.yaml(default CNPG naming, no override) — they match. No blocking issues found.🟡 Suggestions
deployment.yaml:68— the init container pinsimage: ghcr.io/home-assistant/home-assistant:2026.8.2without the# {"$imagepolicy": "flux-system:home-assistant"}marker that the main container carries (deployment.yaml:143). Perkubernetes/infrastructure/image-automation/README.md, Flux'sSettersstrategy patches only lines carrying the marker, so future automated bumps will update the main container's tag but leave this init container pinned to2026.8.2. That's a silent version-skew that also undercuts the comment's own stated rationale ("no extra image is pulled — it is already on the node for the main container"): once the tags diverge, the node has to pull a second image. Low functional risk today (the script only usesjson/os/sys/urllib.parse/yamlfrom the standard library), but worth either adding the marker to both lines or explicitly noting the pin is intentional-and-manual (as the repo already does fortrmnl/terminus.yaml's two containers).deployment.yaml:100-101—before = yaml.safe_load(old) or {} if old.strip() else {}is correct (theor {}binds inside the ternary's true-branch, so it can't accidentally returnNone) but is hard to read at a glance.yaml.safe_load(old) or {}alone already handlesold == ""(sincesafe_load("")returnsNone), so theif old.strip() else {}guard is redundant. Not a bug, just worth simplifying if this is touched again.Tests
No automated tests (there's no test harness for these Kustomize manifests / init-container scripts in this repo). The PR body documents manual verification of the rewrite logic (guard behavior, idempotency, preservation of unrelated keys) run against a sample file using HA's own Python runtime, matching the init container's runtime — reasonable given the constraints. The one thing not exercised per the description is a
secrets.yamlthat fails to parse at all (pre-existing corruption) — in that caseyaml.safe_load(old)atdeployment.yaml:106raises unhandled, and the init container crashes (blocking the pod from starting rather than corrupting the file). That's a defensible fail-closed behavior, not a defect, but wasn't explicitly called out in the verification notes.Recommendation
Looks reasonable — no blocking issues; comments above are optional.
Phase 2b is done: 12.77M statistics rows copied SQLite -> Postgres with HA running and no downtime (read-only mount; SQLite WAL gives readers a consistent snapshot). statistics_meta 1449 OK statistics 10799904 OK 153.9s statistics_short_term 1969804 OK 27.0s span in Postgres: 2024-03-13 -> 2026-08-19 1449 entities with long-term history, 2627859 rows carrying sum database size 2252 MB against a 20Gi PVC Records the high-water marks the cutover top-up must resume from, and two schema details that cost a failed first attempt: - has_mean/has_sum are INTEGER in SQLite, BOOLEAN in Postgres, and Postgres will not coerce them. - the legacy created/start/last_reset DATETIME columns are 100% NULL in both bulk tables, which is what makes a tab-separated COPY safe. Also documents why the schema was built from HA's own models with schema_changes left EMPTY: HA's _initialize_database detects a new database via the events.time_fired_ts index and writes SCHEMA_VERSION itself, so pre-stamping that row would bypass HA's own initialisation. ANALYZE was run on all three tables. After a bulk COPY the planner has no statistics and HA seq-scans 10.8M rows on every history/energy view. Refs #434 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>🤖 Automated review — model
CLI default(complexity: complex), commit39885b94007fd60b8b7ea10d28c317983f449a1f.Summary
Adds a
render-db-urlinit container to the Home Assistant Deployment that rendersrecorder_db_urlinto/config/secrets.yamlfrom the CNPG-generatedha-pg-appSecret, plus matching updates toMIGRATION-recorder-postgres.md. The change is inert (nothing reads the new key yet) and the edit-in-place logic (filter-by-prefix, append, validate parsed YAML round-trip, atomic replace) is careful about the one real risk here — corrupting the hand-maintained, non-config-sync-managedsecrets.yaml. I traced the embedded Python line by line against the checkout and found no correctness or security defects. One documentation gap below.🟡 Suggestions
kubernetes/apps/home-assistant/MIGRATION-recorder-postgres.md: the doc now has## Phase 1,## Phase 2b,## Phase 2c, but no## Phase 2aheading — even though this PR's own commit history / PR body call the init container "phase 2a", and the doc itself references "phase 2a" by name at line 140 ("rendered intosecrets.yamlby therender-db-urlinit container indeployment.yaml(phase 2a)"). A reader following the runbook top-to-bottom will hit a reference to a phase that's never introduced as its own section, unlike Phase 1 which gets an explicit "(this PR)" heading. Worth adding a short## Phase 2a — render the secret (this PR)section (mirroring the Phase 1 pattern) so the numbering is self-consistent, since this file is the operational source of truth for the cutover sequencing.deployment.yaml:16(secret.reloader.stakater.com/reload) only watcheshome-assistant-config-sync-token. The init container's own justification is "the password is generated by CNPG and can be rotated... this re-renders it on every pod start, which is also what makes a rotation take effect" — but nothing currently triggers a pod restart whenha-pg-approtates, so in practice a rotation only takes effect on the next unrelated restart, not automatically. If the rotation-triggers-refresh property described in the comment is meant to hold, consider addingha-pg-appto the Reloader annotation (comma-separated, matching thewoodpecker-secret/home-assistant-config-sync-tokenpattern already used in this repo). Not blocking since the whole feature is inert until phase 2c, but worth deciding before then.Tests
No automated tests exist for this (it's a Kubernetes manifest + inline script, not application code with a test harness in this repo). The PR body describes a manual dry run of the rendering logic against a sample file in HA's own Alpine/busybox-class shell, checking idempotency and preservation of unrelated keys (
influxdb_token,mqtt_password) — reasonable verification for this kind of change, though it's an out-of-band claim I can't re-run from the diff alone. The script's own internal guard (abort if any pre-existing key would be dropped, plus a round-trip check thatrecorder_db_urlparses back to the exact value written) is the real safety net here and is sound: it fails closed (non-zero exit, noos.replace) rather than risking a silent truncation ofsecrets.yaml.Recommendation
Looks reasonable — no blocking issues; comments above are optional.
🤖 Automated review — model
CLI default(complexity: complex), commit6ae734eef4a354090619f195b1f0e002ae73f146.Summary
Adds a
render-db-urlinit container to the Home AssistantDeploymentthat writesrecorder_db_urlinto/config/secrets.yamlfrom the CNPG-generatedha-pg-appSecret, plus documentation of the completed Phase 2b bulk-copy and the Phase 2c cutover steps. The mechanism (filter-then-append, line-count guard, atomic move) is sound, but the Secret key referenced by the init container does not match this repo's established CNPG convention and will very likely fail at deploy time.🔴 Blocking
kubernetes/apps/home-assistant/deployment.yaml:87—secretKeyRef: { name: ha-pg-app, key: fqdn-uri }. Every other CNPG-backed app in this repo that consumes the generated connection URI useskey: uri, notfqdn-uri:kubernetes/apps/trmnl/terminus.yaml:150,215—secretKeyRef: { name: tr-pg-app, key: uri }kubernetes/apps/trmnl/externalsecret.yaml:14-15— comment explicitly states "it comes straight off the CNPG-generatedtr-pg-appSecret'surikey"All CNPG clusters in this repo (including
ha-pg, seekubernetes/apps/home-assistant/ha-pg-cluster.yaml) are managed by the single cluster-wide CNPG operator installed inkubernetes/infrastructure/cloudnative-pg/helmrelease.yaml, so the generated-appSecret schema is the same across clusters.fqdn-uriis not a key CNPG generates (user,password,dbname,host,port,uri,jdbc-uriare the standard ones, and this repo's own precedent above confirmsuriis the one in actual use here). Since the init container'senv.valueFrom.secretKeyRefhas nooptional: true, a missing key producesCreateContainerConfigError, and an init container that can't start blocks every container after it. Withstrategy: Recreateand a single replica, the next Home Assistant pod rollout will fail to start at all — this is the opposite of the "inert" change the PR describes; the render step runs unconditionally on every pod start regardless of whetherconfiguration.yamlreferences the secret yet. Recommend changingkey: fqdn-uritokey: uri(and re-verifying against the liveha-pg-appSecret, as the PR description says was already done, before merging).🟡 Suggestions
kubernetes/apps/home-assistant/deployment.yaml:127-128— the guard only checks line count (wc -l), not content. That's an intentional, documented trade-off ("cheap guard against the one failure that would actually hurt: losing the other keys") and is reasonable for this use case, but it's worth noting it would not catch a rewrite that drops one real key and coincidentally gains a blank line elsewhere. Given the narrow, single-writer nature of this script, this is a minor residual risk, not a blocker.optional: falseis already the default, but given the blocking issue above, it may be worth having the containercommandfail loudly and distinctly (it already does, viaset -eu) — once the key name is fixed, confirmkubectl -n home-assistant get secret ha-pg-app -o jsonpath='{.data.uri}'actually decodes to a validpostgresql://...URI before relying on the "no escaping needed" claim in the comment (deployment.yaml:49-51), since that claim is central to the safety argument for skipping percent-encoding/validation.🟢 Nits
kubernetes/apps/home-assistant/deployment.yaml:91-96— the init container'sresourcessets amemorylimit but nocpulimit, consistent with several sibling containers in this same file (proxy,code-server,config-sync), so this matches existing repo convention rather than being an issue.Tests
No automated tests exist for this repo's Kubernetes manifests (GitOps/Kustomize repo), consistent with the rest of the codebase — manual verification via
kubectlagainst the live cluster is the norm here (as documented in the PR body andMIGRATION-recorder-postgres.md). Given the blocking issue above, the claimed pre-commit verification ("Run in HA's own Alpine/busybox shell... against a sample file") evidently exercised the script's file-editing logic but not the actualsecretKeyRefresolution against the realha-pg-appSecret — that gap is exactly what let the wrong key name through. Oncekey: uriis used, a real-cluster check that the init container completes successfully (kubectl -n home-assistant logs -c render-db-url ...showingrecorder_db_url rendered, and that the main HA container still becomes Ready) would close that gap.Recommendation
Request changes — 1 blocking issue.
Re: the blocking finding on
key: fqdn-uri(deployment.yaml:87) — the factual premise is incorrect, so I am not changing this. Evidence below.CNPG does generate
fqdn-uriLive
ha-pg-appsecret, full key list:fqdn-uriis there, alongsideuri,jdbc-uriandfqdn-jdbc-uri. CNPG emits both host forms for each URI flavour.It is not specific to this cluster
Same key on the other CNPG clusters served by the same operator:
So the observation that trmnl references
key: uridoes not implyfqdn-uriis absent — both keys exist on every one of these secrets. trmnl choosing one is a preference, not evidence of the other missing.The two differ only in host form
fqdn-uriis the deliberate pick: a fully-qualified cluster-local name rather than a namespace-relative one, matching how every other in-cluster endpoint is written in this repo (e.g.openbao.openbao.svc.cluster.local,woodpecker.woodpecker.svc.cluster.local).The predicted failure mode does not occur
The review predicts
CreateContainerConfigErroron the next rollout. I tested exactly that — a pod whose only env var issecretKeyRef: {name: ha-pg-app, key: fqdn-uri}:Started, resolved the key, exited cleanly. No config error.
What I agree with
Triaging this as security-sensitive was right — it handles generated database credentials and writes the file holding every other HA secret. Two earlier findings on this PR were correct and led to real fixes (missing percent-encoding, then the YAML-corruption risk). This particular claim is not.
Unrelated but worth recording here: the
ha-pg-apppassword was rotated during this work after I leaked it into a working transcript. CNPG regenerated the whole secret; the old password is confirmed rejected and the new one confirmed working. All URI keys above are post-rotation.🤖 Generated with Claude Code