Ran the check I had just documented and it returned kustomization.yaml —
because the comment explaining the rule contains the literal string it
greps for. A guard whose only hit is its own documentation trains people
to ignore it, which is worse than not having one.
Anchored on `image:` instead. Verified both directions: returns nothing
across this directory as it stands, and still flags a file containing a
real `image: ...@sha256:` pin.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review point on #459. Nothing stops the next rotator from copy-pasting a
@sha256 pin from an older example, and the failure only surfaces months
later on the first scheduled run — the same silent, delayed failure this
PR just cleaned up 11 instances of.
Added two copy-pasteable checks to the kustomization header: a grep that
must return nothing before adding a rotator, and a pull test to run BEFORE
committing a tag bump. The second matters because a tag that was never
built fails exactly like the dead digests did, and just as quietly.
Not a CI gate, deliberately. Nothing in CI covers kubernetes/** at all
(deploy-infrastructure.yml only covers opentofu/**), so a real guard means
standing up a lint/validate pipeline for that tree — worth doing, but as
its own change rather than smuggled into a repin PR. Documented here so
the knowledge is not lost while that waits.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes the breakage in #458: 11 of the 14 rotators were pinned to image
digests that no longer exist in the registry. Verified by pulling each
rather than by inspecting metadata (the registry API returns 401 for a
PAT, so metadata checks mislead here):
gone 159e5888 authentik, crowdsec-bouncer-key, paperless,
shelly-manager, woodpecker, home-assistant (x3)
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 survivors are exactly the rotators that run often; every gone
digest belongs to a quarterly one. Untagged manifests are collected once
:latest and the dated tags move past them.
Impact had this not been caught: every quarterly CronJob shows
lastScheduleTime <none> — they were created 2026-07-18..23, after the last
quarterly fire — so none has ever run. Their first fire is 2026-10-01,
where ~10 would have ImagePullBackOff'd at once. A rotation that never
starts looks identical to nothing happening, so credentials would have
silently stopped rotating while the manifests kept asserting they did.
All 16 references (13 files; home-assistant has 3 CronJobs, review-bot 2)
now use secret-rotator:2026-07-30 — dated tag, no digest, no :latest, per
owner directive and matching ci-ansible:2026-04-29 and the alpine-jq /
cni-plugins / alpine-openssl pins.
Also repinned flux-git, claude-oauth and review-bot, whose digests still
resolve today. They are pinned the same way and would decay identically;
converting them now avoids a second pass when they break.
Verified before repinning that one image serves all of them: the modes in
use across these files are exactly
cloudflare-roll gitea-token ha-token netbird-key proxmox-token random
woodpecker-secret
and the 2026-07-30 image's usage string lists exactly those seven. So this
is a single binary and splitting the pin would serve no purpose.
Rationale recorded in kustomization.yaml rather than repeated 16 times,
including WHY digests decayed: the build triggers on
.woodpecker/build-ci-images.yml, so adding an unrelated image republishes
secret-rotator and orphans the manifest every rotator was pinned to. That
is how these went stale without anyone touching the rotator source.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
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 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 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>
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 point on #453. The document claimed 14 rotators, which matches
neither way of counting. Verified directly rather than accepting the
review number on trust:
files in rotation/ (excl kustomization): 13
resources listed in kustomization: 13
actual CronJob objects: 15
Class-A: 5
home-assistant.yaml alone defines three CronJobs, which is where the
discrepancy comes from. Now states 13 manifests defining 15 CronJobs, of
which 5 are Class-A.
Worth fixing beyond pedantry: this document argues for verifying claims
rather than asserting them, and the count was eyeballed from a directory
listing rather than counted. A wrong number in the opening paragraph
undermines the parts that were actually verified.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review points on #453. All four verified against the repo before changing
anything, since a proposal's only value is being precise about exactly
this kind of detail.
Line count: mosquitto-bootstrap.yaml is 508 lines, not 518, and the
ServiceAccount/Role/RoleBinding are inside that same file — the original
wording double-counted them.
Generator blast radius: adopting the ESO Password generator would rotate
the 7 Job-minted credentials, not "all 8". evcc is already
ESO-extract-backed and unaffected.
Rollout failure mode: the broker rolls with maxUnavailable: 0 /
maxSurge: 1 (confirmed in mosquitto-deployment.yaml), so mid-rollout two
broker pods with DIFFERENT passwd files sit behind the LB and a
reconnecting client can land on the old one and fail auth until the
rollout finishes. Self-healing and brief, but it will look alarming in
logs during the first rotation, so it is now called out in advance rather
than discovered live.
ESO refresh latency: added to the open questions. The evcc ExternalSecret
uses refreshInterval: 1h, so a rotation takes up to an hour to land. That
does not widen the outage window — broker and client still move together —
but it affects verifying a rotation completed and the cold-rebuild
ordering, and raises whether these want a shorter interval or a
rotator-triggered sync.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Documentation only — nothing implemented, no manifests changed.
The mosquitto broker passwords are the only Class-A-shaped secrets in the
cluster not enrolled in the rotation framework under
kubernetes/infrastructure/openbao/rotation/. They are minted once by seven
bootstrap Jobs and then never change: no schedule, no compromise response,
no path at all. They also live only in the Secret objects, so they are
outside the OpenBao snapshot and unrecoverable if lost.
Records why the obvious fixes do not work, since each was tried and the
reasons are not visible from the code:
- ExternalSecrets over hand-seeded OpenBao paths depend on content git
cannot recreate; a fresh OpenBao would leave them empty and ESO would
overwrite live credentials with nothing.
- The ESO Password generator OVERWRITES pre-existing Secrets — verified
against a scratch Secret, not assumed — so adopting it rotates all eight
at once, uncoordinated.
- Keeping the Jobs and fixing only the image is blocked by spec.template
immutability; renaming them works but recurs on every future edit.
Documents the two mechanics that actually shape the design: the broker
rebuilds its passwd file from the Secrets on every pod start (so a Secret
change is inert until it restarts, and there is no Reloader annotation
today), and a mosquitto user has exactly one password (so there is no
make-before-break as the LLAT rotators use).
Also records who holds each credential, which is not visible from the
manifests and is what determines cutover cost — including that sml2mqtt
and open3e keep the same password in both a Kubernetes Secret and
ansible-vault, synchronised by hand. That duplication, not any client
limitation, is what makes rotation look impossible today.
Proposes per-user rotators following rotation/paperless.yaml, phased
health -> zigbee2mqtt -> evcc -> edge-host pair -> shellies ->
homeassistant, so blast radius is one client per rotation and the Jobs are
deleted incrementally as their users migrate rather than patched.
Open questions are listed rather than guessed at, notably whether
shelly-manager can push MQTT credentials to device firmware and how the HA
MQTT config entry is updated programmatically.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner directive: tools belong in the image, not installed at container
start. This init container ran `apk add --no-cache openssl` on EVERY
paperless pod start.
Unlike the multus CNI installer, this one genuinely needs an image: the
openssl CLI is absent from the alpine base (verified — the base carries the
libraries but not the command), and busybox has no `s_client` equivalent,
so the STARTTLS certificate capture cannot be done with what is already
there.
Adds ci/images/alpine-openssl, named for what it is rather than for this
consumer, so anything else needing s_client/x509 from a shell script can
reuse it.
Script logic is unchanged — only the apk add line is removed and the image
swapped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review point on #447. The build-time check asserted macvlan, static and
host-local were executable but omitted dhcp, which the sibling
cni-dhcp-daemon DaemonSet runs and which its own comment says ships from
this installer ("the dhcp binary ships from the sibling
cni-plugins-installer DaemonSet ... whole tarball extracted").
Verified that dependency in kubernetes/infrastructure/multus/
cni-dhcp-daemon.yaml before changing anything, rather than taking the
review's word for it.
Consequence of the omission: since the whole /plugins directory is copied
regardless of what the loop checks, nothing is broken today — but the
"fail loudly if upstream restructures the archive" guarantee only covered
three of the four binaries this repo depends on. Upstream dropping dhcp
while keeping the others would have built green and surfaced later as a
broken DHCP daemon on whichever node rolled first.
Confirmed all four are executable in the already-published image, so this
tightens the assertion without changing what ships.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Supersedes the wget-instead-of-curl approach in #446, which removed the
`apk add` but left the real problems in place.
The installer fetched the containernetworking/plugins release tarball from
GitHub on the node and extracted it into /opt/cni/bin with NO integrity
verification of any kind. That directory is executed as root in the CNI
path on every node, so a tampered or MITM'd artifact would have obtained
privileged execution cluster-wide with nothing to detect it. It also put
github.com on the critical path of every node rebuild.
Now the binaries are baked into ci/images/cni-plugins, where the download
happens once and the sha256 IS verified, and the installer is a local file
copy with zero network calls (verified: no wget/curl/apk/https in the
rendered script).
The pinned digest was cross-checked against upstream's own published
cni-plugins-linux-amd64-v1.5.1.tgz.sha256 asset, not merely recorded from a
local download — a download can be tampered with, so comparing against
what upstream states is the point:
77baa2f669980a82255ffa2f2717de823992480271ee778aa51a9c60ae89ff9b
Verified the gate actually gates: with the correct digest the layer logic
downloads, verifies, extracts and asserts macvlan/static/host-local are
executable; with a deliberately wrong digest `sha256sum -c` FAILS and the
extract step is never reached.
Install script changes beyond dropping the fetch:
- Copies unconditionally instead of the old "already installed?" guard.
The guard existed to avoid re-downloading; a local copy is cheap, so
copying every time means a node always matches the version pinned in the
manifest rather than keeping whatever it first installed.
- Copies to a temp name in the same directory then rename(2)s over the
target. `cp` onto a plugin the kubelet is currently executing can fail
with ETXTBSY; rename never does and is atomic, so a concurrent CNI
invocation sees either the old or the new binary, never a partial one.
- LICENSE and README.md are no longer installed into /opt/cni/bin; the old
tar extraction put them there.
The chown 0:0 is kept but re-framed as a REPAIR. Its original cause is
gone — extraction applied the archive's `./` uid/gid (1001/127, the
github-actions runner) to the destination, and `cp` into an existing
directory does not touch its ownership. It still runs because nodes
provisioned by the old installer may already carry the wrong ownership,
which breaks Cilium's apply-sysctl-overwrites init container.
Verified end-to-end with the rendered script, including that repair path:
pre-seeded owner 1001:127 -> 0:0 after run
fresh install -> host-local/macvlan/static present, macvlan executes
and reports "CNI macvlan plugin v1.5.1", exit 0
re-run -> idempotent, exit 0, 18 plugins, no .tmp files left
Note the image carries ~80MB of extracted plugins (18 binaries), matching
what the old installer put on each node. All are kept rather than trimming
to the few in use, to avoid removing plugins already present on nodes as a
side effect of this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review point on #444, which merged before this could be folded in.
The comment claimed the listed shapes yield "null" at rc=0. That describes
BARE `.network_key.key`, not the expression actually written, and it
contradicted its own closing line two lines below. Measured against jq
1.7.1 in the image, `(.network_key?.key? // "")` returns an EMPTY string at
rc=0 for all of:
{} {"devices":[]} {"network_key":{}} {"network_key":null}
{"network_key":"abc"} {"network_key":123}
[] "a string"
and never the literal text null, because `// ""` maps a null result to
empty. Bare `.network_key.key` is what exits 5 on the last four shapes.
Checking that also turned up something the review did not mention: because
KEY can never be "null" with the current expression, the `[ "$KEY" =
"null" ]` arm is dead code today. Kept deliberately as belt-and-braces for
a future edit that drops the `// ""` fallback, and now labelled as such
rather than left looking like the arm that fires.
Comment-only: `git diff` shows zero changed lines outside comments, and the
rendered script is unchanged in behaviour.
NOTE: this text lives in the Deployment's init container command, so it is
part of the pod template — merging rolls the zigbee2mqtt pod (~20s of
Zigbee downtime, replicas: 1 + Recreate). Incidental benefit: the restart
resets Z2M's in-memory configure attempt counter, giving the Philips
devices that are interviewed but not yet bound (both RDM001 Treppenhaus
switches, the RWL022, and Sarah) three fresh configure attempts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round-3 review on #444. Both suggestions were optional; both were cheap to
close properly.
`.network_key.key?` only shielded the leaf selector. If the backup's TOP
level were not an object, `.network_key` itself raises before `?` applies.
Verified in the image with jq 1.7.1:
[] (.network_key.key? // "") -> rc=5 Cannot index array ...
"a string" (.network_key.key? // "") -> rc=5 Cannot index string ...
[] (.network_key?.key? // "") -> rc=0 empty
"a string" (.network_key?.key? // "") -> rc=0 empty
So `?` now sits on both selectors. Not a realistic shape for an Open
Coordinator Backup export and it failed closed either way, but it was the
last path that produced jq's raw error rather than a tailored FATAL line.
Full matrix against the rendered script, on the alpine-jq:2026-07-30 tag:
real backup -> exit 0, hash 27c5c77a945bcecc (matches live)
non-hex key -> FATAL: contains non-hex characters
4-char key -> FATAL: length 4, expected 32 hex chars
no network_key -> FATAL: .network_key.key absent
network_key non-object -> FATAL: .network_key.key absent
network_key number -> FATAL: .network_key.key absent
top-level array -> FATAL: .network_key.key absent
top-level string -> FATAL: .network_key.key absent
device link_key in doc -> correctly ignored; network_key used
On the registry suggestion: re-verified the tag pulls immediately before
merge — jq 1.7.1, alpine 3.21.7. It was published by pipeline 3060, the
push build for this branch. No build ran for the later commits because they
touch only kubernetes/apps/**, which correctly does not match the build
step's when.path; the image content is unchanged by them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round-2 review on #444.
The 🟡 identified a real gap, though not via the input shape it described.
Verified against jq 1.7.1 in the actual image:
{"devices":[]} -> "null", rc=0 (absent path is LENIENT)
{"network_key":{}} -> "null", rc=0
{"network_key":null} -> "null", rc=0
{"network_key":"abc"} -> rc=5, Cannot index string with string "key"
{"network_key":123} -> rc=5, Cannot index number with string "key"
So a missing network_key was already handled correctly and did produce the
intended FATAL line. What was NOT handled is a network_key that exists but
is not an object: plain `.key` exits 5 there, and under `set -e` that
aborts before the checks run, surfacing jq's raw error instead. Fixed with
`.network_key.key?`, which yields empty for those shapes and routes them
into the existing absent-key branch.
Now every malformed shape reports the same readable message:
real backup -> exit 0, hash 27c5c77a945bcecc (matches live)
non-hex key -> FATAL: contains non-hex characters
4-char key -> FATAL: length 4, expected 32 hex chars
no network_key -> FATAL: .network_key.key absent
network_key non-object -> FATAL: .network_key.key absent
network_key number -> FATAL: .network_key.key absent
device link_key in doc -> correctly ignored; network_key used
Verification ran against the alpine-jq:2026-07-30 tag itself.
On the 🔴: the premise does not hold — that tag already exists and is
pullable, verified twice by pulling it. The build step fires on branch
pushes as well as master, so pipeline 3060 published it when this branch
was pushed; the reviewer could not query the registry to see that. The
underlying caution is still worth recording, so the manifest now documents
that TAG is derived from `date` AT RUN TIME, that a date must never be
written here before a build has produced it, and that a later master build
publishes its own tag without deleting this one — so this keeps resolving,
though the image then came from a branch build rather than master. Also
recorded why it matters: Flux reconciles on a 10m poll with no ordering
against kaniko, and replicas: 1 + Recreate means an unpullable init image
would leave the sole coordinator in ImagePullBackOff.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review point on #444. The reviewer cited a precedent I had missed:
ci-ansible:2026-04-29 in kubernetes/infrastructure/netbird-backup/
cronjob.yaml. Verified — for self-built images referenced by tag rather
than digest, this repo already uses DATED tags, with :latest used nowhere.
The argument holds independently of convention: with :latest, rebuilding
this base for some unrelated consumer silently changes what the seed-data
init container runs on its next pod restart, with no diff here to review.
That container renders the Zigbee network key, so "what exactly does it
run" should be visible in the manifest. A dated tag makes a change to it a
deliberate, reviewable act.
This does give up automatic pickup of rebuilds, which is the tradeoff the
previous commit chose the other way round. Given the base is thin and
rarely changes, and the consumer is security-sensitive, reviewability wins
— and it stays consistent with the owner directive that our own images do
not need digest pinning, since a dated tag is not a digest.
Verified gitea.renner.uno/gitea_admin/alpine-jq:2026-07-30 pulls and
carries jq 1.7.1, so the reference resolves before merge rather than after.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner directives, both applied here:
- "z2m-seed is a shit name for the image — name it for what it is, not for
how it is used." Renamed to alpine-jq. Nothing in it is
zigbee-specific; it is alpine plus jq, useful to anything that needs to
read JSON from a shell script. The Containerfile now describes it that
way, with the zigbee2mqtt case recorded as its first consumer rather
than its identity.
- "You do not need to pin the digest for our own images." Referenced by
tag. This also collapses what was going to be two PRs into one, since
the digest no longer has to exist before the Deployment can name it.
The seed-data init container now uses that image instead of
`apk add --no-cache jq` on every pod start.
Two consequences of dropping digest pinning, both good:
- The whole --image-name-with-digest-file / "PIN THIS:" apparatus is gone
from the pipeline. That machinery existed only to hand a digest to a
follow-up commit, and it is what broke every pipeline in the repo (#443)
via an unquoted ": " in a YAML scalar. Less surface, same outcome.
- :latest means Kubernetes defaults imagePullPolicy to Always, so a
rebuilt image is picked up without editing the manifest.
Kept the fail-loudly validation, since a wrong network_key costs the whole
Zigbee network: explicit checks for an absent key (jq prints "null" for a
missing path, so that is checked separately rather than being reported as
merely "non-hex"), non-hex characters, and a length other than 32.
Verified by running the manifest's RENDERED script, against the real
coordinator backup and synthetic fixtures. The alpine-jq tag does not
exist until this merges and builds, so the run used the existing image of
identical content (alpine 3.21.7 + jq 1.7.1, verified present):
real backup -> exit 0, network_key line byte-identical to the
live value (sha256 27c5c77a945bcecc)
non-hex key -> exit 1, "contains non-hex characters"
4-char key -> exit 1, "length 4, expected 32 hex chars"
absent network_key -> exit 1, ".network_key.key absent from ..."
device link_key also
present in the doc -> correctly ignored; the network_key is used
Validated with `woodpecker lint` (not just `yq`, which is what let #443
through), plus kustomize build and a server-side dry run. No stale
z2m-seed references remain.
NOTE: the now-orphaned z2m-seed package is still in the registry and
should be deleted; that needs package-delete rights this account does not
have.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
URGENT: this restores CI for the WHOLE repo. My commit 66dfd56b (merged
via #442) left an unquoted YAML scalar containing ": ", so Woodpecker
failed to compile the config:
yaml: construct errors: line 152: cannot unmarshal
'map[echo "PIN THIS:gitea.renner.uno/gitea_admin/z2m-seed@$DIGEST"]'
of type map[string]interface {} into a string value
Because a compile error rejects the entire pipeline set, every pipeline on
master has errored since that merge — not only build-ci-images. Confirmed
from pipeline history: 3048 (the push that introduced it) is the first
failure, and 3049-3053 follow, including an unrelated merge (#440) and the
alert-monitor cron.
Fixed by quoting the command as a single YAML scalar.
Why my pre-merge validation missed it: I checked with `yq '.' >/dev/null`,
which passed because the unquoted form IS valid YAML — it just parses as a
map instead of a string. Parsing successfully is not the same as matching
the schema. `woodpecker lint` catches it immediately and is now noted in a
comment at the line, along with the specific failure mode, so the trap is
not reinstated.
All 17 .woodpecker/*.yml files now lint clean apart from pre-existing
"Additional property timeout is not allowed" warnings in four deploy-*
files, which are untouched by this change and are warnings rather than
errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review point on #442: kaniko writes one line per --destination to
--image-name-with-digest-file, so with both :$TAG and :latest the file
holds two entries carrying the same digest, and "PIN THIS:" would print
two near-identical lines for whoever does step 2 to choose between.
Rather than just comment on the ambiguity, remove it: reduce to the bare
digest, dedupe, and print one unambiguous copy-pasteable reference. The
multi-line behaviour is documented inline so a future edit does not
reintroduce it.
Verified the reduction against a two-line fixture matching kaniko's
format:
gitea.renner.uno/gitea_admin/z2m-seed:2026-07-30@sha256:abc123
gitea.renner.uno/gitea_admin/z2m-seed:latest@sha256:abc123
-> PIN THIS: gitea.renner.uno/gitea_admin/z2m-seed@sha256:abc123
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses the round-3 review (commit 39616401), which I failed to reply to
before pushing the next change.
Add an unconditional heartbeat logging the outstanding PENDING/FAILED
count. The main loop is purely event-driven, so if the
"Received payload: clusterID=" format ever changes (herdsman upgrade, log
format change) it would match nothing forever while still logging
"following <file>" and looking healthy. The per-frame recount cannot
detect that either, because it only runs when a frame is matched. The
heartbeat runs off a timer instead, so a count that never falls while
devices are demonstrably in use is a visible signal that the matcher
broke.
Document, in the ConfigMap itself rather than only in the PR body, that
the two log_* keys are seed-only and do not reach a running Z2M whose PVC
is already seeded. Both were therefore also applied to the live
configuration.yaml via bridge/request/options; verified they persist there
(lines 29-31 of the on-PVC file) so they survive a restart. Also noted
that log_symlink_current only takes effect on a Z2M restart, since the
symlink is created when the log directory is opened — until then the
sidecar waits and says so.
Not changed: resources.limits.cpu is still unset on the sidecar. The
review flagged it as consistent with the sibling containers in this file
and asked only whether it was intentional — it is. A CPU limit on a
mostly-idle process whose value is reacting inside a device's short wake
window buys nothing and risks throttling at exactly the wrong moment; the
memory limit already bounds the real risk.
Verified in eclipse-mosquitto:2.0.22 as uid 1883 with HEARTBEAT=5: the
heartbeat interleaves with frame handling and the simulated Z2M restart
without disturbing either.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses the blocking review point on #441, which was correct.
`tail -F` follows a PATHNAME: it re-opens that name when the inode behind
it changes, but it can never notice that Z2M has started writing to a
different directory. Z2M creates log/<start-timestamp>/log.log fresh on
every start, so tailing the newest directory breaks as soon as Z2M
restarts — the old file merely goes quiet, so the pipe never ends, the
outer loop never re-resolves, "following <file>" still shows the stale
path, and the sidecar looks healthy while never seeing another frame.
Worse than the review assumed: this is not limited to the planned
"interview a batch, then restart Z2M once" step. The Z2M *container* has
restarted 38 times on its own from DNS flaps, each time into a new log
directory, while the pod (and so the sidecar) kept running.
Fixed at the source rather than with a polling watchdog: set
advanced.log_symlink_current: true so Z2M maintains log/current -> the
active directory, and follow that stable path. Verified that BusyBox
`tail -F` in eclipse-mosquitto:2.0.22 does follow a symlink repointed to a
new target mid-follow.
No fallback to the newest directory, deliberately: falling back could
silently tail a frozen file, which is the failure being fixed. A missing
symlink is instead reported loudly and repeatedly while waiting.
Also from this round:
- Short-circuit while nothing needs interviewing. The namespaced debug log
covers all of zh:controller, so every device on the network produces
frames; without this the script forked awk over database.db for each of
~30 devices' traffic forever. Recounted at most once a minute, so a
device regressing to PENDING is still picked up.
- Bound mosquitto_pub with `timeout`, so a hanging TCP connect cannot
stall the single-threaded read loop.
- Wire MQTT_HOST and MQTT_USER explicitly from the Deployment, like
BASE_TOPIC, instead of relying on the script's defaults.
- imagePullPolicy: IfNotPresent, matching the sibling container.
Two bugs found by testing this, not by inspection:
- `mosquitto_pub -W` does not exist (it is a mosquitto_sub flag). Adding
it made every publish fail on a usage error. Removed; `timeout` alone
bounds the call. Same class as the earlier `grep --line-buffered`
mistake, and again only visible by running it.
- `grep -c` prints "0" AND exits 1 when nothing matches, so
`grep -c ... || echo 0` emitted two lines ("0\n0") and every `-eq` test
died with "bad number". This surfaced only once no devices needed an
interview — the steady end state this feature exists to reach.
Verified in eclipse-mosquitto:2.0.22 as uid 1883, simulating Z2M's layout
and a restart (new directory + repointed symlink):
frame before restart -> interview requested
symlink repointed to a new directory
frame after restart -> interview requested <- the reviewed failure case
zero-pending fixture -> "no devices need an interview; idling", no errors
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 1 of 2 toward removing `apk add` from the zigbee2mqtt seed-data init
container, which currently installs jq on every pod start. Owner
directive: baking tools into a layered image is fine, adding them at
runtime is not.
This commit only adds the image and its build step; nothing consumes it
yet, so it is inert on merge. The Deployment cannot be switched in the
same commit because runtime images in this repo are pinned by DIGEST (as
the secret-rotator CronJobs are), and the digest does not exist until
this has built and pushed. The build therefore writes it out via kaniko's
--image-name-with-digest-file and echoes it as "PIN THIS:", so the
follow-up commit can copy the reference straight from the build log
without a separate registry query.
Why a real JSON parser rather than busybox sed/awk: "key" occurs 12 times
in the actual backup — once per device with an APS link_key — and the
FIRST occurrence is a device key, because "devices" sorts before
"network_key". An extraction that is not correctly anchored yields a
device link key, herdsman then forms a NEW network, and every device
drops off. This was reproduced while prototyping the awk approach, which
is what makes jq worth an image here rather than clever one-liners in the
manifest.
Follows the existing conventions: Containerfile alongside the other
ci/images entries, base pinned by digest to the same alpine:3.21 the
ci-k8s image uses, kaniko build gated on its own path so unrelated pushes
do not rebuild it, and date + latest tags pushed as the other images do.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner directive: `apk add` at container start is not acceptable.
Replace alpine:3.21 + `apk add mosquitto-clients` with the broker image
eclipse-mosquitto:2.0.22, used purely for its bundled mosquitto_pub. No
packages are installed at runtime, so the sidecar no longer depends on
network or package-index availability at start.
Every tool the script needs was verified present in that exact image
rather than assumed: sh, tail (including -F, which BusyBox does support),
awk with match()/RSTART/RLENGTH, cat, date, mkdir, mosquitto_pub. The awk
extraction was re-run there and resolves a PENDING device, skips a
SUCCESSFUL one, and skips an unknown address.
Pinned WITHOUT a Flux $imagepolicy marker on purpose: this sidecar is
temporary and must not drag the broker's image-automation policy with it.
Now genuinely non-root (1883/1883, the image's own mosquitto user). The
previous commit justified running as root because `apk add` needed it;
with that gone the only remaining requirement is reading /app/data, and
those paths are world-readable (dirs 0755, files 0644 — verified against
the live PVC), so runAsNonRoot is viable. Cooldown stamps moved to an
explicit emptyDir at /state rather than the image's /tmp, so they stay
writable as non-root without relying on the container filesystem.
Re-verified end-to-end in this image as uid 1883 with no package install:
running as: uid=1883(mosquitto) gid=1883(mosquitto)
interview-watch: 1 device(s) currently need an interview
interview-watch: following /app/data/log/.../log.log
interview-watch: frame from 0xdeadbeefdeadbeef (nwk=65001 cluster=6) ...
interview-watch: interview requested for 0xdeadbeefdeadbeef
with the negative paths also holding: a frame from an already-SUCCESSFUL
device is ignored, and a malformed address neither fires nor crashes.
NOTE: the pre-existing seed-data init container still runs
`apk add --no-cache jq` (line 49). Untouched here as it is out of scope
for this PR, but it violates the same directive and is raised separately.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second review round on #441. The first point was a real blocking defect
that would have shipped silently.
BusyBox grep does not support --line-buffered. Confirmed by running it:
$ grep --line-buffered -oE '...' <<< 'Received payload: ...'
grep: unrecognized option: line-buffered
GREP_EXIT=2
The sidecar runs alpine:3.21 and `apk add mosquitto-clients` does not
pull GNU grep, so the `tail -F | grep --line-buffered | while read` pipe
would have produced zero lines forever while the container still logged
"following <file>" and appeared healthy — a silent no-op of the entire
feature, undetectable from the diff.
Fixed by removing the grep stage rather than installing GNU grep: match
in the read loop with a `case` glob and extract the fields with parameter
expansion. `tail` writes per line and `read` consumes per line, so there
is no buffering question and no extra package. Added a numeric guard on
the extracted address so a future log-format change cannot feed garbage
downstream.
Also from this round:
- Write the cooldown stamp only on a SUCCESSFUL publish. Stamping before
the result was known burned a device's wake window whenever
mosquitto_pub failed, then refused to retry for the full 300s despite
no interview having been requested.
- Derive the request topic from BASE_TOPIC (env, defaulting to
zigbee2mqtt) instead of hardcoding it, so it cannot silently drift from
mqtt.base_topic. Noted that the MQTT ACL is scoped to zigbee2mqtt/# and
would need changing too.
- Harden the sidecar with allowPrivilegeEscalation: false and
capabilities drop ALL. Deliberately no runAsNonRoot: apk add needs
root, and Z2M writes database.db and the log dir as root, so a non-root
uid could not read either input.
- Runbook callout said "Step 2" where it meant item 2 within Step 5.
Verified end-to-end in an alpine:3.21 pod against this script (the
earlier "verified live" evidence came from the session prototype, which
resolved friendly names and is not this code — see PR discussion):
counts only PENDING devices, fires on a frame from a PENDING device with
mosquitto_pub returning success, ignores a frame from an
already-SUCCESSFUL device, and survives a malformed address without
firing or crashing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review points from review-bot on #441.
Stay resident instead of exiting on log-stream end. Previously the
sidecar exit 1'd so the kubelet would restart it and re-resolve the log
file, but that re-ran `apk add mosquitto-clients` over the network every
time and, worse, incurred kubelet crash-loop backoff — which climbs to
5 minutes unless the container stays up >=10 min. Z2M restarts and log
rotations land closer together than that (notably during the "interview
a batch, then restart Z2M once" workflow), so backoff would have kicked
in exactly when a device's short wake window needs a prompt reaction.
The log file is now re-resolved in an outer loop, which also drops the
post-startup network dependency. Chosen over baking a custom image
because it removes the restart entirely rather than making restarts
cheaper.
Match ieeeAddr/nwkAddr independently rather than with one combined sed
pattern. JSON does not guarantee key order, so the old pattern's
assumption that "ieeeAddr" precedes "nwkAddr" held only by virtue of
herdsman's current serialization. Had that changed, the regex would
capture nothing and a device would silently never be picked up — no
error, no log. Now extracted per-field with awk match(), with the
rationale recorded in a comment. Verified against the live database.db
under busybox awk (the sidecar runs alpine): resolves both still-pending
devices, and correctly returns nothing for one that has since been
interviewed.
Flag Step 5 of the migration runbook as automated, so operators do not
hand-chase devices the sidecar already handles. Includes the two
caveats that cost real debugging time: interviewing does not bind
manuSpecificPhilips (needs one Z2M restart), and a device that never
transmits is offline rather than un-interviewed — check lastSeen before
hunting a Z2M-side fault, because a flat battery presents identically.
Not changed: mosquitto_pub takes a password only via argv (-P, or an -L
URL) — it has no --pw-file or env option, verified against the client in
the cluster — so the 🟢 nit cannot be resolved with this client. Not a
cross-container exposure, as the pod does not set shareProcessNamespace;
noted in a comment at the call site.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ZHA->Z2M cutover adopted the existing Zigbee network, so devices
stayed joined but database.db started empty — every device needs an
interview before Z2M knows its modelId and can match a converter.
Mains routers interviewed immediately. 16 battery end devices did not,
and they do not self-heal: zigbee-herdsman only retries an interview on
a ZDO device announce, which sleepy devices essentially never emit. They
sat PENDING for six days across 38 process restarts, exposing only a
generic "Automatically generated definition" (battery/linkquality)
instead of action/contact/moisture entities.
A device that transmits is awake, which is exactly when an interview can
succeed. The sidecar watches Z2M's log for the zh:controller frame-receipt
line and fires a forced interview for any device still PENDING/FAILED.
Verified in practice: interviews completed in 11-34s, including
Xiaomi/Aqara sensors, and two fired with no human interaction at all
(a periodic OTA check and an illuminance report).
Implementation notes:
- Sidecar, not a separate Deployment: it reads Z2M's log file and
database.db, and the PVC is RWO/single-writer. Mounted read-only.
- Signal comes from the log file, not MQTT: an un-interviewed Xiaomi
device publishes nothing at all (its manufacturer-specific attributes
cannot be mapped by the auto-generated definition), and
bridge/logging carries only info-level messages.
- Needs advanced.log_namespaced_levels {zh:controller: debug}. Namespaced
rather than global debug on purpose: global adds the unpi:parser
byte-level firehose (~90 lines/min measured) vs ~6 lines/min here.
- database.db is newline-delimited JSON despite the extension, so the
nwk->IEEE lookup needs no jq and is re-read per frame, which keeps it
correct across network-address changes and drops devices as they pass.
Interviewing is only half the job: Philips devices also need configure to
bind manuSpecificPhilips (0xFC00), and that cluster is registered only on
Z2M's start event, so a device interviewed mid-run stays unbound until the
next restart. One restart after a batch of interviews completes the binds.
Both the sidecar and the namespaced log level are temporary — remove them
once every device is interviewed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
claude-botandClaude Opus 5
k.renner
issue-fix-bot
security-bot