diff --git a/api/v1alpha1/dnsrecord_types.go b/api/v1alpha1/dnsrecord_types.go index 99dc3ac..13101cf 100644 --- a/api/v1alpha1/dnsrecord_types.go +++ b/api/v1alpha1/dnsrecord_types.go @@ -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. diff --git a/config/crd/bases/bind.unkin.net_dnsrecords.yaml b/config/crd/bases/bind.unkin.net_dnsrecords.yaml index 18ddaeb..10fa6a4 100644 --- a/config/crd/bases/bind.unkin.net_dnsrecords.yaml +++ b/config/crd/bases/bind.unkin.net_dnsrecords.yaml @@ -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: diff --git a/config/crd/install.yaml b/config/crd/install.yaml index 74507d8..19d3b03 100644 --- a/config/crd/install.yaml +++ b/config/crd/install.yaml @@ -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: diff --git a/internal/controller/dnsrecord_controller.go b/internal/controller/dnsrecord_controller.go index a1238bc..97f54e9 100644 --- a/internal/controller/dnsrecord_controller.go +++ b/internal/controller/dnsrecord_controller.go @@ -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 { diff --git a/internal/controller/zone_helpers.go b/internal/controller/zone_helpers.go index 683e180..7dd2eb7 100644 --- a/internal/controller/zone_helpers.go +++ b/internal/controller/zone_helpers.go @@ -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 { diff --git a/internal/controller/zone_helpers_test.go b/internal/controller/zone_helpers_test.go index b00888c..53e43f7 100644 --- a/internal/controller/zone_helpers_test.go +++ b/internal/controller/zone_helpers_test.go @@ -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) + } + } +}