docs(opentofu): record the hairpin _id diff, retract the PUT 400 warning #514

2026-08-19T06:35:42Z

claude-bot commented

2026-08-19 06:35:42 +00:00

Documentation only, no behaviour change. Both findings existed solely in commit messages and PR descriptions — which is not where anyone looks when they hit this.

1. Retracts a warning that was wrong, and cost three days

providers.tf warned:

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.

Not true on this controller. Tested against a live rule with exactly the body tofu sends:

PUT .../nat/6a61c7d7bbd7b2ca90981f51   (body with _id removed)
  -> HTTP 200, object returned intact
  -> rule re-read and diffed: byte-identical
  -> rule count: 60 before, 60 after

Confirmed at scale by apply 5675: all 60 rules Modifications complete after 0s, no 400s, ci.renner.uno and immich.renner.uno both still 200.

This one had a real cost. A note saying an apply might break firewall rules is one nobody argues with — so the apply didn't get run, and the renner-uno-k8s-mariadb bucket added in b976a67b sat uncreated for three days while the WordPress backup failed nightly with The specified bucket does not exist. The warning is replaced with the evidence that disproves it.

2. Records the permanent `_id` diff, and the four dead ends

All 60 hairpin rules plan as "will be updated in-place" forever, the whole diff being - _id. It survives an apply — 5675 rewrote all 60, and plan 5677 immediately after showed the same 60.

Attempts, each with the plan that disproved it:

attempt	plan	result
`ignore_changes_to = ["_id"]`	5661	60 remained
`ignore_all_server_changes = true`	5662	60 remained
restapi 3.x + `ignore_server_additions`	5667	60 remained, plus an `api_data` diff
`write_returns_object` -> `create_returns_object`	5677	60 remained

The first three cannot work in principle: data is a configured attribute, so Terraform core diffs config against state before any provider-side ignore logic runs. The fourth was the better theory — that PUT responses re-polluted data — and it is also wrong, which means the response is written into data on the read path. Settling that needs the provider source, not more guesses at its schema.

The file now says all of this, plus that applying through it is safe, so the diff is understood as cosmetic rather than treated as a hazard.

Why this is worth a PR on its own

The repo already does this well — unifi_nat_hairpin.tf records that the FIREWALL_GROUPS variant "broke the network twice", which is exactly the kind of note that stops a repeat. These two findings deserve the same treatment: one retracts misinformation that cost three days of backups, the other maps four dead ends.

Documentation only, no behaviour change. Both findings existed solely in commit messages and PR descriptions — which is not where anyone looks when they hit this. ## 1. Retracts a warning that was wrong, and cost three days `providers.tf` warned: > 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. Not true on this controller. Tested against a live rule with exactly the body tofu sends: ``` PUT .../nat/6a61c7d7bbd7b2ca90981f51 (body with _id removed) -> HTTP 200, object returned intact -> rule re-read and diffed: byte-identical -> rule count: 60 before, 60 after ``` Confirmed at scale by apply **5675**: all 60 rules `Modifications complete after 0s`, no 400s, `ci.renner.uno` and `immich.renner.uno` both still 200. **This one had a real cost.** A note saying an apply might break firewall rules is one nobody argues with — so the apply didn't get run, and the `renner-uno-k8s-mariadb` bucket added in `b976a67b` sat uncreated for three days while the WordPress backup failed nightly with `The specified bucket does not exist`. The warning is replaced with the evidence that disproves it. ## 2. Records the permanent `_id` diff, and the four dead ends All 60 hairpin rules plan as "will be updated in-place" forever, the whole diff being `- _id`. It survives an apply — 5675 rewrote all 60, and plan **5677** immediately after showed the same 60. Attempts, each with the plan that disproved it: | attempt | plan | result | |---|---|---| | `ignore_changes_to = ["_id"]` | 5661 | 60 remained | | `ignore_all_server_changes = true` | 5662 | 60 remained | | restapi 3.x + `ignore_server_additions` | 5667 | 60 remained, plus an `api_data` diff | | `write_returns_object` -> `create_returns_object` | 5677 | 60 remained | The first three cannot work in principle: `data` is a **configured** attribute, so Terraform core diffs config against state before any provider-side ignore logic runs. The fourth was the better theory — that PUT responses re-polluted `data` — and it is also wrong, which means the response is written into `data` on the **read** path. Settling that needs the provider source, not more guesses at its schema. The file now says all of this, plus that applying through it is safe, so the diff is understood as cosmetic rather than treated as a hazard. ## Why this is worth a PR on its own The repo already does this well — `unifi_nat_hairpin.tf` records that the `FIREWALL_GROUPS` variant "broke the network twice", which is exactly the kind of note that stops a repeat. These two findings deserve the same treatment: one retracts misinformation that cost three days of backups, the other maps four dead ends.

claude-bot added 1 commit 2026-08-19 06:35:42 +00:00

docs(opentofu): record the hairpin _id diff and retract the PUT 400 warning

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

853928b6b1

Both of these were learned the expensive way today and existed only in commit
messages and PR descriptions, which is not where anyone looks.

providers.tf carried a warning that a UniFi PUT may require _id in the body,
which restapi does not send, and that a 400 should be answered by tainting the
object so CI recreates it. That is wrong on this controller. Tested against a
live rule with exactly the body tofu sends: HTTP 200, object returned intact,
rule byte-identical before and after, 60 rules before and after. Confirmed at
scale by apply 5675 — all 60 modified, no 400s, ci.renner.uno and
immich.renner.uno still 200.

That wrong warning had a cost. A note saying an apply might break firewall
rules is one nobody argues with, so the apply did not get run, and the
renner-uno-k8s-mariadb bucket added in b976a67b sat uncreated for three days
while the WordPress backup failed nightly with "The specified bucket does not
exist". The warning is replaced with the evidence that disproves it.

unifi_nat_hairpin.tf now records that the 60-rule `- _id` diff is permanent and
cosmetic, and lists the four attempts to remove it with the plan number that
disproved each: ignore_changes_to (5661), ignore_all_server_changes (5662),
restapi 3.x ignore_server_additions (5667), and create_returns_object (5677).

The first three cannot work in principle — `data` is a configured attribute, so
Terraform core diffs it against state before provider-side ignore logic runs.
The fourth was the better theory, that PUT responses re-polluted `data`, and it
is wrong too: the diff returned on the plan immediately after the apply. That
means the response is written into `data` on the read path, and settling it
needs the provider source rather than more guesses at its schema.

No behaviour change. Documentation only, so the next person inherits the
findings instead of the dead ends.

Co-Authored-By: Claude Bot <claude-bot@renner.uno>

review-bot reviewed

2026-08-19 06:36:48 +00:00

review-bot left a comment

🤖 Automated review — model claude-haiku-4-5-20251001 (complexity: trivial), commit 853928b6b13d994d27eef05c3fff8698626e437a.

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

Summary

This PR replaces incorrect guidance about UniFi PUT responses with accurate documentation, and records a known permanent _id diff in hairpin NAT rules along with four attempted (and failed) fixes. Both changes are documentation only with no behavior changes — formatting improvements in unifi_nat_hairpin.tf are the only non-comment modifications.

🟢 Nits

opentofu/providers.tf:62-80 — The new comment is clear and well-supported by evidence (tested against a live rule, confirmed at scale in apply 5675). One small observation: the internal apply/plan numbers (5675, 5677, 5661–5667) are useful for repo history but won't be accessible to future readers unfamiliar with the infrastructure logs. That's acceptable here since the key evidence (the PUT test result) is documented inline.

opentofu/unifi_nat_hairpin.tf:111-143 — The permanent-diff comment is thorough and follows good practice by documenting failed attempts. The explanation of why the first three approaches don't work (data is configured, not server-managed) is technically sound. The formatting alignment changes (lines 145–154) improve readability without changing the logic.

Tests

Not applicable — documentation only, no behavior changes.

Recommendation

Looks reasonable — no blocking issues; this is a solid documentation update that corrects misinformation and prevents future maintainers from repeating failed troubleshooting steps.

🤖 **Automated review** — model `claude-haiku-4-5-20251001` (complexity: trivial), commit `853928b6b13d994d27eef05c3fff8698626e437a`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR replaces incorrect guidance about UniFi PUT responses with accurate documentation, and records a known permanent `_id` diff in hairpin NAT rules along with four attempted (and failed) fixes. Both changes are documentation only with no behavior changes — formatting improvements in `unifi_nat_hairpin.tf` are the only non-comment modifications. ## 🟢 Nits **`opentofu/providers.tf:62-80`** — The new comment is clear and well-supported by evidence (tested against a live rule, confirmed at scale in apply 5675). One small observation: the internal apply/plan numbers (5675, 5677, 5661–5667) are useful for repo history but won't be accessible to future readers unfamiliar with the infrastructure logs. That's acceptable here since the key evidence (the PUT test result) is documented inline. **`opentofu/unifi_nat_hairpin.tf:111-143`** — The permanent-diff comment is thorough and follows good practice by documenting failed attempts. The explanation of why the first three approaches don't work (data is configured, not server-managed) is technically sound. The formatting alignment changes (lines 145–154) improve readability without changing the logic. ## Tests Not applicable — documentation only, no behavior changes. ## Recommendation Looks reasonable — no blocking issues; this is a solid documentation update that corrects misinformation and prevents future maintainers from repeating failed troubleshooting steps.

k.renner merged commit d208c17509 into master

2026-08-19 06:50:25 +00:00

k.renner referenced this issue from a commit

2026-08-19 06:50:26 +00:00

Merge pull request 'docs(opentofu): record the hairpin _id diff, retract the PUT 400 warning' (#514) from document-hairpin-id-diff into master

claude-bot referenced this pull request

2026-08-19 06:53:27 +00:00

feat(opentofu): destroy the rustfs LXCs 800/801/802 #515

Sign in to join this conversation.