From 53cb7fa0595aec697eb8e13116ec90ac17929357 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Fri, 16 Oct 2026 23:32:09 +1100 Subject: [PATCH 1/2] Render empty forwarders for forward zones A forward zone with no forwarders now renders "forwarders { };", which BIND accepts and treats as disabling forwarding for the zone. --- api/v1alpha1/bindzone_types.go | 2 +- .../crd/bases/bind.unkin.net_bindzones.yaml | 3 ++- config/crd/install.yaml | 3 ++- internal/bind/render.go | 12 ++++++++--- internal/bind/render_test.go | 16 +++++++++++++++ internal/controller/bindzone_controller.go | 5 +---- internal/controller/bindzone_notify_test.go | 20 +++++++++++++++++++ 7 files changed, 51 insertions(+), 10 deletions(-) diff --git a/api/v1alpha1/bindzone_types.go b/api/v1alpha1/bindzone_types.go index e7925b3..a90c64a 100644 --- a/api/v1alpha1/bindzone_types.go +++ b/api/v1alpha1/bindzone_types.go @@ -103,7 +103,7 @@ type BindZoneSpec struct { // +optional DNSSECPolicyRef string `json:"dnssecPolicyRef,omitempty"` - // Forwarders lists upstreams for a forward-type zone. + // Forwarders lists upstreams for a forward-type zone; an empty list disables forwarding for the zone. // +optional Forwarders []string `json:"forwarders,omitempty"` diff --git a/config/crd/bases/bind.unkin.net_bindzones.yaml b/config/crd/bases/bind.unkin.net_bindzones.yaml index 11e193c..ad3cf24 100644 --- a/config/crd/bases/bind.unkin.net_bindzones.yaml +++ b/config/crd/bases/bind.unkin.net_bindzones.yaml @@ -90,7 +90,8 @@ spec: When true, UpdateKeyRef must reference a BindTSIGKey. type: boolean forwarders: - description: Forwarders lists upstreams for a forward-type zone. + description: Forwarders lists upstreams for a forward-type zone; an + empty list disables forwarding for the zone. items: type: string type: array diff --git a/config/crd/install.yaml b/config/crd/install.yaml index 19d3b03..2f23155 100644 --- a/config/crd/install.yaml +++ b/config/crd/install.yaml @@ -2756,7 +2756,8 @@ spec: When true, UpdateKeyRef must reference a BindTSIGKey. type: boolean forwarders: - description: Forwarders lists upstreams for a forward-type zone. + description: Forwarders lists upstreams for a forward-type zone; an + empty list disables forwarding for the zone. items: type: string type: array diff --git a/internal/bind/render.go b/internal/bind/render.go index 0c03a37..92b34f9 100644 --- a/internal/bind/render.go +++ b/internal/bind/render.go @@ -154,9 +154,7 @@ func renderForwardZone(z bindv1alpha1.BindZone, indent string) string { b.WriteString(fmt.Sprintf("%szone \"%s\" {\n", indent, z.Spec.ZoneName)) b.WriteString(indent + " type forward;\n") b.WriteString(indent + " forward only;\n") - if len(z.Spec.Forwarders) > 0 { - b.WriteString(fmt.Sprintf("%s forwarders { %s };\n", indent, terminate(z.Spec.Forwarders))) - } + b.WriteString(indent + " " + ForwardersClause(z.Spec.Forwarders) + ";\n") b.WriteString(indent + "};\n") return b.String() } @@ -398,6 +396,14 @@ func terminate(entries []string) string { return strings.Join(parts, " ") } +// ForwardersClause renders a zone forwarders clause; empty means "forwarders { }" (no forwarding). +func ForwardersClause(entries []string) string { + if list := terminate(entries); list != "" { + return "forwarders { " + list + " }" + } + return "forwarders { }" +} + func yesno(b bool) string { if b { return "yes" diff --git a/internal/bind/render_test.go b/internal/bind/render_test.go index 78808ba..210fcc3 100644 --- a/internal/bind/render_test.go +++ b/internal/bind/render_test.go @@ -261,3 +261,19 @@ func TestRenderDeterministicWithShuffledForwards(t *testing.T) { t.Fatalf("render must be independent of forward-zone input order:\n--- A ---\n%s\n--- B ---\n%s", pa, pb) } } + +func TestRenderForwardZoneForwarders(t *testing.T) { + for _, tc := range []struct { + fwd []string + want string + }{ + {nil, " forwarders { };\n"}, + {[]string{"198.18.19.15", "198.18.19.16;"}, " forwarders { 198.18.19.15; 198.18.19.16; };\n"}, + } { + z := bindv1alpha1.BindZone{Spec: bindv1alpha1.BindZoneSpec{ZoneName: "spamhaus.org", Type: bindv1alpha1.ZoneForward, Forwarders: tc.fwd}} + want := "zone \"spamhaus.org\" {\n type forward;\n forward only;\n" + tc.want + "};\n" + if got := renderForwardZone(z, ""); got != want { + t.Errorf("forwarders %v:\ngot %q\nwant %q", tc.fwd, got, want) + } + } +} diff --git a/internal/controller/bindzone_controller.go b/internal/controller/bindzone_controller.go index 9d02f10..214b5a5 100644 --- a/internal/controller/bindzone_controller.go +++ b/internal/controller/bindzone_controller.go @@ -218,10 +218,7 @@ func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1al parts = append(parts, fmt.Sprintf("primaries { %s }", terminateInline(zone.Spec.Primaries))) } case bindv1alpha1.ZoneForward: - parts = append(parts, "type forward", "forward only") - if len(zone.Spec.Forwarders) > 0 { - parts = append(parts, fmt.Sprintf("forwarders { %s }", terminateInline(zone.Spec.Forwarders))) - } + parts = append(parts, "type forward", "forward only", bind.ForwardersClause(zone.Spec.Forwarders)) case bindv1alpha1.ZoneStub: parts = append(parts, "type stub", fmt.Sprintf("file \"%s\"", bind.ZoneFilePath(zone.Spec.ZoneName))) if len(zone.Spec.Primaries) > 0 { diff --git a/internal/controller/bindzone_notify_test.go b/internal/controller/bindzone_notify_test.go index 806778f..ed95b7e 100644 --- a/internal/controller/bindzone_notify_test.go +++ b/internal/controller/bindzone_notify_test.go @@ -63,3 +63,23 @@ func TestBuildZoneConfigPrimaryNoNotifyTargets(t *testing.T) { t.Errorf("did not expect notify clauses with no targets: %q", cfg) } } + +func TestBuildZoneConfigForwardForwarders(t *testing.T) { + r := &BindZoneReconciler{} + for _, tc := range []struct { + fwd []string + want string + }{ + {nil, "{ type forward; forward only; forwarders { }; };"}, + {[]string{"198.18.19.15"}, "{ type forward; forward only; forwarders { 198.18.19.15; }; };"}, + } { + zone := &bindv1alpha1.BindZone{Spec: bindv1alpha1.BindZoneSpec{ZoneName: "spamhaus.org", Type: bindv1alpha1.ZoneForward, Forwarders: tc.fwd}} + cfg, err := r.buildZoneConfig(context.Background(), zone, "", nil, "") + if err != nil { + t.Fatalf("buildZoneConfig: %v", err) + } + if cfg != tc.want { + t.Errorf("forwarders %v:\ngot %q\nwant %q", tc.fwd, cfg, tc.want) + } + } +} From 651c54cf7f634ffcd30c139e5f7fcec40752e216 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Fri, 16 Oct 2026 23:37:56 +1100 Subject: [PATCH 2/2] Drop unreachable forward-zone addzone config --- internal/bind/render.go | 14 +++++--------- internal/controller/bindzone_controller.go | 5 ++++- internal/controller/bindzone_notify_test.go | 20 -------------------- 3 files changed, 9 insertions(+), 30 deletions(-) diff --git a/internal/bind/render.go b/internal/bind/render.go index 92b34f9..f283f95 100644 --- a/internal/bind/render.go +++ b/internal/bind/render.go @@ -154,7 +154,11 @@ func renderForwardZone(z bindv1alpha1.BindZone, indent string) string { b.WriteString(fmt.Sprintf("%szone \"%s\" {\n", indent, z.Spec.ZoneName)) b.WriteString(indent + " type forward;\n") b.WriteString(indent + " forward only;\n") - b.WriteString(indent + " " + ForwardersClause(z.Spec.Forwarders) + ";\n") + if list := terminate(z.Spec.Forwarders); list != "" { + b.WriteString(fmt.Sprintf("%s forwarders { %s };\n", indent, list)) + } else { + b.WriteString(indent + " forwarders { };\n") + } b.WriteString(indent + "};\n") return b.String() } @@ -396,14 +400,6 @@ func terminate(entries []string) string { return strings.Join(parts, " ") } -// ForwardersClause renders a zone forwarders clause; empty means "forwarders { }" (no forwarding). -func ForwardersClause(entries []string) string { - if list := terminate(entries); list != "" { - return "forwarders { " + list + " }" - } - return "forwarders { }" -} - func yesno(b bool) string { if b { return "yes" diff --git a/internal/controller/bindzone_controller.go b/internal/controller/bindzone_controller.go index 214b5a5..9d02f10 100644 --- a/internal/controller/bindzone_controller.go +++ b/internal/controller/bindzone_controller.go @@ -218,7 +218,10 @@ func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1al parts = append(parts, fmt.Sprintf("primaries { %s }", terminateInline(zone.Spec.Primaries))) } case bindv1alpha1.ZoneForward: - parts = append(parts, "type forward", "forward only", bind.ForwardersClause(zone.Spec.Forwarders)) + parts = append(parts, "type forward", "forward only") + if len(zone.Spec.Forwarders) > 0 { + parts = append(parts, fmt.Sprintf("forwarders { %s }", terminateInline(zone.Spec.Forwarders))) + } case bindv1alpha1.ZoneStub: parts = append(parts, "type stub", fmt.Sprintf("file \"%s\"", bind.ZoneFilePath(zone.Spec.ZoneName))) if len(zone.Spec.Primaries) > 0 { diff --git a/internal/controller/bindzone_notify_test.go b/internal/controller/bindzone_notify_test.go index ed95b7e..806778f 100644 --- a/internal/controller/bindzone_notify_test.go +++ b/internal/controller/bindzone_notify_test.go @@ -63,23 +63,3 @@ func TestBuildZoneConfigPrimaryNoNotifyTargets(t *testing.T) { t.Errorf("did not expect notify clauses with no targets: %q", cfg) } } - -func TestBuildZoneConfigForwardForwarders(t *testing.T) { - r := &BindZoneReconciler{} - for _, tc := range []struct { - fwd []string - want string - }{ - {nil, "{ type forward; forward only; forwarders { }; };"}, - {[]string{"198.18.19.15"}, "{ type forward; forward only; forwarders { 198.18.19.15; }; };"}, - } { - zone := &bindv1alpha1.BindZone{Spec: bindv1alpha1.BindZoneSpec{ZoneName: "spamhaus.org", Type: bindv1alpha1.ZoneForward, Forwarders: tc.fwd}} - cfg, err := r.buildZoneConfig(context.Background(), zone, "", nil, "") - if err != nil { - t.Fatalf("buildZoneConfig: %v", err) - } - if cfg != tc.want { - t.Errorf("forwarders %v:\ngot %q\nwant %q", tc.fwd, cfg, tc.want) - } - } -}