feat(mosquitto): enrol the health broker credential in Class-A rotation #455

2026-07-30T14:13:34Z

claude-bot commented

2026-07-30 14:13:34 +00:00

Phase 1 of #453. One of eight broker users; the other seven are untouched and keep their bootstrap Jobs.

Why `health` first

It's the broker's own liveness/readiness probe credential, consumed by exec probes inside the mosquitto container via the same Secret that seeds the passwd file. Nothing outside the pod holds it — so rotating it needs no coordination with any client. Smallest possible blast radius for proving rotator → OpenBao → ESO → Reloader end to end, before touching credentials held by Shelly firmware, the edge host, or the HA config entry.

What it adds

Following rotation/paperless.yaml exactly:

rotator-mosquitto-health.hcl — per-secret policy, read+patch on that one path. Deliberately not a wildcard over mosquitto-*: users are being enrolled one at a time, and a wildcard would silently grant this rotator write access to the other seven before they're ready.
rotation/mosquitto-health.yaml — SA + quarterly CronJob, secret-rotator random mode. Scheduled a month off paperless so unrelated rotations don't share a window.
mosquitto-health-credentials-externalsecret.yaml — projects back to the same Secret name and key, so mosquitto-deployment.yaml's secretKeyRef and the probes reading $HEALTH_PASSWORD are unchanged. refreshInterval: 10m rather than evcc's 1h, so a rotation lands promptly instead of leaving the old password valid for up to an hour.
A Reloader annotation. Without it rotation is inert — the startup shim rebuilds /mosquitto/auth/passwd from the Secrets on pod start, so a rotated Secret would sit unused until some unrelated restart. Listed per-Secret rather than auto: "true", so enrolling each further user stays a visible edit.

Removes only the health bootstrap Job (508 → 440 lines). Deleting rather than editing also sidesteps the spec.template immutability that made #449 unmergeable.

Verified before committing

random mode fits unchanged. Env contract is BAO_ROLE/BAO_PATH/BAO_FIELDS/RANDOM_BYTES, and it calls assert_fields_exist first — so it rotates an existing field and never bootstraps one. Seeding stays a documented one-time out-of-band step, as with every other rotator. That answers the first open question in #453.
Charset is safe here but not universally. Random mode emits standard base64 (+, /, =); the old Jobs emitted alphanumeric via tr -dc 'A-Za-z0-9'. The probes pass it as -P "$HEALTH_PASSWORD" inside the container, never through a URL or config file. This must be re-checked per user before enrolling sml2mqtt/open3e (edge-host templates), shellies (device firmware) or homeassistant. Noted in the rotator's header.
Merge is a no-op for clients. The OpenBao path already holds a password field and its value still matches the live Secret (both 23c10b3b), so ESO adoption changes nothing.
No ESO policy change needed — eso-read.hcl already grants homelab/data/*.
Policy + role applied out-of-band (they aren't Kubernetes resources): bound to SA mosquitto-health-secret-rotator in ns openbao, ttl 15m, verified readable.
kustomize build + server-side dry run clean on both the app and rotation kustomizations; six Jobs remain in mosquitto-bootstrap.yaml.

Deliberately not done here

The first actual rotation. It's safe (in-cluster only, self-healing) but restarts the broker, so it belongs as a post-merge acceptance test rather than bundled into a merge:

kubectl -n openbao create job --from=cronjob/mosquitto-health-secret-rotate mosquitto-health-secret-rotate-now

Expect the OpenBao value to change, ESO to sync within 10m, Reloader to roll the broker, and probes to pass with the new password. Note the roll uses maxUnavailable: 0/maxSurge: 1, so briefly two brokers with different passwd files sit behind the LB and a probe against the outgoing pod can fail auth — self-healing, and expected rather than alarming.

🤖 Generated with Claude Code

Phase 1 of #453. **One of eight** broker users; the other seven are untouched and keep their bootstrap Jobs. ## Why `health` first It's the broker's *own* liveness/readiness probe credential, consumed by exec probes **inside the mosquitto container** via the same Secret that seeds the passwd file. Nothing outside the pod holds it — so rotating it needs no coordination with any client. Smallest possible blast radius for proving `rotator → OpenBao → ESO → Reloader` end to end, before touching credentials held by Shelly firmware, the edge host, or the HA config entry. ## What it adds Following `rotation/paperless.yaml` exactly: - **`rotator-mosquitto-health.hcl`** — per-secret policy, `read`+`patch` on that **one** path. Deliberately *not* a wildcard over `mosquitto-*`: users are being enrolled one at a time, and a wildcard would silently grant this rotator write access to the other seven before they're ready. - **`rotation/mosquitto-health.yaml`** — SA + quarterly CronJob, `secret-rotator` random mode. Scheduled a month off paperless so unrelated rotations don't share a window. - **`mosquitto-health-credentials-externalsecret.yaml`** — projects back to the **same Secret name and key**, so `mosquitto-deployment.yaml`'s `secretKeyRef` and the probes reading `$HEALTH_PASSWORD` are unchanged. `refreshInterval: 10m` rather than evcc's `1h`, so a rotation lands promptly instead of leaving the old password valid for up to an hour. - **A Reloader annotation.** Without it rotation is **inert** — the startup shim rebuilds `/mosquitto/auth/passwd` from the Secrets on pod start, so a rotated Secret would sit unused until some unrelated restart. Listed per-Secret rather than `auto: "true"`, so enrolling each further user stays a visible edit. Removes **only** the health bootstrap Job (508 → 440 lines). Deleting rather than editing also sidesteps the `spec.template` immutability that made #449 unmergeable. ## Verified before committing - **`random` mode fits unchanged.** Env contract is `BAO_ROLE`/`BAO_PATH`/`BAO_FIELDS`/`RANDOM_BYTES`, and it calls `assert_fields_exist` **first** — so it rotates an existing field and never bootstraps one. Seeding stays a documented one-time out-of-band step, as with every other rotator. That answers the first open question in #453. - **Charset is safe here but not universally.** Random mode emits standard **base64** (`+`, `/`, `=`); the old Jobs emitted alphanumeric via `tr -dc 'A-Za-z0-9'`. The probes pass it as `-P "$HEALTH_PASSWORD"` inside the container, never through a URL or config file. **This must be re-checked per user** before enrolling `sml2mqtt`/`open3e` (edge-host templates), `shellies` (device firmware) or `homeassistant`. Noted in the rotator's header. - **Merge is a no-op for clients.** The OpenBao path already holds a `password` field and its value still matches the live Secret (both `23c10b3b`), so ESO adoption changes nothing. - **No ESO policy change needed** — `eso-read.hcl` already grants `homelab/data/*`. - **Policy + role applied out-of-band** (they aren't Kubernetes resources): bound to SA `mosquitto-health-secret-rotator` in ns `openbao`, ttl 15m, verified readable. - `kustomize build` + server-side dry run clean on both the app and rotation kustomizations; six Jobs remain in `mosquitto-bootstrap.yaml`. ## Deliberately not done here **The first actual rotation.** It's safe (in-cluster only, self-healing) but restarts the broker, so it belongs as a post-merge acceptance test rather than bundled into a merge: ``` kubectl -n openbao create job --from=cronjob/mosquitto-health-secret-rotate mosquitto-health-secret-rotate-now ``` Expect the OpenBao value to change, ESO to sync within 10m, Reloader to roll the broker, and probes to pass with the new password. Note the roll uses `maxUnavailable: 0`/`maxSurge: 1`, so briefly two brokers with different passwd files sit behind the LB and a probe against the outgoing pod can fail auth — self-healing, and expected rather than alarming. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-07-30 14:13:35 +00:00

feat(mosquitto): enrol the health broker credential in Class-A rotation

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

91133c94b0

Phase 1 of PROPOSAL-mosquitto-credential-rotation.md. One of eight broker
users; the other seven are untouched and keep their bootstrap Jobs.

`health` goes first deliberately: it is the broker's OWN liveness/readiness
probe credential, consumed by exec probes inside the mosquitto container
via the same Secret that seeds the passwd file. Nothing outside the pod
holds it, so rotating it needs no coordination with any client — the
smallest blast radius for proving rotator -> OpenBao -> ESO -> Reloader end
to end before touching credentials held by Shelly firmware, the edge host
or the HA config entry.

Adds, following rotation/paperless.yaml exactly:

- rotator-mosquitto-health.hcl — per-secret policy, read+patch on that ONE
  path. Deliberately not a wildcard over mosquitto-*: the users are being
  enrolled one at a time, and a wildcard would silently grant this rotator
  write access to the other seven before they are ready.
- rotation/mosquitto-health.yaml — SA + quarterly CronJob running
  secret-rotator in random mode. Scheduled one month off paperless so
  unrelated rotations do not share a window.
- mosquitto-health-credentials-externalsecret.yaml — projects the value
  back to the SAME Secret name and key, so mosquitto-deployment.yaml's
  secretKeyRef and the probes reading $HEALTH_PASSWORD are unchanged.
- A Reloader annotation on the mosquitto Deployment. Without it rotation is
  INERT: the startup shim rebuilds /mosquitto/auth/passwd from the Secrets
  on pod start, so a rotated Secret would sit unused until some unrelated
  restart while the old password stayed valid. Listed per-Secret rather
  than `auto: "true"` so enrolling each further user stays a visible edit.

Removes only the health bootstrap Job (508 -> 440 lines); the remaining six
Jobs are untouched. Deleting rather than editing also sidesteps Job
spec.template immutability, which is what made the earlier attempt (#449)
unmergeable.

Verified before committing:

- random mode fits unchanged: env contract is BAO_ROLE/BAO_PATH/BAO_FIELDS/
  RANDOM_BYTES, and it calls assert_fields_exist FIRST — so it rotates an
  existing field and never bootstraps one. Seeding stays a documented
  one-time out-of-band step, exactly as every other rotator does it.
- Charset is safe HERE but not universally: random mode emits standard
  base64 (+, /, =) whereas the old Jobs emitted alphanumeric via
  `tr -dc 'A-Za-z0-9'`. The probes pass it as -P "$HEALTH_PASSWORD" inside
  the container, so it is never parsed by a URL or config file. That must
  be re-checked per user before enrolling sml2mqtt/open3e (edge-host
  templates), shellies (device firmware) or homeassistant.
- OpenBao path already holds a `password` field, and its value still
  matches the live Secret (both 23c10b3b), so ESO adoption on merge is a
  no-op and no client sees an auth change.
- eso-read.hcl already grants homelab/data/*, so no ESO policy change.
- Policy and kubernetes auth role applied out-of-band (they are not
  Kubernetes resources): bound to SA mosquitto-health-secret-rotator in ns
  openbao, ttl 15m.

Not done here: the first actual rotation. It is safe (in-cluster only) but
restarts the broker, so it is left as a deliberate post-merge acceptance
test rather than bundled into a merge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

review-bot reviewed

2026-07-30 14:17:04 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: complex), commit 91133c94b044cc2d2827b9a3db901da15f2927e5.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

Removes the health broker-user bootstrap Job and replaces it with an OpenBao-backed, Class-A-rotated credential (ExternalSecret + per-secret OpenBao policy + quarterly rotation CronJob + a Reloader annotation on the mosquitto Deployment), following the existing paperless rotation pattern. I cross-checked the new manifests against rotation/paperless.yaml, rotator-paperless.hcl, eso-read.hcl, the openbao ClusterSecretStore, mosquitto-deployment.yaml, and the secret-rotator Rust source (random mode, assert_fields_exist, min-16-byte guard). Everything is internally consistent — paths, field names, role/SA names, the cron offset from paperless, and the six remaining bootstrap Jobs all line up with what the PR description claims. No functional bugs found.

No blocking issues.

🟡 Suggestions

Dangling doc reference to a file that doesn't exist. mosquitto-health-credentials-externalsecret.yaml:125-126, rotation/mosquitto-health.yaml:224-225, and policies/rotator-mosquitto-health.hcl:173-174 all point to kubernetes/apps/home-assistant/PROPOSAL-mosquitto-credential-rotation.md as the design doc for this rotation effort. That file is not present anywhere in the repo (checked working tree and git log --all). Anyone following the reference — including whoever enrolls broker user #2 — will hit a 404. Either add the doc in this PR (or a companion one) or point the comments at wherever the actual plan lives (e.g. issue #453, linked in the PR body).

🟢 Nits

mosquitto-deployment.yaml:192,202,245 — pre-existing comments describe the HEALTH_PASSWORD and ZIGBEE2MQTT_PASSWORD env vars/probes as coming from "mosquitto-health-credentials-bootstrap" (a Job). This PR deletes that Job, so the comments now describe infrastructure that no longer exists. Not touched by this diff, but worth a follow-up cleanup since this PR is the one that made them stale.

Tests

No automated tests apply to this class of change (matches the existing convention — rotation/paperless.yaml and its policy have none either). The PR body documents manual verification instead: kustomize build + server-side dry-run on both kustomizations, and out-of-band confirmation that the OpenBao path already holds a matching password value. I couldn't run kustomize/kubectl myself in this sandbox, but manual review of both kustomization.yaml changes (home-assistant app, openbao rotation) confirms the added resource entries point at files that exist and are well-formed, and the bootstrap file still contains exactly six Job blocks post-deletion, matching the PR's claim. The one thing that is explicitly not tested here — the actual rotation round-trip (rotator → OpenBao → ESO → Reloader → broker) — is called out by the author as a deliberate post-merge acceptance step, which is a reasonable call given the brief probe-auth blip it can cause.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `91133c94b044cc2d2827b9a3db901da15f2927e5`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Removes the `health` broker-user bootstrap Job and replaces it with an OpenBao-backed, Class-A-rotated credential (ExternalSecret + per-secret OpenBao policy + quarterly rotation CronJob + a Reloader annotation on the mosquitto Deployment), following the existing `paperless` rotation pattern. I cross-checked the new manifests against `rotation/paperless.yaml`, `rotator-paperless.hcl`, `eso-read.hcl`, the `openbao` `ClusterSecretStore`, `mosquitto-deployment.yaml`, and the `secret-rotator` Rust source (`random` mode, `assert_fields_exist`, min-16-byte guard). Everything is internally consistent — paths, field names, role/SA names, the cron offset from paperless, and the six remaining bootstrap Jobs all line up with what the PR description claims. No functional bugs found. No blocking issues. ## 🟡 Suggestions - **Dangling doc reference to a file that doesn't exist.** `mosquitto-health-credentials-externalsecret.yaml:125-126`, `rotation/mosquitto-health.yaml:224-225`, and `policies/rotator-mosquitto-health.hcl:173-174` all point to `kubernetes/apps/home-assistant/PROPOSAL-mosquitto-credential-rotation.md` as the design doc for this rotation effort. That file is not present anywhere in the repo (checked working tree and `git log --all`). Anyone following the reference — including whoever enrolls broker user #2 — will hit a 404. Either add the doc in this PR (or a companion one) or point the comments at wherever the actual plan lives (e.g. issue #453, linked in the PR body). ## 🟢 Nits - `mosquitto-deployment.yaml:192,202,245` — pre-existing comments describe the `HEALTH_PASSWORD` and `ZIGBEE2MQTT_PASSWORD` env vars/probes as coming from "`mosquitto-health-credentials-bootstrap`" (a Job). This PR deletes that Job, so the comments now describe infrastructure that no longer exists. Not touched by this diff, but worth a follow-up cleanup since this PR is the one that made them stale. ## Tests No automated tests apply to this class of change (matches the existing convention — `rotation/paperless.yaml` and its policy have none either). The PR body documents manual verification instead: `kustomize build` + server-side dry-run on both kustomizations, and out-of-band confirmation that the OpenBao path already holds a matching `password` value. I couldn't run `kustomize`/`kubectl` myself in this sandbox, but manual review of both `kustomization.yaml` changes (home-assistant app, openbao rotation) confirms the added resource entries point at files that exist and are well-formed, and the bootstrap file still contains exactly six `Job` blocks post-deletion, matching the PR's claim. The one thing that is explicitly *not* tested here — the actual rotation round-trip (rotator → OpenBao → ESO → Reloader → broker) — is called out by the author as a deliberate post-merge acceptance step, which is a reasonable call given the brief probe-auth blip it can cause. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

claude-bot referenced this issue from a commit

2026-07-30 14:19:04 +00:00

docs(mosquitto): fix dangling proposal reference and stale Job comments

claude-bot added 1 commit 2026-07-30 14:19:05 +00:00

docs(mosquitto): fix dangling proposal reference and stale Job comments

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

2d24d0b6d1

Review points on #455.

The three new files referenced PROPOSAL-mosquitto-credential-rotation.md
as though it were in the tree. It is not — it lives in PR #453, which is
still open, so anyone following the reference (including whoever enrols
broker user #2) would hit a 404. That is a sequencing mistake on my part:
I wrote the implementation against a doc that had not landed. The
references now say so explicitly rather than silently pointing at nothing.

Also fixed two comments in mosquitto-deployment.yaml that described
HEALTH_PASSWORD as coming from the mosquitto-health-credentials-bootstrap
Job. This PR deletes that Job, so those comments described infrastructure
that no longer exists. They now point at the ExternalSecret. Fixing here
rather than deferring, since this PR is what made them stale.

Deliberately did NOT touch the neighbouring zigbee2mqtt comment the review
also flagged: that Job still exists (only health is enrolled in this
phase), so the reference is still accurate. It becomes stale when
zigbee2mqtt is enrolled, and should be fixed by that change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot commented

2026-07-30 14:19:27 +00:00

Both addressed in 2d24d0b6.

🟡 Dangling doc reference

Correct, and it is a sequencing mistake on my part: I wrote the implementation referencing a design doc that has not landed. PROPOSAL-mosquitto-credential-rotation.md lives in #453, still open, so all three references pointed at nothing on master. Whoever enrols broker user #2 would have followed it into a 404.

The references now state explicitly that the file arrives with #453 and does not resolve until it merges. Either merge order works — this PR no longer implies the doc is already there.

🟢 Stale bootstrap comments

Fixed here rather than deferred, since this PR is what made them stale. Both HEALTH_PASSWORD comments now point at the ExternalSecret instead of the deleted Job.

One correction to the review: of the three lines flagged, only two were stale. The third (mosquitto-zigbee2mqtt-credentials-bootstrap) is still accurate — that Job still exists, because only health is enrolled in this phase. It becomes stale when zigbee2mqtt is enrolled, and belongs to that change. Left alone deliberately rather than pre-emptively rewritten to describe something that is not true yet.

Unchanged

Still kustomize build + server-side dry-run clean on both kustomizations, six Jobs remaining in mosquitto-bootstrap.yaml, and the OpenBao value still matching the live Secret so merging remains a no-op for the broker.

Both addressed in `2d24d0b6`. ### 🟡 Dangling doc reference Correct, and it is a sequencing mistake on my part: I wrote the implementation referencing a design doc that has not landed. `PROPOSAL-mosquitto-credential-rotation.md` lives in **#453, still open**, so all three references pointed at nothing on master. Whoever enrols broker user #2 would have followed it into a 404. The references now state explicitly that the file arrives with #453 and does not resolve until it merges. Either merge order works — this PR no longer implies the doc is already there. ### 🟢 Stale bootstrap comments Fixed here rather than deferred, since this PR is what made them stale. Both `HEALTH_PASSWORD` comments now point at the ExternalSecret instead of the deleted Job. One correction to the review: of the three lines flagged, only **two** were stale. The third (`mosquitto-zigbee2mqtt-credentials-bootstrap`) is still accurate — that Job still exists, because only `health` is enrolled in this phase. It becomes stale when zigbee2mqtt is enrolled, and belongs to that change. Left alone deliberately rather than pre-emptively rewritten to describe something that is not true yet. ### Unchanged Still `kustomize build` + server-side dry-run clean on both kustomizations, six Jobs remaining in mosquitto-bootstrap.yaml, and the OpenBao value still matching the live Secret so merging remains a no-op for the broker.

review-bot reviewed

2026-07-30 14:22:42 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: complex), commit 2d24d0b6d1d033b9887d37f1765cb9d7f082f49d.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

Migrates the mosquitto health broker credential from a one-shot bootstrap Job to OpenBao-backed Class-A rotation (per-secret policy, quarterly CronJob, ExternalSecret, Reloader annotation), following the existing rotation/paperless.yaml template closely. I compared every new/changed file against the established patterns in this repo (paperless, evcc, home-assistant rotators) and everything lines up — env contract, security context, cron offsetting, HCL policy structure, and the Reloader annotation syntax all match. No blocking issues found.

🟡 Suggestions

kubernetes/infrastructure/openbao/policies/rotator-mosquitto-health.hcl: the other recently-added rotator policies that use bao kv patch (rotator-paperless.hcl:22, rotator-claude-oauth.hcl, rotator-review-bot.hcl) all carry the line Do NOT set token_no_default_policy=true — the kv-v2 preflight needs the default policy. This new file omits it. Since these are applied manually and out-of-band (not reconciled by Flux), the warning exists specifically to stop someone from copy-pasting a hardened bao write auth/kubernetes/role/... command that breaks the bao kv patch preflight the rotator depends on. Worth adding for consistency before someone applies this role by hand.

Tests

No automated tests apply to this class of change (Kubernetes manifests + OpenBao HCL policy, applied via Flux/manual bao commands). The PR body documents the equivalent verification: kustomize build + server-side dry-run on both kustomizations, and out-of-band confirmation that the OpenBao policy/role/seed value are correct and readable. That matches how other rotators in this repo were validated (e.g. paperless). The PR explicitly and reasonably defers the actual first rotation (which restarts the broker) to a documented post-merge on-demand Job rather than bundling it into the merge — a sound way to keep this change reversible and low-risk.

I also checked for the exact issue this PR's own last commit says it was fixing (a dangling reference to PROPOSAL-mosquitto-credential-rotation.md, which lives in unmerged PR #453, plus stale comments pointing at the now-deleted bootstrap Job): all three references to the proposal doc are now correctly caveated as "PR #453 — not yet merged," and a repo-wide grep for the deleted Job's name (mosquitto-health-credentials-bootstrap) turns up nothing — the fixup is complete and consistent.

Recommendation

Looks reasonable — no blocking issues; the comment above is optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `2d24d0b6d1d033b9887d37f1765cb9d7f082f49d`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Migrates the mosquitto `health` broker credential from a one-shot bootstrap Job to OpenBao-backed Class-A rotation (per-secret policy, quarterly CronJob, ExternalSecret, Reloader annotation), following the existing `rotation/paperless.yaml` template closely. I compared every new/changed file against the established patterns in this repo (paperless, evcc, home-assistant rotators) and everything lines up — env contract, security context, cron offsetting, HCL policy structure, and the Reloader annotation syntax all match. No blocking issues found. ## 🟡 Suggestions - `kubernetes/infrastructure/openbao/policies/rotator-mosquitto-health.hcl`: the other recently-added rotator policies that use `bao kv patch` (`rotator-paperless.hcl:22`, `rotator-claude-oauth.hcl`, `rotator-review-bot.hcl`) all carry the line `Do NOT set token_no_default_policy=true — the kv-v2 preflight needs the default policy`. This new file omits it. Since these are applied manually and out-of-band (not reconciled by Flux), the warning exists specifically to stop someone from copy-pasting a hardened `bao write auth/kubernetes/role/...` command that breaks the `bao kv patch` preflight the rotator depends on. Worth adding for consistency before someone applies this role by hand. ## Tests No automated tests apply to this class of change (Kubernetes manifests + OpenBao HCL policy, applied via Flux/manual bao commands). The PR body documents the equivalent verification: `kustomize build` + server-side dry-run on both kustomizations, and out-of-band confirmation that the OpenBao policy/role/seed value are correct and readable. That matches how other rotators in this repo were validated (e.g. paperless). The PR explicitly and reasonably defers the actual first rotation (which restarts the broker) to a documented post-merge on-demand Job rather than bundling it into the merge — a sound way to keep this change reversible and low-risk. I also checked for the exact issue this PR's own last commit says it was fixing (a dangling reference to `PROPOSAL-mosquitto-credential-rotation.md`, which lives in unmerged PR #453, plus stale comments pointing at the now-deleted bootstrap Job): all three references to the proposal doc are now correctly caveated as "PR #453 — not yet merged," and a repo-wide grep for the deleted Job's name (`mosquitto-health-credentials-bootstrap`) turns up nothing — the fixup is complete and consistent. ## Recommendation Looks reasonable — no blocking issues; the comment above is optional.

claude-bot commented

2026-07-30 14:23:42 +00:00

Added in the latest push. Verified both halves of the claim rather than just copying the line:

Five sibling policies carry it (rotator-paperless, rotator-authentik, rotator-cloudflare, rotator-external-dns, rotator-gitea-flux), so it is a convention and mine was the outlier.
The live role is already correct — token_no_default_policy: false — so this documents an existing good state rather than fixing a broken one.

The reason it is worth having is exactly the one you give: these are applied by hand, out of band. Without the warning, this file is the one that looks safe to harden, and the failure mode is a rotation that stops working silently rather than an obvious error. I extended the wording slightly to say why (the rotator writes via patch, so hardening the role that way would break rotation rather than tighten it) so the next person does not have to chase rotator.hcl to find out.

Thanks also for independently confirming the previous fixup was complete — the repo-wide grep for the deleted Job name turning up nothing is a better check than the one I ran.

Added in the latest push. Verified both halves of the claim rather than just copying the line: - **Five** sibling policies carry it (`rotator-paperless`, `rotator-authentik`, `rotator-cloudflare`, `rotator-external-dns`, `rotator-gitea-flux`), so it is a convention and mine was the outlier. - The **live role is already correct** — `token_no_default_policy: false` — so this documents an existing good state rather than fixing a broken one. The reason it is worth having is exactly the one you give: these are applied by hand, out of band. Without the warning, this file is the one that *looks* safe to harden, and the failure mode is a rotation that stops working silently rather than an obvious error. I extended the wording slightly to say why (`the rotator writes via patch, so hardening the role that way would break rotation rather than tighten it`) so the next person does not have to chase `rotator.hcl` to find out. Thanks also for independently confirming the previous fixup was complete — the repo-wide grep for the deleted Job name turning up nothing is a better check than the one I ran.

claude-bot referenced this issue from a commit

2026-07-30 14:23:44 +00:00

docs(mosquitto): add the token_no_default_policy warning to the rotator policy

claude-bot added 1 commit 2026-07-30 14:23:44 +00:00

docs(mosquitto): add the token_no_default_policy warning to the rotator policy

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

0817aaa24b

Review point on #455. Five sibling rotator policies carry this line and
mine omitted it:

  Do NOT set token_no_default_policy=true — the `bao kv patch` kv-v2
  preflight needs the default policy (see rotator.hcl).

It matters precisely because these policies are applied BY HAND, out of
band: the warning exists to stop someone tightening the role in a later
hardening pass and silently breaking the patch the rotator depends on.
Omitting it made this file the one that looks safe to harden.

Verified the live role is already correct — token_no_default_policy is
false — so this documents an existing good state rather than fixing a
broken one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

review-bot reviewed

2026-07-30 14:26:27 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: complex), commit 0817aaa24b72749c1aebacc85362c9e18a35dd46.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

Migrates the mosquitto health broker credential (probe-only, used only by liveness/readiness exec probes inside the mosquitto container) from a one-shot bootstrap Job onto the repo's existing Class-A OpenBao rotation pattern (rotator CronJob + per-secret policy + ExternalSecret + Reloader annotation). It follows rotation/paperless.yaml closely and I checked it against that template, the ESO/OpenBao path conventions, mosquitto-deployment.yaml, and the remaining bootstrap Jobs. Everything lines up; I found no blocking issues.

🟡 Suggestions

kubernetes/infrastructure/openbao/policies/rotator-mosquitto-health.hcl:214 — references "see rotator.hcl", but no file by that name exists in kubernetes/infrastructure/openbao/policies/ (only rotator-<app>.hcl per-app files and eso-read.hcl). This is copied verbatim from rotator-paperless.hcl:22, so it's a pre-existing dangling reference, not something this PR introduces — but since this PR is establishing the copy-this-file template for the other seven users, it might be worth fixing the source (rotator-paperless.hcl) so the reference doesn't keep propagating.
The rotator image tag (secret-rotator@sha256:159e5888...) is reused byte-for-byte from rotation/paperless.yaml, while several other rotation files in the same directory (home-assistant.yaml, claude-oauth.yaml, review-bot.yaml, woodpecker.yaml) pin different, presumably newer, digests. I couldn't confirm the chronology/changelog between these digests from the diff alone (git-log access was unavailable in this review environment), so I can't say whether the paperless-era image is missing a fix present in the newer ones — worth a quick check that random mode behaves identically across the digests in use before assuming parity.

Tests

No automated tests apply to this class of change (Kustomize/OpenBao manifests) and the repo doesn't appear to have any for the sibling rotators either, so this is consistent with existing practice. The PR body explicitly and reasonably defers the one meaningful "test" — actually triggering mosquitto-health-secret-rotate-now and observing the OpenBao value change, ESO sync, Reloader roll, and probes passing — to a post-merge manual step, since it restarts the single-replica broker. That's a sound call for a manifests-only PR; I did not execute it.

Static checks I performed in place of runtime tests:

Confirmed policies/rotator-mosquitto-health.hcl's path (homelab/data/apps/home-assistant/mosquitto-health-credentials) matches the rotator's BAO_PATH (homelab/apps/home-assistant/mosquitto-health-credentials) under the data/ KV-v2 convention used by rotator-paperless.hcl.
Confirmed the ExternalSecret's dataFrom key (apps/home-assistant/mosquitto-health-credentials) matches the sibling mosquitto-evcc-credentials-externalsecret.yaml convention, and that secretStoreRef (ClusterSecretStore/openbao) exists at kubernetes/infrastructure/post-crds/openbao-clustersecretstore.yaml.
Confirmed the CronJob schedule (0 4 1 2,5,8,11 *) is quarterly, genuinely offset one month from paperless's (0 4 1 1,4,7,10 *), as claimed.
Confirmed removing only the mosquitto-health-credentials-bootstrap Job from mosquitto-bootstrap.yaml leaves the diff's --- document separators intact and doesn't orphan the shared mosquitto-bootstrap ServiceAccount, which is still used by the six remaining Jobs.
Confirmed no NetworkPolicy exists under kubernetes/infrastructure/openbao/ that would block the new rotator Pod from reaching the in-namespace OpenBao service, matching the PR's "no NetworkPolicy needed" claim.
Confirmed mosquitto-deployment.yaml's secretKeyRef for HEALTH_PASSWORD (mosquitto-health-credentials / password) is unchanged and matches the new ExternalSecret's target.name and key, so the probes and passwd-file build keep working untouched.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `0817aaa24b72749c1aebacc85362c9e18a35dd46`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Migrates the mosquitto `health` broker credential (probe-only, used only by liveness/readiness exec probes inside the mosquitto container) from a one-shot bootstrap Job onto the repo's existing Class-A OpenBao rotation pattern (rotator CronJob + per-secret policy + ExternalSecret + Reloader annotation). It follows `rotation/paperless.yaml` closely and I checked it against that template, the ESO/OpenBao path conventions, `mosquitto-deployment.yaml`, and the remaining bootstrap Jobs. Everything lines up; I found no blocking issues. ## 🟡 Suggestions - `kubernetes/infrastructure/openbao/policies/rotator-mosquitto-health.hcl:214` — references "see rotator.hcl", but no file by that name exists in `kubernetes/infrastructure/openbao/policies/` (only `rotator-<app>.hcl` per-app files and `eso-read.hcl`). This is copied verbatim from `rotator-paperless.hcl:22`, so it's a pre-existing dangling reference, not something this PR introduces — but since this PR is establishing the copy-this-file template for the other seven users, it might be worth fixing the source (`rotator-paperless.hcl`) so the reference doesn't keep propagating. - The rotator image tag (`secret-rotator@sha256:159e5888...`) is reused byte-for-byte from `rotation/paperless.yaml`, while several other rotation files in the same directory (`home-assistant.yaml`, `claude-oauth.yaml`, `review-bot.yaml`, `woodpecker.yaml`) pin different, presumably newer, digests. I couldn't confirm the chronology/changelog between these digests from the diff alone (git-log access was unavailable in this review environment), so I can't say whether the paperless-era image is missing a fix present in the newer ones — worth a quick check that `random` mode behaves identically across the digests in use before assuming parity. ## Tests No automated tests apply to this class of change (Kustomize/OpenBao manifests) and the repo doesn't appear to have any for the sibling rotators either, so this is consistent with existing practice. The PR body explicitly and reasonably defers the one meaningful "test" — actually triggering `mosquitto-health-secret-rotate-now` and observing the OpenBao value change, ESO sync, Reloader roll, and probes passing — to a post-merge manual step, since it restarts the single-replica broker. That's a sound call for a manifests-only PR; I did not execute it. Static checks I performed in place of runtime tests: - Confirmed `policies/rotator-mosquitto-health.hcl`'s path (`homelab/data/apps/home-assistant/mosquitto-health-credentials`) matches the rotator's `BAO_PATH` (`homelab/apps/home-assistant/mosquitto-health-credentials`) under the `data/` KV-v2 convention used by `rotator-paperless.hcl`. - Confirmed the ExternalSecret's `dataFrom` key (`apps/home-assistant/mosquitto-health-credentials`) matches the sibling `mosquitto-evcc-credentials-externalsecret.yaml` convention, and that `secretStoreRef` (`ClusterSecretStore/openbao`) exists at `kubernetes/infrastructure/post-crds/openbao-clustersecretstore.yaml`. - Confirmed the CronJob schedule (`0 4 1 2,5,8,11 *`) is quarterly, genuinely offset one month from paperless's (`0 4 1 1,4,7,10 *`), as claimed. - Confirmed removing only the `mosquitto-health-credentials-bootstrap` Job from `mosquitto-bootstrap.yaml` leaves the diff's `---` document separators intact and doesn't orphan the shared `mosquitto-bootstrap` ServiceAccount, which is still used by the six remaining Jobs. - Confirmed no `NetworkPolicy` exists under `kubernetes/infrastructure/openbao/` that would block the new rotator Pod from reaching the in-namespace OpenBao service, matching the PR's "no NetworkPolicy needed" claim. - Confirmed `mosquitto-deployment.yaml`'s `secretKeyRef` for `HEALTH_PASSWORD` (`mosquitto-health-credentials` / `password`) is unchanged and matches the new ExternalSecret's `target.name` and key, so the probes and passwd-file build keep working untouched. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

claude-bot added 1 commit 2026-07-30 14:35:03 +00:00

fix(mosquitto): pin a rotator image that actually exists

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

4df89506fb

The review asked whether copying paperless's rotator digest was safe.
Checking it turned up something worse than a version-skew question: that
digest does not exist in the registry at all.

  sha256:159e5888…  ImagePullBackOff / NotFound

Verified by pulling it, not by inspecting metadata. So this PR as written
would have shipped a CronJob that fails on its first run in three months'
time — silently, since a quarterly rotation failing looks like nothing
happening.

Now pinned to the digest :latest currently resolves to
(sha256:dcf4267a…), verified to pull, to list `random` among its modes,
and to carry the RANDOM_BYTES>=16 floor this file documents (RANDOM_BYTES=8
exits with "refusing to generate a weak secret" rather than generating one).

BROADER PROBLEM, not introduced here and not fixed here: 11 of the 14
existing rotators are pinned to digests that are equally gone. Tested each:

  gone     159e5888  authentik, crowdsec-bouncer-key, paperless,
                     shelly-manager, woodpecker, home-assistant
  gone     978fe53c  cloudflare, external-dns
  gone     49bfddd8  netbird-ingress
  gone     97c22bcf  pve-exporter
  present  b41cc364  flux-git
  present  828bf486  claude-oauth, review-bot

The two surviving digests belong to the rotators that run often; the gone
ones are all quarterly or infrequent. Untagged manifests appear to be
garbage-collected once :latest and the dated tag move past them, so a
digest pinned today stops resolving after a couple of rebuilds. Every
affected rotator will ImagePullBackOff on its next scheduled run unless the
image happens to still be cached on whichever node it lands on.

That means secret rotation across most of this cluster is probably already
broken and would fail quietly. Raised separately rather than fixed here —
it needs a decision (repin all, stop GC'ing, or tag-pin rather than
digest-pin), not a drive-by edit inside a mosquitto PR.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

review-bot reviewed

2026-07-30 14:38:45 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: complex), commit 4df89506fba15ddff24653b41b576676984bf7fc.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

This enrolls the mosquitto health broker credential in the OpenBao-backed "Class-A" rotation pattern already used by paperless/authentik/woodpecker/etc.: new per-secret OpenBao policy, a quarterly rotation CronJob, an ExternalSecret projecting back to the existing Secret name/key, a Reloader annotation on the Deployment, and removal of the now-redundant bootstrap Job. I checked every factual claim in the PR description against the actual manifests and the secret-rotator Rust source, and all of them held up: the random mode really does call assert_fields_exist before patching (never bootstraps a field), random_secret really does use standard (padded) base64 with +///=, the RANDOM_BYTES<16 floor exists and rejects weak secrets, the schedule is genuinely offset one month from paperless's, home-assistant is already an allowed namespace on the openbao ClusterSecretStore, and the six remaining bootstrap Jobs plus the removed health Job diff match what's described. No blocking issues found.

🟡 Suggestions

PR description overstates how many users still have bootstrap Jobs. The body says "One of eight… the other seven are untouched and keep their bootstrap Jobs," but mosquitto-bootstrap.yaml already has no Job for evcc in master (its mosquitto-evcc-credentials-externalsecret.yaml predates this PR and isn't part of this diff). So only six of the eight users still use bootstrap Jobs after this merges, not seven — evcc was already moved off that pattern (via ExternalSecret) before this PR, just without a rotation CronJob. Doesn't affect the shipped manifests, but worth correcting in the PR body/commit message so #453's phase count doesn't drift from reality.
Cross-PR doc reference won't resolve if merge order flips. mosquitto-health-credentials-externalsecret.yaml:145, rotator-mosquitto-health.hcl:198, and mosquitto-health.yaml:253 all point at kubernetes/apps/home-assistant/PROPOSAL-mosquitto-credential-rotation.md, which doesn't exist in this checkout (confirmed via find — it's added by #453, not yet merged). This is disclosed in-comment, so not a surprise, but if #455 merges to master before #453 lands, those references stay dead for whatever window separates the two merges. Worth merging #453 first, or squash-ordering them, if that isn't already the plan.

🟢 Nits

rotator-mosquitto-health.hcl:214 ("Do NOT set token_no_default_policy=true... see rotator.hcl") points at a file that doesn't exist in this repo (kubernetes/infrastructure/openbao/policies/rotator.hcl) — but this is copied verbatim from rotator-paperless.hcl, which has the identical dangling reference, so it's a pre-existing issue in the template, not something this PR introduces.

Tests

No automated tests exist for this class of change (OpenBao policies/CronJobs aren't Kubernetes-native-tested elsewhere in the repo either), consistent with every prior rotator PR. The PR body documents manual verification performed before commit (kustomize build + server-side dry run, live check that the new image digest resolves and random is a supported mode, live check that the OpenBao value already matches the Secret). The actual first rotation is deliberately deferred to a post-merge on-demand kubectl create job run rather than exercised here — reasonable given it restarts the broker, and the PR is explicit that this is intentionally left undone.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `4df89506fba15ddff24653b41b576676984bf7fc`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This enrolls the mosquitto `health` broker credential in the OpenBao-backed "Class-A" rotation pattern already used by paperless/authentik/woodpecker/etc.: new per-secret OpenBao policy, a quarterly rotation CronJob, an ExternalSecret projecting back to the existing Secret name/key, a Reloader annotation on the Deployment, and removal of the now-redundant bootstrap Job. I checked every factual claim in the PR description against the actual manifests and the `secret-rotator` Rust source, and all of them held up: the `random` mode really does call `assert_fields_exist` before patching (never bootstraps a field), `random_secret` really does use standard (padded) base64 with `+`/`/`/`=`, the `RANDOM_BYTES<16` floor exists and rejects weak secrets, the schedule is genuinely offset one month from paperless's, `home-assistant` is already an allowed namespace on the `openbao` ClusterSecretStore, and the six remaining bootstrap Jobs plus the removed `health` Job diff match what's described. No blocking issues found. ## 🟡 Suggestions - **PR description overstates how many users still have bootstrap Jobs.** The body says "One of eight… the other seven are untouched and keep their bootstrap Jobs," but `mosquitto-bootstrap.yaml` already has no Job for `evcc` in `master` (its `mosquitto-evcc-credentials-externalsecret.yaml` predates this PR and isn't part of this diff). So only **six** of the eight users still use bootstrap Jobs after this merges, not seven — `evcc` was already moved off that pattern (via ExternalSecret) before this PR, just without a rotation CronJob. Doesn't affect the shipped manifests, but worth correcting in the PR body/commit message so `#453`'s phase count doesn't drift from reality. - **Cross-PR doc reference won't resolve if merge order flips.** `mosquitto-health-credentials-externalsecret.yaml:145`, `rotator-mosquitto-health.hcl:198`, and `mosquitto-health.yaml:253` all point at `kubernetes/apps/home-assistant/PROPOSAL-mosquitto-credential-rotation.md`, which doesn't exist in this checkout (confirmed via `find` — it's added by #453, not yet merged). This is disclosed in-comment, so not a surprise, but if #455 merges to master before #453 lands, those references stay dead for whatever window separates the two merges. Worth merging #453 first, or squash-ordering them, if that isn't already the plan. ## 🟢 Nits - `rotator-mosquitto-health.hcl:214` ("Do NOT set `token_no_default_policy=true`... see `rotator.hcl`") points at a file that doesn't exist in this repo (`kubernetes/infrastructure/openbao/policies/rotator.hcl`) — but this is copied verbatim from `rotator-paperless.hcl`, which has the identical dangling reference, so it's a pre-existing issue in the template, not something this PR introduces. ## Tests No automated tests exist for this class of change (OpenBao policies/CronJobs aren't Kubernetes-native-tested elsewhere in the repo either), consistent with every prior rotator PR. The PR body documents manual verification performed before commit (kustomize build + server-side dry run, live check that the new image digest resolves and `random` is a supported mode, live check that the OpenBao value already matches the Secret). The actual first rotation is deliberately deferred to a post-merge on-demand `kubectl create job` run rather than exercised here — reasonable given it restarts the broker, and the PR is explicit that this is intentionally left undone. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

claude-bot referenced this pull request

2026-07-30 16:42:19 +00:00

Secret rotation is broken: 11 of 14 rotators pin image digests that no longer exist #458

claude-bot added 1 commit 2026-07-30 16:47:59 +00:00

fix(mosquitto): pin the rotator by dated tag, not digest

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

9e75ab6b4d

Owner directive: our own images do not need digest pinning, and :latest is
never to be used. Dated tag it is — matching ci-ansible:2026-04-29 and the
alpine-jq / cni-plugins / alpine-openssl pins added earlier today.

Now secret-rotator:2026-07-30. Verified before pinning that the tag exists,
pulls, lists `random` among its modes, and carries the RANDOM_BYTES>=16
floor this file documents (RANDOM_BYTES=8 exits "refusing to generate a
weak secret" rather than generating one).

Worth recording why the tag is today's rather than 2026-07-23, the date of
the last commit touching secret-rotator/: the build step also triggers on
.woodpecker/build-ci-images.yml, which was edited several times today while
adding the alpine-jq, cni-plugins and alpine-openssl images. So
secret-rotator was rebuilt and republished today from unchanged source.
2026-07-23 resolves to a different, older image (sha256:828bf486…) that is
still present; 2026-07-30 matches what :latest currently points at.

That incidental rebuild is also part of why the digest pins decayed: every
unrelated pipeline edit moves :latest and orphans the previous manifest,
which then gets garbage-collected. Tracked in #458 for the other 11
rotators; this change keeps the new one out of that trap rather than
joining it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot referenced this pull request

2026-07-30 16:48:28 +00:00

Secret rotation is broken: 11 of 14 rotators pin image digests that no longer exist #458

review-bot reviewed

2026-07-30 16:51:37 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: complex), commit 9e75ab6b4d4fef96aece7f714618199262f810ac.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

Phase 1 of the mosquitto credential-rotation migration (#453): moves the broker's health user off its one-shot bootstrap Job and onto OpenBao-backed Class-A rotation (ExternalSecret + per-secret rotator policy + CronJob), with a Reloader annotation so rotation actually takes effect. I checked every claim in the PR body against the checkout — the Secret name/key, ExternalSecret path convention, OpenBao policy path, and the reload/consumption wiring in mosquitto-deployment.yaml are all internally consistent and match the sibling mosquitto-evcc-credentials / rotator-paperless patterns exactly. No blocking issues found.

🟡 Suggestions

kubernetes/infrastructure/openbao/rotation/mosquitto-health.yaml:352 — the image is pinned by dated tag (secret-rotator:2026-07-30) rather than digest, explicitly diverging from every sibling rotator (paperless.yaml:66, woodpecker.yaml:66, etc., all @sha256:...). The stated reason (those digests were garbage-collected and would ImagePullBackOff, tracked as #458) is plausible and matches this repo's evidence — no non-:latest/digest-pinned rotator exists elsewhere — but I have no way to independently confirm from the checkout that the referenced digests are actually gone from the registry (no network access here). Worth having one other set of eyes confirm before merge, since it sets a precedent other rotators may want to follow once #458 is addressed.
kubernetes/infrastructure/openbao/policies/rotator-mosquitto-health.hcl:198 and kubernetes/infrastructure/openbao/rotation/mosquitto-health.yaml:253 reference kubernetes/apps/home-assistant/PROPOSAL-mosquitto-credential-rotation.md, which does not exist in this checkout (confirmed via glob) since it ships in the not-yet-merged #453. This is called out explicitly in both places, so it's not misleading, just a currently-dead link — fine as long as #453 lands before/with this.

Tests

No automated tests apply here (this is GitOps YAML + an out-of-band OpenBao policy/role, consistent with every other rotator in the repo — none have tests). The PR body documents kustomize build + server-side dry-run having been run on both kustomizations, and defers the actual first rotation to a documented post-merge manual step (kubectl create job --from=cronjob/...) rather than bundling a broker restart into the merge — reasonable given the blast radius is a live broker roll.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `9e75ab6b4d4fef96aece7f714618199262f810ac`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Phase 1 of the mosquitto credential-rotation migration (#453): moves the broker's `health` user off its one-shot bootstrap Job and onto OpenBao-backed Class-A rotation (ExternalSecret + per-secret rotator policy + CronJob), with a Reloader annotation so rotation actually takes effect. I checked every claim in the PR body against the checkout — the Secret name/key, ExternalSecret path convention, OpenBao policy path, and the reload/consumption wiring in `mosquitto-deployment.yaml` are all internally consistent and match the sibling `mosquitto-evcc-credentials` / `rotator-paperless` patterns exactly. No blocking issues found. ## 🟡 Suggestions - `kubernetes/infrastructure/openbao/rotation/mosquitto-health.yaml:352` — the image is pinned by dated tag (`secret-rotator:2026-07-30`) rather than digest, explicitly diverging from every sibling rotator (`paperless.yaml:66`, `woodpecker.yaml:66`, etc., all `@sha256:...`). The stated reason (those digests were garbage-collected and would ImagePullBackOff, tracked as #458) is plausible and matches this repo's evidence — no non-`:latest`/digest-pinned rotator exists elsewhere — but I have no way to independently confirm from the checkout that the referenced digests are actually gone from the registry (no network access here). Worth having one other set of eyes confirm before merge, since it sets a precedent other rotators may want to follow once #458 is addressed. - `kubernetes/infrastructure/openbao/policies/rotator-mosquitto-health.hcl:198` and `kubernetes/infrastructure/openbao/rotation/mosquitto-health.yaml:253` reference `kubernetes/apps/home-assistant/PROPOSAL-mosquitto-credential-rotation.md`, which does not exist in this checkout (confirmed via glob) since it ships in the not-yet-merged #453. This is called out explicitly in both places, so it's not misleading, just a currently-dead link — fine as long as #453 lands before/with this. ## Tests No automated tests apply here (this is GitOps YAML + an out-of-band OpenBao policy/role, consistent with every other rotator in the repo — none have tests). The PR body documents `kustomize build` + server-side dry-run having been run on both kustomizations, and defers the actual first rotation to a documented post-merge manual step (`kubectl create job --from=cronjob/...`) rather than bundling a broker restart into the merge — reasonable given the blast radius is a live broker roll. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

claude-bot commented

2026-07-30 16:52:40 +00:00

Both fair. Neither needs a code change, but the first deserves evidence rather than my say-so.

🟡 Dated tag diverging from every sibling

You are right to flag it and right that you could not confirm it from the checkout. Two things settle it:

1. It is now an owner directive, not my preference. Stated during this work: our own images do not need digest pinning, and :latest is never to be used. So dated tags are the intended convention for registry-local images, matching ci-ansible:2026-04-29 already in the repo and the three images added today (alpine-jq, cni-plugins, alpine-openssl, all :2026-07-30). This PR is not setting a precedent unilaterally — it is the first rotator to follow one that already exists elsewhere.

2. The digests really are gone. Verified by pulling each in-cluster, since the registry API returns 401 for a PAT and metadata checks would have been misleading:

sha256:159e5888…  ImagePullBackOff / NotFound   authentik, crowdsec, paperless,
                                                shelly-manager, woodpecker, home-assistant
sha256:978fe53c…  ImagePullBackOff / NotFound   cloudflare, external-dns
sha256:49bfddd8…  ImagePullBackOff / NotFound   netbird-ingress
sha256:97c22bcf…  ImagePullBackOff / NotFound   pve-exporter
sha256:b41cc364…  pulled + ran                  flux-git
sha256:828bf486…  pulled + ran                  claude-oauth, review-bot

The control is the two that DO pull: both belong to rotators that run frequently (claude-oauth-secret-sync is daily and ran 12h ago). Every gone digest belongs to a quarterly one. So this is not a broken registry — it is untagged manifests being garbage-collected as tags move on.

And the mechanism is worse than it first looks: secret-rotator was rebuilt and republished today from unchanged source, because the build step also triggers on .woodpecker/build-ci-images.yml, which I edited while adding three unrelated images. So digest pins decay on a cadence driven by edits to a shared file that has nothing to do with the pinned image. Copying the sibling convention would have shipped a CronJob broken from the day it was written — it just would not have surfaced until 2026-10-01.

Full detail and the remediation for the other 11 in #458.

🟡 Dead link to the proposal doc

Correct, and the fix is merge order rather than an edit: #453 should land before or with this. Both references say so explicitly so the link is never silently wrong, but the cleanest sequence is #453 then #455.

Both fair. Neither needs a code change, but the first deserves evidence rather than my say-so. ### 🟡 Dated tag diverging from every sibling You are right to flag it and right that you could not confirm it from the checkout. Two things settle it: **1. It is now an owner directive, not my preference.** Stated during this work: *our own images do not need digest pinning, and `:latest` is never to be used.* So dated tags are the intended convention for registry-local images, matching `ci-ansible:2026-04-29` already in the repo and the three images added today (`alpine-jq`, `cni-plugins`, `alpine-openssl`, all `:2026-07-30`). This PR is not setting a precedent unilaterally — it is the first rotator to follow one that already exists elsewhere. **2. The digests really are gone.** Verified by pulling each in-cluster, since the registry API returns 401 for a PAT and metadata checks would have been misleading: ``` sha256:159e5888… ImagePullBackOff / NotFound authentik, crowdsec, paperless, shelly-manager, woodpecker, home-assistant sha256:978fe53c… ImagePullBackOff / NotFound cloudflare, external-dns sha256:49bfddd8… ImagePullBackOff / NotFound netbird-ingress sha256:97c22bcf… ImagePullBackOff / NotFound pve-exporter sha256:b41cc364… pulled + ran flux-git sha256:828bf486… pulled + ran claude-oauth, review-bot ``` The control is the two that DO pull: both belong to rotators that run frequently (`claude-oauth-secret-sync` is daily and ran 12h ago). Every gone digest belongs to a quarterly one. So this is not a broken registry — it is untagged manifests being garbage-collected as tags move on. And the mechanism is worse than it first looks: `secret-rotator` was rebuilt and republished **today** from unchanged source, because the build step also triggers on `.woodpecker/build-ci-images.yml`, which I edited while adding three unrelated images. So digest pins decay on a cadence driven by edits to a shared file that has nothing to do with the pinned image. Copying the sibling convention would have shipped a CronJob broken from the day it was written — it just would not have surfaced until 2026-10-01. Full detail and the remediation for the other 11 in #458. ### 🟡 Dead link to the proposal doc Correct, and the fix is merge order rather than an edit: **#453 should land before or with this.** Both references say so explicitly so the link is never silently wrong, but the cleanest sequence is #453 then #455.

k.renner merged commit 31a1179537 into master

2026-07-30 16:54:03 +00:00

k.renner referenced this issue from a commit

2026-07-30 16:54:04 +00:00

Merge pull request 'feat(mosquitto): enrol the health broker credential in Class-A rotation' (#455) from mosquitto-rotate-health into master

claude-bot referenced this pull request

2026-07-30 17:27:08 +00:00

fix(rotation): repin every rotator to a dated tag, not a digest #459

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#455

feat(mosquitto): enrol the health broker credential in Class-A rotation #455

Why health first

What it adds

Verified before committing

Deliberately not done here

Summary

🟡 Suggestions

🟢 Nits

Tests

Recommendation

🟡 Dangling doc reference

🟢 Stale bootstrap comments

Unchanged

Summary

🟡 Suggestions

Tests

Recommendation

Summary

🟡 Suggestions

Tests

Recommendation

Summary

🟡 Suggestions

🟢 Nits

Tests

Recommendation

Summary

🟡 Suggestions

Tests

Recommendation

🟡 Dated tag diverging from every sibling

🟡 Dead link to the proposal doc

Why `health` first