From 7f7bfcb4e8df136478f63bd3e2c9c77e8edfda06 Mon Sep 17 00:00:00 2001 From: Kevin Williams Date: Tue, 29 Sep 2026 05:54:45 -0700 Subject: [PATCH] fix: publish nameservers on a zone waiting for domain verification The replicator holds a DNSZone back until its Domain is verified, but the Domain controller verifies by nameserver delegation only against a DNSZone that already has status.nameservers. Those were filled in only after the zone was provisioned, so a domain pointed at Datum could never verify and its zone never provisioned. markPendingDomainVerification now publishes the DNSZoneClass nameservers on a waiting zone without provisioning it. updateStatus no longer blanks them while the new downstream zone has yet to report its own. The static nameserver lookup moves to dnsutils.ClassNameservers so the replicator and the PowerDNS client share it. --- .../dnszone_replicator_controller.go | 38 ++++++++---- .../dnszone_replicator_controller_test.go | 60 +++++++++++++++++++ internal/dns/pdns/client.go | 9 +-- internal/dns/utils/nameservers.go | 15 +++++ 4 files changed, 104 insertions(+), 18 deletions(-) create mode 100644 internal/dns/utils/nameservers.go diff --git a/internal/controller/dnszone_replicator_controller.go b/internal/controller/dnszone_replicator_controller.go index 4e7ef988..6e43a831 100644 --- a/internal/controller/dnszone_replicator_controller.go +++ b/internal/controller/dnszone_replicator_controller.go @@ -261,15 +261,7 @@ func (r *DNSZoneReplicator) Reconcile(ctx context.Context, req mcreconcile.Reque } if !verified { base := upstream.DeepCopy() - msg := "Waiting for domain ownership verification before provisioning DNS" - if apimeta.SetStatusCondition(&upstream.Status.Conditions, metav1.Condition{ - Type: CondAccepted, - Status: metav1.ConditionFalse, - Reason: ReasonPendingDomainVerification, - Message: msg, - ObservedGeneration: upstream.Generation, - LastTransitionTime: metav1.NewTime(time.Now()), - }) { + if markPendingDomainVerification(&upstream, zoneClass) { if perr := upstreamCl.GetClient().Status().Patch(ctx, &upstream, client.MergeFrom(base)); perr != nil { return ctrl.Result{}, perr } @@ -645,7 +637,9 @@ func (r *DNSZoneReplicator) updateStatus(ctx context.Context, c client.Client, s if err := r.DownstreamClient.Get(ctx, client.ObjectKey{Namespace: md.Namespace, Name: md.Name}, &shadow); err == nil { currentNS := dnsutils.NormalizeStringSlice(upstream.Status.Nameservers) desiredNS := dnsutils.NormalizeStringSlice(shadow.Status.Nameservers) - if !equality.Semantic.DeepEqual(currentNS, desiredNS) { + // A shadow that hasn't reported yet must not blank the class + // nameservers published while the zone waited on verification. + if len(desiredNS) > 0 && !equality.Semantic.DeepEqual(currentNS, desiredNS) { upstream.Status.Nameservers = desiredNS changed = true } @@ -854,6 +848,30 @@ func (r *DNSZoneReplicator) isDomainVerified(ctx context.Context, c client.Clien return false, nil } +// markPendingDomainVerification records on upstream that the zone is waiting +// for its Domain to be verified, and reports whether the status changed. +// +// It also publishes the nameservers the zone will be served from, without +// serving it. Pointing the domain at them is one way to prove ownership, and +// the Domain controller compares against these to verify it. Leaving them +// empty until the zone is provisioned would deadlock that check. +func markPendingDomainVerification(upstream *dnsv1alpha1.DNSZone, zoneClass dnsv1alpha1.DNSZoneClass) bool { + changed := apimeta.SetStatusCondition(&upstream.Status.Conditions, metav1.Condition{ + Type: CondAccepted, + Status: metav1.ConditionFalse, + Reason: ReasonPendingDomainVerification, + Message: "Waiting for domain ownership verification before provisioning DNS", + ObservedGeneration: upstream.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + }) + ns := dnsutils.ClassNameservers(zoneClass) + if !equality.Semantic.DeepEqual(dnsutils.NormalizeStringSlice(upstream.Status.Nameservers), ns) { + upstream.Status.Nameservers = ns + changed = true + } + return changed +} + // ---- Watches / mapping helpers -------------------------------------------- func (r *DNSZoneReplicator) SetupWithManager(mgr mcmanager.Manager, downstreamCl cluster.Cluster) error { diff --git a/internal/controller/dnszone_replicator_controller_test.go b/internal/controller/dnszone_replicator_controller_test.go index b055da96..8387a8fc 100644 --- a/internal/controller/dnszone_replicator_controller_test.go +++ b/internal/controller/dnszone_replicator_controller_test.go @@ -646,3 +646,63 @@ func TestCleanupReleasesAZoneWithLegacyAccounting(t *testing.T) { t.Fatalf("the accounting configmap survived teardown, so the domain stays claimed forever") } } + +func TestMarkPendingDomainVerificationPublishesClassNameservers(t *testing.T) { + t.Parallel() + + staticClass := dnsv1alpha1.DNSZoneClass{ + Spec: dnsv1alpha1.DNSZoneClassSpec{ + NameServerPolicy: &dnsv1alpha1.NameServerPolicy{ + Mode: dnsv1alpha1.NameServerPolicyModeStatic, + Static: &dnsv1alpha1.StaticNS{Servers: []string{"ns2.example.net", "ns1.example.net"}}, + }, + }, + } + + tests := []struct { + name string + class dnsv1alpha1.DNSZoneClass + existingNS []string + wantNS []string + wantChanged bool + }{ + { + name: "publishes the class nameservers on a new zone", + class: staticClass, + wantNS: []string{"ns1.example.net", "ns2.example.net"}, + wantChanged: true, + }, + { + name: "class without static nameservers publishes none", + class: dnsv1alpha1.DNSZoneClass{}, + wantChanged: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + zone := &dnsv1alpha1.DNSZone{Status: dnsv1alpha1.DNSZoneStatus{Nameservers: tt.existingNS}} + + if changed := markPendingDomainVerification(zone, tt.class); changed != tt.wantChanged { + t.Fatalf("changed = %v, want %v", changed, tt.wantChanged) + } + if !reflect.DeepEqual(zone.Status.Nameservers, tt.wantNS) { + t.Fatalf("nameservers = %v, want %v", zone.Status.Nameservers, tt.wantNS) + } + cond := apimeta.FindStatusCondition(zone.Status.Conditions, CondAccepted) + if cond == nil || cond.Status != metav1.ConditionFalse || cond.Reason != ReasonPendingDomainVerification { + t.Fatalf("Accepted condition = %+v, want False/%s", cond, ReasonPendingDomainVerification) + } + }) + } + + t.Run("a second pass changes nothing", func(t *testing.T) { + t.Parallel() + zone := &dnsv1alpha1.DNSZone{} + markPendingDomainVerification(zone, staticClass) + if markPendingDomainVerification(zone, staticClass) { + t.Fatal("expected no change on an already-marked zone") + } + }) +} diff --git a/internal/dns/pdns/client.go b/internal/dns/pdns/client.go index 432568ad..2aa2ea3c 100644 --- a/internal/dns/pdns/client.go +++ b/internal/dns/pdns/client.go @@ -239,14 +239,7 @@ func (c *Client) clearZoneComments(ctx context.Context, zoneName string) error { } func (c *Client) GetZoneNameservers(ctx context.Context, zone dnsv1alpha1.DNSZone, class dnsv1alpha1.DNSZoneClass) []string { - var desiredNS []string - if class.Spec.NameServerPolicy != nil && - class.Spec.NameServerPolicy.Mode == dnsv1alpha1.NameServerPolicyModeStatic && - class.Spec.NameServerPolicy.Static != nil { - desiredNS = append(desiredNS, class.Spec.NameServerPolicy.Static.Servers...) - } - - return dnsutils.NormalizeStringSlice(desiredNS) + return dnsutils.ClassNameservers(class) } // EnsureRecordSet makes the RRsets a DNSRecordSet declares match PowerDNS, and diff --git a/internal/dns/utils/nameservers.go b/internal/dns/utils/nameservers.go new file mode 100644 index 00000000..5927e76e --- /dev/null +++ b/internal/dns/utils/nameservers.go @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package utils + +import dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" + +// ClassNameservers returns the nameservers a DNSZoneClass assigns to its zones, +// normalized. It returns nil when the class assigns none. +func ClassNameservers(class dnsv1alpha1.DNSZoneClass) []string { + policy := class.Spec.NameServerPolicy + if policy == nil || policy.Mode != dnsv1alpha1.NameServerPolicyModeStatic || policy.Static == nil { + return nil + } + return NormalizeStringSlice(policy.Static.Servers) +}