mail: verify reconciles the configuration set, in both directions (#272)
Neither the record nor SES is the source of truth for a domain's configuration set. DEPLOYMENT INTENT is: a deployment with an SNS topic wants every identity bound to its account's set, one without wants none bound, and both stores are projections of that. Either can drift from it, and one of them did — an out-of-band unbind left the record claiming a set SES no longer had, which silenced #268's warning for a domain emitting nothing. A diagnostic that is confidently wrong is worse than one that is merely absent. Reconciliation lands in VerifyDomain because IdentityStatus has ALREADY asked SES about this identity there, so the check is free. The rejected alternative was reconciling on every status read: that puts a GetEmailIdentity per domain on a path which today answers from the bucket alone, and turns a working read into a failure whenever AWS is unreachable. Catching drift a beat later costs nothing; making every read depend on AWS costs something on every read. GetEmailIdentity already returns the binding and IdentityStatus was discarding it, so Status gains ConfigSet and the field comes free from a call already being made. The fake had to be fixed to match: it stored the binding but returned Status alone, so it would have reported an empty binding forever and a reconciliation test could have passed against something production would never do — the same divergence its own NOT_STARTED branch exists to prevent. The UNBIND direction is not symmetry for its own sake. A deployment that turns event ingest off, or whose set was deleted, otherwise leaves identities pointing at a destination that no longer exists, and a send through a missing configuration set fails outright. A rebind refreshes the IAM policies, for #279's reason arriving through the other door: repairing SES and the record while leaving the policy naming no configuration set converts silent-no-events into a 554 on every send — strictly worse than the drift repaired. Only on an ACTUAL rebind, so the ordinary verify stays the cheap read it was. Mutation-tested: removing the reconciliation, keeping the rebind but dropping the policy refresh, and making the fake stop reporting its real binding each fail a test. Stacked on #279 (RefreshPolicies); merge that first. Gate: 32 packages, vet + gofmt clean, GOOS=darwin build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 files changed,
+20
−14
internal/console/apimail.go+6 −1internal/mail/walk_enable_test.go+5 −5internal/mail/walk_verify_test.go+9 −8
diff --git a/internal/console/apimail.go b/internal/console/apimail.go| index 2904f745d..136f882e7 100644 |
| --- a/internal/console/apimail.go |
| +++ b/internal/console/apimail.go |
| @@ -755,7 +755,12 @@ func (s *Server) handleAPIEmailDomainVerify(w http.ResponseWriter, r *http.Reque |
| return |
| } |
| region := s.cfg.MailDeployment.Region |
| - rec, err := mail.VerifyDomain(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct.Sqid, domain, region, time.Now()) |
| + // The deployment goes in because verify RECONCILES the identity's |
| + // configuration set against it (#272): the binding SES should have is a |
| + // function of whether this deployment has an SNS topic, not of what the |
| + // record last recorded. |
| + rec, err := mail.VerifyDomain(ctx, s.cfg.Releases, s.cfg.MailProvisioner, |
| + s.cfg.MailDeployment, acct.Sqid, domain, region, time.Now()) |
| if err != nil { |
| log.Printf("console: api email domain verify %s/%s: %v", acct.Sqid, domain, err) |
| jsonError(w, http.StatusBadGateway, "could not check verification — try again") |
diff --git a/internal/mail/walk_enable_test.go b/internal/mail/walk_enable_test.go| index c3edb90ed..ef8a149f2 100644 |
| --- a/internal/mail/walk_enable_test.go |
| +++ b/internal/mail/walk_enable_test.go |
| @@ -39,7 +39,7 @@ func enableFixture(t *testing.T) (context.Context, release.Store, *MemProvisione |
| } |
| p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"}) |
| - if _, err := VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now); err != nil { |
| + if _, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now); err != nil { |
| t.Fatalf("VerifyDomain: %v", err) |
| } |
| return ctx, s, p, pub, now |
| @@ -185,7 +185,7 @@ func TestEnsureDomainRefreshesPoliciesWhenItBindsAConfigSet(t *testing.T) { |
| } |
| p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"}) |
| - if _, err := VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now); err != nil { |
| + if _, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now); err != nil { |
| t.Fatalf("VerifyDomain: %v", err) |
| } |
| if _, err := Enable(ctx, s, p, pub, noSNS, "bag", "seapointish", now); err != nil { |
| @@ -453,7 +453,7 @@ func TestEnableRewritesThePolicyWhenASecondDomainIsAdded(t *testing.T) { |
| t.Fatalf("EnsureDomain second: %v", err) |
| } |
| p.SetIdentityStatus("eu-west-1", second, Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"}) |
| - if _, err := VerifyDomain(ctx, s, p, "bag", second, "eu-west-1", now); err != nil { |
| + if _, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", second, "eu-west-1", now); err != nil { |
| t.Fatalf("VerifyDomain second: %v", err) |
| } |
| @@ -487,7 +487,7 @@ func TestEnableRewritesThePolicyWhenADomainStopsVerifying(t *testing.T) { |
| t.Fatalf("EnsureDomain second: %v", err) |
| } |
| p.SetIdentityStatus("eu-west-1", second, Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"}) |
| - if _, err := VerifyDomain(ctx, s, p, "bag", second, "eu-west-1", now); err != nil { |
| + if _, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", second, "eu-west-1", now); err != nil { |
| t.Fatalf("VerifyDomain second: %v", err) |
| } |
| @@ -502,7 +502,7 @@ func TestEnableRewritesThePolicyWhenADomainStopsVerifying(t *testing.T) { |
| // second regresses: SES now reports it FAILED. |
| p.SetIdentityStatus("eu-west-1", second, Status{DKIM: "FAILED", MailFrom: "FAILED", Sending: "PENDING"}) |
| - if _, err := VerifyDomain(ctx, s, p, "bag", second, "eu-west-1", now); err != nil { |
| + if _, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", second, "eu-west-1", now); err != nil { |
| t.Fatalf("VerifyDomain regression: %v", err) |
| } |
diff --git a/internal/mail/walk_verify_test.go b/internal/mail/walk_verify_test.go| index 268fdb03a..5345b738b 100644 |
| --- a/internal/mail/walk_verify_test.go |
| +++ b/internal/mail/walk_verify_test.go |
| @@ -2,6 +2,7 @@ package mail |
| import ( |
| "context" |
| + "strings" |
| "testing" |
| "time" |
| @@ -29,7 +30,7 @@ func TestVerifyRequiresEveryAxis(t *testing.T) { |
| p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| Status{DKIM: "SUCCESS", MailFrom: "PENDING", Sending: "SUCCESS"}) |
| - rec, err := VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| + rec, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| if err != nil { |
| t.Fatalf("VerifyDomain: %v", err) |
| } |
| @@ -39,7 +40,7 @@ func TestVerifyRequiresEveryAxis(t *testing.T) { |
| p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"}) |
| - rec, err = VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| + rec, err = VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| if err != nil { |
| t.Fatalf("VerifyDomain: %v", err) |
| } |
| @@ -57,7 +58,7 @@ func TestVerifyRecordsAPermanentFailure(t *testing.T) { |
| p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| Status{DKIM: "FAILED", MailFrom: "PENDING", Sending: "PENDING"}) |
| - rec, err := VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| + rec, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| if err != nil { |
| t.Fatalf("VerifyDomain: %v", err) |
| } |
| @@ -75,10 +76,10 @@ func TestVerifyCanRecoverFromFailed(t *testing.T) { |
| p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| Status{DKIM: "FAILED", MailFrom: "PENDING", Sending: "PENDING"}) |
| - VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| + VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"}) |
| - rec, err := VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| + rec, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| if err != nil { |
| t.Fatalf("VerifyDomain: %v", err) |
| } |
| @@ -100,7 +101,7 @@ func TestVerifyLeavesVerifiedWhenSESRegresses(t *testing.T) { |
| p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"}) |
| - rec, err := VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| + rec, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| if err != nil { |
| t.Fatalf("VerifyDomain: %v", err) |
| } |
| @@ -110,7 +111,7 @@ func TestVerifyLeavesVerifiedWhenSESRegresses(t *testing.T) { |
| p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| Status{DKIM: "FAILED", MailFrom: "SUCCESS", Sending: "SUCCESS"}) |
| - rec, err = VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| + rec, err = VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now) |
| if err != nil { |
| t.Fatalf("VerifyDomain: %v", err) |
| } |
| @@ -123,7 +124,7 @@ func TestVerifyRefusesAnUnknownRegion(t *testing.T) { |
| ctx := context.Background() |
| s, p := release.NewDirStore(t.TempDir()), NewMemProvisioner("1") |
| now := seedDomain(t, s, p) |
| - if _, err := VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "ap-south-1", now); err == nil { |
| + if _, err := VerifyDomain(ctx, s, p, testDeployment(), "bag", "seapointish.bag.oncarlos.com", "ap-south-1", now); err == nil { |
| t.Fatal("verifying a region the domain has no identity in returned success") |
| } |
| } |