fix(web): restart the container when a podman secret is rotated #276

Sammanfogat

supernaut sammanfogade 1 incheckning från fix/274-web-secret-rotation-restart in i main

2026-07-31 10:01:54 +00:00

supernaut kommenterad

2026-07-31 09:40:36 +00:00

Ägare

Closes #274.

Rotating any web podman secret reported changed and left the running container on the old value.
Each secret task now registers its result; a set_fact reduces them to web_secrets_changed; the
restart condition consumes it alongside the existing unit-change and image-drift triggers.

 state: >-
   {{ 'restarted'
      if (web_unit is changed)
+        or (web_secrets_changed | default(false) | bool)
         or ((web_running_image.stdout | default('') | trim) != (web_image ~ ':' ~ web_image_tag))
      else 'started' }}

Why not the forgejo pattern

The forgejo role embeds a content hash in the secret name, so a rotation renames the secret, which
rewrites the unit, which trips web_unit is changed. That would have been the more consistent
choice, and I went the other way for two reasons:

No orphans. Renaming on every rotation leaves the old podman secret behind, and nothing in this
repo prunes them. Seven secrets rotating over a service's lifetime accumulates.
The detection already exists. podman_secret compares the data itself, so the change signal is
available without encoding it in a name.

Happy to switch to the hashed-name pattern if consistency is preferred — it is a small change either
way.

The `force: true` risk, checked rather than assumed

Every one of these tasks sets force: true. If that made the module report changed unconditionally,
this fix would restart the portal on every converge — a much worse bug than the one being fixed.
It does not; the module compares content first. Verified against two real applies from last night:

Apply	Vault state	Secret tasks
03:10	unchanged	4/4 sampled `ok`
03:36	Sweego key rotated	1 `changed`, 3 `ok`

Reducer test

The expression has to be false for every not-actually-rotated shape, including ones that arise only
in odd runs. Exercised standalone:

Case	Result
All vars undefined (role short-circuited)	`False`
All present, none changed	`False`
Skipped-task dicts, one missing `.changed` entirely	`False`
One rotated (sweego)	`True`
Session secret rotated only	`True`

Hence the double default on each item — (_var | default({})).changed | default(false) — which
covers both an undefined var and a dict with no changed key.

Verification

ansible-playbook site.yml --syntax-check passes; ansible-lint roles/web/ passes on the
production profile.
--check --diff --tags web against prod: ok=12 changed=1 failed=0, the single changed task being
Enable and start bitborg-web. That is pre-existing check-mode noise, not this change —
web_running_image is registered from a command, which does not run under --check, so empty
stdout trips the image-drift branch. The dry-run before this change showed the identical single
task.
Check mode cannot exercise the new path at all. Podman commands do not run under --check, so
the secret tasks report skipping and the fact comes out false. Documented in the role so the next
reader does not mistake that for a broken fix. The real proof is a rotation on a live apply.

Suggested post-merge check

On the next apply with unchanged vault values, confirm Enable and start bitborg-web reports ok
(no spurious restart). The positive case gets proven for free the next time any web secret is
genuinely rotated.

Closes #274. Rotating any web podman secret reported `changed` and left the running container on the old value. Each secret task now registers its result; a `set_fact` reduces them to `web_secrets_changed`; the restart condition consumes it alongside the existing unit-change and image-drift triggers. ```diff state: >- {{ 'restarted' if (web_unit is changed) + or (web_secrets_changed | default(false) | bool) or ((web_running_image.stdout | default('') | trim) != (web_image ~ ':' ~ web_image_tag)) else 'started' }} ``` ## Why not the forgejo pattern The forgejo role embeds a content hash in the secret name, so a rotation renames the secret, which rewrites the unit, which trips `web_unit is changed`. That would have been the more consistent choice, and I went the other way for two reasons: - **No orphans.** Renaming on every rotation leaves the old podman secret behind, and nothing in this repo prunes them. Seven secrets rotating over a service's lifetime accumulates. - **The detection already exists.** `podman_secret` compares the data itself, so the change signal is available without encoding it in a name. Happy to switch to the hashed-name pattern if consistency is preferred — it is a small change either way. ## The `force: true` risk, checked rather than assumed Every one of these tasks sets `force: true`. If that made the module report `changed` unconditionally, this fix would restart the portal on **every** converge — a much worse bug than the one being fixed. It does not; the module compares content first. Verified against two real applies from last night: | Apply | Vault state | Secret tasks | | --- | --- | --- | | 03:10 | unchanged | 4/4 sampled `ok` | | 03:36 | Sweego key rotated | 1 `changed`, 3 `ok` | ## Reducer test The expression has to be false for every not-actually-rotated shape, including ones that arise only in odd runs. Exercised standalone: | Case | Result | | --- | --- | | All vars undefined (role short-circuited) | `False` | | All present, none changed | `False` | | Skipped-task dicts, one missing `.changed` entirely | `False` | | One rotated (sweego) | `True` | | Session secret rotated only | `True` | Hence the double default on each item — `(_var | default({})).changed | default(false)` — which covers both an undefined var and a dict with no `changed` key. ## Verification - `ansible-playbook site.yml --syntax-check` passes; `ansible-lint roles/web/` passes on the production profile. - `--check --diff --tags web` against prod: `ok=12 changed=1 failed=0`, the single changed task being `Enable and start bitborg-web`. **That is pre-existing check-mode noise, not this change** — `web_running_image` is registered from a `command`, which does not run under `--check`, so empty stdout trips the image-drift branch. The dry-run before this change showed the identical single task. - **Check mode cannot exercise the new path at all.** Podman commands do not run under `--check`, so the secret tasks report `skipping` and the fact comes out false. Documented in the role so the next reader does not mistake that for a broken fix. The real proof is a rotation on a live apply. ## Suggested post-merge check On the next apply with unchanged vault values, confirm `Enable and start bitborg-web` reports `ok` (no spurious restart). The positive case gets proven for free the next time any web secret is genuinely rotated.

supernaut lade till 1 incheckning

2026-07-31 09:40:36 +00:00

fix(web): restart the container when a podman secret is rotated

ci / ci (pull_request) Successful in 1m33s

Detaljer

509f9f53f2

Closes #274.

Podman injects secrets into the container environment at container creation, so
rotating a secret's value does nothing to a running container. The web secrets
use static names, so a rotation leaves the Quadlet unit byte-identical and
`web_unit is changed` stays false — the restart never fired.

That is how the Sweego API key rotation reported a clean `changed=1 failed=0`
apply while the portal kept serving the old key. The rotation only took effect
after an explicit `systemctl --user restart gitborg-web`.

All seven web secrets were affected — DATABASE_URL, the Kanidm provision token,
the Sweego key, the sign-up preview token, the Cap captcha secret, the OIDC
client secret and the session signing secret. The shared failure mode is a
rotation that reports success and silently does not apply, which is worst for
the ones rotated deliberately for security reasons.

Each secret task now registers its result, a set_fact reduces them to
`web_secrets_changed`, and the restart condition consumes it alongside the
existing unit-change and image-drift triggers.

Chose this over the forgejo role's content-hashed secret names (which make a
rotation rewrite the unit) because it needs no name churn and leaves no orphaned
secrets behind, and because podman_secret already detects content changes
itself.

Verified `force: true` does not make those tasks always report changed — an
apply with unchanged vault values reports every secret task `ok`, so this cannot
bounce the portal on an unrelated converge. The reducer is unit-tested across
five shapes: all-undefined, present-and-unchanged, skipped dicts missing
`.changed`, one rotated, and session-only rotated. First three yield false, last
two true.

Check mode cannot exercise this — podman commands do not run under `--check`, so
the secret tasks skip and the fact comes out false. Documented in the role.

supernaut sammanfogade incheckning 47d799e4d5 till main

2026-07-31 10:01:54 +00:00

supernaut tog bort grenen fix/274-web-secret-rotation-restart

2026-07-31 10:01:54 +00:00

supernaut refererade denna ändringsförfrågan från en incheckning

2026-07-31 10:01:56 +00:00

fix(web): restart the container when a podman secret is rotated (#276)

supernaut refererade denna ändringsförfrågan från en incheckning

2026-08-03 09:41:35 +00:00

fix(web): restart the container when a podman secret is rotated (#276)

Logga in för att delta i denna konversation.

Inga granskare