fix(vpn-ingress): new-peer.sh counted its own commented-out example as an allocated address #64

Merged
jakman85 merged 1 commits from fix/new-peer-ignores-template-comments into main 2026-10-06 07:31:03 +00:00
Owner

Your first real enrolment failed, and it was my bug — not a real collision:

$ ./infra/vpn-ingress/new-peer.sh keith-phone 2
new-peer.sh: 10.90.0.2/32 is already allocated in platform/vpn-ingress/peers.yaml. Pick another octet.

Nothing was allocated. peers.yaml had zero peers, as the pod's own init container reported (wg0.conf assembled: 0 peer(s)). The duplicate-address check grepped the whole file, and line 66 of peers.yaml is a commented-out example stanza I wrote, containing literally # AllowedIPs = 10.90.0.2/32.

So .2 — the first address anyone would ask for, and the one my own runbook uses as its example — was permanently un-allocatable, with a message asserting the opposite of the truth.

Fixed on both sides

Either alone leaves the trap half-set:

  • The check strips comment lines before matching. That's the correct predicate, not a patch: a real stanza's AllowedIPs line is never commented, while the template and every peer's # <name> — <owner> label always are.
  • The example now uses placeholders that cannot match anything real — 10.90.0.<octet>/32, <device-name>, <YYYY-MM-DD> — so a future grep can't be fooled by it either.

The bug class, named

A documentation example that is byte-identical to real data will eventually be parsed as real data. This is its third instance in this component:

  1. flux-postbuild-substitution flagged a ${...} I wrote inside a comment explaining that hazard.
  2. no-plaintext-secrets flagged wg-server-key.sops.yaml.example because the kustomization named it in a comment.
  3. This one.

The rule going forward: make examples structurally unmatchable, not merely commented out.

Verified by running it

  • .2 now works — renders a profile with Address 10.90.0.2/32, full tunnel, at .cache/wg-peers/keith-phone.conf (0600). The throwaway keypair was deleted afterwards.
  • This also closes a gap left open in #60: the profile could only be rendered by deriving the server public key via sops -d --extract … | wg pubkey, so that pipeline is now proven against your real key, not just against the placeholder it was expected to reject.
  • A real duplicate is still refused — appending an uncommented AllowedIPs = 10.90.0.5/32 stanza and asking for octet 5 produced the correct refusal; tree restored afterwards.
  • Gate: rc 3, all runnable stages green, two environmental skips. Not green — reported as rc 3.

Not verified

No peer is enrolled by this branch. The keypair I generated was a throwaway and was deleted — the device's private key must exist only on the device, so you have to run the script yourself. The end of the path (stanza merged, Flux rolls the DaemonSet, wg show lists a peer) is still unexercised.

No sensitive paths — platform/vpn-ingress/**, infra/vpn-ingress/**, decisions/**. Merges on a green gate, no override.

🤖 Generated with Claude Code

Your first real enrolment failed, and it was my bug — not a real collision: ``` $ ./infra/vpn-ingress/new-peer.sh keith-phone 2 new-peer.sh: 10.90.0.2/32 is already allocated in platform/vpn-ingress/peers.yaml. Pick another octet. ``` **Nothing was allocated.** `peers.yaml` had zero peers, as the pod's own init container reported (`wg0.conf assembled: 0 peer(s)`). The duplicate-address check grepped the **whole file**, and line 66 of `peers.yaml` is a **commented-out example stanza** I wrote, containing literally `# AllowedIPs = 10.90.0.2/32`. So `.2` — the first address anyone would ask for, and the one my own runbook uses as its example — was permanently un-allocatable, with a message asserting the opposite of the truth. ## Fixed on both sides Either alone leaves the trap half-set: - **The check strips comment lines before matching.** That's the correct predicate, not a patch: a real stanza's `AllowedIPs` line is never commented, while the template *and* every peer's `# <name> — <owner>` label always are. - **The example now uses placeholders that cannot match anything real** — `10.90.0.<octet>/32`, `<device-name>`, `<YYYY-MM-DD>` — so a future grep can't be fooled by it either. ## The bug class, named **A documentation example that is byte-identical to real data will eventually be parsed as real data.** This is its *third* instance in this component: 1. `flux-postbuild-substitution` flagged a `${...}` I wrote inside a comment *explaining* that hazard. 2. `no-plaintext-secrets` flagged `wg-server-key.sops.yaml.example` because the kustomization named it in a comment. 3. This one. The rule going forward: make examples **structurally unmatchable**, not merely commented out. ## Verified by running it - **`.2` now works** — renders a profile with `Address 10.90.0.2/32`, full tunnel, at `.cache/wg-peers/keith-phone.conf` (0600). The throwaway keypair was deleted afterwards. - **This also closes a gap left open in #60**: the profile could only be rendered by deriving the server public key via `sops -d --extract … | wg pubkey`, so that pipeline is now proven **against your real key**, not just against the placeholder it was expected to reject. - **A real duplicate is still refused** — appending an *uncommented* `AllowedIPs = 10.90.0.5/32` stanza and asking for octet 5 produced the correct refusal; tree restored afterwards. - Gate: **rc 3**, all runnable stages green, two environmental skips. **Not green — reported as rc 3.** ## Not verified **No peer is enrolled by this branch.** The keypair I generated was a throwaway and was deleted — the device's private key must exist only on the device, so you have to run the script yourself. The end of the path (stanza merged, Flux rolls the DaemonSet, `wg show` lists a peer) is still unexercised. No sensitive paths — `platform/vpn-ingress/**`, `infra/vpn-ingress/**`, `decisions/**`. **Merges on a green gate, no override.** 🤖 Generated with [Claude Code](https://claude.com/claude-code)
gitea-admin added 1 commit 2026-10-06 07:29:01 +00:00
fix(vpn-ingress): new-peer.sh counted its own commented-out example as an allocated address
gate / gate (pull_request) Successful in 1m51s
gate / conformance (pull_request) Has been skipped
146dd9a906
The first real enrolment failed:

    $ ./infra/vpn-ingress/new-peer.sh keith-phone 2
    new-peer.sh: 10.90.0.2/32 is already allocated ... Pick another octet.

Nothing was allocated -- peers.yaml had zero peers, as the pod's init
container reported (`0 peer(s)`). The duplicate-address check grepped the
whole file, and peers.yaml line 66 is a COMMENTED-OUT example stanza I wrote
containing literally `#   AllowedIPs = 10.90.0.2/32`. So .2 -- the first
address anyone would ask for, and the one my own runbook uses as its example
-- was permanently un-allocatable, with a message asserting the opposite of
the truth.

Fixed on both sides, because either alone leaves the trap half-set:

  * the check strips comment lines before matching, which is the correct
    predicate rather than a patch: a real stanza's AllowedIPs line is never
    commented, while the template and every peer's `# <name> — <owner>` label
    always are.
  * the example now uses placeholders that cannot match anything real
    (`10.90.0.<octet>/32`, `<device-name>`), so a future grep cannot be fooled
    by it either.

The bug class is worth naming: a documentation example that is byte-identical
to real data will eventually be parsed as real data. This is its third
instance in this component -- flux-postbuild-substitution flagged a `${...}`
written inside a comment explaining that hazard, and no-plaintext-secrets
flagged the .sops.yaml.example because a kustomization named it in a comment.
Make examples structurally unmatchable, not merely commented out.

Verified by running it: .2 now renders a profile (via the real
`sops -d | wg pubkey` path, which this also proves against the operator's
actual key for the first time), and an uncommented duplicate stanza is still
correctly refused.

No sensitive paths; merges on a green gate.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
jakman85 merged commit 5928d8153c into main 2026-10-06 07:31:03 +00:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Home-Network/home-network-k3s-impl#64