amadan
mail, console: the cap, the pause and the day-roll resume are per sender (plan 2, task 12)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6 files changed,
+192
−92
internal/console/apimail.go+10 −3internal/console/mailcounts.go+46 −42internal/console/mailcounts_test.go+54 −19internal/console/sesevents.go+2 −2internal/mail/walk.go+48 −26internal/mail/walk_lifecycle_test.go+32 −0
diff --git a/internal/console/apimail.go b/internal/console/apimail.go| index 20905304f..9ebbbb5a0 100644 |
| --- a/internal/console/apimail.go |
| +++ b/internal/console/apimail.go |
| @@ -647,9 +647,16 @@ func (s *Server) handleAPIEmailStatus(w http.ResponseWriter, r *http.Request) { |
| // enforceMailLimits' day-roll check — a member running `carlos email |
| // status` is exactly the moment a rolled-over daily budget should |
| // clear. Shares resumeStaleCapPause with enforceMailLimits so the two |
| - // can never drift; a RATE pause or a kind-less pause is left untouched |
| - // by that function, fail-safe. |
| - rec, _, err = s.resumeStaleCapPause(ctx, acct.Sqid, app, rec, time.Now()) |
| + // can never drift; a RATE pause, a PER-INSTANCE pause or a kind-less |
| + // pause is left untouched by that function, fail-safe. |
| + sn, _, err := mail.GetSender(ctx, s.cfg.Releases, acct.Sqid, app, mail.SenderAppID) |
| + if err == nil { |
| + if _, resumed, rerr := s.resumeStaleCapPause(ctx, acct.Sqid, app, mail.SenderAppID, sn, time.Now()); rerr != nil { |
| + err = rerr |
| + } else if resumed { |
| + rec, _, err = mail.Get(ctx, s.cfg.Releases, acct.Sqid, app) |
| + } |
| + } |
| if err != nil { |
| log.Printf("console: api email status resume %s/%s: %v", acct.Sqid, app, err) |
| jsonError(w, http.StatusBadGateway, "could not read the email record — try again") |
diff --git a/internal/console/mailcounts.go b/internal/console/mailcounts.go| index f82013324..eabf11d0b 100644 |
| --- a/internal/console/mailcounts.go |
| +++ b/internal/console/mailcounts.go |
| @@ -157,52 +157,54 @@ FROM mail_sender_counts WHERE account = ? AND app = ? AND day = ?` |
| // Both callers share this one function so they can never drift. |
| // |
| // Fail-safe by construction, matching "RATE AND MANUAL PAUSES NEVER |
| -// AUTO-RESUME" below: resumes ONLY when rec is currently paused (Active == |
| -// false), the pause is a CAP pause (PauseKind == mail.PauseKindCap — a |
| -// RATE pause or a kind-less manual/pre-existing pause, PauseKind == "", |
| -// both fall through untouched), and the pause was applied on a UTC day |
| -// STRICTLY EARLIER than now — same-day resumes would make the cap |
| -// meaningless. |
| +// AUTO-RESUME" below: resumes ONLY when sn is currently paused (Active == |
| +// false), the pause is a CAP pause on a sender that is not PerInstance |
| +// (PauseKind == mail.PauseKindCap — a RATE pause, a PER-INSTANCE pause or a |
| +// kind-less manual/pre-existing pause, PauseKind == "", all fall through |
| +// untouched), and the pause was applied on a UTC day STRICTLY EARLIER than |
| +// now — same-day resumes would make the cap meaningless. |
| // |
| -// Returns the freshly re-read record when it resumes (rec's SetPaused |
| +// Returns the freshly re-read sender when it resumes (sn's SetSenderPaused |
| // write moves Active, PauseKind and At, so callers must not patch the |
| -// in-memory copy by hand) and whether a resume happened; rec is passed |
| +// in-memory copy by hand) and whether a resume happened; sn is passed |
| // straight back, and resumed is false, in every other case, including |
| -// rec == nil or rec.Provisioned == nil. |
| -func (s *Server) resumeStaleCapPause(ctx context.Context, acct, app string, rec *mail.Record, now time.Time) (*mail.Record, bool, error) { |
| - if rec == nil || rec.Provisioned == nil || rec.Provisioned.Active { |
| - return rec, false, nil |
| +// sn == nil or sn.Provisioned == nil. |
| +func (s *Server) resumeStaleCapPause(ctx context.Context, acct, app, sender string, sn *mail.Sender, now time.Time) (*mail.Sender, bool, error) { |
| + if sn == nil || sn.Provisioned == nil || sn.Provisioned.Active { |
| + return sn, false, nil |
| } |
| - // A RATE pause or a MANUAL (operator) pause — PauseKind is anything |
| - // other than mail.PauseKindCap, including "" — never auto-resumes: see |
| - // "RATE AND MANUAL PAUSES NEVER AUTO-RESUME" below. |
| - if rec.Provisioned.PauseKind != mail.PauseKindCap { |
| - return rec, false, nil |
| + // A RATE pause, a MANUAL pause and a PER-INSTANCE pause all fall through |
| + // untouched. The first two never auto-resume by design; the third must |
| + // not, because resuming reactivates every key including the standalones |
| + // and the app credential every instance still holds. |
| + if sn.Provisioned.PauseKind != mail.PauseKindCap || sn.PerInstance { |
| + return sn, false, nil |
| } |
| - pausedDay := rec.Provisioned.At.UTC().Format("2006-01-02") |
| + pausedDay := sn.Provisioned.At.UTC().Format("2006-01-02") |
| today := now.UTC().Format("2006-01-02") |
| if pausedDay >= today { |
| // Paused earlier TODAY (or a clock oddity puts the pause in the |
| // future) — not a day roll yet. |
| - return rec, false, nil |
| + return sn, false, nil |
| } |
| - if _, err := mail.SetPaused(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct, app, false, "", now); err != nil { |
| + if _, err := mail.SetSenderPaused(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct, app, sender, false, "", now); err != nil { |
| return nil, false, err |
| } |
| if a, err := s.store.AccountBySqid(acct); err != nil { |
| log.Printf("console: mail resume audit: resolving account %s: %v", acct, err) |
| } else if a != nil { |
| - s.audit(a, app, "ses", "mail.resumed", fmt.Sprintf("%s: daily cap pause cleared on the UTC day roll", app)) |
| + s.audit(a, app, "ses", "mail.resumed", fmt.Sprintf("%s/%s: daily cap pause cleared on the UTC day roll", app, sender)) |
| } |
| - fresh, _, err := mail.Get(ctx, s.cfg.Releases, acct, app) |
| + fresh, _, err := mail.GetSender(ctx, s.cfg.Releases, acct, app, sender) |
| if err != nil { |
| return nil, false, err |
| } |
| return fresh, true, nil |
| } |
| -// enforceMailLimits pauses an app that has exceeded its daily cap or is past |
| -// either SES threshold, and reports why. |
| +// enforceMailLimits pauses a sender that has exceeded its daily cap or is |
| +// past either SES threshold, and reports why. The cap and the pause are the |
| +// sender's own, so one sender's breach leaves its siblings sending. |
| // |
| // The rate thresholds are applied only once MinSendsForRates messages have |
| // gone out that day: without that floor, one bounce on a two-message morning |
| @@ -213,42 +215,44 @@ func (s *Server) resumeStaleCapPause(ctx context.Context, acct, app string, rec |
| // counts that caused it and surfaced on the app page — a silent pause |
| // presents to the member as "email stopped working", which is the failure |
| // this whole mechanism is supposed to prevent someone else's tenant causing. |
| -func (s *Server) enforceMailLimits(ctx context.Context, acct, app string, now time.Time) (bool, string, error) { |
| - rec, _, err := mail.Get(ctx, s.cfg.Releases, acct, app) |
| +func (s *Server) enforceMailLimits(ctx context.Context, acct, app, sender string, now time.Time) (bool, string, error) { |
| + sn, _, err := mail.GetSender(ctx, s.cfg.Releases, acct, app, sender) |
| if err != nil { |
| return false, "", err |
| } |
| - if rec == nil || rec.Provisioned == nil { |
| + if sn == nil || sn.Provisioned == nil { |
| return false, "", nil |
| } |
| - if !rec.Provisioned.Active { |
| - fresh, resumed, err := s.resumeStaleCapPause(ctx, acct, app, rec, now) |
| + if !sn.Provisioned.Active { |
| + fresh, resumed, err := s.resumeStaleCapPause(ctx, acct, app, sender, sn, now) |
| if err != nil { |
| return false, "", err |
| } |
| if !resumed { |
| return false, "", nil |
| } |
| - // Re-read via resumeStaleCapPause: SetPaused just wrote a fresh |
| - // record (Active, PauseKind and At all moved), and everything |
| - // below reads rec fresh rather than patching the in-memory copy by |
| + // Re-read via resumeStaleCapPause: SetSenderPaused just wrote a |
| + // fresh sender (Active, PauseKind and At all moved), and everything |
| + // below reads sn fresh rather than patching the in-memory copy by |
| // hand. |
| - rec = fresh |
| - if rec == nil || rec.Provisioned == nil || !rec.Provisioned.Active { |
| + sn = fresh |
| + if sn == nil || sn.Provisioned == nil || !sn.Provisioned.Active { |
| return false, "", nil |
| } |
| } |
| day := now.UTC().Format("2006-01-02") |
| - // "" and not mail.SenderAppID: enforceMailLimits takes no sender yet, so |
| - // the only cap it can enforce is the app's whole day, same as before |
| - // this table split (task 12 gives this function a sender). |
| - c, err := s.store.MailCounts(ctx, acct, app, "", day) |
| + // The real sender, not "": MailCounts treats "" as every sender the app |
| + // has, and this cap belongs to ONE sender — summing every sibling's |
| + // traffic into it would let one sender's cap trip on another sender's |
| + // mail, which is exactly what keying the cap to the sender is meant to |
| + // stop. |
| + c, err := s.store.MailCounts(ctx, acct, app, sender, day) |
| if err != nil { |
| return false, "", err |
| } |
| cap := TierCap("") |
| - if rec.Grant != nil && rec.Grant.DailyCap > 0 { |
| - cap = rec.Grant.DailyCap |
| + if sn.Grant != nil && sn.Grant.DailyCap > 0 { |
| + cap = sn.Grant.DailyCap |
| } |
| var reason, kind string |
| @@ -270,7 +274,7 @@ func (s *Server) enforceMailLimits(ctx context.Context, acct, app string, now ti |
| return false, "", nil |
| } |
| - if _, err := mail.SetPaused(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct, app, true, kind, now); err != nil { |
| + if _, err := mail.SetSenderPaused(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct, app, sender, true, kind, now); err != nil { |
| return false, "", err |
| } |
| // Audit AFTER the write it announces, and never before — a pause the |
| @@ -279,7 +283,7 @@ func (s *Server) enforceMailLimits(ctx context.Context, acct, app string, now ti |
| if a, err := s.store.AccountBySqid(acct); err != nil { |
| log.Printf("console: mail pause audit: resolving account %s: %v", acct, err) |
| } else if a != nil { |
| - s.audit(a, app, "ses", "mail.paused", fmt.Sprintf("%s: %s", app, reason)) |
| + s.audit(a, app, "ses", "mail.paused", fmt.Sprintf("%s/%s: %s", app, sender, reason)) |
| } |
| return true, reason, nil |
| } |
diff --git a/internal/console/mailcounts_test.go b/internal/console/mailcounts_test.go| index 34464dd82..1c0ea4663 100644 |
| --- a/internal/console/mailcounts_test.go |
| +++ b/internal/console/mailcounts_test.go |
| @@ -96,7 +96,7 @@ func seedCounts(t *testing.T, fx *consoleFixture, day string, c MailCounts) { |
| func TestEnforcePausesOnCapBreach(t *testing.T) { |
| fx, now := mailFixture(t) |
| seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 50}) |
| - paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", now) |
| + paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -117,7 +117,7 @@ func TestEnforcePausesOnCapBreach(t *testing.T) { |
| func TestEnforceIgnoresRatesBelowTheSendFloor(t *testing.T) { |
| fx, now := mailFixture(t) |
| seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 2, Bounces: 1}) |
| - paused, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", now) |
| + paused, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -137,7 +137,7 @@ func TestEnforcePausesOnBounceRate(t *testing.T) { |
| mail.ApplyGrant(context.Background(), fx.rel, release.OpsAccount, "harbourmail", |
| []string{"x.com"}, 100000, "operator", "owner@example.test", now) |
| seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 1000, Bounces: 60}) |
| - paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", now) |
| + paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -167,7 +167,7 @@ func TestEnforcePausesOnComplaintRate(t *testing.T) { |
| mail.ApplyGrant(context.Background(), fx.rel, release.OpsAccount, "harbourmail", |
| []string{"x.com"}, 100000, "operator", "owner@example.test", now) |
| seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 1000, Complaints: 5}) |
| - paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", now) |
| + paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -207,7 +207,7 @@ func TestSESThresholdConstants(t *testing.T) { |
| func TestEnforceCapPauseResumesOnTheNextUTCDay(t *testing.T) { |
| fx, now := mailFixture(t) |
| seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 50}) |
| - if _, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", now); err != nil { |
| + if _, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now); err != nil { |
| t.Fatal(err) |
| } |
| rec, _, _ := mail.Get(context.Background(), fx.rel, release.OpsAccount, "harbourmail") |
| @@ -219,7 +219,7 @@ func TestEnforceCapPauseResumesOnTheNextUTCDay(t *testing.T) { |
| } |
| next := now.AddDate(0, 0, 1) // a fresh UTC day, no counts at all |
| - paused, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", next) |
| + paused, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, next) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -262,7 +262,7 @@ func TestEnforceRatePauseNeverAutoResumes(t *testing.T) { |
| t.Fatal(err) |
| } |
| seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 1000, Bounces: 60}) |
| - if _, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", now); err != nil { |
| + if _, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now); err != nil { |
| t.Fatal(err) |
| } |
| rec, _, _ := mail.Get(context.Background(), fx.rel, release.OpsAccount, "harbourmail") |
| @@ -274,7 +274,7 @@ func TestEnforceRatePauseNeverAutoResumes(t *testing.T) { |
| } |
| next := now.AddDate(0, 0, 1) // a fresh UTC day |
| - paused, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", next) |
| + paused, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, next) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -302,14 +302,14 @@ func TestEnforceAlreadyPausedDoesNotReprocess(t *testing.T) { |
| fx, now := mailFixture(t) |
| day := now.UTC().Format("2006-01-02") |
| seedCounts(t, fx, day, MailCounts{Sends: 50}) |
| - if _, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", now); err != nil { |
| + if _, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now); err != nil { |
| t.Fatal(err) |
| } |
| // More sends land on the SAME day, well past the cap — a re-evaluating |
| // enforce would trip again. |
| seedCounts(t, fx, day, MailCounts{Sends: 25}) |
| - paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", now) |
| + paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -339,7 +339,7 @@ func TestEnforceAlreadyPausedDoesNotReprocess(t *testing.T) { |
| func TestEnforceRecordsTheReasonInTheAuditLog(t *testing.T) { |
| fx, now := mailFixture(t) |
| seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 50}) |
| - fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", now) |
| + fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now) |
| entries, err := fx.st.Events(OpsAccountID, "harbourmail", 10) |
| if err != nil { |
| @@ -379,11 +379,11 @@ func pauseDirectly(t *testing.T, fx *consoleFixture, app, kind string, at time.T |
| func TestResumeStaleCapPauseClearsACapPauseFromYesterday(t *testing.T) { |
| fx, now := mailFixture(t) |
| pauseDirectly(t, fx, "harbourmail", mail.PauseKindCap, now.AddDate(0, 0, -1)) |
| - rec, _, err := mail.Get(context.Background(), fx.rel, release.OpsAccount, "harbourmail") |
| + sn, _, err := mail.GetSender(context.Background(), fx.rel, release.OpsAccount, "harbourmail", mail.SenderAppID) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - got, resumed, err := fx.srv.resumeStaleCapPause(context.Background(), release.OpsAccount, "harbourmail", rec, now) |
| + got, resumed, err := fx.srv.resumeStaleCapPause(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, sn, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -401,11 +401,11 @@ func TestResumeStaleCapPauseClearsACapPauseFromYesterday(t *testing.T) { |
| func TestResumeStaleCapPauseLeavesATodayCapPauseAlone(t *testing.T) { |
| fx, now := mailFixture(t) |
| pauseDirectly(t, fx, "harbourmail", mail.PauseKindCap, now) // paused earlier the SAME UTC day |
| - rec, _, err := mail.Get(context.Background(), fx.rel, release.OpsAccount, "harbourmail") |
| + sn, _, err := mail.GetSender(context.Background(), fx.rel, release.OpsAccount, "harbourmail", mail.SenderAppID) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - got, resumed, err := fx.srv.resumeStaleCapPause(context.Background(), release.OpsAccount, "harbourmail", rec, now) |
| + got, resumed, err := fx.srv.resumeStaleCapPause(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, sn, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -420,11 +420,11 @@ func TestResumeStaleCapPauseLeavesATodayCapPauseAlone(t *testing.T) { |
| func TestResumeStaleCapPauseNeverTouchesARatePause(t *testing.T) { |
| fx, now := mailFixture(t) |
| pauseDirectly(t, fx, "harbourmail", mail.PauseKindRate, now.AddDate(0, 0, -1)) |
| - rec, _, err := mail.Get(context.Background(), fx.rel, release.OpsAccount, "harbourmail") |
| + sn, _, err := mail.GetSender(context.Background(), fx.rel, release.OpsAccount, "harbourmail", mail.SenderAppID) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - got, resumed, err := fx.srv.resumeStaleCapPause(context.Background(), release.OpsAccount, "harbourmail", rec, now) |
| + got, resumed, err := fx.srv.resumeStaleCapPause(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, sn, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -442,11 +442,11 @@ func TestResumeStaleCapPauseNeverTouchesARatePause(t *testing.T) { |
| func TestResumeStaleCapPauseNeverTouchesAKindlessPause(t *testing.T) { |
| fx, now := mailFixture(t) |
| pauseDirectly(t, fx, "harbourmail", "", now.AddDate(0, 0, -1)) |
| - rec, _, err := mail.Get(context.Background(), fx.rel, release.OpsAccount, "harbourmail") |
| + sn, _, err := mail.GetSender(context.Background(), fx.rel, release.OpsAccount, "harbourmail", mail.SenderAppID) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - got, resumed, err := fx.srv.resumeStaleCapPause(context.Background(), release.OpsAccount, "harbourmail", rec, now) |
| + got, resumed, err := fx.srv.resumeStaleCapPause(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, sn, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -571,3 +571,38 @@ func TestMailCountsAreKeptAndReadPerSender(t *testing.T) { |
| t.Fatalf("a quiet day = %+v %v", quiet, err) |
| } |
| } |
| + |
| +// The cap applies per sender, so one sender's breach pauses that sender and |
| +// leaves its siblings sending. |
| +func TestTheCapIsEnforcedPerSender(t *testing.T) { |
| + fx, now := mailFixture(t) |
| + ctx := context.Background() |
| + day := now.UTC().Format("2006-01-02") |
| + for i := 0; i < 50; i++ { |
| + if err := fx.st.RecordMailEvent(ctx, release.OpsAccount, "harbourmail", mail.SenderAppID, day, mail.EventSend); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + // A second sender's traffic does not count towards the first one's cap. |
| + for i := 0; i < 50; i++ { |
| + if err := fx.st.RecordMailEvent(ctx, release.OpsAccount, "harbourmail", "book.example.com", day, mail.EventSend); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + paused, reason, err := fx.srv.enforceMailLimits(ctx, release.OpsAccount, "harbourmail", mail.SenderAppID, now) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !paused || !strings.Contains(reason, "daily cap 50") { |
| + t.Fatalf("app sender = %v %q, want a cap pause", paused, reason) |
| + } |
| + sn, _, err := mail.GetSender(ctx, fx.rel, release.OpsAccount, "harbourmail", mail.SenderAppID) |
| + if err != nil || sn.Provisioned.Active || sn.Provisioned.PauseKind != mail.PauseKindCap { |
| + t.Fatalf("the app sender's record = %+v %v", sn.Provisioned, err) |
| + } |
| + // The day roll clears a cap pause, still, and only a cap pause. |
| + fresh, resumed, err := fx.srv.resumeStaleCapPause(ctx, release.OpsAccount, "harbourmail", mail.SenderAppID, sn, now.AddDate(0, 0, 1)) |
| + if err != nil || !resumed || fresh == nil || !fresh.Provisioned.Active { |
| + t.Fatalf("the day roll did not clear the cap pause: %+v %v %v", fresh, resumed, err) |
| + } |
| +} |
diff --git a/internal/console/sesevents.go b/internal/console/sesevents.go| index e00233481..a4c3f5c4a 100644 |
| --- a/internal/console/sesevents.go |
| +++ b/internal/console/sesevents.go |
| @@ -521,7 +521,7 @@ func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event, notice |
| 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 { |
| + if _, reason, err := s.enforceMailLimits(ctx, callerAcct, callerApp, callerSender, 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) |
| @@ -568,7 +568,7 @@ func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event, notice |
| log.Printf("console: ses events: recording %s for %s/%s: %v", ev.Type, a.Sqid, app, err) |
| continue |
| } |
| - if _, reason, err := s.enforceMailLimits(ctx, a.Sqid, app, at); err != nil { |
| + if _, reason, err := s.enforceMailLimits(ctx, a.Sqid, app, mail.SenderAppID, at); err != nil { |
| log.Printf("console: ses events: enforcing limits for %s/%s: %v", a.Sqid, app, err) |
| } else if reason != "" { |
| log.Printf("console: ses events: paused %s/%s: %s", a.Sqid, app, reason) |
diff --git a/internal/mail/walk.go b/internal/mail/walk.go| index e43b745cf..b250b831d 100644 |
| --- a/internal/mail/walk.go |
| +++ b/internal/mail/walk.go |
| @@ -967,20 +967,35 @@ func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app |
| return rec, nil |
| } |
| -// SetPaused stops or resumes sending — every principal this app owns, not |
| -// just the delivered one. |
| -// |
| -// It toggles the KEY, never the policy. A policy rewrite would have to be |
| -// undone correctly to resume, and a half-failed rewrite leaves a grant that |
| -// is neither the old one nor the new one. Deactivating a key is atomic at |
| -// IAM and trivially reversible. |
| +// ErrPerInstance refuses a resume aimed at an app sender whose app has moved |
| +// to per-instance senders (design section 6.3). Nothing sets PerInstance in this |
| +// release; the refusal exists so that nothing can set it later without this |
| +// guard already being in place and tested. |
| +var ErrPerInstance = errors.New("mail: the app sender is in per-instance mode") |
| + |
| +// SetPaused stops or resumes the APP sender, which is every sender an app |
| +// has until instance senders exist. Its signature is unchanged so the |
| +// operator door and the limiter keep calling it exactly as they did. |
| +func SetPaused(ctx context.Context, s release.Store, p Provisioner, |
| + acct, app string, paused bool, kind string, now time.Time) (*Record, error) { |
| + |
| + sn, err := SetSenderPaused(ctx, s, p, acct, app, SenderAppID, paused, kind, now) |
| + if err != nil { |
| + return nil, err |
| + } |
| + return sn.record(app), nil |
| +} |
| + |
| +// SetSenderPaused toggles one sender's keys, never its policy. A policy |
| +// rewrite would have to be undone correctly to resume, and a half-failed |
| +// rewrite leaves a grant that is neither the old one nor the new one; |
| +// deactivating a key is atomic at IAM and trivially reversible. |
| // |
| -// BOTH of the delivered user's keys are toggled: an app mid-rotation still |
| -// holds the previous credential, and pausing only the current one would |
| -// leave it sending. Every standalone credential (I2) is toggled too — each |
| -// is its OWN IAM user, so there is no two-key ceiling interaction, and |
| -// skipping them would leave a tenant paused for abuse still sending from the |
| -// same verified domain through, say, a laptop credential. |
| +// BOTH of the delivered user's keys are toggled, because an app |
| +// mid-rotation still holds the previous credential, and every standalone |
| +// credential of this sender is toggled too — each is its own IAM user, so |
| +// there is no two-key ceiling interaction, and skipping them would leave a |
| +// tenant paused for abuse still sending through a laptop credential. |
| // |
| // kind records WHICH automatic pause this is (mail.PauseKindCap or |
| // mail.PauseKindRate) so mailcounts.go's enforceMailLimits can tell a |
| @@ -990,18 +1005,25 @@ func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app |
| // (paused == false): PauseKind is always cleared back to "" on resume, |
| // whatever resumed it, so a record never carries a stale kind from a pause |
| // that is already lifted. |
| -func SetPaused(ctx context.Context, s release.Store, p Provisioner, |
| - acct, app string, paused bool, kind string, now time.Time) (*Record, error) { |
| +// |
| +// A resume is refused while PerInstance is set: the app's instances still |
| +// hold this credential, and re-arming it is exactly what per-instance mode |
| +// exists to prevent. A pause is always allowed. |
| +func SetSenderPaused(ctx context.Context, s release.Store, p Provisioner, |
| + acct, app, sender string, paused bool, kind string, now time.Time) (*Sender, error) { |
| - rec, etag, err := Get(ctx, s, acct, app) |
| + sn, etag, err := GetSender(ctx, s, acct, app, sender) |
| if err != nil { |
| return nil, err |
| } |
| - if rec == nil || rec.Provisioned == nil { |
| + if sn == nil || sn.Provisioned == nil { |
| return nil, ErrNotProvisioned |
| } |
| - user := rec.Provisioned.IAMUser |
| - for _, id := range []string{rec.Provisioned.KeyID, rec.Provisioned.PrevKeyID} { |
| + if !paused && sn.PerInstance { |
| + return nil, fmt.Errorf("mail: %s/%s: resume its instance senders instead: %w", acct, app, ErrPerInstance) |
| + } |
| + user := sn.Provisioned.IAMUser |
| + for _, id := range []string{sn.Provisioned.KeyID, sn.Provisioned.PrevKeyID} { |
| if id == "" { |
| continue |
| } |
| @@ -1009,22 +1031,22 @@ func SetPaused(ctx context.Context, s release.Store, p Provisioner, |
| return nil, err |
| } |
| } |
| - for _, sa := range rec.Standalone { |
| + for _, sa := range sn.Standalone { |
| if err := p.SetKeyActive(ctx, sa.IAMUser, sa.KeyID, !paused); err != nil { |
| return nil, err |
| } |
| } |
| - rec.Provisioned.Active = !paused |
| + sn.Provisioned.Active = !paused |
| if paused { |
| - rec.Provisioned.PauseKind = kind |
| + sn.Provisioned.PauseKind = kind |
| } else { |
| - rec.Provisioned.PauseKind = "" |
| + sn.Provisioned.PauseKind = "" |
| } |
| - rec.Provisioned.At = now |
| - if _, err := Put(ctx, s, acct, app, rec, etag); err != nil { |
| + sn.Provisioned.At = now |
| + if _, err := PutSender(ctx, s, acct, app, sn, etag); err != nil { |
| return nil, err |
| } |
| - return rec, nil |
| + return sn, nil |
| } |
| // CreateStandalone mints a displayed credential on its OWN IAM principal. |
diff --git a/internal/mail/walk_lifecycle_test.go b/internal/mail/walk_lifecycle_test.go| index 2b48de825..cce966e1f 100644 |
| --- a/internal/mail/walk_lifecycle_test.go |
| +++ b/internal/mail/walk_lifecycle_test.go |
| @@ -578,4 +578,36 @@ func TestTeardownDeletesTheLegacyRecordToo(t *testing.T) { |
| } |
| } |
| +// An app sender in per-instance mode refuses to resume. Nothing sets the |
| +// flag in this release, but the refusal has to be in place before anything |
| +// can: resumeStaleCapPause reactivates every key it touches, including every |
| +// standalone, so a cap-paused app sender would otherwise re-arm on the next |
| +// UTC day roll the credential every instance still holds. |
| +func TestPerInstanceModeRefusesAResume(t *testing.T) { |
| + ctx, s, p, pub, now := enableFixture(t) |
| + if _, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "harbourmail", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := SetPaused(ctx, s, p, "bag", "harbourmail", true, PauseKindCap, now); err != nil { |
| + t.Fatal(err) |
| + } |
| + sn, etag, err := GetSender(ctx, s, "bag", "harbourmail", SenderAppID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + sn.PerInstance = true |
| + sn.Provisioned.PauseKind = PauseKindPerInstance |
| + if _, err := PutSender(ctx, s, "bag", "harbourmail", sn, etag); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := SetPaused(ctx, s, p, "bag", "harbourmail", false, "", now); !errors.Is(err, ErrPerInstance) { |
| + t.Fatalf("resume in per-instance mode = %v, want ErrPerInstance", err) |
| + } |
| + // Pausing harder is still allowed: the refusal is about re-arming a |
| + // credential, not about turning one off. |
| + if _, err := SetPaused(ctx, s, p, "bag", "harbourmail", true, PauseKindRate, now); err != nil { |
| + t.Fatalf("pausing an app sender in per-instance mode was refused: %v", err) |
| + } |
| +} |
| + |
| func memWriterForTest() dnsrec.Writer { return dnsrec.NewMemWriter() } |