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

fix(opentofu): correct swapped proxy-1/proxy-2 IPv6 attribution #431

Merged
k.renner merged 1 commits from claude-bot/fix-proxy-v6-labels into master 2026-07-27 20:15:57 +00:00
Conversation 1 Commits 1 Files Changed 2
+13 -7
claude-bot commented 2026-07-27 19:28:29 +00:00
Member
Copy Link
Copy Source

Surfaced while triaging #392 / #420 — both of which turned out to be already fixed in production.

The problem

The two v6 addresses in hairpin_endpoints were attributed to the opposite proxy members. DNS is the operational truth, verified independently for this PR:

getent ahosts proxy-1.renner.uno  ->  2a03:4000:28:f6e:a4c7:59ff:fe1e:9a29   (v6-only member, single AAAA)
getent ahosts proxy-2.renner.uno  ->  2a03:4000:4b:c1e:4474:43ff:fedf:8f85
                                      45.157.177.26                          (dual-stack member)

Impact: none functionally — but it mattered

Both entries DNAT to the same target = "fd99::86", so the hairpin behaved identically before and after. This is purely a correctness/labelling fix.

It's worth doing because these labels were the reference other code had to actively warn people away from: probes-blackbox.yaml carried a NOTE saying the .tf labels were backwards and "tracked for a separate relabel". That NOTE is now removed — the two files agree.

Slots are untouched; the file's "never reuse a freed slot" rule applies to the keys, not the values.

Verification

  • tofu validate → Success! The configuration is valid.
  • tofu fmt reports pre-existing alignment drift further down this file (the jsonencode block around line 108). Confirmed present on master before this change by re-running fmt against the stashed version — deliberately left alone so this PR stays a one-concern change.

🤖 Generated with Claude Code

Surfaced while triaging #392 / #420 — both of which turned out to be already fixed in production. ### The problem The two v6 addresses in `hairpin_endpoints` were attributed to the **opposite** proxy members. DNS is the operational truth, verified independently for this PR: ``` getent ahosts proxy-1.renner.uno -> 2a03:4000:28:f6e:a4c7:59ff:fe1e:9a29 (v6-only member, single AAAA) getent ahosts proxy-2.renner.uno -> 2a03:4000:4b:c1e:4474:43ff:fedf:8f85 45.157.177.26 (dual-stack member) ``` ### Impact: none functionally — but it mattered Both entries DNAT to the same `target = "fd99::86"`, so the hairpin behaved identically before and after. This is purely a correctness/labelling fix. It's worth doing because these labels were the reference other code had to actively warn people away from: `probes-blackbox.yaml` carried a NOTE saying the `.tf` labels were backwards and "tracked for a separate relabel". That NOTE is now removed — the two files agree. Slots are untouched; the file's *"never reuse a freed slot"* rule applies to the keys, not the values. ### Verification - `tofu validate` → `Success! The configuration is valid.` - `tofu fmt` reports **pre-existing** alignment drift further down this file (the `jsonencode` block around line 108). Confirmed present on master before this change by re-running `fmt` against the stashed version — deliberately left alone so this PR stays a one-concern change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude-bot added 1 commit 2026-07-27 19:28:29 +00:00
fix(opentofu): correct swapped proxy-1/proxy-2 IPv6 attribution
ci/woodpecker/pr/pr-review Pipeline was successful
Details
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
Details
5a46db78ce 
The two v6 addresses in hairpin_endpoints were attributed to the opposite
proxy members. DNS is the operational truth, verified independently:

  getent ahosts proxy-1.renner.uno -> 2a03:4000:28:f6e:a4c7:59ff:fe1e:9a29
                                      (v6-only member, single AAAA)
  getent ahosts proxy-2.renner.uno -> 2a03:4000:4b:c1e:4474:43ff:fedf:8f85
                                      45.157.177.26   (dual-stack member)

Purely a labelling fix with zero functional impact: both entries DNAT to the
same target (fd99::86), so the hairpin behaved identically before and after.
Slots are untouched — the file's "never reuse a freed slot" rule applies to
the keys, not the values.

Also trims the now-stale NOTE in probes-blackbox.yaml that existed only to warn
readers away from the wrong labels; the two files now agree.

Surfaced while triaging #392/#420, both of which turned out to be already
fixed. `tofu validate` passes. (Note: `tofu fmt` reports pre-existing
alignment drift further down this file, at the jsonencode block around line
108 — present on master before this change and deliberately left alone.)

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

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

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

Summary

Swaps the two proxy-1 v6 / proxy-2 v6 address values in opentofu/unifi_nat_hairpin.tf to match DNS, and updates the now-stale "these are backwards" comment in kubernetes/apps/monitoring-rules/probes-blackbox.yaml to reflect the fix. The change is internally consistent — the addresses now line up with probes-blackbox.yaml's existing (unchanged) blackbox-edge-wan targets, which already had the correct hostname↔address attribution. No blocking issues found; one operational risk worth flagging before applying.

🟡 Suggestions

  • opentofu/unifi_nat_hairpin.tf:64-73 (provider "restapi" block, unchanged by this PR): the file itself documents a "UniFi PUT quirk: updates may require _id in the body, which restapi does not send — if an in-place update 400s, taint the object so CI recreates it." This PR only changes the address field of the "proxy-1 v6" / "proxy-2 v6" endpoint entries, and since the for_each keys (combo[0]/net_name/combo[1]) are untouched, tofu apply will attempt an in-place update (PUT) on every existing restapi_object.hairpin resource for those two endpoints across all 10 networks × 2 ports (40 resources), rather than a create/destroy. Given the documented quirk, that's exactly the kind of change likely to hit the 400 the comment warns about. Worth running tofu plan to confirm it shows updates (not creates) and being ready to taint+reapply per the documented workaround if any of the 40 PUTs fail, rather than assuming a clean apply.
  • The PR body's "Verification" section only cites tofu validate/tofu fmt — neither exercises the actual API call path, so the above risk isn't something those checks would have caught. Worth a one-line mention in the PR body (or just confirming tofu plan output) since this touches live NAT rules on the UDM.

Tests

No automated tests apply to this OpenTofu/YAML config repo (consistent with the rest of the codebase — no test suite for .tf/probe YAML). Verification here is necessarily manual: tofu validate/tofu fmt as stated, plus (per the suggestion above) a tofu plan review before apply given the live-update risk. DNS attribution (getent ahosts) could not be independently re-verified in this sandboxed review environment (no network egress available), so I relied on internal consistency between the two changed files and the unchanged blackbox-edge-wan probe targets, which already matched the corrected attribution — that cross-check supports the PR's claim.

Recommendation

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

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `5a46db78ce550af1edc4b622bb49022a9dc284e3`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Swaps the two `proxy-1 v6` / `proxy-2 v6` address values in `opentofu/unifi_nat_hairpin.tf` to match DNS, and updates the now-stale "these are backwards" comment in `kubernetes/apps/monitoring-rules/probes-blackbox.yaml` to reflect the fix. The change is internally consistent — the addresses now line up with `probes-blackbox.yaml`'s existing (unchanged) `blackbox-edge-wan` targets, which already had the correct hostname↔address attribution. No blocking issues found; one operational risk worth flagging before applying. ## 🟡 Suggestions - **`opentofu/unifi_nat_hairpin.tf:64-73` (`provider "restapi"` block, unchanged by this PR)**: the file itself documents a "UniFi PUT quirk: updates may require `_id` in the body, which restapi does not send — if an in-place update 400s, taint the object so CI recreates it." This PR only changes the `address` field of the `"proxy-1 v6"` / `"proxy-2 v6"` endpoint entries, and since the `for_each` keys (`combo[0]`/`net_name`/`combo[1]`) are untouched, `tofu apply` will attempt an **in-place update (PUT)** on every existing `restapi_object.hairpin` resource for those two endpoints across all 10 networks × 2 ports (40 resources), rather than a create/destroy. Given the documented quirk, that's exactly the kind of change likely to hit the 400 the comment warns about. Worth running `tofu plan` to confirm it shows updates (not creates) and being ready to taint+reapply per the documented workaround if any of the 40 PUTs fail, rather than assuming a clean apply. - The PR body's "Verification" section only cites `tofu validate`/`tofu fmt` — neither exercises the actual API call path, so the above risk isn't something those checks would have caught. Worth a one-line mention in the PR body (or just confirming `tofu plan` output) since this touches live NAT rules on the UDM. ## Tests No automated tests apply to this OpenTofu/YAML config repo (consistent with the rest of the codebase — no test suite for `.tf`/probe YAML). Verification here is necessarily manual: `tofu validate`/`tofu fmt` as stated, plus (per the suggestion above) a `tofu plan` review before apply given the live-update risk. DNS attribution (`getent ahosts`) could not be independently re-verified in this sandboxed review environment (no network egress available), so I relied on internal consistency between the two changed files and the unchanged `blackbox-edge-wan` probe targets, which already matched the corrected attribution — that cross-check supports the PR's claim. ## Recommendation Looks reasonable — no blocking issues; comments above are optional. <!-- pr-review:sha=5a46db78ce550af1edc4b622bb49022a9dc284e3 -->
k.renner merged commit 2ed257bc6a into master 2026-07-27 20:15:57 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
review-bot
Labels
Clear labels
apollo
Hermes expert: Immich, Audiobookshelf, Libation, Navidrome, Music Assistant
 atlas
Hermes expert: Talos, Flux, network, Authentik, Matrix, monitoring
 clio
Hermes expert: Paperless, wiki, ical-proxy, TRMNL, websites
 daedalus
Hermes expert: Gitea, Woodpecker, Renovate, review-bot, Hermes
 hestia
Hermes expert: Home Assistant, IoT, MQTT, ESPHome, voice
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
alert-monitor-bot claude-bot (Claude Bot) flux hermes-bot issue-fix-bot k.renner renovate-bot review-bot (Review Bot) security-bot update-audit-bot
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: konairius/homelab#431