Reconcile zone apex NS off the pod IP placeholder #22
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: unkin/bind-operator#22
Reference in New Issue
Block a user
Delete Branch "benvin/zone-nameservers"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Every primary zone was seeded with an apex
NS ns1.<zone>glued to the primary pod IP, and nothing replaced it: BIND ignores an RRset-wide delete at a zone apex, so neither aspec.recordsapex NS nor a later reconcile could displace it. acme.unkin.net has served a stale rke2 pod IP to public resolvers for 54 days.BindZone.spec.nameservers, the names published at the apexspec.recordsapex NS into that path, so it replaces instead of appendingCloses #21
apexNSUpdateshas no live-state input (pure function of spec) andbind.NSUpdatealways emitsupdate delete+update addunconditionally; every primary zone with resolvable creds gets a real dynamic-update transaction every reconcile (requeueLong = 2m,internal/controller/helpers.go:20) forever, whether or not the apex NS RRset actually changed → bump the SOA serial / send the delete+add only when the desired values differ from what is currently published (query current RRset, or hash-compare against a status field) before calling NSUpdate.len(zone.Spec.Nameservers) > 0; a zone that never setsspec.nameservers(the exact acme.unkin.net scenario in the PR description, using only the primary-address fallback) gets its apex NS repointed but the stalens1 A <pod-ip>record is never deleted → also run the glue cleanup when nameservers falls back to the default, not only when explicitly declared.Ready/Provisionedwhile the delegation bug this PR closes is still unfixed for it, with no operator-visible signal → surface at least a status condition or count of failed apex syncs.owns()compares record/nameserver names viafqdn()with no case-folding (unlike the type comparison, which usesEqualFold); mixed-case zone or record names inspec.nameservers/spec.recordswill fail to match and reintroduce the ns1/@ NS churn.ApexNSqueriesdig +short @127.0.0.1 <zone> NSwith no view selector, unlike every other rndc-based call in this file (ZoneSerial/AddZone/DelZone/ZoneExistsall take/threadview). For a zone bound to a BindView whose match-clients does not route the pods own loopback query into that view (exactlyconfig/samples/04-view.yamls split-horizon setup), the unsigned dig gets REFUSED with exit 0 and empty output — verified live against named 9.20 with a two-view config matching that sample.ApexNSreturns(nil, nil), soapexNSUpdates(zone_helpers.go:158) always computeslive=[], meaningdel := missing(live, desired)is always empty: the stale placeholder apex NS is never deleted and the ns1 glue A never retires (zone_helpers.go:176-179 never fires). Confirmed the signedNSUpdate(TSIG key in match-clients) does land in the right view while the unsigned read never sees it — the two paths are on different sides of the view ACL, so convergence never completes for any BindView-scoped zone. → threadzone.Spec.ViewRefintoApexNSthe same way the other rndc.go calls do (rndc has azonestatus ... in <view>equivalent for NS; dig has none, so this likely needs to go through rndc/an authenticated query path instead of bare dig).zoneNameserversfolding an apex NS fromspec.Recordsinto the declared set copies onlyrec.Values, droppingrec.TTL.apexNSUpdatesis always called withzone.Spec.DefaultTTL(bindzone_controller.go:135), so a user-set TTL on that record (a supported, tested combination perTestApexNSUpdatesLeavesRecordOwnedGlue) is silently overridden by the zone default once the record is routed through the apex path. → carry the records TTL through toapexNSUpdates, or document that an apex NS entry in spec.Records cannot set its own TTL.renderSeedZone(seed.go) glues every declared nameserver that resolves in-zone, not just one named ns1. A zone ever seeded/declared with more than one in-zone nameserver leaves any non-ns1 glue A record permanently orphaned once that name drops out ofspec.nameservers.if nsDeclaredwithif truein Reconcile fails no existing test — this transition has zero coverage → converge toward the fallback (or previously-recorded status) even when undeclared, and add a Reconcile-level test for both transitions.namesis non-empty from spec.Nameservers (line 140 break), fine today, but the two sources are otherwise unordered/undocumented as to precedence beyond "nameservers wins" — worth a one-line doc note given a later refactor could invert the loop order silently.spec.nameserverson an already-existing zone (created=false) emitsupdate add <apex> NS <newname>with no glue A record — glue is only ever written by seed.go'srenderSeedZone, at zone creation only. named rejects the add outright ("post update name server sanity check failed" / REFUSED, verified live against bind9 9.20) → reconcile fails withApexNSSyncFailedon every subsequent pass, permanently. The apex-sync transaction (bindzone_controller.go:126-145) runs alone, before and separately from the records-sync transaction (bindzone_controller.go:146-155), so adding the matchingspec.recordsA record in the same edit does not help: the apex transaction still fails first and the records transaction never runs. Have apexNSUpdates seed/add glue for any newly-declared in-zone nameserver in the same transaction as the NS add.go test ./internal/... ./api/...fully green. Add a unit test that feeds the parser real dig output shapes directly (REFUSED/NXDOMAIN/SERVFAIL empty output, a real NS answer, a;;-comment line) without going through Exec/k8s..", not "line is a dig comment";strings /usr/bin/dig(9.20) turns up at least one comment that itself ends in a period (;; WARNING -- TSIG key was not used.), which this would append to the NS list as if it were an answer. Filter on the actual dig comment marker (leading;) instead of inferring it from message wording.len(live) == 0 ||is dead:del := missing(live, desired)ranges overlive, sodelis already nil wheneverliveis empty;len(del) == 0alone is equivalent. Confirmed by deleting the clause: no test fails. Drop it, or add a test that actually needs it.zoneUpdateCreds(a Secret GET) now runs unconditionally on every primary-zone reconcile, including zones with neithernsDeclarednor anyspec.records(previously gated behindlen(zone.Spec.Records) > 0). Gate it behindnsDeclared || len(zone.Spec.Records) > 0to skip the wasted API call on the common no-op zone.apexNSUpdatesdiffsdesired/liveby name only, andApexNScannot observe TTL at all (dig +shortprints bare names, no TTL). Changingspec.defaultTTLor aspec.recordsapex-NS TTL override on an already-converged zone silently never republishes the new TTL, despite the field doc's "kept in sync on every reconcile."