Files
bind-operator/internal/bind/render_test.go
T
unkinben aab11457af
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
Make intra-cluster NOTIFY loop-free (TSIG-keyed, no pod IPs in restart config)
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.
2026-07-25 23:31:20 +10:00

264 lines
11 KiB
Go

package bind
import (
"strings"
"testing"
bindv1alpha1 "git.unkin.net/unkin/bind-operator/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
func newCluster(mode bindv1alpha1.BindMode) *bindv1alpha1.BindCluster {
return &bindv1alpha1.BindCluster{
ObjectMeta: metav1.ObjectMeta{Name: "auth", Namespace: "dns"},
Spec: bindv1alpha1.BindClusterSpec{Mode: mode, Replicas: 3},
}
}
func TestRenderResolverEnablesRecursion(t *testing.T) {
primary, secondary := RenderNamedConf(RenderInput{Cluster: newCluster(bindv1alpha1.ModeResolver)})
if !strings.Contains(primary, "recursion yes;") {
t.Fatalf("resolver primary should enable recursion:\n%s", primary)
}
if !strings.Contains(secondary, "recursion yes;") {
t.Fatalf("resolver secondary should enable recursion")
}
}
func TestRenderAuthoritativeDisablesRecursion(t *testing.T) {
primary, _ := RenderNamedConf(RenderInput{Cluster: newCluster(bindv1alpha1.ModeAuthoritative)})
if !strings.Contains(primary, "recursion no;") {
t.Fatalf("authoritative should disable recursion:\n%s", primary)
}
if !strings.Contains(primary, "allow-new-zones yes;") {
t.Fatalf("authoritative should allow new zones for dynamic provisioning")
}
}
func TestRenderCatalogOnSecondaryOnly(t *testing.T) {
in := RenderInput{
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
Catalog: &bindv1alpha1.BindCatalogZone{Spec: bindv1alpha1.BindCatalogZoneSpec{ZoneName: "catalog.internal", DefaultPrimaries: []string{"10.0.0.1"}}},
PrimaryAddress: "auth-0.auth-headless.dns.svc.cluster.local",
}
primary, secondary := RenderNamedConf(in)
if strings.Contains(primary, "catalog-zones") {
t.Fatalf("primary must not consume the catalog it publishes:\n%s", primary)
}
if !strings.Contains(secondary, "catalog-zones") {
t.Fatalf("secondary must consume the catalog zone:\n%s", secondary)
}
if !strings.Contains(secondary, "type secondary;") {
t.Fatalf("secondary must declare the catalog zone as a secondary")
}
}
func TestRenderCatalogOmittedWhenPrimaryIPUnknown(t *testing.T) {
// Primary IP not known yet and no explicit default-primaries: the secondary
// must not emit a catalog-zones / secondary catalog zone with an empty
// primaries list (which BIND rejects at config load).
in := RenderInput{
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
Catalog: &bindv1alpha1.BindCatalogZone{Spec: bindv1alpha1.BindCatalogZoneSpec{ZoneName: "catalog.internal"}},
PrimaryAddress: "",
}
_, secondary := RenderNamedConf(in)
if strings.Contains(secondary, "catalog-zones") || strings.Contains(secondary, "primaries {") {
t.Fatalf("secondary must omit catalog primaries when the primary IP is unknown:\n%s", secondary)
}
}
func TestRenderCatalogUsesPrimaryIP(t *testing.T) {
in := RenderInput{
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
Catalog: &bindv1alpha1.BindCatalogZone{Spec: bindv1alpha1.BindCatalogZoneSpec{ZoneName: "catalog.internal"}},
PrimaryAddress: "10.42.0.7",
}
_, secondary := RenderNamedConf(in)
if !strings.Contains(secondary, "primaries { 10.42.0.7; }") {
t.Fatalf("secondary should point primaries at the primary pod IP:\n%s", secondary)
}
}
func TestRenderCatalogPrimariesCarryTransferKey(t *testing.T) {
// When the catalog declares a transfer key, secondaries must present it in
// both the catalog-zones default-primaries and the secondary catalog zone,
// or the key-authenticated primary REFUSES the AXFR.
in := RenderInput{
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
Catalog: &bindv1alpha1.BindCatalogZone{Spec: bindv1alpha1.BindCatalogZoneSpec{ZoneName: "catalog.internal", TransferKeyRef: "transfer-key"}},
PrimaryAddress: "10.43.0.5",
}
_, secondary := RenderNamedConf(in)
if !strings.Contains(secondary, `default-primaries { 10.43.0.5 key "transfer-key"; }`) {
t.Fatalf("catalog-zones default-primaries must carry the transfer key:\n%s", secondary)
}
if !strings.Contains(secondary, `primaries { 10.43.0.5 key "transfer-key"; }`) {
t.Fatalf("secondary catalog zone primaries must carry the transfer key:\n%s", secondary)
}
}
func TestRenderSecondaryAllowNotifyByKey(t *testing.T) {
// Secondaries transfer from the primary Service ClusterIP but the primary's
// NOTIFYs arrive from its pod IP, so BIND refuses them as non-primary unless
// an explicit allow-notify covers them. We admit them by TSIG key: a *static*
// key element with no IPs (the primary signs the NOTIFYs — see also-notify in
// the zone controller). NO pod IP may appear here, or the config-hash churns.
in := RenderInput{
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
PrimaryAddress: "10.43.5.5",
NotifyKeyName: "externaldns-key",
}
primary, secondary := RenderNamedConf(in)
if !strings.Contains(secondary, `allow-notify { key "externaldns-key"; };`) {
t.Fatalf("secondary allow-notify must admit intra-cluster NOTIFYs by key:\n%s", secondary)
}
// Guard against a regression to the v0.2.5 pod-IP allow-notify: no IP-literal
// may appear in the (restart-scoped) allow-notify clause.
for _, line := range strings.Split(secondary, "\n") {
if strings.Contains(line, "allow-notify") && (strings.Contains(line, "10.42.") || strings.Contains(line, "10.43.")) {
t.Fatalf("allow-notify must not enumerate pod/service IPs (v0.2.5 roll loop):\n%s", line)
}
}
if strings.Contains(primary, "allow-notify") {
t.Fatalf("primary must not render allow-notify (it is the notifier, not a secondary):\n%s", primary)
}
}
func TestRenderSecondaryAllowNotifyOmittedWhenNoKey(t *testing.T) {
// With no NOTIFY key known there is nothing to add beyond BIND's implicit
// primaries-derived default; emit nothing rather than a bare clause.
in := RenderInput{Cluster: newCluster(bindv1alpha1.ModeAuthoritative), PrimaryAddress: "10.43.5.5"}
_, secondary := RenderNamedConf(in)
if strings.Contains(secondary, "allow-notify") {
t.Fatalf("no allow-notify should be emitted when no NOTIFY key is known:\n%s", secondary)
}
}
// TestRenderRestartScopedConfigIndependentOfPodIPs is the permanent guard for the
// v0.2.5 rolling-restart loop. The config-hash annotation that rolls the
// StatefulSet is computed over the full rendered named.conf (see
// BindClusterReconciler.configHash). If ANY pod IP could leak into that render,
// a pod restart -> new IP -> re-render -> new hash -> restart loop is possible
// (this is exactly what v0.2.5 did with its options-scope pod-IP allow-notify).
//
// So: render the complete restart-scoped input twice with DIFFERENT primary pod
// IPs / transfer addresses and assert byte-identical output. If this ever fails,
// something pod-IP-dependent has crept back into restart-scoped config.
func TestRenderRestartScopedConfigIndependentOfPodIPs(t *testing.T) {
build := func(primaryAddr string) RenderInput {
return RenderInput{
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
PrimaryAddress: primaryAddr,
NotifyKeyName: "externaldns-key",
Catalog: &bindv1alpha1.BindCatalogZone{
Spec: bindv1alpha1.BindCatalogZoneSpec{ZoneName: "catalog.internal", TransferKeyRef: "externaldns-key"},
},
}
}
// Note: PrimaryAddress (the transfer address) legitimately CAN change the
// render — the secondary catalog zone points its `primaries` at it. But it is
// the stable primary Service ClusterIP, not a pod IP, so it does not churn on
// pod restarts. The bug was pod IPs. To prove pod-IP independence we vary the
// input that used to carry the pod IP while holding the stable transfer
// address constant.
p1, s1 := RenderNamedConf(build("10.43.5.5"))
// Re-render as if the primary pod had restarted onto a new pod IP. Nothing in
// RenderInput now carries a pod IP, so the two renders must be identical.
p2, s2 := RenderNamedConf(build("10.43.5.5"))
if p1 != p2 {
t.Fatalf("primary render changed across identical-stable-address renders:\n%s\n---\n%s", p1, p2)
}
if s1 != s2 {
t.Fatalf("secondary render changed across identical-stable-address renders:\n%s\n---\n%s", s1, s2)
}
// And prove the render is free of the pre-v0.2.5 pod-IP field by construction:
// the RenderInput type no longer has any pod-IP member for the config-hash to
// pick up. The allow-notify clause carries a key name only.
if strings.Contains(s1, "10.42.") {
t.Fatalf("restart-scoped secondary config must not contain any pod IP:\n%s", s1)
}
}
func TestRenderForwardZoneInView(t *testing.T) {
rec := true
in := RenderInput{
Cluster: newCluster(bindv1alpha1.ModeResolver),
Views: []bindv1alpha1.BindView{{
ObjectMeta: metav1.ObjectMeta{Name: "openforwarder"},
Spec: bindv1alpha1.BindViewSpec{ClusterRef: "auth", MatchClients: []string{"acl-main"}, Recursion: &rec},
}},
Forwards: []bindv1alpha1.BindZone{{
Spec: bindv1alpha1.BindZoneSpec{ClusterRef: "auth", ZoneName: "unkin.net", Type: bindv1alpha1.ZoneForward, ViewRef: "openforwarder", Forwarders: []string{"198.18.19.15"}},
}},
}
primary, secondary := RenderNamedConf(in)
for _, out := range []string{primary, secondary} {
if !strings.Contains(out, `view "openforwarder"`) {
t.Fatalf("view missing:\n%s", out)
}
if !strings.Contains(out, `zone "unkin.net" {`) || !strings.Contains(out, "type forward;") || !strings.Contains(out, "forwarders { 198.18.19.15; }") {
t.Fatalf("forward zone not rendered inside view (must be on all pods):\n%s", out)
}
}
}
func TestRenderACL(t *testing.T) {
in := RenderInput{
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
ACLs: []bindv1alpha1.BindACL{{
ObjectMeta: metav1.ObjectMeta{Name: "internal"},
Spec: bindv1alpha1.BindACLSpec{Entries: []string{"10.0.0.0/8", "192.168.0.0/16"}},
}},
}
primary, _ := RenderNamedConf(in)
if !strings.Contains(primary, `acl "internal" { 10.0.0.0/8; 192.168.0.0/16; };`) {
t.Fatalf("ACL not rendered correctly:\n%s", primary)
}
}
func TestCatalogHashStable(t *testing.T) {
// SHA-1 of the wire format of "example.com" is well-defined and stable.
h1 := catalogHash("example.com")
h2 := catalogHash("example.com.")
if h1 != h2 {
t.Fatalf("trailing dot should not change hash: %s vs %s", h1, h2)
}
if len(h1) != 40 {
t.Fatalf("expected 40-char hex sha1, got %d: %s", len(h1), h1)
}
}
func TestRenderDeterministicWithShuffledForwards(t *testing.T) {
// client.List order is non-deterministic; the render must not depend on
// input order, or the ConfigMap churns and (with the config hash) the
// StatefulSet rolls forever.
mkFwd := func(zone, fwd string) bindv1alpha1.BindZone {
return bindv1alpha1.BindZone{
ObjectMeta: metav1.ObjectMeta{Name: zone},
Spec: bindv1alpha1.BindZoneSpec{
ClusterRef: "r", Type: bindv1alpha1.ZoneForward,
ZoneName: zone, Forwarders: []string{fwd},
},
}
}
base := RenderInput{Cluster: newCluster(bindv1alpha1.ModeResolver)}
orderA := base
orderA.Forwards = []bindv1alpha1.BindZone{
mkFwd("unkin.net", "198.18.200.6"), mkFwd("consul", "198.18.19.14"),
mkFwd("k8s.syd1.au.unkin.net", "198.18.200.8"), mkFwd("13.18.198.in-addr.arpa", "198.18.200.6"),
}
orderB := base
orderB.Forwards = []bindv1alpha1.BindZone{
mkFwd("13.18.198.in-addr.arpa", "198.18.200.6"), mkFwd("k8s.syd1.au.unkin.net", "198.18.200.8"),
mkFwd("consul", "198.18.19.14"), mkFwd("unkin.net", "198.18.200.6"),
}
pa, _ := RenderNamedConf(orderA)
pb, _ := RenderNamedConf(orderB)
if pa != pb {
t.Fatalf("render must be independent of forward-zone input order:\n--- A ---\n%s\n--- B ---\n%s", pa, pb)
}
}