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) +}