From aab11457afc2a74952a702d9a3c7841f27a004c3 Mon Sep 17 00:00:00 2001 From: Ben Vincent Date: Sat, 25 Jul 2026 23:22:45 +1000 Subject: [PATCH] Make intra-cluster NOTIFY loop-free (TSIG-keyed, no pod IPs in restart config) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ""; };` — 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 ""`. - 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. --- internal/bind/render.go | 78 +++++++------ internal/bind/render_test.go | 82 ++++++++++--- internal/controller/bindcluster_controller.go | 18 +-- internal/controller/bindzone_controller.go | 22 +++- internal/controller/bindzone_notify_test.go | 25 +++- internal/controller/config_hash_podip_test.go | 108 ++++++++++++++++++ internal/controller/zone_helpers.go | 32 +++++- 7 files changed, 295 insertions(+), 70 deletions(-) create mode 100644 internal/controller/config_hash_podip_test.go diff --git a/internal/bind/render.go b/internal/bind/render.go index 620f91a..0c03a37 100644 --- a/internal/bind/render.go +++ b/internal/bind/render.go @@ -22,15 +22,20 @@ type RenderInput struct { Forwards []bindv1alpha1.BindZone // PrimaryAddress is the in-cluster address secondaries transfer from. PrimaryAddress string - // PrimaryPodAddresses are the primary pod's own IP(s). Secondaries transfer - // from PrimaryAddress (the stable primary Service ClusterIP) but the primary - // pod's NOTIFYs egress with its *pod* IP as the source — k8s Services only - // NAT the inbound direction — so BIND, whose implicit allow-notify is the - // zone's primaries list (the ClusterIP), REFUSES them as "non-primary" and - // replication falls back to the SOA refresh timer. Secondaries render these - // into an options-scope allow-notify so intra-cluster NOTIFYs are accepted - // immediately. Empty leaves BIND's default behaviour unchanged. - PrimaryPodAddresses []string + // NotifyKeyName is the TSIG key name secondaries accept intra-cluster NOTIFYs + // under. The primary pod's NOTIFYs egress with its *pod* IP as the source — k8s + // Services only NAT the inbound direction — so BIND, whose implicit + // allow-notify is the zone's primaries list (the stable ClusterIP), REFUSES + // them as "non-primary" and replication falls back to the SOA refresh timer. + // + // The primary signs its outgoing NOTIFYs with this key (the zone-scope + // also-notify entries, applied via rndc, carry `key ""`), and + // secondaries render `allow-notify { key ""; }` — an address-match-list + // with a *key* element and NO IPs. Because it names only the key, this clause + // is fully static: it never changes when a pod IP churns, so it cannot feed a + // changed config-hash and cannot roll the StatefulSet (the v0.2.5 loop). Empty + // leaves BIND's default behaviour unchanged. + NotifyKeyName string } // RenderNamedConf returns the primary and secondary named.conf contents for a @@ -95,11 +100,13 @@ func render(in RenderInput, isPrimary bool) string { b.WriteString(" allow-new-zones yes;\n") } b.WriteString(" dnssec-validation auto;\n") - // Secondaries accept NOTIFY from the primary's pod IP(s). Catalog member and - // plain secondary zones take their implicit allow-notify from their primaries - // (the primary Service ClusterIP), but the primary's NOTIFYs are sourced from - // its pod IP, so an options-scope allow-notify covering the pod IP(s) is - // needed or every NOTIFY is refused and replication waits for the SOA refresh. + // Secondaries accept NOTIFY authenticated by the cluster's NOTIFY TSIG key. + // Catalog member and plain secondary zones take their implicit allow-notify + // from their primaries (the primary Service ClusterIP), but the primary's + // NOTIFYs are sourced from its pod IP, so they are refused as "non-primary" + // unless an explicit allow-notify covers them. Rather than enumerate pod IPs + // (restart-scoped config that depends on pod IPs — the v0.2.5 roll loop), we + // accept any NOTIFY signed with the cluster key: `allow-notify { key "X"; }`. b.WriteString(allowNotifyClause(in, isPrimary, " ")) for _, o := range c.Spec.ExtraOptions { b.WriteString(" " + strings.TrimRight(o, ";") + ";\n") @@ -293,36 +300,31 @@ func transferPrimaries(in RenderInput) []string { } // allowNotifyClause renders an options-scope allow-notify on secondaries that -// permits the primary pod IP(s). Zones (catalog members and plain secondaries) -// point their primaries at the primary Service ClusterIP for stable AXFR, which -// also becomes their implicit allow-notify — but NOTIFYs leave the primary pod -// with its pod IP as source, so without this they are refused as "non-primary". -// Emitted only on secondaries and only when the primary pod IP(s) are known. +// accepts intra-cluster NOTIFYs authenticated by the cluster's NOTIFY TSIG key. +// Zones (catalog members and plain secondaries) point their primaries at the +// primary Service ClusterIP for stable AXFR, which also becomes their implicit +// allow-notify — but NOTIFYs leave the primary pod with its *pod* IP as source, +// so without this they are refused as "non-primary". The primary signs those +// NOTIFYs with NotifyKeyName (see the also-notify entries applied via rndc), and +// this clause admits them by key. +// +// The rendered clause is `allow-notify { key ""; };` — a key element with +// NO IP addresses. It is therefore fully static: it depends only on the key +// name, never on any pod IP, so it can never change the config-hash and can +// never trigger a rolling restart. This is the fix for the v0.2.5 loop, where an +// options-scope allow-notify enumerating the primary pod IP re-rendered on every +// pod churn, flipped the hash, rolled the pods, changed the IP, and looped. +// +// Emitted only on secondaries and only when the NOTIFY key name is known. func allowNotifyClause(in RenderInput, isPrimary bool, indent string) string { if isPrimary { return "" } - addrs := make([]string, 0, len(in.PrimaryPodAddresses)+1) - seen := map[string]bool{} - for _, a := range in.PrimaryPodAddresses { - a = strings.TrimSpace(strings.TrimRight(a, ";")) - if a == "" || seen[a] { - continue - } - seen[a] = true - addrs = append(addrs, a) - } - // Keep the transfer address (the Service ClusterIP, or the pod IP when no - // primary Service exists) in the set: an explicit allow-notify replaces the - // implicit primaries-derived default, so it must still cover that source. - if a := strings.TrimSpace(strings.TrimRight(in.PrimaryAddress, ";")); a != "" && !seen[a] { - addrs = append(addrs, a) - } - if len(addrs) == 0 { + key := strings.TrimSpace(in.NotifyKeyName) + if key == "" { return "" } - sort.Strings(addrs) - return fmt.Sprintf("%sallow-notify { %s };\n", indent, terminate(addrs)) + return fmt.Sprintf("%sallow-notify { key \"%s\"; };\n", indent, key) } func catalogZonesClause(in RenderInput, isPrimary bool, indent string) string { diff --git a/internal/bind/render_test.go b/internal/bind/render_test.go index 2418ad8..78808ba 100644 --- a/internal/bind/render_test.go +++ b/internal/bind/render_test.go @@ -98,31 +98,87 @@ func TestRenderCatalogPrimariesCarryTransferKey(t *testing.T) { } } -func TestRenderSecondaryAllowNotifyPrimaryPodIP(t *testing.T) { +func TestRenderSecondaryAllowNotifyByKey(t *testing.T) { // Secondaries transfer from the primary Service ClusterIP but the primary's - // NOTIFYs arrive from its pod IP, so an options allow-notify must cover the - // pod IP (and keep the transfer address) or BIND refuses them as non-primary. + // 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", - PrimaryPodAddresses: []string{"10.42.3.197"}, + Cluster: newCluster(bindv1alpha1.ModeAuthoritative), + PrimaryAddress: "10.43.5.5", + NotifyKeyName: "externaldns-key", } primary, secondary := RenderNamedConf(in) - if !strings.Contains(secondary, "allow-notify { 10.42.3.197; 10.43.5.5; };") { - t.Fatalf("secondary allow-notify must cover the primary pod IP and transfer address:\n%s", secondary) + 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 TestRenderSecondaryAllowNotifyOmittedWhenPodIPUnknown(t *testing.T) { - // With no primary pod IP known there is nothing to add beyond BIND's implicit - // primaries-derived default; emit nothing rather than a bare/duplicate clause. - in := RenderInput{Cluster: newCluster(bindv1alpha1.ModeAuthoritative)} +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 primary addresses are known:\n%s", secondary) + 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) } } diff --git a/internal/controller/bindcluster_controller.go b/internal/controller/bindcluster_controller.go index 9918774..8cf8c56 100644 --- a/internal/controller/bindcluster_controller.go +++ b/internal/controller/bindcluster_controller.go @@ -176,14 +176,6 @@ func (r *BindClusterReconciler) reconcileConfigMap(ctx context.Context, c *bindv // across primary pod restarts (falls back to the pod IP when no primary // Service exists; the Pod/Service watches re-render when it changes). in := bind.RenderInput{Cluster: c, PrimaryAddress: primaryTransferAddress(ctx, r.Client, c)} - // Secondaries transfer from the stable primary Service ClusterIP, but the - // primary pod's NOTIFYs are sourced from its pod IP, which BIND refuses as - // "non-primary" unless it appears in allow-notify. Render the primary pod IP - // so intra-cluster NOTIFYs are accepted immediately (the Pod watch re-renders - // the ConfigMap when the pod IP changes across restarts). - if ip := primaryPodIP(ctx, r.Client, c); ip != "" { - in.PrimaryPodAddresses = []string{ip} - } var acls bindv1alpha1.BindACLList if err := r.List(ctx, &acls, client.InNamespace(c.Namespace)); err == nil { @@ -239,6 +231,16 @@ func (r *BindClusterReconciler) reconcileConfigMap(ctx context.Context, c *bindv } } + // Secondaries accept intra-cluster NOTIFYs signed with the cluster's catalog + // transfer TSIG key (the primary signs its also-notify NOTIFYs with it — see + // bindzone_controller). Render `allow-notify { key ""; }` — a static key + // element, NO pod IPs — so it never changes on pod churn and cannot re-render + // the restart-scoped config (the v0.2.5 roll loop). Resolve the catalog's + // TransferKeyRef to the BIND key name (KeyName override, else the ref). + if in.Catalog != nil && in.Catalog.Spec.TransferKeyRef != "" { + in.NotifyKeyName = tsigKeyName(ctx, r.Client, c.Namespace, in.Catalog.Spec.TransferKeyRef) + } + primaryConf, secondaryConf := bind.RenderNamedConf(in) data := map[string]string{ "named.conf.primary": primaryConf, diff --git a/internal/controller/bindzone_controller.go b/internal/controller/bindzone_controller.go index 1c9a0f6..b06a61b 100644 --- a/internal/controller/bindzone_controller.go +++ b/internal/controller/bindzone_controller.go @@ -88,7 +88,13 @@ func (r *BindZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c notifyTargets = secondaryPodIPs(ctx, r.Client, cluster) } - zoneConfig, err := r.buildZoneConfig(ctx, &zone, r.zoneTransferKeyRef(ctx, &zone, cluster), notifyTargets) + // The catalog transfer TSIG key doubles as the intra-cluster NOTIFY key: the + // primary signs its also-notify NOTIFYs with it and secondaries accept them + // via `allow-notify { key ""; }`. Keying the NOTIFYs is what lets the + // secondary's allow-notify be a static key element (no pod IPs), so pod-IP + // churn never re-renders restart-scoped config (the v0.2.5 roll loop). + transferKey := r.zoneTransferKeyRef(ctx, &zone, cluster) + zoneConfig, err := r.buildZoneConfig(ctx, &zone, transferKey, notifyTargets, transferKey) if err != nil { return r.setPhase(ctx, &zone, "Error", "ConfigError", err.Error()) } @@ -142,8 +148,10 @@ func (r *BindZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // buildZoneConfig renders the inner clause passed to rndc addzone/modzone. // transferKey, when set, is the catalog transfer TSIG key name; catalog member -// primary zones must allow AXFR with it so secondaries can pull them. -func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1alpha1.BindZone, transferKey string, notifyTargets []string) (string, error) { +// primary zones must allow AXFR with it so secondaries can pull them. notifyKey, +// when set, is the TSIG key each also-notify entry is signed with, so +// secondaries can accept the NOTIFYs by key rather than by (churning) pod IP. +func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1alpha1.BindZone, transferKey string, notifyTargets []string, notifyKey string) (string, error) { zType := zone.Spec.Type if zType == "" { zType = bindv1alpha1.ZonePrimary @@ -164,9 +172,13 @@ func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1al } // NOTIFY only the secondaries we know about (their apex NS is the primary // itself, so default `notify yes` would reach no one). `notify explicit` - // keeps NOTIFY off the query-serving VIP and scoped to the pod IPs. + // keeps NOTIFY off the query-serving VIP and scoped to the pod IPs. Each + // entry is signed with notifyKey so secondaries can admit the NOTIFY by key + // (`allow-notify { key ... }`) instead of by pod IP. This zone config is + // applied via rndc addzone/modzone — no pod restart — so listing pod IPs + // here is safe; only *restart-scoped* config must never depend on pod IPs. if len(notifyTargets) > 0 { - parts = append(parts, "notify explicit", fmt.Sprintf("also-notify { %s }", terminateInline(notifyTargets))) + parts = append(parts, "notify explicit", fmt.Sprintf("also-notify { %s }", alsoNotifyList(notifyTargets, notifyKey))) } if zone.Spec.DNSSECPolicyRef != "" { parts = append(parts, fmt.Sprintf("dnssec-policy \"%s\"", zone.Spec.DNSSECPolicyRef), "inline-signing yes") diff --git a/internal/controller/bindzone_notify_test.go b/internal/controller/bindzone_notify_test.go index a5502a8..806778f 100644 --- a/internal/controller/bindzone_notify_test.go +++ b/internal/controller/bindzone_notify_test.go @@ -19,15 +19,32 @@ func TestBuildZoneConfigPrimaryAlsoNotify(t *testing.T) { }, } - cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", []string{"10.42.2.6", "10.42.1.5"}) + cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", []string{"10.42.2.6", "10.42.1.5"}, "externaldns-key") if err != nil { t.Fatalf("buildZoneConfig: %v", err) } if !strings.Contains(cfg, "notify explicit") { t.Errorf("expected notify explicit in %q", cfg) } - if !strings.Contains(cfg, "also-notify { 10.42.2.6; 10.42.1.5; }") { - t.Errorf("expected also-notify with the secondary IPs in %q", cfg) + // also-notify entries are keyed so secondaries can admit the NOTIFY by TSIG + // key (allow-notify { key ... }) instead of by (churning) pod IP. + if !strings.Contains(cfg, `also-notify { 10.42.2.6 key "externaldns-key"; 10.42.1.5 key "externaldns-key"; }`) { + t.Errorf("expected keyed also-notify with the secondary IPs in %q", cfg) + } +} + +// also-notify entries carry no key when none is configured (unkeyed NOTIFY). +func TestBuildZoneConfigPrimaryAlsoNotifyUnkeyed(t *testing.T) { + r := &BindZoneReconciler{} + zone := &bindv1alpha1.BindZone{ + Spec: bindv1alpha1.BindZoneSpec{ZoneName: "main.unkin.net", Type: bindv1alpha1.ZonePrimary}, + } + cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", []string{"10.42.2.6"}, "") + if err != nil { + t.Fatalf("buildZoneConfig: %v", err) + } + if !strings.Contains(cfg, "also-notify { 10.42.2.6; }") { + t.Errorf("expected unkeyed also-notify in %q", cfg) } } @@ -38,7 +55,7 @@ func TestBuildZoneConfigPrimaryNoNotifyTargets(t *testing.T) { Spec: bindv1alpha1.BindZoneSpec{ZoneName: "main.unkin.net", Type: bindv1alpha1.ZonePrimary}, } - cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", nil) + cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", nil, "externaldns-key") if err != nil { t.Fatalf("buildZoneConfig: %v", err) } diff --git a/internal/controller/config_hash_podip_test.go b/internal/controller/config_hash_podip_test.go new file mode 100644 index 0000000..d79e6d9 --- /dev/null +++ b/internal/controller/config_hash_podip_test.go @@ -0,0 +1,108 @@ +package controller + +import ( + "context" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + bindv1alpha1 "git.unkin.net/unkin/bind-operator/api/v1alpha1" +) + +// TestConfigHashIndependentOfPrimaryPodIP is the permanent regression guard for +// the v0.2.5 rolling-restart loop. +// +// v0.2.5 rendered an options-scope `allow-notify { ; ... }` into +// the (restart-scoped) named.conf. The config-hash annotation that rolls the +// StatefulSet is computed over that render (BindClusterReconciler.configHash), so +// a config change rolled the pods, the primary pod came back on a NEW IP, the +// operator re-rendered with the new IP, the hash changed, the pods rolled again, +// and so on — an infinite roll loop across every BindCluster. +// +// The fix removed all pod IPs from restart-scoped config (secondaries now admit +// intra-cluster NOTIFYs by TSIG key: `allow-notify { key "X"; }`). This test +// reconciles the ConfigMap with the primary pod on one IP, computes the hash, +// then does it again with the primary pod on a DIFFERENT IP, and asserts the +// hash is byte-identical. If any pod-IP dependency ever creeps back into +// restart-scoped config, this fails and the loop-class bug is caught. +func TestConfigHashIndependentOfPrimaryPodIP(t *testing.T) { + scheme := runtime.NewScheme() + if err := clientgoscheme.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + if err := bindv1alpha1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + const ns = "dns" + svcIP := "10.43.5.5" // stable primary Service ClusterIP (does NOT churn) + + cluster := &bindv1alpha1.BindCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "auth", Namespace: ns}, + Spec: bindv1alpha1.BindClusterSpec{ + Mode: bindv1alpha1.ModeAuthoritative, + Replicas: 3, + PrimaryService: &bindv1alpha1.ClusterServiceSpec{}, + }, + } + primarySvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: primaryServiceName(cluster.Name), Namespace: ns}, + Spec: corev1.ServiceSpec{ClusterIP: svcIP}, + } + catalog := &bindv1alpha1.BindCatalogZone{ + ObjectMeta: metav1.ObjectMeta{Name: "cat", Namespace: ns}, + Spec: bindv1alpha1.BindCatalogZoneSpec{ + ClusterRef: cluster.Name, + ZoneName: "catalog.internal", + TransferKeyRef: "externaldns-key", + }, + } + tsig := &bindv1alpha1.BindTSIGKey{ + ObjectMeta: metav1.ObjectMeta{Name: "externaldns-key", Namespace: ns}, + Spec: bindv1alpha1.BindTSIGKeySpec{ClusterRef: cluster.Name}, + } + + // hashWithPrimaryIP reconciles the ConfigMap with the primary pod carrying the + // given IP, then returns the resulting config-hash. + hashWithPrimaryIP := func(ip string) string { + primaryPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: primaryPodName(cluster.Name), Namespace: ns, Labels: commonLabels(cluster.Name)}, + Status: corev1.PodStatus{PodIP: ip}, + } + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(cluster, primarySvc, catalog, tsig, primaryPod). + Build() + r := &BindClusterReconciler{Client: c, Scheme: scheme} + ctx := context.Background() + if err := r.reconcileConfigMap(ctx, cluster); err != nil { + t.Fatalf("reconcileConfigMap: %v", err) + } + // Sanity: the pod IP must not have leaked into the rendered config. + var cm corev1.ConfigMap + if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: configMapName(cluster.Name)}, &cm); err != nil { + t.Fatalf("get configmap: %v", err) + } + for k, v := range cm.Data { + if ip != "" && strings.Contains(v, ip) { + t.Fatalf("primary pod IP %s leaked into restart-scoped config %s:\n%s", ip, k, v) + } + } + return r.configHash(ctx, cluster) + } + + h1 := hashWithPrimaryIP("10.42.3.197") + h2 := hashWithPrimaryIP("10.42.9.42") // primary pod restarted onto a new IP + if h1 == "" { + t.Fatal("config hash should not be empty") + } + if h1 != h2 { + t.Fatalf("config hash changed when only the primary pod IP changed — the v0.2.5 roll loop:\n%s\n%s", h1, h2) + } +} diff --git a/internal/controller/zone_helpers.go b/internal/controller/zone_helpers.go index 340184f..7af8c5f 100644 --- a/internal/controller/zone_helpers.go +++ b/internal/controller/zone_helpers.go @@ -2,6 +2,7 @@ package controller import ( "context" + "fmt" "strings" "sigs.k8s.io/controller-runtime/pkg/client" @@ -58,12 +59,19 @@ func recordsToUpdates(zone string, records []bindv1alpha1.Record, defaultTTL int // updateKeyName returns the TSIG key name (as used in named.conf) for a zone's // update key, falling back to the object name. func updateKeyName(ctx context.Context, c client.Client, zone *bindv1alpha1.BindZone) string { - ref := zone.Spec.UpdateKeyRef + return tsigKeyName(ctx, c, zone.Namespace, zone.Spec.UpdateKeyRef) +} + +// tsigKeyName resolves a BindTSIGKey object reference to the TSIG key name used +// in named.conf (the KeyName override when set, otherwise the object name). +// Returns "" for an empty ref, and falls back to the ref if the object cannot be +// read. +func tsigKeyName(ctx context.Context, c client.Client, namespace, ref string) string { if ref == "" { return "" } var key bindv1alpha1.BindTSIGKey - if err := c.Get(ctx, client.ObjectKey{Namespace: zone.Namespace, Name: ref}, &key); err != nil { + if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ref}, &key); err != nil { return ref } if key.Spec.KeyName != "" { @@ -86,3 +94,23 @@ func terminateInline(entries []string) string { } return strings.Join(parts, " ") } + +// alsoNotifyList renders also-notify entries, each optionally annotated with a +// TSIG key so the primary signs its NOTIFYs and secondaries can accept them by +// key (`allow-notify { key ... }`) rather than by pod IP. An entry that already +// carries a `key` clause is left untouched. +func alsoNotifyList(addrs []string, key string) string { + key = strings.TrimSpace(key) + var parts []string + for _, a := range addrs { + a = strings.TrimSpace(strings.TrimRight(a, ";")) + if a == "" { + continue + } + if key != "" && !strings.Contains(a, " key ") { + a = fmt.Sprintf("%s key \"%s\"", a, key) + } + parts = append(parts, a+";") + } + return strings.Join(parts, " ") +}