Reject an apex NS DNSRecord instead of silently appending #24

Merged
benvin merged 1 commits from benvin/apex-ns-dnsrecord-reject into main 2026-09-27 11:23:47 +10:00
6 changed files with 56 additions and 3 deletions
+1
View File
@@ -6,6 +6,7 @@ import (
// DNSRecordSpec defines a single record set applied to a zone via TSIG dynamic
// update (nsupdate) — the external-dns write path expressed as a CRD.
// +kubebuilder:validation:XValidation:rule="!(self.type.lowerAscii() == 'ns' && (!has(self.name) || self.name == '@' || self.name.size() == 0))",message="an apex NS RRset cannot be managed by a DNSRecord: BIND ignores an RRset-wide delete at a zone apex, so this would only append. Use BindZone.spec.nameservers"
type DNSRecordSpec struct {
// ZoneRef names the BindZone this record belongs to. The cluster, view and
// update key are derived from the referenced zone.
@@ -86,6 +86,12 @@ spec:
- values
- zoneRef
type: object
x-kubernetes-validations:
- message: 'an apex NS RRset cannot be managed by a DNSRecord: BIND ignores
an RRset-wide delete at a zone apex, so this would only append. Use
BindZone.spec.nameservers'
rule: '!(self.type.lowerAscii() == ''ns'' && (!has(self.name) || self.name
== ''@'' || self.name.size() == 0))'
status:
description: DNSRecordStatus reports observed record state.
properties:
+6
View File
@@ -3022,6 +3022,12 @@ spec:
- values
- zoneRef
type: object
x-kubernetes-validations:
- message: 'an apex NS RRset cannot be managed by a DNSRecord: BIND ignores
an RRset-wide delete at a zone apex, so this would only append. Use
BindZone.spec.nameservers'
rule: '!(self.type.lowerAscii() == ''ns'' && (!has(self.name) || self.name
== ''@'' || self.name.size() == 0))'
status:
description: DNSRecordStatus reports observed record state.
properties:
+8 -1
View File
@@ -44,6 +44,8 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
}
primaryPod := primaryPodName(cluster.Name)
name := fqdn(record.Spec.Name, zone.Spec.ZoneName)
// CEL on the spec catches "@" and ""; a zone-qualified apex name reaches here.
apexNS := isApexNS(record.Spec.Name, record.Spec.Type, zone.Spec.ZoneName)
creds, err := resolveTSIG(ctx, r.Client, record.Namespace, zone.Spec.UpdateKeyRef)
if err != nil {
@@ -53,7 +55,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
// Deletion via finalizer: remove the RRset.
if !record.DeletionTimestamp.IsZero() {
if controllerutil.ContainsFinalizer(&record, finalizer) {
if primaryReady(ctx, r.Client, cluster) && r.Exec != nil {
if primaryReady(ctx, r.Client, cluster) && r.Exec != nil && !apexNS {
_ = r.Exec.NSUpdate(ctx, record.Namespace, primaryPod, zone.Spec.ZoneName, creds,
[]bind.RecordUpdate{{FQDN: name, Type: record.Spec.Type, Delete: true}})
}
@@ -65,6 +67,11 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
return ctrl.Result{}, nil
}
if apexNS {
return r.setPhase(ctx, &record, "Error", "ApexNSUnsupported",
"BIND ignores an RRset-wide delete at a zone apex, so this record can only append; publish the apex NS via BindZone.spec.nameservers")
}
if !controllerutil.ContainsFinalizer(&record, finalizer) {
controllerutil.AddFinalizer(&record, finalizer)
if err := r.Update(ctx, &record); err != nil {
+9 -2
View File
@@ -39,12 +39,19 @@ func fqdn(name, zone string) string {
return name + "." + zone
}
// isApexNS reports whether an owner/type pair addresses a zone's apex NS RRset.
// BIND ignores an RRset-wide delete there, so such a record can only append:
// the apex NS is converged per rdata from BindZone.spec.nameservers.
func isApexNS(name, typ, zone string) bool {
return strings.EqualFold(typ, "NS") && strings.EqualFold(fqdn(name, zone), fqdn("@", zone))
}
func recordsToUpdates(zone string, records []bindv1alpha1.Record, defaultTTL int32) []bind.RecordUpdate {
updates := make([]bind.RecordUpdate, 0, len(records))
for _, rec := range records {
// The apex NS RRset is converged by apexNSUpdates: an RRset-wide delete
// here is ignored by BIND and would only append to the live set.
if strings.EqualFold(rec.Type, "NS") && fqdn(rec.Name, zone) == fqdn("@", zone) {
if isApexNS(rec.Name, rec.Type, zone) {
continue
}
ttl := defaultTTL
@@ -140,7 +147,7 @@ func zoneNameservers(zone *bindv1alpha1.BindZone, cluster *bindv1alpha1.BindClus
if len(names) > 0 {
break
}
if !strings.EqualFold(rec.Type, "NS") || fqdn(rec.Name, zone.Spec.ZoneName) != fqdn("@", zone.Spec.ZoneName) {
if !isApexNS(rec.Name, rec.Type, zone.Spec.ZoneName) {
continue
}
for _, v := range rec.Values {
+26
View File
@@ -77,3 +77,29 @@ func TestCatalogEnabledDefault(t *testing.T) {
t.Error("secondary zone should never be a catalog member")
}
}
func TestIsApexNS(t *testing.T) {
const zone = "acme.unkin.net"
cases := []struct {
name, typ string
want bool
}{
{"@", "NS", true},
{"", "NS", true},
{"acme.unkin.net.", "NS", true},
{"ACME.UNKIN.NET.", "ns", true},
{"@", "ns", true},
// No trailing dot means relative: acme.unkin.net.acme.unkin.net.
{"acme.unkin.net", "NS", false},
{"sub", "NS", false},
{"ns1", "NS", false},
{"@", "TXT", false},
{"@", "MX", false},
{"@", "SOA", false},
}
for _, c := range cases {
if got := isApexNS(c.name, c.typ, zone); got != c.want {
t.Errorf("isApexNS(%q,%q)=%v want %v", c.name, c.typ, got, c.want)
}
}
}