fix/wordpress-php85-php2shell
4354
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  claude-botandClaude Opus 5
|
9cc20251fd |
fix(edge): retire the legacy compose project before any role starts a container
Review on #474 found two blocking defects, both in exactly the place the --check verification is blind: command and shell tasks are skipped, not simulated, under check mode. So "verified with --check --diff" covered everything except the two things that would actually have broken. 1. Ordering: a real run would have aborted. The legacy-project teardown lived in power-meter-sml, which the play runs last. zwave-gateway runs before it and does `docker compose up -d` for a container named zwave-js -- while the old shared project still owns that name. Docker enforces container names globally, not per project, so the command fails with "Conflict: container name already in use". That is not the missing-USB-device string the task tolerates, so the task fails and the play aborts before ever reaching the cleanup. Moved to pre_tasks, which is its correct home anyway: it is a migration step for both services, and putting it inside either one meant whichever ran first hit the conflict. The play now also declares edge_base_dir and edge_legacy_compose in its own vars rather than borrowing them from a role's defaults, since pre_tasks must not depend on role default resolution. And it removes the now-orphaned zwavejs.env left at the old shared project root. 2. Dangling handler. The libeatmydata task still notified "Restart edge compose services", a handler that was deleted with the old role. Masked today only because `creates:` makes the task report ok -- but the role's own comment says to delete that .so when the Alpine base bumps, and the next time it does, Ansible would fatally error with "handler not found". Now notifies "Restart zwave-js". Swept every notify in the new roles against every defined handler; that was the only unresolved one. Also from review: kubernetes/bootstrap.sh referenced the deleted role. Repointed to ansible/docs/edge-host.md. Re-verified: ok=66 changed=25. The teardown moved from the power-meter-sml group into pre_tasks and gained the orphaned-env removal, so the count goes 5 + 5 + 14 -> 5 pre-existing + 6 migration + 14 header repointing. Nothing else moved. The limit is worth stating plainly: this re-run still cannot prove the teardown works, for the same reason it missed the bug. The first real apply is the test. Co-Authored-By: Claude Bot <claude-bot@renner.uno> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
2d9c3ba213 |
refactor(edge): split the edge-host role into one role per concern
edge-host was a single role doing eight unrelated jobs: flash-wear
mitigation, log shipping, VLAN networking, an MQTT bridge, a CAN interface,
the heating CAN services, a Z-Wave gateway and a power-meter reader.
That is not just untidy. It is what blocked a second thin client from
joining as an NTP peer: the host-generic work (journald to RAM, Alloy, the
VLAN sub-interface) could not be applied without also handing the box a
Z-Wave radio and a CAN bus it does not have. The role bound its services to
specific USB device paths, so a second host would have tried to start
containers for hardware that is not there.
Now:
edge-flash-wear journald -> RAM, noatime/lazytime, TRIM, eMMC health
edge-log-shipper Alloy: journal -> Loki
edge-vlan tagged VLAN sub-interface
mqtt-bridge loopback mosquitto -> cluster broker over TLS
socketcan can0 only
heating-can open3e (Vitocal/VX3) + e3oncan (E380 meter)
zwave-gateway zwave-js-ui, own compose project
power-meter-sml sml2mqtt, own compose project
The E380 reader lives with open3e rather than in its own role: it is part
of the same heating installation -- it feeds the heat pump's own energy
management -- not a separate system that happens to share a bus. can0 does
stay separate, because an interface is not the same concern as the services
consuming it.
Each role installs its own packages, so it stands alone. An edge host with
no peripherals takes the first five and none of the rest.
docker-daemon.json moves to docker-install with its own docker_log_driver
variable and Restart docker handler, since configuring the docker daemon was
never an edge-host concern. Handlers split ten ways; defaults split per
role; all sixteen templates and files moved with git mv.
Two deliberate behaviour changes on the node, both accepted:
1. Compose split. zwave-js and sml2mqtt shared a compose file, so touching
either recreated both. They are now separate projects in their own
directories. container_name is fixed, so the old project must release the
names first -- a guarded one-time task downs it and removes the file.
First apply costs a brief down/up on both. Data paths are unchanged; the
zwave store is not migrated.
2. Header repointing. Templates and scripts carried "Managed by
roles/edge-host" headers, which render onto the host -- they would have
sent an operator to a role that no longer exists. Updating them changes
file content, so it restarts the services that consume those files.
Verified by --check --diff against the live host. 24 changed items, and
every one is accounted for:
5 pre-existing, unchanged from the baseline before this work:
common timesyncd; open3e venv; E3onCAN venv; and the open3e /
e3oncan restart handlers. The venv tasks install from a git URL,
which pip cannot verify, so they report changed on every run --
a separate defect, not introduced here.
5 the compose split: zwave env + compose and sml compose to their
new paths, plus the two legacy-project retirement tasks.
14 the header repointing: journald conf, eMMC health script, alloy
config, mosquitto bridge config, 80-can0.network, open3e overlay /
env / unit, e3oncan unit, zwave backup script + env, sml2mqtt
config -- and the resulting restarts of journald, alloy, mosquitto,
open3e and e3oncan.
Nothing changed outside those three groups.
One consequence check mode cannot show: 80-can0.network.j2 changed, so the
"Reload networkd" handler fires on a real run and reconfigures can0,
briefly dropping the bus under open3e and e3oncan. It is absent from the
check-mode output only because shell handlers are skipped there. The VLAN
templates did NOT change, so the riskier reconfigure of the parent uplink
on this headless host is not triggered.
Also fixed in passing: the keepalived-ntp guard told operators to recover
with `--limit <host> --tags vlan40`, and referred to the edge-host role.
Both corrected. The role README becomes ansible/docs/edge-host.md -- it
documents the host, not any single role now.
Co-Authored-By: Claude Bot <claude-bot@renner.uno>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
claude-botandClaude Opus 5
|
07faf0ae69 |
fix(edge-host-iso): carry the bootstrap fixes into the ISO side too
Review on #474 caught that the previous commit fixed the procedure in ansible/roles/edge-host/README.md and left the same two defects in the two places an operator actually reads them first. That happened because my grep for the stale patterns ran with the working directory set to ansible/, so `images/edge-host-iso/README.md` was a path that did not exist. grep returned nothing and I read that as clean. A false negative from a bad path, not a clean sweep. Fixed now, verified from the repository root: airootfs/usr/local/bin/edge-install Printed the superseded two-command sequence at the console immediately after imaging -- the very first instructions anyone sees, telling them to run common.yml with -l edge_host. Now prints the single unrestricted run, plus the two steps that precede it (confirm the host resolves by name, add it to [edge_host]). README.md Described the image as being for `edge-1.srv.renner.uno`, a name that does not resolve, and repeated the same two-command bootstrap. Both replaced. This file is the link target of step 2 in the ansible-side README, so following the procedure end to end landed straight back on the defect #474 claims to close. Also corrected while there: the ISO README called the target "the bare-metal N100". edge-1 is a Wyse 3040 (Atom x5-Z8350), and the installer text further down already says it "covers the Wyse 3040". Rewritten to describe the image as not tied to one box or model, since the installer autodetects the target disk -- which is what makes it usable for the second thin client. Also from review: a comment on why `become: true` is set when the connection is already root. Not taken -- ignore_unreachable, which common.yml sets. The review's own reasoning applies: Ansible already skips an unreachable host and continues with the others, so it adds nothing here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
e288ebe39d |
fix(edge-host): make the bootstrap self-contained and the runbook followable
The documented base-install procedure had three defects, two of which are
standing house rules and one of which simply does not work.
1. It required a DHCP reservation.
"Add the DHCP reservation/DNS entry for <host> on the UDM" was step 3 --
a reservation as a documented precondition. Reservations are off the
table here. The UDM registers DHCP leases in DNS on its own, so the
record appears without one, and the host is identified by name
everywhere: inventory, plays, README. Its address is never recorded.
2. It required --limit, because the plays were split wrong.
Step 4 was:
ansible-playbook plays/common.yml -l edge_host
ansible-playbook plays/install-edge-host.yml
The --limit was load-bearing, not sloppiness: common.yml is
`hosts: all:!...`, so running it unrestricted while bootstrapping a
single new box would reconfigure the entire fleet. The split left no
other option. But --limit narrows the target list rather than staging
anything, and leaves every other host outside convergence with no report
that they were skipped.
Fixed structurally rather than by deleting the flag: install-edge-host.yml
now carries `certificates` and `common` itself -- the parts of common.yml
that apply to an edge host -- so the bootstrap is one unrestricted run
over the edge_host group. No second command, no limit.
Not carried over: qemu-guest-agent (these are bare metal, not VMs) and
monitoring-agent (Debian-oriented; edge hosts ship logs via edge-alloy).
common.yml's loki/prometheus vars go with monitoring-agent and are not
needed -- verified that the common role does not reference them.
3. It named a host that does not exist.
The README said `edge-1.srv.renner.uno` in both the base-install steps
and the Z-Wave cutover checklist. The host is `edge-1.iot.renner.uno`;
the .srv name does not resolve, so anyone following the runbook hits a
dead name at step 3.
Verified idempotent against the live host before committing:
`ansible-playbook plays/install-edge-host.yml --check --diff` over the whole
edge_host group -> ok=53 changed=5 failed=0, with all five changes inside
the edge-host role (open3e venv, E3onCAN checkout/venv), which was already
in this play. certificates and common contributed no changes at all.
Also checked the common role branches only on Debian/RedHat os_family, so
its OS-specific tasks skip cleanly on Arch.
Prompted by preparing a second Wyse 3040 as an NTP peer -- this is the
procedure that would have been followed to build it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
 k.renner
|
5b03d6b7af |
Merge pull request 'refactor(prometheus): address the PVE nodes by name in scrape targets' (#473) from prometheus-pve-hostnames into master
Reviewed-on: k.renner/homelab#473 |
||
 security-bot
|
343e4845e2 |
chore(image-automation): bump gotenberg/gotenberg:8.35.0
Updates managed by image-automation-controller. |
||
|
security-bot
|
5d56c35017 |
chore(image-automation): bump ghcr.io/home-assistant/home-assistant:2026.8.1
Updates managed by image-automation-controller. |
||
|
claude-botandClaude Opus 5
|
17c12fe13f |
refactor(prometheus): address the PVE nodes by name in scrape targets
Follow-up to #472, which changed the inventory but left the same three IPv4 literals in the monitoring config and a runbook. Raised in review there. prometheus.yml.j2: - pve job targets -> FQDNs - chrony job targets -> FQDNs, and restructured to one static_config per host so each carries its own hostname label The chrony restructure is the substantive part. The previous form listed all three targets together and recovered the hostname in relabel_configs by regex-matching __address__ against each node's IP: - source_labels: [__address__] regex: '10\.2\.40\.201:.*' target_label: hostname replacement: 'jupiter' That silently coupled the labels to the addressing. Swapping a target for a name would have left every regex unmatched and the hostname label simply absent, with the scrape still succeeding and nothing failing -- a find-and-replace on the addresses alone would have broken the labels quietly. Per-host static_configs remove the coupling entirely. nodes.yml.j2: targets -> FQDNs. Already one entry per host with an explicit hostname label, so nothing else to change. claude-workstation/RUNBOOK.md: the kubeconfig command pointed --server at https://10.2.40.201:6443. That is jupiter, a Proxmox host, not a control plane node -- stale from when k8s ran as VMs on it. The cluster's endpoint is https://k8s.renner.uno:6443 (kubernetes/talos/patches/cluster.yaml), and the name is also what the API server certificate carries as a SAN, so an address there fails TLS verification even when reachable. The documented command was very likely broken as written. Deliberately NOT changed: kubernetes/apps/pve/endpointslice.yaml. An EndpointSlice takes literal addresses; hostnames are not valid in that field, so those three are correct as they are. Note this changes the `instance` label on the pve job from an address to a name, since the relabel chain feeds __address__ into __param_target and on to instance. Anything selecting on the old value needs updating. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
k.renner
|
332950b564 |
Merge pull request 'refactor(inventory): address the PVE nodes by name, not by IPv4 literal' (#472) from inventory-pve-hostnames into master
ci/woodpecker/cron/alert-monitor Pipeline was successful
Reviewed-on: k.renner/homelab#472 |
||
|
claude-botandClaude Opus 5
|
ba369a37b0 |
refactor(inventory): address the PVE nodes by name, not by IPv4 literal
jupiter, neptune and saturn were the last entries in pve_hosts.yml still reached via ansible_host=<IPv4 literal>. Every other host here -- authentik, netbird, proxy, nas-1, grafana, edge-1 -- has always used its FQDN, so these three were the outlier rather than the convention. An address in an inventory is allocator state standing in for identity, and it pins access to the legacy family: the names resolve IPv6-first, which is the primary path on this network. DNS is authoritative for where a host is; the inventory should say which host. The short inventory_hostname is kept deliberately -- plays key on it (ntp_servers[inventory_hostname] in install-ntp-ha.yml, node names in opentofu, the prometheus targets) -- so only ansible_host changes. No claim that this fixes the connection stalls seen while applying the NTP work on 2026-08-07. Those were the IPS reacting to the connection pattern I was generating (a port sweep, then rapid repeated SSH across hosts interleaved with Ansible's own churn), not an address-family problem. This change stands on convention and on not encoding a lease as identity. claude-ws is left alone: its comment documents that obg.renner.uno is split-horizon and resolves to the public proxy from home. That explains why that particular name is unusable, not why a literal is right -- a name that resolves to the wrong target calls for one that resolves correctly. Out of scope here, and I do not know whether such a record exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
k.renner
|
eacc821222 |
Merge pull request 'fix(chrony): allow IPv6 clients -- the allow list was IPv4-only' (#471) from chrony-allow-ipv6 into master
Reviewed-on: k.renner/homelab#471 |
||
|
claude-botandClaude Opus 5
|
24ed2485c9 |
fix(chrony): allow IPv6 clients -- the allow list was IPv4-only
chrony_allowed_networks contained only 10.2.0.0/16, so the rendered
chrony.conf permitted IPv4 clients and nothing else. IPv6 is the primary
address family here: ntp.srv.renner.uno resolves to fd40::123, so an
IPv4-only allow list refuses every client that uses the documented name.
This was live but invisible. The hosts carried `allow fd00::/8` in
chrony.conf for a long time without it ever being in this repository.
Applying the role on 2026-08-07 rendered the template over the live config
and dropped the line on all four NTP servers at once, taking internal NTP
down over IPv6 until it was added here.
Same class of untracked host state as the anycast service address, and the
same failure: config that exists only on the box survives right up until
something re-renders it. Reading the drifted file is not enough -- the rest
of it has to be diffed against what the template would produce.
The outage was not caught by the play, which reported the v6 master as
"serving ntp.srv.renner.uno" throughout. It was caught by querying the
service:
chronyd -Q "server ntp.srv.renner.uno iburst"
-> No suitable source for synchronisation
and after the fix:
chronyd -Q "server ntp.srv.renner.uno iburst"
-> System clock wrong by 0.001999 seconds (ignored)
with a control query against ptbtime1.ptb.de to confirm the method rather
than the target was sound.
fe80::/10 is included as well: a client on the same segment may source its
query from a link-local address, and without it chrony drops the request,
which presents as the server being unreachable rather than as a policy
refusal.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
k.renner
|
6628f6eaef |
Merge pull request 'fix(ntp): link-local VRRP primary, and sequence the migration around DAD' (#470) from ntp-vrrp-linklocal-and-dad-sequencing into master
ci/woodpecker/cron/alert-monitor Pipeline was successful
Reviewed-on: k.renner/homelab#470 |
||
|
claude-botandClaude Opus 5
|
ea439b110d |
fix(ntp): link-local VRRP primary, and sequence the migration around DAD
Applying #469 to edge-1 exposed two defects that only show up on real hardware. 1. The VRRP address was unusable. keepalived took MASTER and assigned fd40::123, but the kernel left it: inet6 fd40::123/128 scope global dadfailed deprecated tentative proto keepalived valid_lft forever preferred_lft 0sec `-nodad` did not take effect -- no nodad flag on the address, and net.ipv6.conf.enp1s0/40.accept_dad = 1 -- so DAD ran and found a real duplicate: jupiter, saturn and neptune still held the address statically. edge-1 believed it was the v6 master while serving nothing. The flag is dropped rather than fixed. It was inherited from the previous design, where every NTP server carried the address at once and duplicate detection had to be suppressed for that to work. Under single-master VRRP the opposite holds: exactly one host should have it, so a duplicate is a genuine fault and detecting it is the point. What that costs is that the migration can no longer be done host by host. A static copy anywhere and a VRRP copy elsewhere cannot coexist, and the dadfailed state is sticky -- the kernel does not retry. install-ntp-ha.yml is therefore split in two: a first play withdraws every static assignment across the whole group, then the second hands the address to keepalived. Deliberately not serial, since it must finish everywhere before any host claims the address. There is a gap between the phases where nobody holds it, bounded by one play's run time; clients retry. The withdrawal tasks move to roles/keepalived-ntp/tasks/withdraw-static.yml for the first play to include. main.yml keeps the detect-and-assert guards, which still run every time and fail loudly if any static assignment survives. 2. keepalived warned: (NTP_VIP6) the first IPv6 VIP address should be link local RFC 5798 gives an IPv6 virtual router a link-local primary. keepalived carried on regardless, but the instance was sourcing VRRPv3 from something the protocol does not expect. fe80::123 is now the first virtual_ipaddress: it mirrors the service address and cannot collide with a NIC's EUI-64 link-local. ntp_vip6 and ntp_vip6_linklocal move to group_vars/ntp_cluster.yml so both plays read one definition rather than duplicating a service address. Verified: `keepalived -t` on edge-1 (keepalived 2.4.1) parses the two-address form clean, and the playbook passes --syntax-check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
k.renner
|
b1c06b9d43 |
Merge pull request 'feat(ntp): hold the IPv6 service address in keepalived, gated on clock health' (#469) from ntp-service-address-health-gated into master
Reviewed-on: k.renner/homelab#469 |
||
|
security-bot
|
436c24a394 |
chore(image-automation): bump ghcr.io/music-assistant/server:2.9.11
Updates managed by image-automation-controller. |
||
|
claude-botandClaude Opus 5
|
a1e3b65cbf |
fix(ntp): verify the ifupdown side unconditionally, not only after a match
Review on #469 found the Debian path still fails open, and was right that
my previous commit message was wrong to claim otherwise.
The whole Debian chain -- detect, withdraw, confirm, assert -- was gated on
`keepalived_ntp_static6_removed.changed`, which is only true when the
lineinfile regex matches exactly, prefix length included. The hook it
targets is hand-written and was never in version control, so any deviation
(a different mask, unusual whitespace, a second assignment elsewhere)
leaves .changed false and silently skips every following task. The static
assignment then coexists with the VRRP-managed one, which is the
double-assignment failure this PR exists to prevent -- just relocated from
edge-1 to a PVE host.
Same fail-open class as the Arch grep fixed in
|
||
|
claude-botandClaude Opus 5
|
5e01e2f1b5 |
fix(ntp): detect a static networkd assignment at any prefix length
The Arch guard added in the previous commit grepped for the literal
"fd40::123/128" -- the full address/prefixlen from ntp_vip6. A hand-written
[Address] using any other mask (/64, say) would not have matched, the
assert would have passed, and keepalived would have been handed an address
networkd was still assigning.
That fails open, which is the wrong direction for this particular check.
It is detect-only: there is no delete afterwards whose result gets
re-verified, so a miss is silent, and the outcome is the dual-ownership
state the guard exists to prevent.
The Debian path does not share the problem. `ip -6 addr show ... to
<prefix>` matches by range regardless of the prefix length the address was
actually assigned with, and its `ip addr del` -- which IS prefix-sensitive
-- is re-verified by a following assert, so a mismatch there fails loudly.
Now matches the bare address, anchored with \b so it cannot instead hit a
longer address sharing the prefix. Verified both directions:
grep -rlE 'fd40::123\b' /etc/systemd/network/ on edge-1
-> /etc/systemd/network/10-enp1s0.40.network (still detected)
grep -cE 'fd40::123\b' against 'Address=fd40::1234/64'
-> 0 (no false positive)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
claude-botandClaude Opus 5
|
40ef53fa67 |
fix(ntp): assert the static service address is really gone before handing over
Second review round on #469 found two ways the migration could silently leave the address outside VRRP control -- both of which would defeat the health gating while the play reported success. 1. The one-shot migration had no verification. Gating the withdrawal on `keepalived_ntp_static6_removed.changed` is what makes it idempotent, but it also means it gets exactly one attempt and never retries. Combined with `failed_when: false` on the delete, a mismatch would pass silently: `ip addr del` matches on address AND prefix length, so a host whose static assignment used a different mask or interface than ntp_vip6 / keepalived_ntp_interface would see the delete no-op, keep the address permanently outside VRRP control, and still report success. The idempotency fix from the previous commit created this: making the task run once removed the retry that would otherwise have masked it. Now asserted explicitly after the delete, with a message naming the likely cause and the commands to inspect it. 2. edge-1 -- the host the incident actually happened on -- was not covered. The removal tasks are gated to Debian, so they never touch edge-1 (Arch). Its static [Address] block belongs to the edge-host role, and install-edge-host.yml is a separate play with no ordering relationship to this one. Nothing guaranteed it had been re-applied. edge_host already sets ntp_keepalived_enabled: true, so the next run of install-ntp-ha.yml gives edge-1 the NTP_VIP6 instance immediately. With the [Address] block still present it would hold fd40::123 statically AND run VRRP for it: keepalived would believe it had withdrawn the address while networkd kept it assigned, and the host would go on answering NTP with a clock check_ntp had already rejected. That is the failure of 2026-08-04, on the same host. This is not hypothetical -- edge-1 still has the block right now: grep -rl fd40::123/128 /etc/systemd/network/ -> /etc/systemd/network/10-enp1s0.40.network The role now detects this and fails fast, naming the play to run first, rather than proceeding into a split-ownership state. Also documents why the post-deploy presence check is meaningful: presence alone cannot distinguish a VRRP-held address from a static leftover, and only means what it claims because the role asserts no static assignment survives on either distro. Not taken: narrowing serial: 1 so routine runs are not serialised through the chrony settle pause. Real cost, but correctness of the address handover comes first at four hosts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
fbf5c4dcb8 |
fix(ntp): make the static-address withdrawal run once, not every play
Review on #469 caught a real defect. The withdrawal task used ansible.builtin.command with no changed_when, so it reported changed and fired `notify: restart keepalived` on every run where the address was present. That is fine exactly once. After this lands, fd40::123 is legitimately present on whichever host is the elected v6 master, put there by keepalived -- and an ungated delete cannot tell that apart from a static assignment, because the address looks identical either way. So every later run of this play (adding a host, changing a priority, plain re-convergence) would have deleted the master's address and restarted keepalived, dropping the service address and forcing a re-election each time. That is precisely the flapping this change exists to prevent. Both the check and the delete are now gated on the ifupdown hook having just been removed, so they run once during the migration off static config and never again. Steady-state runs leave the master's address alone. Also from review: - failed_when: false on the delete. The desired end state is "address absent". If check_ntp fails in the window between the check and the delete, keepalived withdraws the address first and `ip addr del` errors on an address that is already gone -- the outcome the task wants, and no reason to abort the play for that host. - The play's post-deploy verification only ever checked the IPv4 VIP. The v6 address is the path clients actually use, so a broken v6 rollout -- address never claimed, or -nodad not behaving as expected on a given kernel -- would not have surfaced in the play at all. It now gets the same check, and the message points at check_ntp and the health log for the case where no host holds it. Not taken: the suggestion to narrow serial: 1 so it does not also serialise chrony. With four hosts it costs nothing, and scoping it per-role would mean splitting the play. Worth revisiting if the cluster grows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
112299da49 |
fix(ntp): remove the static service address so keepalived can own it
Without this the health gating is defeated on three of the four NTP
servers. fd40::123 was assigned statically on every one of them, by hand
and in two different ways, none of it in version control:
PVE nodes `up ip -6 addr replace fd40::123/128 dev vmbr1.40 nodad`
in /etc/network/interfaces
edge-1 an [Address] block in /etc/systemd/network/
A statically assigned copy does not disappear when keepalived decides the
host is unfit to serve time. Leaving it in place means the host keeps
answering NTP on an address VRRP believes it has withdrawn -- exactly the
failure this whole change exists to prevent, with the added confusion of
keepalived reporting the address as released.
The ifupdown hook is removed here. edge-1 needs nothing: the edge-host
networkd template has never carried the address, so applying that role
drops it.
The live address is deleted as well, because removing the config line alone
does not take effect until the next network reload, and a reload on a PVE
node interrupts guest connectivity including the Talos VM and its Ceph OSD.
Deleting it is safe where keepalived already holds it: the deletion
notifies a keepalived restart, and the elected master re-adds it on the way
back up. Doing it unconditionally avoids having to tell "assigned
statically" from "assigned by keepalived", which the address alone does not
reveal.
The play now runs serial: 1. Withdrawing the address and restarting
keepalived on every NTP server at once would leave the fleet with nobody
holding fd40::123 until the elections settle; serialised, the remaining
members answer throughout.
Verified against the live hosts rather than assumed:
- all three PVE nodes carry the identical hook (jupiter:54, saturn:55,
neptune:56), so one task covers them
- the removal regexp matches exactly one line per host
- `ip -6 addr show dev <if> to <addr>` prints the matching address when
present and nothing at all when absent, so the guard condition is sound
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
claude-botandClaude Opus 5
|
01e843beef |
fix(ntp): apply the v6 service address to every ntp_cluster member
The previous commit set keepalived_ntp_vip6 in group_vars/edge_host, which would have given the v6 VRRP instance to edge-1 alone. The other NTP servers would have rendered no NTP_VIP6 instance at all and stayed silently outside the election -- so the address could never fail over to them, and the health gating would have amounted to "edge-1 withdraws and nobody takes over". Moves it alongside ntp_vip in plays/install-ntp-ha.yml, which is already where the service-wide address lives, and passes it to the role for every host in ntp_cluster. Verified that edge-1 is currently BACKUP and does not hold the IPv4 VIP: a VRRP BACKUP receiving no advertisements promotes itself to MASTER, so the other members are alive and one of them holds it. They are real participants, not retired hosts. Not verified: whether the other members also carry fd40::123 statically, as edge-1 did. They have no DNS records -- jupiter/saturn/neptune resolve only through the *.renner.uno wildcard to the proxy -- so there is no hostname path to them to check. If they do carry it statically, that address must be removed at the same time this lands, or two hosts will hold it outside VRRP control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
2fbce12b0b |
feat(ntp): hold the IPv6 service address in keepalived, gated on clock health
ntp.srv.renner.uno resolves to fd40::123 (IPv6) and to the keepalived VIP
(IPv4, legacy). IPv6 is the primary family here, so the v6 address is the
path clients actually use -- and it was the one with no health gating at
all.
It was assigned statically in /etc/systemd/network/ on edge-1, so networkd
carried it unconditionally from boot. That is how the host answered NTP
queries on 2026-08-04 while its clock was ~16 days in the past: chronyd was
running, udp/123 was listening, the address was up, and nothing anywhere
was checking whether the clock was right. Fixing the keepalived health
check does not help, because keepalived was not holding that address.
This moves it into keepalived as a second VRRP instance tracking the same
check_ntp script, so the address exists only while the host is fit to serve
time.
Three details that are load-bearing rather than stylistic:
- A separate vrrp_instance is mandatory. keepalived requires that "all
addresses in virtual_ipaddress must be of the same family", so the v6
address cannot join NTP_VIP.
- No authentication block. VRRPv3 removed authentication (RFC 5798) and
keepalived always uses version 3 for IPv6 instances.
- track_script uses `weight 0`, overriding the script's -20. A non-zero
weight only subtracts from priority on failure, so the last remaining
host -- having nobody to lose the election to -- would stay MASTER and
keep serving a clock it had already reported as wrong. Weight 0
transitions the instance to FAULT instead, releasing the address
unconditionally. That is the required behaviour: a host unfit to serve
time must stop answering whether or not a replacement exists.
Semantic change, deliberate and worth stating: VRRP is a single-master
unicast VIP, not anycast. Previously every NTP server carried fd40::123 and
clients reached whichever answered neighbour discovery first. Now exactly
one host holds it. For NTP this is the better trade -- query volume makes
load spreading pointless, and on failover keepalived sends unsolicited
Neighbour Advertisements so client neighbour caches update in seconds,
where the static address had no withdrawal signal at all and clients kept
using a dead holder until their own neighbour unreachability detection
expired. True routed anycast with health withdrawal would mean announcing
the /128 via an IGP, which the gateway would have to speak.
The edge-host role is deliberately untouched: its template already has no
[Address] block, so applying it removes the hand-placed address, and
keepalived then adds it back under health control. The service address is
NTP-role configuration and now lives there.
Validated with `keepalived -t` against keepalived 2.4.1 on edge-1 using the
config as it renders for that host; parses clean apart from the pre-existing
auth_pass truncation warning on the IPv4 instance.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
k.renner
|
ea54e558fd |
Merge pull request 'fix(ntp): make the keepalived health check verify the clock, not liveness' (#467) from ntp-health-clock-correctness into master
Reviewed-on: k.renner/homelab#467 |
||
|
k.renner
|
d39115a55d |
Merge pull request 'docs(chrony): correct the root cause recorded for the makestep fix' (#466) from chrony-comment-correction into master
Reviewed-on: k.renner/homelab#466 |
||
|
claude-botandClaude Opus 5
|
601290dcd9 |
fix(ntp): make the keepalived health check verify the clock, not liveness
The NTP health check asked "is chronyd alive", never "is this host safe to
take time from". Those came apart on 2026-08-04.
edge-1's CMOS battery is dead. After a power loss its clock came up ~16
days in the past and, with no makestep directive, chronyd never corrected
it. All three health checks passed continuously for ~18h:
- chronyd was active
- `chronyc tracking` responded
- udp/123 was listening
and `local stratum 10` meant chronyd advertised itself to clients as a
valid reference the whole time. keepalived would have kept the VIP on a
host ready to serve a clock from July.
The sharpest part: check 2 ran `chronyc tracking >/dev/null 2>&1` and
discarded the output. The one command that knew the answer was executed
and its answer thrown away. During the incident that output read:
System time : 1376235.375000000 seconds slow of NTP time
This captures the output and adds two assertions:
- offset above keepalived_ntp_max_offset (default 1.0s) fails, so a host
that knows its clock is wrong drops the VIP
- no upstream source selected in `chronyc sources` fails, because with
`local stratum N` chronyd advertises itself as a usable reference even
when it has never synchronised to anything -- upstream chrony docs are
explicit that the local reference activates immediately in that case
Both predicates were validated against live output from edge-1 rather than
assumed: the offset comparison against the real incident value, and the
`chronyc sources` selected-source column against a real run.
Failing the check costs the configured weight (-20). When the WAN is down
every host fails together and loses the same weight, so relative priority
is unchanged and the VIP stays put -- the local-reference fallback still
works. What this prevents is one host with a broken clock winning.
Scope: this hardens the keepalived path, which is legacy. It does not
address the documented end state in plays/install-ntp-ha.yml, where edge-1
becomes the SOLE NTP server -- there, a failing check means no host serves
time at all, converting "serves the wrong time" into "serves no time".
That needs an architectural answer, not a better health check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
claude-botandClaude Opus 5
|
32d12f93d8 |
docs(chrony): correct the root cause recorded for the makestep fix
#465 landed the makestep directive with the wrong root cause in its
comment. It claimed edge-1 "rebooted with an RTC ~16 days in the past".
That is false, and the real cause matters for what to do next.
edge-1's CMOS battery is dead. After a power loss the RTC came up reading
2018-08-20 -- roughly eight years stale, not 16 days:
PM: RTC time: 09:18:56, date: 2018-08-20
rtc_cmos 00:01: setting system clock to 2018-08-20T09:18:58 UTC
The clock was then reconstructed entirely in software. systemd floored it
forward to its own build date:
systemd[1]: System time advanced to built-in epoch: Fri 2026-06-26 11:27:23 UTC
and it settled at ~2026-07-19, about 16 days behind. So the 16 days is
incidental -- it is just where the flooring landed. There is no 16-day
mechanism, and nothing "drifted". The two real faults are a dead RTC
battery and a chrony config that never steps. #465 fixed the second.
Also collapses the duplicated rationale in chrony.conf.j2 to a pointer at
defaults/main.yml, as suggested in review on #465, so the explanation
lives in exactly one place.
No behaviour change: the makestep directive and its defaults are
untouched. Comments only.
The commit message on
|
||
|
security-bot
|
0fd92f2fa2 |
chore(image-automation): bump ghcr.io/home-assistant/home-assistant:2026.8.0
Updates managed by image-automation-controller. |
||
|
k.renner
|
d41f4870ad |
Merge pull request 'fix(chrony): step the clock at boot so a bad RTC self-corrects' (#465) from worktree-chrony-makestep into master
ci/woodpecker/cron/alert-monitor Pipeline was successful
Reviewed-on: k.renner/homelab#465 |
||
|
claude-botandClaude Opus 5
|
7fcaa4b035 |
fix(chrony): step the clock at boot so a bad RTC self-corrects
chrony.conf.j2 had no makestep directive, so chronyd only ever slewed. A large offset is therefore never closed in practice. On 2026-08-04 edge-1 rebooted with its RTC ~16 days in the past. chronyd was synchronised to ptbtime1 at stratum 2 with a sub-millisecond RMS offset, yet reported "System time: 1376235.375 seconds slow of NTP time" and corrected none of it. With host time at 2026-07-20, every outbound TLS handshake failed x509 NotBefore validation against a cluster certificate issued 2026-07-23: tls: failed to verify certificate: x509: certificate has expired or is not yet valid: current time 2026-07-20T16:15:36+02:00 is before 2026-07-23T06:15:01Z That took out the local mosquitto -> cluster MQTT bridge (all Vitocal 250-A / Vitocharge VX3 / E380 heating telemetry went unavailable in Home Assistant for ~18h) and the Alloy -> Loki log shipper simultaneously. Because the same skew broke log shipping, the outage was invisible to log-based alerting. Recovery required a manual `chronyc makestep`. Adding the directive lets this correct itself at boot instead. The limit keeps steps confined to the first few updates after chronyd starts, so a long-running NTP server is never jumped out from under its workloads mid-flight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
k.renner
|
11bf13883f |
Merge pull request 'ci: validate the GitOps tree and pipeline configs on every PR' (#460) from ci-k8s-validate into master
ci/woodpecker/cron/update-all Pipeline failed
ci/woodpecker/cron/update-audit Pipeline was successful
ci/woodpecker/cron/e2e Pipeline failed
ci/woodpecker/cron/security-scan Pipeline was successful
ci/woodpecker/cron/issue-fix Pipeline was successful
ci/woodpecker/cron/alert-monitor Pipeline was successful
Reviewed-on: k.renner/homelab#460 |
||
|
security-bot
|
4b502daa98 |
chore(image-automation): bump woodpeckerci/woodpecker-server:v3.17.0
Updates managed by image-automation-controller. |
||
|
security-bot
|
83a20bfa80 |
chore(image-automation): bump woodpeckerci/woodpecker-agent:v3.17.0
Updates managed by image-automation-controller. |
||
|
security-bot
|
c1a1a59ecc |
chore(image-automation): bump redis:8.10.0-alpine
Updates managed by image-automation-controller. |
||
|
claude-botandClaude Opus 5
|
39dc23a999 |
ci: document why the shellcheck discovery floor stays at 1
Review flagged that shellcheck's floor (1) is far weaker than kustomize-build's
(50), so it catches total discovery failure but not partial loss.
Correct, and kept as-is with the reasoning written down. The kustomize floor is
loose relative to the tree it guards, which is what lets it catch gross partial
loss without tripping on churn. The shell-script set is small enough that any
floor tight enough to catch partial loss would sit inside normal churn:
deleting one script would fail CI spuriously, and the fix a hurried person
reaches for is lowering the floor — which is how a guard becomes decoration. A
floor that must be edited whenever the tree changes is not a guard, it is a
snapshot with an exit code, and this file just had all its snapshots removed
for the same reason.
So it catches the failure that actually occurred (discovery silently returning
nothing, step green having checked nothing) and not partial loss. Noted as
worth revisiting if kubernetes/ grows a lot more shell, when a loose
proportionate floor becomes possible.
The review's other point — that the image pins and the /bin/sh and
/bin/woodpecker-cli claims are not checkable from a static checkout, and that a
green run is the real confirmation — is now settled by pipeline 3185 on
|
||
|
claude-botandClaude Opus 5
|
b1f5d6dcbc |
ci: explain the ci-k8s tag split and track the stale pins (#461)
Review noted this file pins ci-k8s at a newer dated tag than e2e.yml, e2e-test.yml, alert-monitor.yml and issue-fix.yml, and asked for either a stated reason or a follow-up so it does not read as an oversight. There is no reason — the other four are stale. Per the latest-stable rule all five should sit on the newest dated tag. Not bumped here on purpose. Those four pipelines exercise different tooling in the image and three are cron- or event-triggered, so none of them run on this PR. Bumping them in this branch would ship four untested changes under cover of a green run that never touched them — the same shape as the digest decay in #458, where breakage sat unnoticed precisely because the affected jobs did not run for months. Each wants its own verification, so it is tracked as #461 rather than smuggled in here. Comment added above kustomize-build so the split is explained where a reader meets it, pointing at the issue rather than restating the whole argument. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
ba052332bf |
docs(ci): drop snapshot counts from comments — they decay and prove nothing
The file count was meaningless and would be wrong tomorrow. Correcting it from
197 to 180 last commit was the wrong fix: a number that has to be re-derived
every time the tree changes does not belong in a comment at all. It cost two
review rounds — one to flag that 197 did not reproduce, one to correct it —
and neither round was about anything that mattered, because the count was
never the argument. Widening update.path is pointless because Flux cannot
patch non-Kubernetes YAML at all; the size of the walk was only ever a
secondary aside.
Swept every snapshot figure I had introduced, not just the one flagged:
* "180 unrelated ansible/esphome YAMLs (179 + 1)" -> "the whole ansible/ and
esphome/ trees", in both the pipeline comment and the README.
* "builds 60 kustomizations" -> "builds every kustomization in the tree".
* "verified green across all 60 kustomizations" -> "across every
kustomization", keeping the as-of-writing hedge that made it a historical
claim rather than a live one.
* "expected ~60" in the discovery-floor failure message -> "floor is 50",
which cites the actual guard on the line above instead of a tree size that
drifts away from it. The message already prints the real $count.
* "scratch-based (19 MB, 2 layers, no shell)" -> "ships no shell", plus what
the reader actually needs: a step on the plain tag fails to START rather
than failing a check, which is the confusing symptom. The byte size would
have gone stale on the next upstream release.
The floor of 50 itself stays — it is a deliberately loose guard against
discovery breaking, not a description of the tree, and it is asserted in code
where a wrong value fails loudly instead of merely misinforming.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
claude-botandClaude Opus 5
|
1379813459 |
docs(ci): correct the file count in the update.path argument (197 -> 180)
Review could not reproduce "197 unrelated ansible/esphome YAML files" and counted 180. The reviewer is right, and the number was wrong twice over. Recounted in this checkout: ansible/ has 179, esphome/ has 1 -> 180, which matches the review exactly. My 197 came from also folding in .woodpecker/, which is wrong on both counts: those files are neither ansible nor esphome, and they are precisely the directory you WOULD want in the walk, so counting them as "unrelated" inverts the argument the sentence is making. The composite is also 198 rather than 197 today — the original count was taken against master, where .woodpecker/ still had 17 files because ci-manifests-test.yml did not exist yet. The conclusion is unchanged (widening update.path is pointless because Flux cannot patch non-Kubernetes YAML at all, and it would pull in a large number of unrelated files), but the supporting figure now reproduces, and both places that cite it are fixed with the derivation shown so it can be re-checked. Also recorded the review's rule-of-three note where it will actually be read: kustomize-build and shellcheck share a discovery shape, two copies is deliberate because the two traps commented around are different, and a third instance is the signal to factor it into scripts/ci/ — which would pick up unit tests via ci-scripts-test.yml on the way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
5115378835 |
ci: address review on #460 — drop an unverifiable claim, dedupe rationale
Review raised two 🟡 points and one nit. Both 🟡 points turned out to be
already answered by this PR's own CI run, but one of them exposed a real
accuracy problem in a comment, which is fixed here.
1. ci-k8s:2026-07-30 might never have been built. Good reasoning — build-k8s
only mints a dated tag when ci/images/k8s/** or build-ci-images.yml changes
on a push, and this PR touches neither; secret-rotator:2026-07-30 does not
transfer, being a separately-gated step. Checked rather than assumed: the
registry API is closed to this token (no read:package), so the evidence is
pipeline 3178 on
|
||
|
claude-botandClaude Opus 5
|
8a580e341d |
docs(ci): record why woodpecker-cli cannot be enrolled in image-automation
I proposed widening ImageUpdateAutomation's `update.path` to `./` and adding a woodpecker-cli ImagePolicy so automation would own the lint pin the way it owns the server and agent. Checking that before building it killed the plan, and corrected two things I had asserted without evidence. WRONG #1 — my reason for hesitating. I claimed the 7 ansible-vault encrypted `.yml` files would break the walk and take all 33 automated images down with them. That was a guess about kyaml I never verified. Objects that are not Kubernetes manifests are SKIPPED, not fatal. WRONG #2 — and this is the one that matters: the plan cannot work at all. Image-automation only patches YAML that is a Kubernetes manifest. Maintainer, fluxcd/flux2#2826: "Flux can only patch Kubernetes manifests. This means the YAML manifest must have `apiVersion` and `kind` fields." The docs agree — markers go "in any Kubernetes custom resource stored in Git". A Woodpecker pipeline file is plain YAML with no apiVersion/kind, so a marker in it is silently ignored. That is a property of the FILE, not of `update.path`. Widening the path buys exactly nothing and would drag 197 unrelated ansible/esphome YAMLs into the walk. So no policy, no second automation, no path change — all three would have been motion that looked like a fix and changed nothing. The failure mode would have been the quiet kind this repo keeps getting bitten by: a marker sitting in the file looking maintained, never once updated. What actually closes the gap, given the pin must stay manual: * README gains a "Not enrolled (structurally impossible)" section holding the quote and the explicit "do not try widening the path" warning, so the next person does not re-derive this. * update-audit item 5 now covers `.woodpecker/*.yml` step images and is told to flag woodpecker-cli whenever it trails the DEPLOYED SERVER, not merely the newest CLI release — the server auto-bumps and the CLI does not, so that skew is the thing worth reporting. Also told to compare only the vX.Y.Z part (the tag carries `-alpine`; the plain tag is scratch with no shell and would break the step) and to ignore the `next-*`/`pull_*` dev tags, which are the newest by date on that repo and are not releases. * the step comment states the real constraint instead of my earlier wrong one, which told the reader that widening the path was the fix. Verified: v3.16.0-alpine is the newest 3.x alpine release tag, and matches the deployed server v3.16.0. Config still parses; step image unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
fbfaf90923 |
docs(ci): frame the CLI pin as the latest-stable rule, not a separate one
I had written that the CLI "MUST track the deployed woodpecker-server", which invents a rule that does not exist and misstates the one that does. The standing rule already covers it: any software uses the latest stable version. CLI tools are software. If the CLI and the server both follow it, they match by construction — matching is a CONSEQUENCE, not a separate requirement. I had read "any software" as meaning installs and images but not CLIs, and was about to propose adding a redundant rule to the wiki off the back of that misreading. The comment now says that, and keeps the reason the skew mattered here: the CLI compiles against its own schema, so one behind the server can pass a config the server rejects, and the gate stops predicting server behaviour. Also recorded a real limitation found while checking this. The server and agent are bumped automatically by image-automation within the 3.x range, and their policy comment notes they must stay version-matched. This pin is manual, because ImageUpdateAutomation is scoped to `update.path: ./kubernetes` and cannot maintain a tag under .woodpecker/. So the CLI will lag a server upgrade until someone bumps it. Widening that path would let a policy own this line the same way it owns the server and agent — noted rather than done, since it changes automation scope repo-wide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
7b5f244b7f |
ci: use latest stable versions, and match the woodpecker CLI to the server
Violated the standing rule: "use the latest stable version. Old versions must not be hardcoded without a specific, documented reason." woodpecker-cli was pinned at v3.9.0 while the deployed server and agent run v3.16.0 — seven minor versions behind. That version was never chosen; I picked it while probing for any tag that had a shell and then never revisited it. For this step the skew is not cosmetic. The CLI compiles the config against its own schema, so one behind the server can pass a config the server later rejects, or reject one the server accepts. Either way the gate stops predicting what the server will do, which is the only reason it exists — and the failure it was added to catch (#443) is precisely a server-side compile error. Now v3.16.0-alpine, matched to the running deployment (checked against the live Deployment, not assumed), with a comment saying it must be bumped alongside the server. Verified it still behaves correctly: rc=0 on a valid config, rc=1 on the #443 construct. shellcheck was pinned at v0.10.0 for the same non-reason — it was simply the newer of the two tags I happened to probe. v0.11.0 exists, so bumped. Verified first that the existing scripts still pass at -S error under the newer version, since a shellcheck bump can add checks and I did not want to ship a gate that fails on the current tree. ci-k8s stays on the dated-tag convention for registry-local images. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
888aa60fe0 |
ci: run the four checks in parallel so every failure shows at once
Review points on #460. Woodpecker runs steps sequentially by default, so a failing kustomize-build hid woodpecker-lint, shellcheck and no-digest-pins entirely — a contributor would fix one thing, repush, and only then discover the next. All four are independent, cheap and offline, with no ordering between them, so depends_on: [] fans them out and one run reports everything. Verified the structure rather than the diff: all four steps now render with depends_on=[]. Also made the discovery-floor failure actionable. The message said only "Discovery is broken", which is wrong advice if the tree legitimately shrank below 50 kustomizations — it now tells the reader to lower the floor in that case, and explicitly not to delete it, since removing the guard restores the silent-green no-op it exists to prevent. On the ci-k8s:2026-07-30 tag, raised for the third time: confirmed by execution, not inspection. It resolved in pipeline 3161, whose kustomize-build and no-digest-pins steps both ran that image and logged "discovered 60 kustomizations" with 60 ok lines. An image that failed to pull could not have produced that output. The review is right that nothing in THIS PR rebuilds the tag — it was published earlier today by build-ci-images runs triggered by edits to .woodpecker/build-ci-images.yml, which is also exactly the drift that left other pipelines pinned to ci-k8s:2026-05-01. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
9d3dc152c1 |
fix(ci): repair the block scalar I broke in the previous commit
The previous commit set IFS to a literal newline by writing
IFS="
"
inside a `- |` block scalar. The closing quote sits at column 0, which
terminates the block — so the config no longer parsed:
go-yaml load error in scanner (while scanning a simple key) at
L70.C1-L89.C3: could not find expected ':'
Same class as #443, in the very pipeline written to catch #443, and I
pushed it: `woodpecker lint` printed the error in the same command as the
push, and I read the push output rather than the lint output.
Replaced with `while IFS= read -r` fed from a FILE rather than a pipe.
That handles paths with spaces without needing a literal newline anywhere,
and the file redirection keeps the loop in the current shell so `fail`
still propagates — a pipe would have run it in a subshell and made a
failing kustomize build exit 0, which is the silent-pass shape this step
already shipped once with -printf.
Verified both properties in the actual image rather than reasoning about
them: a directory named "has space" iterates once intact, and a `fail=1`
set inside the loop is still 1 after it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
claude-botandClaude Opus 5
|
1911a20db5 |
ci: path-filter the manifest gate and harden its loops
Review points on #460. Path-filter the pipeline to kubernetes/** and .woodpecker/**. It was unfiltered, matching ci-scripts-test.yml, but that comparison does not hold: ci-scripts-test is one step on one already-cached image, whereas this pulls three images and builds 60 kustomizations — real cost to impose on a docs-only PR. The filter still covers everything the steps read, and including .woodpecker/** means an edit to this file re-runs its own checks. Pin IFS to newline in both find-driven loops so a path containing spaces is passed as one argument rather than split into fragments. Verified in the actual image: a directory named "has space" now iterates once, intact. Deliberately NOT rewritten as `printf | while read`, which is the usual advice: that puts the loop in a subshell, so `fail` would not propagate and a failing kustomize build would exit 0. That is the same silent-pass shape this step already shipped once with -printf, and fixing a latent whitespace bug by reintroducing a live correctness bug would be a poor trade. Also corrected the file header, which still claimed the gate runs on every pull_request. On the ci-k8s:2026-07-30 tag the review asked about again: confirmed by execution rather than inspection. Pipeline 3161 ran this pipeline against a real checkout with all five steps green, which requires all three images to have pulled in the runner, and its kustomize-build step logged "discovered 60 kustomizations" with 60 ok lines — so the image both resolved and did the work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
ddf5f5007b |
docs(ci): correct how the digest guard avoids self-matching
Review point on #460. The comment claimed "`image:` anchors the match so the kustomization comment documenting this rule does not flag itself". That is wrong about the mechanism, and the reviewer proved it by running the bare grep — kustomization.yaml DOES match, because the comment documenting the rule contains the literal string `image:.*@sha256`. What actually excludes it is the `grep -v kustomization.yaml` filter. The step's behaviour was always correct; only the explanation was wrong. Worth fixing rather than shrugging at: this is the third time today the same self-matching trap has caught me. Twice while writing the guard in rotation/kustomization.yaml (where the final version documents it accurately), and now once more in the comment describing it here. A comment that misidentifies which line is load-bearing invites someone to delete the filter as redundant tidying, at which point the step fails permanently on its own documentation. The comment now says the filter is load-bearing and why. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
790fd1933a |
fix(ci): kustomize-build validated nothing and reported green
Blocking review point on #460, and it was exactly right. `find -printf` is a GNU findutils extension. ci-k8s is alpine-based and never installs findutils, so find is BusyBox — confirmed by running it in the image: find: unrecognized: -printf BusyBox v1.37.0 Because the result fed a command substitution used as a word list, the failure was SILENT: empty output, zero loop iterations, fail stayed 0, and the step exited green having run kubectl kustomize on nothing at all. The proof is in this PR's own pipeline run (3158). The step is recorded as "success", and its log contains ZERO `ok` lines — sixty were expected. So the gate I added to catch silent no-ops was itself a silent no-op, and CI reported it as passing. Worth stating why neither of my checks caught it: I dry-ran the loop on my workstation, which has GNU find, and the CI run cannot distinguish a zero-iteration loop from a working one when every kustomization already builds — both exit 0. The review's suggested test (paste the per-directory log lines, not the exit code) is the only one that separates them. Fixed by replacing -printf with a portable pipe through sed. Verified in the actual image, with a synthetic tree, that the loop now iterates. Also added a discovery assertion to both find-based steps: fail if the count is implausible (kustomize-build expects >=50 of ~60; shellcheck expects >=1) rather than proceeding with an empty list. That is the general fix — without it, ANY future breakage of discovery degrades back into a green no-op instead of a failure, which is the trap this whole commit is about rather than the -printf detail specifically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
claude-botandClaude Opus 5
|
cd12bc16c9 |
ci: validate the GitOps tree and pipeline configs on every PR
Nothing covered kubernetes/** before this. deploy-infrastructure.yml gates only opentofu/**, and ci-scripts-test.yml runs only the scripts/ci/ unit tests — so a broken manifest reached the cluster and was caught by Flux failing to reconcile, or by someone noticing. Reviewers flagged the gap on three separate PRs today. Four static checks, all offline (no cluster, no registry, no secrets): kustomize-build every kustomization under kubernetes/ must build woodpecker-lint every .woodpecker/*.yml must compile shellcheck standalone .sh under kubernetes/, severity=error no-digest-pins rotators must not reintroduce @sha256 image pins Each was verified GREEN against the tree as it stands before being added, so this gates new breakage rather than failing on pre-existing debt: all 60 kustomizations build, all pipeline configs lint, all 6 shell scripts are clean at error severity, and no digest pins remain after #459. The woodpecker-lint step is the one that matters most: it exits 1 on the exact defect that took every pipeline in this repo down earlier today (#443) — an unquoted YAML scalar containing ": ", which `yq` accepts because it IS valid YAML, just a map rather than a string. Verified both directions in the image the step actually uses: rc=1 on that construct, rc=0 on the pre-existing "Additional property timeout is not allowed" warnings in four deploy-*.yml files, so it fails on real breakage without blocking on legacy noise. Image choices, all version-pinned with no floating tags: ci-k8s:2026-07-30 kubectl for kustomize; also the shell for the grep-based guard woodpecker-cli:v3.9.0-alpine the non-alpine image is distroless and has no /bin/sh, so Woodpecker `commands:` cannot run in it; the binary is /bin/woodpecker-cli, not on PATH as `woodpecker` shellcheck-alpine:v0.10.0 pinned rather than :stable, which floats like :latest Deliberately NOT included: kubeconform. It needs CRD schemas for ExternalSecret, Certificate, HelmRelease and friends, so it either needs a schema-fetching step (network, and a new failure mode) or -ignore-missing-schemas, which would skip precisely the CRDs most of this tree is made of. Worth doing properly as its own change. Known gap, called out in the file rather than implied away: shellcheck cannot see shell embedded in manifests (init containers, probe exec blocks), which is where several real bugs lived this year. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
k.renner
|
971ea6dc09 |
Merge pull request 'fix(rotation): repin every rotator to a dated tag, not a digest' (#459) from rotator-repin into master
Reviewed-on: k.renner/homelab#459 |
||
|
claude-botandClaude Opus 5
|
5d32065b8f |
fix(rotation): make the documented guard actually pass
Second attempt, and the first one was wrong for a subtler reason than I fixed. Anchoring on `image:` did not help: this file documents the pattern, so the comment text `image:.*@sha256` matches the regex `image:.*@sha256`. The guard kept reporting its own documentation. Added the `| grep -v kustomization.yaml` filter and this time ran the command exactly as written rather than a paraphrase of it. Verified both directions: no output across this directory as it stands, and it still reports a file carrying a real `image: ...@sha256:` pin. Lesson worth the two commits: a documented check has to be executed in the form it is documented in. I had verified a variant twice and shipped a guard whose only hit was itself — which trains people to ignore it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |