amadan
console: SES events are counted against the sender SES named, not every app on the domain (plan 2, task 11)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 files changed,
+166
−17
internal/console/mailevents_test.go+60 −4internal/console/sesevents.go+57 −13internal/console/sesevents_test.go+49 −0
diff --git a/internal/console/mailevents_test.go b/internal/console/mailevents_test.go| index c982a34d1..7c64489b7 100644 |
| --- a/internal/console/mailevents_test.go |
| +++ b/internal/console/mailevents_test.go |
| @@ -160,11 +160,16 @@ func TestTheMailLogGoesToTheAppSESSaysSentIt(t *testing.T) { |
| if got := logOf("jb-site"); len(got) != 0 { |
| t.Errorf("the other app on the domain was shown another app's recipient: %+v", got) |
| } |
| + // THE VISIBLE CHANGE, and it belongs in the roll note (design §8). |
| + // Before senders, every app on a shared domain was charged for every |
| + // event on that domain. Now only the app SES named is charged, so a |
| + // sibling's cap stops being consumed by another app's sends. |
| day := now.UTC().Format("2006-01-02") |
| - for _, app := range []string{"jb-book", "jb-site"} { |
| - if c, _ := fx.st.MailCounts(ctx, release.OpsAccount, app, "", day); c.Bounces != 1 { |
| - t.Errorf("%s bounces = %d — the counts' attribution is not this change's to narrow", app, c.Bounces) |
| - } |
| + if c, _ := fx.st.MailCounts(ctx, release.OpsAccount, "jb-book", "", day); c.Bounces != 1 { |
| + t.Errorf("jb-book bounces = %d, want the 1 SES attributed to it", c.Bounces) |
| + } |
| + if c, _ := fx.st.MailCounts(ctx, release.OpsAccount, "jb-site", "", day); c.Bounces != 0 { |
| + t.Errorf("jb-site bounces = %d, want 0 — a sibling on the same domain is no longer charged", c.Bounces) |
| } |
| // SES naming one of CARLOS's own identities that is not a live app gives |
| @@ -204,6 +209,57 @@ func TestTheMailLogGoesToTheAppSESSaysSentIt(t *testing.T) { |
| } |
| } |
| } |
| + |
| + // An unknown caller keeps today's behaviour exactly: the domain is all |
| + // there is to go on, so every app on it is charged. |
| + for _, app := range []string{"jb-book", "jb-site"} { |
| + if c, _ := fx.st.MailCounts(ctx, release.OpsAccount, app, "", day); c.Bounces < 2 { |
| + t.Errorf("%s bounces = %d, want the two domain-attributed ones", app, c.Bounces) |
| + } |
| + } |
| + |
| + // The stray caller parses into a well-formed account and app, so it is |
| + // "named" — but "not-an-app" is not a live app, and a count row keyed on |
| + // a name no app answers to is a row nothing reads and nothing reaps. |
| + if c, _ := fx.st.MailCounts(ctx, release.OpsAccount, "not-an-app", "", day); c.Bounces != 0 { |
| + t.Errorf("a name that is not an app was given %d bounces", c.Bounces) |
| + } |
| +} |
| + |
| +// An event whose caller names a slugged sender is charged to that sender |
| +// alone, resolved through the caller index rather than by parsing. |
| +func TestAnIndexedCallerIsChargedToItsOwnSender(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + ctx := context.Background() |
| + now := time.Date(2026, 9, 15, 12, 0, 0, 0, time.UTC) |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, "jb-book", "bill@tito.io", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + names, err := mail.SenderNames(release.OpsAccount, "jb-book", "book.example.com", "") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := mail.PutCallerIndex(ctx, fx.rel, names.IAMUser, release.OpsAccount, "jb-book", "book.example.com", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + ev := bounceTo("ann@example.com", "m-instance", now) |
| + // bounceTo sets At but not SentAt, and the count buckets by SentAt |
| + // (falling back to time.Now() when it's zero) — set it explicitly so |
| + // this test's fixed date is the day the count actually lands in, |
| + // rather than depending on which real day the suite happens to run. |
| + ev.SentAt = now |
| + ev.Caller = names.IAMUser |
| + fx.srv.recordAndEnforceMail(ctx, ev, "sns-instance") |
| + |
| + day := now.UTC().Format("2006-01-02") |
| + one, _ := fx.st.MailCounts(ctx, release.OpsAccount, "jb-book", "book.example.com", day) |
| + if one.Bounces != 1 { |
| + t.Fatalf("the named sender's counts = %+v, want 1 bounce", one) |
| + } |
| + appSender, _ := fx.st.MailCounts(ctx, release.OpsAccount, "jb-book", mail.SenderAppID, day) |
| + if appSender.Bounces != 0 { |
| + t.Fatalf("the app sender was charged for an instance sender's bounce: %+v", appSender) |
| + } |
| } |
| // A trashed app is told nothing new, and a name that is freed — by a purge, |
diff --git a/internal/console/sesevents.go b/internal/console/sesevents.go| index 094dfeaeb..e00233481 100644 |
| --- a/internal/console/sesevents.go |
| +++ b/internal/console/sesevents.go |
| @@ -457,12 +457,13 @@ func domainMatchesEvent(d *mail.DomainRecord, domain string) bool { |
| // account, one level up. |
| // |
| // Each notification is also kept in the per-message log (mailevents.go), |
| -// under notice — SNS's own id for it. The log is attributed more narrowly |
| -// than the counts where SES allows: when the notification names a sending |
| -// identity CARLOS minted, the row goes to that app alone; only when it names |
| -// none does the row go to every app on the matching domain. The counts keep |
| -// their domain-wide attribution untouched — they enforce a cap, and changing |
| -// what they measure is not this change's business. |
| +// under notice — SNS's own id for it, and, since senders (design §8), the |
| +// counts follow the same rule: where the caller identity resolves to a |
| +// sender CARLOS minted, that sender alone is counted, once, and the |
| +// domain-wide loop below is skipped entirely for it. Only when the caller |
| +// resolves to nothing does the count fall back to every app on the matching |
| +// domain, against its app sender, which is the whole of what this endpoint |
| +// did before senders existed. |
| func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event, notice string) { |
| accts, err := s.store.Accounts() |
| if err != nil { |
| @@ -490,16 +491,47 @@ func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event, notice |
| now := time.Now() |
| key := mailNoticeKey(notice, ev, now) |
| - // The per-message log. When SES names one of CARLOS's own sending |
| - // identities, that app — and nobody else — gets the row, whatever domain |
| - // the message went out on: the credential is minted per app, so the name |
| - // is the sender. Only when SES names nobody CARLOS minted does the row |
| - // fall back to every app on the matching domain, below. |
| - callerAcct, callerApp, named := mail.AppOfCaller(ev.Caller) |
| + // ATTRIBUTION BY CALLER (senders design §8). Every sender has its own |
| + // IAM user and SES stamps its name on Send, Bounce, Complaint, Delivery |
| + // and Reject, so where the sending domain can only name every app that |
| + // shares it, the caller names the one sender that actually sent. The |
| + // resolver parses before it reads, so an app enabled before senders |
| + // existed still resolves with no index entry of its own. |
| + callerAcct, callerApp, callerSender, named := mail.SenderOfCaller(ctx, s.cfg.Releases, ev.Caller) |
| if named { |
| + // The COUNT is gated on the app being live, exactly as the log row |
| + // already is (recordMailMessage, sesevents.go:558). A caller name |
| + // can parse into an account and an app that is not a live app — |
| + // mail.UserName(acct, "not-an-app") is a well-formed name for an |
| + // app that was trashed, or never existed — and counting it would |
| + // insert a mail_sender_counts row keyed on a name no app answers |
| + // to, which nothing would ever read or reap. Returning rather than |
| + // falling through is deliberate: SES named a sender, so the |
| + // domain-wide loop is not the right answer for it either. |
| + if !s.mailAppLive(callerAcct, callerApp) { |
| + return |
| + } |
| s.recordMailMessage(ctx, callerAcct, callerApp, key, ev, true, now) |
| + // Counted ONCE, against the sender SES named, and the domain loop |
| + // below is skipped entirely. This is the visible change in the roll |
| + // note: a sibling app on the same domain is no longer charged for |
| + // this send, which is the intent, but a tenant watching its own |
| + // numbers will see them move. |
| + if err := s.store.RecordMailEvent(ctx, callerAcct, callerApp, callerSender, day, ev.Type); err != nil { |
| + log.Printf("console: ses events: recording %s for %s/%s/%s -> %v", ev.Type, callerAcct, callerApp, callerSender, err) |
| + return |
| + } |
| + if _, reason, err := s.enforceMailLimits(ctx, callerAcct, callerApp, at); err != nil { |
| + log.Printf("console: ses events: enforcing limits for %s/%s/%s -> %v", callerAcct, callerApp, callerSender, err) |
| + } else if reason != "" { |
| + log.Printf("console: ses events: paused %s/%s/%s -> %s", callerAcct, callerApp, callerSender, reason) |
| + } |
| + return |
| } |
| + // The fallback, unchanged: SES named nobody CARLOS minted, so the |
| + // sending domain is all there is to go on and every app on it is |
| + // charged, against its app sender. |
| for _, a := range accts { |
| domains, err := mail.ListDomains(ctx, s.cfg.Releases, a.Sqid) |
| if err != nil { |
| @@ -515,8 +547,20 @@ func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event, notice |
| // addresses. The domain claim refuses a second account the same |
| // name, but not another account's MAIL FROM host claimed as a |
| // domain of its own and never verified. |
| - logHere := !named && domainVerifiedForEvent(d, ev.Domain) |
| + logHere := domainVerifiedForEvent(d, ev.Domain) |
| for _, app := range d.Apps { |
| + // An app in per-instance mode is skipped (design §8). Its |
| + // app sender is deliberately dead — no instance sends |
| + // through it — so charging it for a domain-wide event it |
| + // cannot have caused would pause a credential nobody holds |
| + // and hide the instance that actually sent. A read failure |
| + // is not a reason to drop the event: fall through and |
| + // charge, which is today's behaviour. |
| + if sn, _, err := mail.GetSender(ctx, s.cfg.Releases, a.Sqid, app, mail.SenderAppID); err != nil { |
| + log.Printf("console: ses events: reading the app sender for %s/%s -> %v", a.Sqid, app, err) |
| + } else if sn != nil && sn.PerInstance { |
| + continue |
| + } |
| if logHere { |
| s.recordMailMessage(ctx, a.Sqid, app, key, ev, false, now) |
| } |
diff --git a/internal/console/sesevents_test.go b/internal/console/sesevents_test.go| index 47f96b6a0..d26a02233 100644 |
| --- a/internal/console/sesevents_test.go |
| +++ b/internal/console/sesevents_test.go |
| @@ -783,3 +783,52 @@ func TestRecordAndEnforceMailAttributesTheRewrittenMailFromEnvelope(t *testing.T |
| t.Fatalf("Sends = %d, want 1 — the rewritten MAIL FROM envelope must still attribute to the app", counts.Sends) |
| } |
| } |
| + |
| +// The domain fallback skips an app sender in per-instance mode (senders |
| +// design §8). Nothing sets PerInstance yet, so this hand-sets it: the app |
| +// sender is deliberately dead once its app moves to per-instance senders, |
| +// and charging it for a domain-wide event it cannot have sent would pause a |
| +// credential nobody holds. |
| +func TestRecordAndEnforceMailSkipsAPerInstanceAppInTheDomainFallback(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + ctx := context.Background() |
| + now := time.Now() |
| + for _, app := range []string{"jb-book", "jb-site"} { |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, app, "bill@tito.io", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + rec := &mail.DomainRecord{ |
| + Domain: "lgc.li", Kind: mail.KindCustom, Apps: []string{"jb-book", "jb-site"}, |
| + Regions: map[string]*mail.RegionState{"eu-west-1": { |
| + IdentityARN: "arn:aws:ses:eu-west-1:1:identity/lgc.li", MailFrom: "mail-eu-west-1.lgc.li", Status: mail.StatusVerified, |
| + }}, |
| + CreatedAt: now, |
| + } |
| + if _, err := mail.PutDomain(ctx, fx.rel, release.OpsAccount, rec, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + // Hand-set jb-site's app sender to per-instance mode: nothing in this |
| + // release writes PerInstance itself, so the only way to exercise the |
| + // skip is to put the sender record directly. |
| + perInstance := &mail.Sender{ID: mail.SenderAppID, Scope: mail.ScopeApp, Source: mail.SourceDomain, PerInstance: true} |
| + if _, err := mail.PutSender(ctx, fx.rel, release.OpsAccount, "jb-site", perInstance, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + ev := bounceTo("ann@example.com", "m-domain-fallback", now) |
| + // An unnamed caller, so this falls back to the domain loop rather than |
| + // resolving to a sender directly. |
| + fx.srv.recordAndEnforceMail(ctx, ev, "sns-fallback") |
| + |
| + day := now.UTC().Format("2006-01-02") |
| + if c, _ := fx.st.MailCounts(ctx, release.OpsAccount, "jb-book", "", day); c.Bounces != 1 { |
| + t.Errorf("jb-book bounces = %d, want 1 — it has no PerInstance app sender", c.Bounces) |
| + } |
| + if c, _ := fx.st.MailCounts(ctx, release.OpsAccount, "jb-site", "", day); c.Bounces != 0 { |
| + t.Errorf("jb-site bounces = %d, want 0 — its app sender is in per-instance mode and must not be charged", c.Bounces) |
| + } |
| + if got, _ := fx.st.MailEvents(ctx, release.OpsAccount, "jb-site", MailEventQuery{}, now); len(got) != 0 { |
| + t.Errorf("jb-site's log got a row from a per-instance app sender's skipped domain fallback: %+v", got) |
| + } |
| +} |