Make intra-cluster NOTIFY loop-free (TSIG-keyed allow-notify, no pod IPs in restart config) #15

2026-07-25T23:23:14+10:00

unkinben commented

2026-07-25 23:23:14 +10:00

Why

v0.2.5 (PR #14) added an options-scope allow-notify { <primaryPodIP>; <transferAddr>; } on secondaries so intra-cluster NOTIFYs (which egress with the primary's pod IP as source, since k8s Services only NAT inbound) would be accepted. But options-scope config feeds the config-hash annotation that rolls the StatefulSet. So:

config change → pods roll → primary comes back on a new pod IP → operator re-renders allow-notify with the new IP → new hash → pods roll → new IP → infinite roll loop across every BindCluster (externaldns, authoritative, resolvers all cycled).

The production deployment was reverted to v0.2.4 (argocd revert #291); the v0.2.5 code stayed on main. This PR fixes it in place for v0.2.6.

Constraint learned: nothing rendered into restart-scoped (config-hash) config may depend on pod IPs.

Chosen design — A: TSIG-signed NOTIFYs, fully static secondary config

Secondary: render allow-notify { key "<name>"; }; — an address-match-list key element with NO IPs. It depends only on the key name, so pod-IP churn can never change the render, the hash, or trigger a restart. (BIND 9.20 accepts a key element in an address-match-list.)
Primary: sign outgoing NOTIFYs. The zone-scope also-notify entries (already enumerating replica pod IPs, applied via rndc addzone/modzone with no restart) now carry key "<name>": also-notify { <replicaIP> key "<name>"; ... };. (BIND 9.20 supports keyed also-notify entries.)
Key choice: reuse the cluster's catalog transfer TSIG key (BindCatalogZone.spec.transferKeyRef). Secondaries already present this key for AXFR (it is in the secondary's catalog-zones default-primaries { <ip> key "<name>"; }), it is materialised into keys.conf on every pod (primary and secondary), and the primary already requires allow-transfer { key "<name>"; }. No new key plumbing — the cluster's existing replication key doubles as the NOTIFY key.

Why this is loop-free

also-notify lives in zone-scope config applied via rndc (BindZoneReconciler.buildZoneConfig → Exec.AddZone), which never touches named.conf and never rolls pods — so listing pod IPs there is safe. The only pod-IP-bearing input was the v0.2.5 options-scope allow-notify; it is gone. RenderInput no longer has any pod-IP field.

Regression test (permanent, loop class killed)

TestConfigHashIndependentOfPrimaryPodIP (controller): reconciles the ConfigMap with the primary pod on two different IPs (stable primary Service ClusterIP held constant) and asserts the config-hash is byte-identical. Fault-injection verified: reintroducing any primary-pod-IP dependency into restart-scoped config makes this test FAIL.
TestRenderRestartScopedConfigIndependentOfPodIPs + TestRenderSecondaryAllowNotifyByKey (render): assert no pod/service IP appears in allow-notify and the render is stable.
Existing also-notify tests updated to assert keyed entries; NOTIFY-acceptance behaviour stays covered.

go build ./..., go test ./..., go vet ./..., gofmt — all clean.

Rollout

Tag v0.2.6 → bump the argocd image. The deployment is currently pinned to v0.2.4 by revert #291; the bump to v0.2.6 re-enables immediate intra-cluster NOTIFY without the roll loop. (This PR does not tag or release.)

## Why v0.2.5 (PR #14) added an **options-scope** `allow-notify { <primaryPodIP>; <transferAddr>; }` on secondaries so intra-cluster NOTIFYs (which egress with the primary's *pod* IP as source, since k8s Services only NAT inbound) would be accepted. But options-scope config feeds the **config-hash annotation** that rolls the StatefulSet. So: > config change → pods roll → primary comes back on a **new pod IP** → operator re-renders `allow-notify` with the new IP → **new hash** → pods roll → new IP → **infinite roll loop across every BindCluster** (externaldns, authoritative, resolvers all cycled). The production **deployment** was reverted to v0.2.4 (argocd revert #291); the v0.2.5 **code** stayed on `main`. This PR fixes it in place for v0.2.6. **Constraint learned:** nothing rendered into restart-scoped (config-hash) config may depend on pod IPs. ## Chosen design — A: TSIG-signed NOTIFYs, fully static secondary config - **Secondary:** render `allow-notify { key "<name>"; };` — an address-match-list **key element with NO IPs**. It depends only on the key name, so pod-IP churn can never change the render, the hash, or trigger a restart. (BIND 9.20 accepts a `key` element in an address-match-list.) - **Primary:** sign outgoing NOTIFYs. The zone-scope `also-notify` entries (already enumerating replica pod IPs, applied via `rndc addzone`/`modzone` with **no restart**) now carry `key "<name>"`: `also-notify { <replicaIP> key "<name>"; ... };`. (BIND 9.20 supports keyed `also-notify` entries.) - **Key choice:** reuse the cluster's **catalog transfer TSIG key** (`BindCatalogZone.spec.transferKeyRef`). Secondaries already present this key for AXFR (it is in the secondary's `catalog-zones default-primaries { <ip> key "<name>"; }`), it is materialised into `keys.conf` on **every** pod (primary and secondary), and the primary already requires `allow-transfer { key "<name>"; }`. No new key plumbing — the cluster's existing replication key doubles as the NOTIFY key. ### Why this is loop-free `also-notify` lives in **zone-scope** config applied via rndc (`BindZoneReconciler.buildZoneConfig` → `Exec.AddZone`), which never touches named.conf and never rolls pods — so listing pod IPs there is safe. The **only** pod-IP-bearing input was the v0.2.5 options-scope `allow-notify`; it is gone. `RenderInput` no longer has any pod-IP field. ## Regression test (permanent, loop class killed) - `TestConfigHashIndependentOfPrimaryPodIP` (controller): reconciles the ConfigMap with the primary pod on **two different IPs** (stable primary Service ClusterIP held constant) and asserts the **config-hash is byte-identical**. Fault-injection verified: reintroducing any primary-pod-IP dependency into restart-scoped config makes this test FAIL. - `TestRenderRestartScopedConfigIndependentOfPodIPs` + `TestRenderSecondaryAllowNotifyByKey` (render): assert no pod/service IP appears in `allow-notify` and the render is stable. - Existing also-notify tests updated to assert **keyed** entries; NOTIFY-acceptance behaviour stays covered. `go build ./...`, `go test ./...`, `go vet ./...`, gofmt — all clean. ## Rollout Tag `v0.2.6` → bump the argocd image. The deployment is currently pinned to v0.2.4 by revert #291; the bump to v0.2.6 re-enables immediate intra-cluster NOTIFY without the roll loop. (This PR does not tag or release.)

unkinben added 1 commit 2026-07-25 23:31:21 +10:00

Make intra-cluster NOTIFY loop-free (TSIG-keyed, no pod IPs in restart config)

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

ci/woodpecker/pr/test Pipeline was successful

Details

ci/woodpecker/pr/build Pipeline was successful

Details

aab11457af

v0.2.5 (PR #14) added an options-scope allow-notify enumerating the primary
pod IP on secondaries. Options-scope config feeds the config-hash annotation
that rolls the StatefulSet, so any config change rolled the pods, the primary
came back on a new pod IP, the operator re-rendered with the new IP, the hash
changed, the pods rolled again — an infinite roll loop across every
BindCluster. The prod deployment was reverted to v0.2.4.

Replace the pod-IP allow-notify with TSIG-authenticated NOTIFY:

- Secondaries render `allow-notify { key "<name>"; };` — a static key element
  with NO IPs. It depends only on the key name, so pod-IP churn can never
  change the render, the config-hash, or trigger a restart.
- The primary signs its outgoing NOTIFYs: the zone-scope also-notify entries
  (already enumerating replica pod IPs, applied via rndc addzone/modzone with
  NO restart) now carry `key "<name>"`.
- Key choice: reuse the cluster's catalog transfer TSIG key (TransferKeyRef).
  Secondaries already present it for AXFR and it is in keys.conf on every pod,
  so no new key plumbing is needed.

Add a permanent regression guard for the loop class:
- controller: reconcile the ConfigMap with the primary pod on two different
  IPs and assert the config-hash is byte-identical.
- render: render restart-scoped input and assert no pod IP appears in
  allow-notify; RenderInput no longer has any pod-IP field.

Zone-scope also-notify (rndc, no restart) legitimately still lists pod IPs;
only restart-scoped config must be pod-IP-independent.

unkinben force-pushed benvin/notify-fix-loopfree from 61324ae89a to aab11457af

2026-07-25 23:31:21 +10:00

Compare

benvin merged commit 2894d85c60 into main

2026-07-25 23:45:45 +10:00

benvin deleted branch benvin/notify-fix-loopfree

2026-07-25 23:45:46 +10:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/bind-operator#15