Merge pull request 'Accept intra-cluster NOTIFY on secondaries via allow-notify' (#14) from benvin/allow-notify-intra-cluster into main

Reviewed-on: #14

internal/bind/render.go

@@ -22,6 +22,15 @@ 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
 }
 
 // RenderNamedConf returns the primary and secondary named.conf contents for a
@@ -86,6 +95,12 @@ 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.
+	b.WriteString(allowNotifyClause(in, isPrimary, "    "))
 	for _, o := range c.Spec.ExtraOptions {
 		b.WriteString("    " + strings.TrimRight(o, ";") + ";\n")
 	}
@@ -277,6 +292,39 @@ func transferPrimaries(in RenderInput) []string {
 	return out
 }
 
+// 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.
+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 {
+		return ""
+	}
+	sort.Strings(addrs)
+	return fmt.Sprintf("%sallow-notify { %s };\n", indent, terminate(addrs))
+}
+
 func catalogZonesClause(in RenderInput, isPrimary bool, indent string) string {
 	// Only secondaries consume the catalog to auto-provision member zones.
 	if in.Catalog == nil || isPrimary {

internal/bind/render_test.go

@@ -98,6 +98,34 @@ func TestRenderCatalogPrimariesCarryTransferKey(t *testing.T) {
 	}
 }
 
+func TestRenderSecondaryAllowNotifyPrimaryPodIP(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.
+	in := RenderInput{
+		Cluster:             newCluster(bindv1alpha1.ModeAuthoritative),
+		PrimaryAddress:      "10.43.5.5",
+		PrimaryPodAddresses: []string{"10.42.3.197"},
+	}
+	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(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)}
+	_, 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)
+	}
+}
+
 func TestRenderForwardZoneInView(t *testing.T) {
 	rec := true
 	in := RenderInput{

internal/bind/seed.go

@@ -35,13 +35,19 @@ func (e *Executor) ZoneExists(ctx context.Context, namespace, pod, zone, view st
 func (e *Executor) WriteSeedZone(ctx context.Context, namespace, pod, zone, path, primaryIP string, serial int64) error {
 	origin := dot(zone)
 	ns := "ns1." + origin
+	// Short refresh/retry so a secondary that misses a NOTIFY (e.g. its pod IP
+	// changed and the primary's also-notify was briefly stale) still converges
+	// in minutes, not the hour a 3600s refresh would impose. minimum is the
+	// negative-cache TTL: keep it low so a stale-secondary NXDOMAIN does not
+	// stick in downstream resolvers for long. NOTIFY (also-notify on the
+	// primary) remains the fast path; these are the fallback.
 	content := fmt.Sprintf(`$TTL 3600
 @ IN SOA %s hostmaster.%s (
     %d ; serial
-    3600       ; refresh
-    900        ; retry
+    300        ; refresh
+    60         ; retry
     1209600    ; expire
-    300 )      ; minimum
+    60 )       ; minimum
 @ IN NS %s
 ns1 IN A %s
 `, ns, origin, serial, ns, primaryIP)

internal/controller/bindcluster_controller.go

@@ -176,6 +176,14 @@ 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 {

internal/controller/bindzone_controller.go

@@ -80,7 +80,15 @@ func (r *BindZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		return r.setPhase(ctx, &zone, "Pending", "PrimaryNotReady", "waiting for cluster primary to be ready")
 	}
 
-	zoneConfig, err := r.buildZoneConfig(ctx, &zone, r.zoneTransferKeyRef(ctx, &zone, cluster))
+	// Primary zones replicated to secondaries (catalog members) get an
+	// also-notify pointing at the secondary pods, so a dynamic update NOTIFYs
+	// them immediately rather than waiting for the SOA refresh.
+	var notifyTargets []string
+	if isPrimaryType(zone.Spec.Type) && catalogEnabled(&zone) {
+		notifyTargets = secondaryPodIPs(ctx, r.Client, cluster)
+	}
+
+	zoneConfig, err := r.buildZoneConfig(ctx, &zone, r.zoneTransferKeyRef(ctx, &zone, cluster), notifyTargets)
 	if err != nil {
 		return r.setPhase(ctx, &zone, "Error", "ConfigError", err.Error())
 	}
@@ -135,7 +143,7 @@ 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) (string, error) {
+func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1alpha1.BindZone, transferKey string, notifyTargets []string) (string, error) {
 	zType := zone.Spec.Type
 	if zType == "" {
 		zType = bindv1alpha1.ZonePrimary
@@ -154,6 +162,12 @@ func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1al
 			// Catalog member: permit key-authenticated AXFR from secondaries.
 			parts = append(parts, fmt.Sprintf("allow-transfer { key \"%s\"; }", transferKey))
 		}
+		// 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.
+		if len(notifyTargets) > 0 {
+			parts = append(parts, "notify explicit", fmt.Sprintf("also-notify { %s }", terminateInline(notifyTargets)))
+		}
 		if zone.Spec.DNSSECPolicyRef != "" {
 			parts = append(parts, fmt.Sprintf("dnssec-policy \"%s\"", zone.Spec.DNSSECPolicyRef), "inline-signing yes")
 		}

internal/controller/bindzone_notify_test.go

@@ -0,0 +1,48 @@
+package controller
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	bindv1alpha1 "git.unkin.net/unkin/bind-operator/api/v1alpha1"
+)
+
+// A primary zone with known secondaries renders notify explicit + also-notify
+// so a dynamic update NOTIFYs the secondaries immediately.
+func TestBuildZoneConfigPrimaryAlsoNotify(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", "10.42.1.5"})
+	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)
+	}
+}
+
+// With no secondaries, no also-notify is emitted (single-replica cluster).
+func TestBuildZoneConfigPrimaryNoNotifyTargets(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", nil)
+	if err != nil {
+		t.Fatalf("buildZoneConfig: %v", err)
+	}
+	if strings.Contains(cfg, "also-notify") || strings.Contains(cfg, "notify explicit") {
+		t.Errorf("did not expect notify clauses with no targets: %q", cfg)
+	}
+}

internal/controller/helpers.go

@@ -3,6 +3,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"sort"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
@@ -126,6 +127,34 @@ func primaryTransferAddress(ctx context.Context, c client.Client, cluster *bindv
 	return primaryPodIP(ctx, c, cluster)
 }
 
+// secondaryPodIPs returns the pod IPs of a cluster's secondary pods (every pod
+// except the ordinal-0 primary) that currently have an address. The primary
+// uses this list as its zone `also-notify` set, so a change to a primary zone
+// (in particular a dynamic update) triggers an immediate NOTIFY -> IXFR to the
+// secondaries instead of leaving them stale until the next SOA refresh. The
+// list is sorted so the rendered zone config is stable and does not churn
+// modzone on every reconcile. Pod IPs change across restarts, so the caller
+// relies on the zone controller's periodic requeue to refresh the set (a
+// restarted secondary re-transfers the whole zone on load regardless). Returns
+// nil for a single-replica cluster.
+func secondaryPodIPs(ctx context.Context, c client.Client, cluster *bindv1alpha1.BindCluster) []string {
+	var pods corev1.PodList
+	if err := c.List(ctx, &pods, client.InNamespace(cluster.Namespace), client.MatchingLabels(commonLabels(cluster.Name))); err != nil {
+		return nil
+	}
+	primary := primaryPodName(cluster.Name)
+	var ips []string
+	for i := range pods.Items {
+		p := &pods.Items[i]
+		if p.Name == primary || p.Status.PodIP == "" {
+			continue
+		}
+		ips = append(ips, p.Status.PodIP)
+	}
+	sort.Strings(ips)
+	return ips
+}
+
 // resolveTSIG reads the material of a BindTSIGKey into TSIG credentials.
 func resolveTSIG(ctx context.Context, c client.Client, namespace, keyRef string) (bind.TSIGCreds, error) {
 	var creds bind.TSIGCreds