diff --git a/services/protocols/cmd/lambdamail-protocols/main.go b/services/protocols/cmd/lambdamail-protocols/main.go index f7e367d..1e61461 100644 --- a/services/protocols/cmd/lambdamail-protocols/main.go +++ b/services/protocols/cmd/lambdamail-protocols/main.go @@ -209,10 +209,14 @@ func run(cfg config) { // Folders the mailbox owner keeps for themselves, which the filing rules // can then name as a destination. // The console's reconcile button reaches this: the same sync the timer - // runs, for one named domain, on demand. - router.SetDnsReconciler(domainReconciler{run: func(c context.Context, domain string) (*usecase.SyncDnsRecordsOutput, error) { - return reconcileDomainOnDemand(c, cfg, aliasRepo, dkimRepo, domain) - }}) + // runs, for one named domain, on demand. Only wired when there is a + // provider to talk to, so the route can say it is unavailable instead of + // reaching Cloudflare with an empty token and reporting a refusal. + if cfg.CloudflareToken != "" { + router.SetDnsReconciler(domainReconciler{run: func(c context.Context, domain string) (*usecase.SyncDnsRecordsOutput, error) { + return reconcileDomainOnDemand(c, cfg, aliasRepo, dkimRepo, domain) + }}) + } router.SetFolderAdmin(usecase.NewManageFoldersUseCase( postgres.NewWebmailRepository(pool), postgres.NewWebmailRepository(pool))) if dkimRepo != nil { diff --git a/services/protocols/internal/presentation/http/admin_dns_api.go b/services/protocols/internal/presentation/http/admin_dns_api.go index 059843f..8748e51 100644 --- a/services/protocols/internal/presentation/http/admin_dns_api.go +++ b/services/protocols/internal/presentation/http/admin_dns_api.go @@ -81,7 +81,7 @@ func (a *adminDnsAPI) handleReconcile(w http.ResponseWriter, r *http.Request) { } if a.reconciler == nil { writeError(w, http.StatusServiceUnavailable, "RECONCILER_UNAVAILABLE", - "DNS reconciliation needs a provider token to be configured") + "DNS reconciliation is not available: the service started without a DNS provider") return } @@ -183,7 +183,13 @@ func (r *Router) SetAdminDnsAPI(spec DnsSpecSource, verifier DnsRecordVerifier, if spec == nil || verifier == nil || sessionSecret == "" { return } - r.dns = &adminDnsAPI{spec: spec, verifier: verifier, sessions: NewWebSessionVerifier(sessionSecret)} + r.dns = &adminDnsAPI{ + spec: spec, verifier: verifier, sessions: NewWebSessionVerifier(sessionSecret), + // Carried over from the router so the two setters can be called in + // either order. They could not before, and main.go called them in the + // order that lost the reconciler. + reconciler: r.dnsReconciler, + } } // handleAdminDnsReconcile publishes the records a domain is missing. @@ -203,6 +209,12 @@ func (r *Router) handleAdminDnsReconcile(w http.ResponseWriter, req *http.Reques // SetDnsReconciler enables the reconcile route to actually publish records. // Without it the route reports that reconciliation is not configured. func (r *Router) SetDnsReconciler(reconciler DomainReconciler) { + // Kept on the router as well as on the API. Assigning only to r.dns meant + // that calling this before SetAdminDnsAPI silently did nothing: the field + // was written to a struct that did not exist yet, and the console then + // reported reconciliation as unconfigured while the provider token was + // present and the background sweep was publishing records with it. + r.dnsReconciler = reconciler if r.dns != nil { r.dns.reconciler = reconciler } diff --git a/services/protocols/internal/presentation/http/admin_dns_reconcile_test.go b/services/protocols/internal/presentation/http/admin_dns_reconcile_test.go index 84c3cdc..7f3ffae 100644 --- a/services/protocols/internal/presentation/http/admin_dns_reconcile_test.go +++ b/services/protocols/internal/presentation/http/admin_dns_reconcile_test.go @@ -8,6 +8,9 @@ import ( "net/http/httptest" "strings" "testing" + "time" + + "lambdamail/protocols/internal/domain/entity" ) // Making the reconcile button do what its name says. @@ -144,3 +147,60 @@ func adminSurfaceToken2() string { IssuedAt: interopIssuedAt, ExpiresAt: interopIssuedAt + 3600, }) } + +// stubSpec is the minimum a wired DNS API needs; the wiring, not the spec, is +// what these tests are about. +type stubSpec struct{} + +func (stubSpec) ExpectedRecords(context.Context, string) ([]entity.DnsRecord, error) { + return nil, nil +} + +type stubVerifier struct{} + +func (stubVerifier) VerifyRecord(context.Context, entity.DnsRecord) (bool, string) { + return true, "" +} + +// The console reported "DNS reconciliation needs a provider token to be +// configured" while the token was configured and working - the background +// sweep was publishing records with it at that very moment. +// +// SetDnsReconciler only assigned when r.dns already existed, and main.go calls +// it before SetAdminDnsAPI creates r.dns. So it silently did nothing, and the +// API was then built with no reconciler at all. The earlier tests missed it by +// constructing adminDnsAPI directly, which is exactly the wiring that broke. +func TestReconcilerSurvivesBeingWiredBeforeTheDnsAPI(t *testing.T) { + for _, order := range []string{"reconciler first", "dns api first"} { + t.Run(order, func(t *testing.T) { + router := NewRouter(nil, func() error { return nil }) + stub := &stubReconciler{created: 11} + + wireAPI := func() { router.SetAdminDnsAPI(stubSpec{}, stubVerifier{}, interopSecret) } + wireReconciler := func() { router.SetDnsReconciler(stub) } + + if order == "reconciler first" { + wireReconciler() + wireAPI() + } else { + wireAPI() + wireReconciler() + } + + // Only the clock is pinned: the captured admin token has a fixed + // validity window. The wiring itself is left exactly as main.go + // builds it, because the wiring is what is under test. + router.dns.sessions.now = func() time.Time { return time.Unix(interopIssuedAt+60, 0) } + + rec := httptest.NewRecorder() + router.handleAdminDnsReconcile(rec, adminRequest("/api/v1/admin/dns/reconcile?domain=example.test")) + + if rec.Code != http.StatusOK { + t.Fatalf("answered %d (%s): the route lost its reconciler", rec.Code, rec.Body) + } + if len(stub.called) != 1 { + t.Fatalf("reconciled %v, want one call: the route answered without reconciling", stub.called) + } + }) + } +} diff --git a/services/protocols/internal/presentation/http/router.go b/services/protocols/internal/presentation/http/router.go index 153e1d6..974b691 100644 --- a/services/protocols/internal/presentation/http/router.go +++ b/services/protocols/internal/presentation/http/router.go @@ -37,6 +37,8 @@ type Router struct { // dns verifies the published records against public resolvers. It lives // here because this is the process with the resolver and the record spec. dns *adminDnsAPI + // dnsReconciler is held here so it survives being set before dns exists. + dnsReconciler DomainReconciler // degradedFunc reports a condition that leaves the service running but // not fit for production, the clearest case being a self-signed // certificate standing in for one Traefik never issued