fix(dns): a lista dizia PENDING, nunca verificado, com tudo verificado
O crocha na lista de dominios lia-se da coluna dns_status, e nada lhe escrevia depois do onboarding. A verificacao corre no servico de protocolos, que tem o resolvedor, e a resposta ia directa para o browser sem ficar registada -- por isso um dominio com 13 de 13 registos publicados e a resolver continuava listado como PENDING, ultima verificacao "Nunca", ao lado da pagina que dizia o contrario. Quem verificou passa a escrever o que encontrou. Falhar a gravar nao transforma uma verificacao boa num erro para o operador: fica no log. A coluna aceita apenas PENDING, VERIFIED, PARTIAL, DRIFT e ERROR, e o verificador tambem diz MISSING e UNKNOWN. Escrever qualquer uma dessas violava a check constraint, a actualizacao era rejeitada e o crocha ficava velo na mesma -- exactamente o sintoma que isto vem corrigir. Ha um teste so para essa traducao. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
47c3700380
commit
9689f26dda
6 files changed
+139
No files matched your search
@@ -226,6 +226,9 @@ func run(cfg config) {
|
||||
// like this: the auth service answered it by re-reading the stored status,
|
||||
// because the resolver and the record spec both live here.
|
||||
if cfg.CloudflareToken != "" || cfg.PublicIPv4 != "" {
|
||||
// Verification writes down what it found, so the badge in the domain
|
||||
// list stops disagreeing with the page next to it.
|
||||
router.SetDnsStatusWriter(postgres.NewDomainRepository(pool))
|
||||
router.SetAdminDnsAPI(
|
||||
&dnsSpecSource{cfg: cfg, dkim: dkimRepo},
|
||||
netdns.NewPublicVerifier(),
|
||||
|
||||
@@ -3,6 +3,7 @@ package postgres
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
)
|
||||
@@ -40,3 +41,39 @@ func (r *DomainRepository) ActiveDomainNames(ctx context.Context) ([]string, err
|
||||
}
|
||||
return out, rows.Err()
|
||||
}
|
||||
|
||||
// SaveDnsStatus files the outcome of a verification against the domain.
|
||||
//
|
||||
// The console reads its badge from this column, and nothing ever wrote to it
|
||||
// after onboarding: a domain whose records were all published and resolving
|
||||
// was still listed as PENDING, last checked "never".
|
||||
func (r *DomainRepository) SaveDnsStatus(ctx context.Context, domain, status string) error {
|
||||
_, err := r.pool.Exec(ctx, `
|
||||
UPDATE domains
|
||||
SET dns_status = $2, dns_last_checked_at = now()
|
||||
WHERE lower(name) = lower($1)
|
||||
`, domain, columnStatus(status))
|
||||
if err != nil {
|
||||
return fmt.Errorf("save dns status for %s: %w", domain, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// columnStatus maps the verifier's vocabulary onto the one the column accepts.
|
||||
//
|
||||
// dns_status is constrained to PENDING, VERIFIED, PARTIAL, DRIFT and ERROR,
|
||||
// while verification also reports MISSING and UNKNOWN. Writing either of those
|
||||
// violates the check constraint, so the update fails and the badge silently
|
||||
// stays stale - the exact symptom this was meant to fix.
|
||||
func columnStatus(status string) string {
|
||||
switch strings.ToUpper(strings.TrimSpace(status)) {
|
||||
case "VERIFIED", "PARTIAL", "DRIFT", "ERROR", "PENDING":
|
||||
return strings.ToUpper(strings.TrimSpace(status))
|
||||
case "MISSING":
|
||||
// Nothing resolved: the zone is not serving what it should.
|
||||
return "ERROR"
|
||||
default:
|
||||
// UNKNOWN, and anything new the verifier learns to say.
|
||||
return "PENDING"
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,27 @@
|
||||
package postgres
|
||||
|
||||
import "testing"
|
||||
|
||||
// dns_status is constrained by the schema to a fixed set. The verifier answers
|
||||
// with a wider vocabulary, and writing one of the extra words fails the check
|
||||
// constraint - the update is rejected and the console goes on showing the
|
||||
// stale badge this was written to fix.
|
||||
func TestColumnStatus_OnlyEverProducesAcceptedValues(t *testing.T) {
|
||||
allowed := map[string]bool{
|
||||
"PENDING": true, "VERIFIED": true, "PARTIAL": true, "DRIFT": true, "ERROR": true,
|
||||
}
|
||||
|
||||
for _, in := range []string{"VERIFIED", "PARTIAL", "MISSING", "UNKNOWN", "", "something new"} {
|
||||
got := columnStatus(in)
|
||||
if !allowed[got] {
|
||||
t.Errorf("columnStatus(%q) = %q, which the check constraint rejects", in, got)
|
||||
}
|
||||
}
|
||||
|
||||
if got := columnStatus("MISSING"); got != "ERROR" {
|
||||
t.Errorf("MISSING became %q; nothing resolved, so it is an error, not a pending check", got)
|
||||
}
|
||||
if got := columnStatus("VERIFIED"); got != "VERIFIED" {
|
||||
t.Errorf("VERIFIED became %q", got)
|
||||
}
|
||||
}
|
||||
@@ -47,6 +47,15 @@ type DomainReconciler interface {
|
||||
ReconcileDomain(ctx context.Context, domain string) (ReconcileResult, error)
|
||||
}
|
||||
|
||||
// DnsStatusWriter records the outcome of a verification.
|
||||
//
|
||||
// Verification lives here, with the resolver; the badge the console shows is
|
||||
// read from the domains table. Without this the two never met: a domain could
|
||||
// verify 13 of 13 and still be listed as PENDING, never checked.
|
||||
type DnsStatusWriter interface {
|
||||
SaveDnsStatus(ctx context.Context, domain, status string) error
|
||||
}
|
||||
|
||||
type adminDnsAPI struct {
|
||||
spec DnsSpecSource
|
||||
verifier DnsRecordVerifier
|
||||
@@ -54,6 +63,9 @@ type adminDnsAPI struct {
|
||||
// reconciler is optional: without it the route reports that the feature
|
||||
// is not configured rather than answering as though it had run.
|
||||
reconciler DomainReconciler
|
||||
// status is optional: without it verification still answers, it just is
|
||||
// not written down.
|
||||
status DnsStatusWriter
|
||||
}
|
||||
|
||||
// handleReconcile publishes the records a domain is missing.
|
||||
@@ -168,6 +180,15 @@ func (a *adminDnsAPI) handleVerify(w http.ResponseWriter, r *http.Request) {
|
||||
status = "PARTIAL"
|
||||
}
|
||||
|
||||
// Written down before answering, so the list badge and this page agree.
|
||||
if a.status != nil {
|
||||
if err := a.status.SaveDnsStatus(r.Context(), domain, status); err != nil {
|
||||
// The check itself succeeded; failing to file it must not turn a
|
||||
// good answer into an error for the operator.
|
||||
log.Printf("dns: verified %s but could not record the status: %v", domain, err)
|
||||
}
|
||||
}
|
||||
|
||||
writeJSON(w, http.StatusOK, map[string]any{
|
||||
"domain": domain,
|
||||
"status": status,
|
||||
@@ -189,6 +210,7 @@ func (r *Router) SetAdminDnsAPI(spec DnsSpecSource, verifier DnsRecordVerifier,
|
||||
// either order. They could not before, and main.go called them in the
|
||||
// order that lost the reconciler.
|
||||
reconciler: r.dnsReconciler,
|
||||
status: r.dnsStatus,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -208,6 +230,14 @@ 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.
|
||||
// SetDnsStatusWriter lets verification record what it found.
|
||||
func (r *Router) SetDnsStatusWriter(w DnsStatusWriter) {
|
||||
r.dnsStatus = w
|
||||
if r.dns != nil {
|
||||
r.dns.status = w
|
||||
}
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
@@ -204,3 +204,44 @@ func TestReconcilerSurvivesBeingWiredBeforeTheDnsAPI(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
type stubStatusWriter struct {
|
||||
domain string
|
||||
status string
|
||||
calls int
|
||||
}
|
||||
|
||||
func (s *stubStatusWriter) SaveDnsStatus(_ context.Context, domain, status string) error {
|
||||
s.calls++
|
||||
s.domain, s.status = domain, status
|
||||
return nil
|
||||
}
|
||||
|
||||
// The console listed a fully verified domain as PENDING, "last checked: never".
|
||||
//
|
||||
// Verification ran in the protocols service, which owns the resolver, and its
|
||||
// answer went straight to the browser. Nothing wrote it down, and the badge in
|
||||
// the list is read from the domains table - so it stayed at the value set when
|
||||
// the domain was first added, no matter how many times it verified 13 of 13.
|
||||
func TestVerifyRecordsWhatItFound(t *testing.T) {
|
||||
writer := &stubStatusWriter{}
|
||||
api := &adminDnsAPI{
|
||||
spec: stubSpec{}, verifier: stubVerifier{},
|
||||
sessions: testVerifier(interopSecret), status: writer,
|
||||
}
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/dns/verify?domain=example.test", nil)
|
||||
req.Header.Set("Authorization", "Bearer "+adminSurfaceToken2())
|
||||
api.handleVerify(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("answered %d: %s", rec.Code, rec.Body)
|
||||
}
|
||||
if writer.calls != 1 {
|
||||
t.Fatalf("wrote the status %d times, want 1: the console keeps showing a stale badge", writer.calls)
|
||||
}
|
||||
if writer.domain != "example.test" {
|
||||
t.Errorf("recorded status for %q, want example.test", writer.domain)
|
||||
}
|
||||
}
|
||||
@@ -39,6 +39,7 @@ type Router struct {
|
||||
dns *adminDnsAPI
|
||||
// dnsReconciler is held here so it survives being set before dns exists.
|
||||
dnsReconciler DomainReconciler
|
||||
dnsStatus DnsStatusWriter
|
||||
// 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
|
||||
|
||||
Reference in new issue
Block a user