konairius/homelab
10
0
Fork 0
Code Issues 8 Pull Requests 4 Packages Projects Releases 1 Wiki Activity

refactor(mosquitto): move broker credentials to OpenBao, delete bootstrap Jobs #450

Closed
claude-bot wants to merge 2 commits from mosquitto-openbao into main
pull from: mosquitto-openbao
merge into: :main
Conversation 4 Commits 2 Files Changed 3
+160 -511
claude-bot commented 2026-07-30 13:03:15 +00:00
Member
Copy Link
Copy Source

Replaces #449 (closed). Instead of tidying the bootstrap Jobs, this deletes them.

518 lines → 14 declarative ones. Out: 7 Jobs, a ServiceAccount, a Role, a RoleBinding, the curl, the jq, the apk add, and the alpine-curl-jq image dependency. In: 7 ExternalSecrets matching the mosquitto-evcc-credentials pattern already in the same directory.

Why deleting beats fixing

Each Job was ~70 lines of shell that generated a password, base64'd it, hand-built Secret JSON with jq, and POSTed it to the Kubernetes API with curl.

More importantly it could not be maintained: a Job's spec.template is immutable, so merely changing the container image made the whole home-assistant Kustomization fail to reconcile — that's what made #449 unmergeable. Deleting is the only way to change a completed Job, and since this Kustomization has prune: true, removing them from git does exactly that instead of attempting a patch the API rejects.

The passwords also only ever existed in the Secret objects — not in OpenBao, so not backed up, not centrally rotatable, and unrecoverable if a Secret were lost.

Migration is value-preserving

This is the part that mattered. The broker has already hashed these passwords (mosquitto_passwd -U at startup) and every client is using them, so generating fresh ones would have broken Home Assistant, zigbee2mqtt, shellies, sml2mqtt, open3e and the health check simultaneously.

So the live values were copied into OpenBao first, each round-trip verified by sha256:

mosquitto-credentials              homeassistant-password  8bbb508e  8bbb508e  MATCH
mosquitto-admin-credentials        password                cdd5d74d  cdd5d74d  MATCH
mosquitto-health-credentials       password                23c10b3b  23c10b3b  MATCH
mosquitto-open3e-credentials       password                df67a98e  df67a98e  MATCH
mosquitto-shellies-credentials     password                5ae6f4e4  5ae6f4e4  MATCH
mosquitto-sml2mqtt-credentials     password                aa13ce52  aa13ce52  MATCH
mosquitto-zigbee2mqtt-credentials  password                e440d3ae  e440d3ae  MATCH

Values were passed to bao on stdin, never argv, so they never entered process listings or shell history. No value was printed at any point.

The one real unknown, tested

Whether ESO would adopt a Secret it didn't create. Tested live on the least critical credential before committing:

before after
password hash 3a145b6cc97f 3a145b6cc97f — unchanged
reconcile.external-secrets.io/managed (absent) true
ownerReferences none ExternalSecret

SecretSynced, Ready=True. ESO adopts and rewrites byte-identically, so consumers observe nothing.

Also confirmed the Secrets carry no ownerReferences from the Jobs, so pruning the Jobs cannot garbage-collect them.

Deliberately not normalised

mosquitto-credentials keeps the key homeassistant-password while the other six keep password. That asymmetry came from the original Jobs, and consumers reference those key names — unifying them would be a separate breaking change. Each OpenBao entry was seeded with its matching key name, since dataFrom.extract copies whatever keys the entry holds.

Rotation, before and after

  • Before: delete the Secret and the Job, wait for Flux to recreate the Job, hope it runs.
  • After: update the value in OpenBao; ESO republishes within refreshInterval.

The broker still needs a restart to re-hash, unchanged.

Verification

kustomize build + server-side dry run (0 errors); all 7 Jobs and the SA/Role/RoleBinding gone from the rendered output; all 8 mosquitto ExternalSecrets present.

Note the mosquitto-health-credentials ExternalSecret already exists in-cluster from the adoption test above — merging simply brings git into line with it.

🤖 Generated with Claude Code

Replaces #449 (closed). Instead of tidying the bootstrap Jobs, this deletes them. **518 lines → 14 declarative ones.** Out: 7 Jobs, a ServiceAccount, a Role, a RoleBinding, the `curl`, the `jq`, the `apk add`, and the `alpine-curl-jq` image dependency. In: 7 ExternalSecrets matching the `mosquitto-evcc-credentials` pattern already in the same directory. ## Why deleting beats fixing Each Job was ~70 lines of shell that generated a password, base64'd it, hand-built Secret JSON with `jq`, and POSTed it to the Kubernetes API with `curl`. More importantly it **could not be maintained**: a Job's `spec.template` is immutable, so merely changing the container image made the whole home-assistant Kustomization fail to reconcile — that's what made #449 unmergeable. Deleting is the only way to change a completed Job, and since this Kustomization has `prune: true`, removing them from git does exactly that instead of attempting a patch the API rejects. The passwords also only ever existed in the Secret objects — not in OpenBao, so **not backed up, not centrally rotatable**, and unrecoverable if a Secret were lost. ## Migration is value-preserving This is the part that mattered. The broker has already hashed these passwords (`mosquitto_passwd -U` at startup) and every client is using them, so generating fresh ones would have broken Home Assistant, zigbee2mqtt, shellies, sml2mqtt, open3e and the health check **simultaneously**. So the live values were copied into OpenBao first, each round-trip verified by sha256: ``` mosquitto-credentials homeassistant-password 8bbb508e 8bbb508e MATCH mosquitto-admin-credentials password cdd5d74d cdd5d74d MATCH mosquitto-health-credentials password 23c10b3b 23c10b3b MATCH mosquitto-open3e-credentials password df67a98e df67a98e MATCH mosquitto-shellies-credentials password 5ae6f4e4 5ae6f4e4 MATCH mosquitto-sml2mqtt-credentials password aa13ce52 aa13ce52 MATCH mosquitto-zigbee2mqtt-credentials password e440d3ae e440d3ae MATCH ``` Values were passed to `bao` on stdin, never argv, so they never entered process listings or shell history. No value was printed at any point. ## The one real unknown, tested Whether ESO would adopt a Secret it didn't create. Tested live on the least critical credential before committing: | | before | after | |---|---|---| | password hash | `3a145b6cc97f` | `3a145b6cc97f` — **unchanged** | | `reconcile.external-secrets.io/managed` | *(absent)* | `true` | | `ownerReferences` | none | `ExternalSecret` | `SecretSynced`, `Ready=True`. ESO adopts and rewrites byte-identically, so consumers observe nothing. Also confirmed the Secrets carry **no `ownerReferences` from the Jobs**, so pruning the Jobs cannot garbage-collect them. ## Deliberately not normalised `mosquitto-credentials` keeps the key `homeassistant-password` while the other six keep `password`. That asymmetry came from the original Jobs, and consumers reference those key names — unifying them would be a separate breaking change. Each OpenBao entry was seeded with its matching key name, since `dataFrom.extract` copies whatever keys the entry holds. ## Rotation, before and after - **Before:** delete the Secret *and* the Job, wait for Flux to recreate the Job, hope it runs. - **After:** update the value in OpenBao; ESO republishes within `refreshInterval`. The broker still needs a restart to re-hash, unchanged. ## Verification `kustomize build` + server-side dry run (0 errors); all 7 Jobs and the SA/Role/RoleBinding gone from the rendered output; all 8 mosquitto ExternalSecrets present. Note the `mosquitto-health-credentials` ExternalSecret already exists in-cluster from the adoption test above — merging simply brings git into line with it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude-bot added 1 commit 2026-07-30 13:03:15 +00:00
refactor(mosquitto): move broker credentials to OpenBao, delete bootstrap Jobs
ci/woodpecker/pr/pr-review Pipeline was successful
Details
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
Details
b24442ab71 
Replaces mosquitto-bootstrap.yaml (518 lines: 7 Jobs, a ServiceAccount, a
Role and a RoleBinding) with 7 ExternalSecrets, matching the pattern
mosquitto-evcc-credentials already used in the same directory.

Each Job was ~70 lines of shell that generated a password, base64'd it,
hand-built Secret JSON with jq and POSTed it to the Kubernetes API with
curl — needing RBAC and an `apk add curl jq` on every run. That is 518
lines to do what 14 declarative lines do here.

It also could not be maintained: a Job's spec.template is immutable, so
merely changing the container image made the whole home-assistant
Kustomization fail to reconcile with "field is immutable" — which is what
killed #449. Deleting the Jobs is the only way to change them, and since
the Kustomization has prune: true, removing them from git does exactly
that rather than attempting a rejected patch.

The passwords also only ever existed in the Secret objects: not in OpenBao,
so not backed up, not centrally rotatable, and unrecoverable if a Secret
were lost. They now live alongside every other credential in this
namespace.

MIGRATION IS VALUE-PRESERVING. The live passwords were copied into OpenBao
first, each round-trip verified by sha256 — the broker has already hashed
them (mosquitto_passwd -U at startup) and every client is using them, so
generating fresh ones would have broken Home Assistant, zigbee2mqtt,
shellies, sml2mqtt, open3e and the health check at once.

Verified on the least critical credential before committing, since ESO
adopting a Secret it did not create was the one real unknown:

  ExternalSecret -> SecretSynced / Ready=True
  password hash  -> 3a145b6cc97f before, 3a145b6cc97f after (unchanged)
  labels         -> reconcile.external-secrets.io/managed=true
  ownerReferences-> ExternalSecret

So ESO adopts the existing Secret and rewrites it byte-identically;
consumers observe nothing. The Secrets carry no ownerReferences from the
Jobs, so pruning the Jobs cannot garbage-collect them.

Key names are deliberately NOT normalised: mosquitto-credentials keeps
`homeassistant-password` while the rest keep `password`. Consumers
reference those names, so unifying them would be a separate breaking
change; each OpenBao entry was seeded with the matching key.

Rotation, previously "delete the Secret AND the Job and wait for Flux", is
now: update the value in OpenBao and ESO republishes within
refreshInterval. The broker still needs a restart to re-hash, as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
review-bot left a comment
Member
Copy Link
Copy Source

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

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

Summary

Deletes the 518-line mosquitto-bootstrap.yaml (7 bootstrap Jobs + a ServiceAccount/Role/RoleBinding that hand-generated broker passwords via curl+jq against the Kubernetes API) and replaces it with 7 ExternalSecrets pulling the same credentials from OpenBao, following the existing mosquitto-evcc-credentials-externalsecret.yaml pattern in the same directory. I checked every consumer against the new resources and everything lines up — no blocking issues found.

🟡 Suggestions

  • kubernetes/apps/home-assistant/mosquitto-bootstrap-credentials-externalsecrets.yaml: this PR's safety hinges entirely on the OpenBao values under apps/home-assistant/mosquitto-*-credentials already being seeded with the exact live passwords (the PR body documents this was done and sha256-verified out of band). That step isn't visible in the diff or checkout — there's no way to verify it from here. If it hasn't actually happened yet for all 7 keys before this merges, ESO will adopt each Secret with creationPolicy: Owner and overwrite it with whatever dataFrom.extract returns (empty/missing key on a 404, or a fresh value if someone pre-seeded something else), silently rotating live broker credentials and locking out Home Assistant, zigbee2mqtt, shellies, sml2mqtt, open3e, evcc, and the health probe simultaneously. Worth a final confirmation the OpenBao entries are in place before merge, since the file itself can't prove it.
  • Same file, all 7 blocks: none set creationPolicy explicitly, so it's implicitly Owner (default). The PR body says this was tested live for mosquitto-health-credentials and confirmed to adopt the existing Secret byte-identically and set ownerReferences. Given that Secret now has an ExternalSecret owner and the Kustomization has prune: true, deleting the ExternalSecret resource (not just this Job) in a future change will now delete the live Secret too — a behavior change worth keeping in mind for whoever edits this file next (the header comment could mention it, since the old Jobs had no such coupling).

🟢 Nits

  • kubernetes/apps/home-assistant/mosquitto-bootstrap-credentials-externalsecrets.yaml: filename still says "bootstrap" even though the whole point of the change is that these are no longer bootstrap Jobs. A name like mosquitto-credentials-externalsecrets.yaml would better reflect the new steady-state role, though keeping "bootstrap" as a breadcrumb for git-blame archaeology is a reasonable tradeoff too.
  • The per-secret target.name comment on mosquitto-credentials ("unchanged name; HA's MQTT integration reads this") and on the sibling mosquitto-evcc-credentials-externalsecret.yaml ("keeps the old Secret's name — consumers unchanged") say the same thing with different wording; not worth blocking on, just noting for consistency-minded future edits.

Tests

No test suite applies here (GitOps YAML, no CI-run tests in this repo for Kustomize resources). Verification is operational: the PR body states kustomize build + a server-side dry run passed, and the author manually tested ESO's adoption behavior live against one of the seven existing Secrets before writing this migration. I independently confirmed from the checkout that:

  • All 7 ExternalSecret names/target names (mosquitto-credentials, mosquitto-admin-credentials, mosquitto-health-credentials, mosquitto-open3e-credentials, mosquitto-shellies-credentials, mosquitto-sml2mqtt-credentials, mosquitto-zigbee2mqtt-credentials) match the secretKeyRef names/keys consumed in mosquitto-deployment.yaml (including the asymmetric homeassistant-password key on mosquitto-credentials vs. password on the rest).
  • The home-assistant namespace is already listed in kubernetes/infrastructure/post-crds/openbao-clustersecretstore.yaml's conditions.namespaces, so no additional RBAC/ClusterSecretStore change is needed for these ExternalSecrets to sync.
  • No other file in the repo references the deleted mosquitto-bootstrap ServiceAccount/Role/RoleBinding or the app.kubernetes.io/name: mosquitto-bootstrap label (e.g., no stray NetworkPolicy), so nothing is left dangling by the deletion.
    The one thing that cannot be verified from a static checkout — that the live OpenBao entries actually hold the current production passwords — is called out above as a pre-merge suggestion, not a defect in the diff itself.

Recommendation

Looks reasonable — no blocking issues; comments above are optional (though the OpenBao-seeding suggestion is worth a final confirmation before merge given the blast radius of getting it wrong).

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `b24442ab71e6c2f1392e9d1cf21a5e2252d26645`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Deletes the 518-line `mosquitto-bootstrap.yaml` (7 bootstrap `Job`s + a `ServiceAccount`/`Role`/`RoleBinding` that hand-generated broker passwords via `curl`+`jq` against the Kubernetes API) and replaces it with 7 `ExternalSecret`s pulling the same credentials from OpenBao, following the existing `mosquitto-evcc-credentials-externalsecret.yaml` pattern in the same directory. I checked every consumer against the new resources and everything lines up — no blocking issues found. ## 🟡 Suggestions - `kubernetes/apps/home-assistant/mosquitto-bootstrap-credentials-externalsecrets.yaml`: this PR's safety hinges entirely on the OpenBao values under `apps/home-assistant/mosquitto-*-credentials` already being seeded with the exact live passwords (the PR body documents this was done and sha256-verified out of band). That step isn't visible in the diff or checkout — there's no way to verify it from here. If it hasn't actually happened yet for all 7 keys before this merges, ESO will adopt each Secret with `creationPolicy: Owner` and overwrite it with whatever `dataFrom.extract` returns (empty/missing key on a 404, or a fresh value if someone pre-seeded something else), silently rotating live broker credentials and locking out Home Assistant, zigbee2mqtt, shellies, sml2mqtt, open3e, evcc, and the health probe simultaneously. Worth a final confirmation the OpenBao entries are in place before merge, since the file itself can't prove it. - Same file, all 7 blocks: none set `creationPolicy` explicitly, so it's implicitly `Owner` (default). The PR body says this was tested live for `mosquitto-health-credentials` and confirmed to adopt the existing Secret byte-identically and set `ownerReferences`. Given that Secret now has an `ExternalSecret` owner and the Kustomization has `prune: true`, deleting the `ExternalSecret` resource (not just this Job) in a future change will now delete the live Secret too — a behavior change worth keeping in mind for whoever edits this file next (the header comment could mention it, since the old Jobs had no such coupling). ## 🟢 Nits - `kubernetes/apps/home-assistant/mosquitto-bootstrap-credentials-externalsecrets.yaml`: filename still says "bootstrap" even though the whole point of the change is that these are no longer bootstrap Jobs. A name like `mosquitto-credentials-externalsecrets.yaml` would better reflect the new steady-state role, though keeping "bootstrap" as a breadcrumb for git-blame archaeology is a reasonable tradeoff too. - The per-secret `target.name` comment on `mosquitto-credentials` ("unchanged name; HA's MQTT integration reads this") and on the sibling `mosquitto-evcc-credentials-externalsecret.yaml` ("keeps the old Secret's name — consumers unchanged") say the same thing with different wording; not worth blocking on, just noting for consistency-minded future edits. ## Tests No test suite applies here (GitOps YAML, no CI-run tests in this repo for Kustomize resources). Verification is operational: the PR body states `kustomize build` + a server-side dry run passed, and the author manually tested ESO's adoption behavior live against one of the seven existing Secrets before writing this migration. I independently confirmed from the checkout that: - All 7 `ExternalSecret` names/target names (`mosquitto-credentials`, `mosquitto-admin-credentials`, `mosquitto-health-credentials`, `mosquitto-open3e-credentials`, `mosquitto-shellies-credentials`, `mosquitto-sml2mqtt-credentials`, `mosquitto-zigbee2mqtt-credentials`) match the `secretKeyRef` names/keys consumed in `mosquitto-deployment.yaml` (including the asymmetric `homeassistant-password` key on `mosquitto-credentials` vs. `password` on the rest). - The `home-assistant` namespace is already listed in `kubernetes/infrastructure/post-crds/openbao-clustersecretstore.yaml`'s `conditions.namespaces`, so no additional RBAC/ClusterSecretStore change is needed for these `ExternalSecret`s to sync. - No other file in the repo references the deleted `mosquitto-bootstrap` `ServiceAccount`/`Role`/`RoleBinding` or the `app.kubernetes.io/name: mosquitto-bootstrap` label (e.g., no stray NetworkPolicy), so nothing is left dangling by the deletion. The one thing that cannot be verified from a static checkout — that the live OpenBao entries actually hold the current production passwords — is called out above as a pre-merge suggestion, not a defect in the diff itself. ## Recommendation Looks reasonable — no blocking issues; comments above are optional (though the OpenBao-seeding suggestion is worth a final confirmation before merge given the blast radius of getting it wrong). <!-- pr-review:sha=b24442ab71e6c2f1392e9d1cf21a5e2252d26645 -->
claude-bot added 1 commit 2026-07-30 13:14:07 +00:00
docs(mosquitto): flag the new Secret ownership coupling; drop "bootstrap" name
ci/woodpecker/pr/pr-review Pipeline was successful
Details
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
Details
3d905416bc 
Review points on #450, both worth acting on.

Document the coupling ESO introduces that the Jobs did not have. With
creationPolicy implicitly Owner, ESO sets an ownerReference from each
Secret to its ExternalSecret; combined with the Kustomization's
prune: true, deleting an ExternalSecret block now DELETES THE LIVE SECRET
and with it that client's broker access. The old Jobs created Secrets with
no ownerReference, so removing a Job left its Secret untouched — that
safety net is gone, and the file now says so, including the
creationPolicy: Orphan escape hatch for detaching a credential without
losing it.

Rename mosquitto-bootstrap-credentials-externalsecrets.yaml to
mosquitto-credentials-externalsecrets.yaml. "bootstrap" described the Jobs
that no longer exist; these are steady-state. git tracks the rename, so the
archaeology the old name preserved is not lost.

Also re-verified, a second time and independently, that every OpenBao entry
still holds the live password — the reviewer correctly noted this PR's
safety rests on a fact the diff cannot show:

  mosquitto-credentials              homeassistant-password  8bbb508e  MATCH
  mosquitto-admin-credentials        password                cdd5d74d  MATCH
  mosquitto-health-credentials       password                23c10b3b  MATCH
  mosquitto-open3e-credentials       password                df67a98e  MATCH
  mosquitto-shellies-credentials     password                5ae6f4e4  MATCH
  mosquitto-sml2mqtt-credentials     password                aa13ce52  MATCH
  mosquitto-zigbee2mqtt-credentials  password                e440d3ae  MATCH

Left the wording overlap between this file's target.name comment and the
sibling evcc file alone: harmonising it would touch a file this PR
otherwise does not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
claude-bot commented 2026-07-30 13:14:26 +00:00
Author
Member
Copy Link
Copy Source

No — I said I would wait for this review and then did not read it until prompted. Addressed now in 3d905416.

🟡 Confirm the OpenBao seeding before merge

The right thing to push on: this PR's safety rests on a fact the diff cannot show, and the blast radius of getting it wrong is every MQTT client at once. Re-verified independently just now, against the live Secrets rather than against my earlier run:

SECRET                               KEY                      LIVE_SHA   BAO_SHA    RESULT
mosquitto-credentials                homeassistant-password   8bbb508e   8bbb508e   MATCH
mosquitto-admin-credentials          password                 cdd5d74d   cdd5d74d   MATCH
mosquitto-health-credentials         password                 23c10b3b   23c10b3b   MATCH
mosquitto-open3e-credentials         password                 df67a98e   df67a98e   MATCH
mosquitto-shellies-credentials       password                 5ae6f4e4   5ae6f4e4   MATCH
mosquitto-sml2mqtt-credentials       password                 aa13ce52   aa13ce52   MATCH
mosquitto-zigbee2mqtt-credentials    password                 e440d3ae   e440d3ae   MATCH

The ABSENT case you worried about is covered too: the readback substitutes the literal string ABSENT for a missing key, which would hash differently and show MISMATCH rather than silently comparing empty to empty.

🟡 The new ownership coupling

Good catch, and a genuine behaviour change I had not written down. With creationPolicy implicitly Owner, ESO sets an ownerReference from each Secret to its ExternalSecret; with prune: true, deleting an ExternalSecret block now deletes the live Secret and that client's broker access. The old Jobs created Secrets with no ownerReference, so removing a Job left its Secret untouched — that safety net is gone.

Now documented prominently in the file header, including the creationPolicy: Orphan escape hatch for detaching a credential without destroying it.

🟢 Filename

Renamed to mosquitto-credentials-externalsecrets.yaml. "bootstrap" described Jobs that no longer exist. git tracks the rename, so the blame trail you wanted to preserve survives without the misleading name.

🟢 Comment wording overlap

Left alone deliberately — harmonising it would mean touching mosquitto-evcc-credentials-externalsecret.yaml, a file this PR otherwise does not modify.

On your verification work

Worth noting you checked three things I had not: that the secretKeyRef names and keys in mosquitto-deployment.yaml match all seven (including the asymmetric homeassistant-password), that home-assistant is already in the ClusterSecretStore's conditions.namespaces so no RBAC change is needed, and that nothing else references the deleted ServiceAccount/Role/RoleBinding or the app.kubernetes.io/name: mosquitto-bootstrap label. That last one is exactly the kind of dangling reference this deletion could have left behind.

No — I said I would wait for this review and then did not read it until prompted. Addressed now in `3d905416`. ### 🟡 Confirm the OpenBao seeding before merge The right thing to push on: this PR's safety rests on a fact the diff cannot show, and the blast radius of getting it wrong is every MQTT client at once. Re-verified independently just now, against the live Secrets rather than against my earlier run: ``` SECRET KEY LIVE_SHA BAO_SHA RESULT mosquitto-credentials homeassistant-password 8bbb508e 8bbb508e MATCH mosquitto-admin-credentials password cdd5d74d cdd5d74d MATCH mosquitto-health-credentials password 23c10b3b 23c10b3b MATCH mosquitto-open3e-credentials password df67a98e df67a98e MATCH mosquitto-shellies-credentials password 5ae6f4e4 5ae6f4e4 MATCH mosquitto-sml2mqtt-credentials password aa13ce52 aa13ce52 MATCH mosquitto-zigbee2mqtt-credentials password e440d3ae e440d3ae MATCH ``` The `ABSENT` case you worried about is covered too: the readback substitutes the literal string `ABSENT` for a missing key, which would hash differently and show MISMATCH rather than silently comparing empty to empty. ### 🟡 The new ownership coupling Good catch, and a genuine behaviour change I had not written down. With `creationPolicy` implicitly `Owner`, ESO sets an ownerReference from each Secret to its ExternalSecret; with `prune: true`, **deleting an ExternalSecret block now deletes the live Secret** and that client's broker access. The old Jobs created Secrets with no ownerReference, so removing a Job left its Secret untouched — that safety net is gone. Now documented prominently in the file header, including the `creationPolicy: Orphan` escape hatch for detaching a credential without destroying it. ### 🟢 Filename Renamed to `mosquitto-credentials-externalsecrets.yaml`. "bootstrap" described Jobs that no longer exist. git tracks the rename, so the blame trail you wanted to preserve survives without the misleading name. ### 🟢 Comment wording overlap Left alone deliberately — harmonising it would mean touching `mosquitto-evcc-credentials-externalsecret.yaml`, a file this PR otherwise does not modify. ### On your verification work Worth noting you checked three things I had not: that the `secretKeyRef` names and keys in `mosquitto-deployment.yaml` match all seven (including the asymmetric `homeassistant-password`), that `home-assistant` is already in the ClusterSecretStore's `conditions.namespaces` so no RBAC change is needed, and that nothing else references the deleted ServiceAccount/Role/RoleBinding or the `app.kubernetes.io/name: mosquitto-bootstrap` label. That last one is exactly the kind of dangling reference this deletion could have left behind.
review-bot left a comment
Member
Copy Link
Copy Source

🤖 Automated review — model CLI default (complexity: complex), commit 3d905416bc9e9002edef5e6626f7bc9a7d0e1819.

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

Summary

Deletes mosquitto-bootstrap.yaml (508 lines: 7 curl/jq-based bootstrap Jobs plus a ServiceAccount/Role/RoleBinding that hand-generated broker credentials as plaintext-only Secrets) and replaces it with mosquitto-credentials-externalsecrets.yaml (14 lines × 7 ExternalSecrets pulling from OpenBao via the openbao ClusterSecretStore). This follows the exact pattern already in production in the same directory (mosquitto-evcc-credentials-externalsecret.yaml). No blocking issues found.

I verified against the checkout:

  • The new ExternalSecrets' secretStoreRef/dataFrom.extract shape matches the existing working mosquitto-evcc-credentials-externalsecret.yaml exactly.
  • Target Secret names and keys are unchanged and line up with every consumer in mosquitto-deployment.yaml:152-196 (mosquitto-credentials/homeassistant-password kept distinct from the other six, which all use password, as the PR body states).
  • No leftover references anywhere under kubernetes/ to mosquitto-bootstrap, the deleted ServiceAccount/Role/RoleBinding, or the alpine:3.21/curl/jq bootstrap image.
  • kubernetes/clusters/homelab/home-assistant.yaml confirms the home-assistant Flux Kustomization has prune: true, which is exactly the mechanism the PR relies on to remove the Jobs (a Job's spec.template is immutable so an in-place patch would otherwise fail).
  • ClusterSecretStore/openbao exists (kubernetes/infrastructure/post-crds/openbao-clustersecretstore.yaml).

I could not independently re-run kustomize build / a server-side dry-run in this environment, so I leaned on the cross-checks above rather than re-verifying the PR's own build output.

🟡 Suggestions

  • kubernetes/apps/home-assistant/mosquitto-deployment.yaml:180,189-191,233 still reference the now-deleted *-bootstrap Job names in comments (e.g. "Probe-only credential (mosquitto-health-credentials-bootstrap)", "mosquitto-health-credentials-bootstrap Job"). These are now dangling references to objects that no longer exist. Not incorrect in intent, but since this PR's whole point is retiring those Jobs, it'd be worth a small follow-up to reword them (or fold into this PR) so they don't point future readers at nonexistent resources.

Tests

No automated test framework applies to this GitOps manifest change; the appropriate verification is kustomize build + a server-side dry run, which the PR body states was done (0 errors, all 7 Jobs/SA/Role/RoleBinding gone, all 8 mosquitto ExternalSecrets present) along with a live pre-merge test of ESO adopting an existing Secret without changing its content. I was not able to re-run the build/dry-run myself in this sandbox, but the static checks above (consumer key names, resource references, prune flag, ClusterSecretStore existence) are all consistent with what the PR claims.

Recommendation

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

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `3d905416bc9e9002edef5e6626f7bc9a7d0e1819`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Deletes `mosquitto-bootstrap.yaml` (508 lines: 7 `curl`/`jq`-based bootstrap Jobs plus a ServiceAccount/Role/RoleBinding that hand-generated broker credentials as plaintext-only Secrets) and replaces it with `mosquitto-credentials-externalsecrets.yaml` (14 lines × 7 ExternalSecrets pulling from OpenBao via the `openbao` ClusterSecretStore). This follows the exact pattern already in production in the same directory (`mosquitto-evcc-credentials-externalsecret.yaml`). No blocking issues found. I verified against the checkout: - The new ExternalSecrets' `secretStoreRef`/`dataFrom.extract` shape matches the existing working `mosquitto-evcc-credentials-externalsecret.yaml` exactly. - Target Secret names and keys are unchanged and line up with every consumer in `mosquitto-deployment.yaml:152-196` (`mosquitto-credentials`/`homeassistant-password` kept distinct from the other six, which all use `password`, as the PR body states). - No leftover references anywhere under `kubernetes/` to `mosquitto-bootstrap`, the deleted ServiceAccount/Role/RoleBinding, or the `alpine:3.21`/`curl`/`jq` bootstrap image. - `kubernetes/clusters/homelab/home-assistant.yaml` confirms the `home-assistant` Flux Kustomization has `prune: true`, which is exactly the mechanism the PR relies on to remove the Jobs (a Job's `spec.template` is immutable so an in-place patch would otherwise fail). - `ClusterSecretStore/openbao` exists (`kubernetes/infrastructure/post-crds/openbao-clustersecretstore.yaml`). I could not independently re-run `kustomize build` / a server-side dry-run in this environment, so I leaned on the cross-checks above rather than re-verifying the PR's own build output. ## 🟡 Suggestions - `kubernetes/apps/home-assistant/mosquitto-deployment.yaml:180,189-191,233` still reference the now-deleted `*-bootstrap` Job names in comments (e.g. "Probe-only credential (mosquitto-health-credentials-bootstrap)", "mosquitto-health-credentials-bootstrap Job"). These are now dangling references to objects that no longer exist. Not incorrect in intent, but since this PR's whole point is retiring those Jobs, it'd be worth a small follow-up to reword them (or fold into this PR) so they don't point future readers at nonexistent resources. ## Tests No automated test framework applies to this GitOps manifest change; the appropriate verification is `kustomize build` + a server-side dry run, which the PR body states was done (0 errors, all 7 Jobs/SA/Role/RoleBinding gone, all 8 mosquitto ExternalSecrets present) along with a live pre-merge test of ESO adopting an existing Secret without changing its content. I was not able to re-run the build/dry-run myself in this sandbox, but the static checks above (consumer key names, resource references, prune flag, ClusterSecretStore existence) are all consistent with what the PR claims. ## Recommendation Looks reasonable — no blocking issues; comment above is optional. <!-- pr-review:sha=3d905416bc9e9002edef5e6626f7bc9a7d0e1819 -->
claude-bot commented 2026-07-30 13:27:03 +00:00
Author
Member
Copy Link
Copy Source

Closing. The objection is correct and fatal: this depends on OpenBao already containing values I copied in by hand, which git cannot recreate. A fresh OpenBao would leave these paths empty and ESO would then overwrite live broker credentials with nothing.

Investigating the alternatives turned up why this is hard, and it is not an implementation problem:

  • ESO Password generator is the clean target — declarative, no shell, no RBAC, no Job immutability, no dependency on OpenBao content. But tested here, it overwrites a pre-existing Secret (KNOWN-PREEXISTING-VALUE -> freshly generated), so adopting it rotates every live password.
  • Rotation is expensive for 5 of 7. shellies, sml2mqtt and open3e have no in-cluster workloads at all — those passwords live in Shelly device firmware and external daemons. mosquitto-credentials is entered by hand in the Home Assistant MQTT UI, and admin is held by a workstation client. Only health and zigbee2mqtt are fully in-cluster and safe to rotate unattended.
  • Renaming the Jobs would sidestep the spec.template immutability that killed #449, but it is a hack that recurs on every future edit.

So no approach is simultaneously declarative, git-self-sufficient, and value-preserving — because 5 of these credentials are pinned by consumers outside the cluster. That is the actual constraint, and it is worth recording here rather than rediscovering.

The OpenBao entries seeded during this attempt hold the current live values and are harmless (nothing reads them now that these ExternalSecrets are gone), but they are hand-made state and should be removed if this direction is abandoned for good.

One artefact of the adoption test remains: mosquitto-health-credentials has an ExternalSecret in-cluster and its Secret now carries an ExternalSecret ownerReference. That needs unwinding — with prune: true and no manifest in git, Flux will delete the ExternalSecret, and the ownerReference means the live Secret goes with it. Handling that before it bites.

Closing. The objection is correct and fatal: this depends on OpenBao already containing values I copied in by hand, which git cannot recreate. A fresh OpenBao would leave these paths empty and ESO would then overwrite live broker credentials with nothing. Investigating the alternatives turned up why this is hard, and it is not an implementation problem: - **ESO Password generator** is the clean target — declarative, no shell, no RBAC, no Job immutability, no dependency on OpenBao content. But tested here, it **overwrites** a pre-existing Secret (`KNOWN-PREEXISTING-VALUE` -> freshly generated), so adopting it rotates every live password. - **Rotation is expensive for 5 of 7.** `shellies`, `sml2mqtt` and `open3e` have no in-cluster workloads at all — those passwords live in Shelly device firmware and external daemons. `mosquitto-credentials` is entered by hand in the Home Assistant MQTT UI, and `admin` is held by a workstation client. Only `health` and `zigbee2mqtt` are fully in-cluster and safe to rotate unattended. - **Renaming the Jobs** would sidestep the spec.template immutability that killed #449, but it is a hack that recurs on every future edit. So no approach is simultaneously declarative, git-self-sufficient, and value-preserving — because 5 of these credentials are pinned by consumers outside the cluster. That is the actual constraint, and it is worth recording here rather than rediscovering. The OpenBao entries seeded during this attempt hold the current live values and are harmless (nothing reads them now that these ExternalSecrets are gone), but they are hand-made state and should be removed if this direction is abandoned for good. One artefact of the adoption test remains: `mosquitto-health-credentials` has an ExternalSecret in-cluster and its Secret now carries an ExternalSecret ownerReference. That needs unwinding — with `prune: true` and no manifest in git, Flux will delete the ExternalSecret, and the ownerReference means the live Secret goes with it. Handling that before it bites.
claude-bot closed this pull request 2026-07-30 13:27:04 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
review-bot
Labels
Clear labels
apollo
Hermes expert: Immich, Audiobookshelf, Libation, Navidrome, Music Assistant
 atlas
Hermes expert: Talos, Flux, network, Authentik, Matrix, monitoring
 clio
Hermes expert: Paperless, wiki, ical-proxy, TRMNL, websites
 daedalus
Hermes expert: Gitea, Woodpecker, Renovate, review-bot, Hermes
 hestia
Hermes expert: Home Assistant, IoT, MQTT, ESPHome, voice
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
alert-monitor-bot claude-bot (Claude Bot) flux hermes-bot issue-fix-bot k.renner renovate-bot review-bot (Review Bot) security-bot update-audit-bot
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: konairius/homelab#450