amadan

carlos / platform Public

mail: associate each account's tenant with its identities and its configuration set (plan 2, task 7)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by claude bf04ed984247403fcb3f65f3133694cc671113a4 parent 0e56440
8 files changed, +244 −22
  • internal/mail/aws.go +50 −12
  • internal/mail/aws_test.go +8 −0
  • internal/mail/domain.go +17 −4
  • internal/mail/memprovisioner.go +43 −6
  • internal/mail/memprovisioner_test.go +28 −0
  • internal/mail/provisioner.go +12 −0
  • internal/mail/walk.go +33 −0
  • internal/mail/walk_domain_test.go +53 −0
diff --git a/internal/mail/aws.go b/internal/mail/aws.go
index f97c7f9e6..0c68bd433 100644
--- a/internal/mail/aws.go
+++ b/internal/mail/aws.go
@@ -19,10 +19,12 @@
// provisioner. EnsureTenant below therefore does the one thing the Provisioner
// interface actually asks of it — create the account's named tenant container,
// idempotently — and stops there: it does not call
-// CreateTenantResourceAssociation, because the interface gives EnsureTenant no
-// identity or config-set to associate. Isolation is unaffected either way:
-// the tenant buys REPUTATION isolation, and AUTHORITY isolation is the IAM
-// policy in policy.go (spec §4).
+// CreateTenantResourceAssociation itself, because the interface gives
+// EnsureTenant no identity or config-set to associate; that association is
+// wired separately, by walk.go's EnsureDomain, once it has both (plan 2,
+// task 7). Isolation is unaffected either way: the tenant buys REPUTATION
+// isolation, and AUTHORITY isolation is the IAM policy in policy.go (spec
+// §4).
package mail
import (
@@ -60,6 +62,8 @@ type sesAPI interface {
CreateConfigurationSet(context.Context, *sesv2.CreateConfigurationSetInput, ...func(*sesv2.Options)) (*sesv2.CreateConfigurationSetOutput, error)
CreateConfigurationSetEventDestination(context.Context, *sesv2.CreateConfigurationSetEventDestinationInput, ...func(*sesv2.Options)) (*sesv2.CreateConfigurationSetEventDestinationOutput, error)
PutEmailIdentityConfigurationSetAttributes(context.Context, *sesv2.PutEmailIdentityConfigurationSetAttributesInput, ...func(*sesv2.Options)) (*sesv2.PutEmailIdentityConfigurationSetAttributesOutput, error)
+ CreateTenantResourceAssociation(context.Context, *sesv2.CreateTenantResourceAssociationInput, ...func(*sesv2.Options)) (*sesv2.CreateTenantResourceAssociationOutput, error)
+ DeleteTenantResourceAssociation(context.Context, *sesv2.DeleteTenantResourceAssociationInput, ...func(*sesv2.Options)) (*sesv2.DeleteTenantResourceAssociationOutput, error)
}
// iamAPI is the subset of *iam.Client awsProvisioner calls.
@@ -168,17 +172,19 @@ func isNotFound(err error) bool {
// unaffected by that gap: the tenant buys reputation isolation, and
// authority isolation is the IAM policy (spec §4), which this does not touch.
//
-// PRECONDITION FOR SENDING, NOT YET MET HERE: per
+// PRECONDITION FOR SENDING, NOW MET BY EnsureDomain, NOT HERE: per
// docs.aws.amazon.com/ses/latest/dg/tenants.html, a tenant needs at least
// one verified identity AND one configuration set associated via
// CreateTenantResourceAssociation before SES will accept a send attributed
-// to it — SES validates that association on every send. Nothing in this
-// package wires that association yet. Consequently: the send path MUST NOT
-// emit the `X-SES-TENANT: <name>` header until associations are wired,
-// wherever that lands (identities and config sets are ensured by separate
-// calls, so it is not naturally EnsureTenant's job). Emitting the header
-// against a tenant with no associated resources fails every send, the
-// opposite of what tenants exist to do.
+// to it — SES validates that association on every send. walk.go's
+// EnsureDomain wires both associations now, after PutDomain, using
+// AssociateTenantResource below (plan 2, task 7). This still does not make
+// any send carry the `X-SES-TENANT: <name>` header: nothing in step 2
+// composes an outgoing message or writes a tenant-conditioned policy, so
+// that header waits for step 3. The spike's answer (design §4.1, confirmed
+// against the live API): an association is inert for the identity's owner
+// in every respect except deletion, and `ses:TenantName` IS enforced over
+// SMTP.
func (p *awsProvisioner) EnsureTenant(ctx context.Context, name string) error {
_, err := p.ses.CreateTenant(ctx, &sesv2.CreateTenantInput{
TenantName: aws.String(name),
@@ -189,6 +195,38 @@ func (p *awsProvisioner) EnsureTenant(ctx context.Context, name string) error {
return nil
}
+// AssociateTenantResource associates an identity or a configuration set with
+// a tenant. This is the call the file header said nothing here made yet: SES
+// validates the association on every tenant-attributed send, so it must exist
+// before any send carries X-SES-TENANT. Associating changes nothing about the
+// resource itself, so it is safe on an identity another product owns.
+func (p *awsProvisioner) AssociateTenantResource(ctx context.Context, tenant, resourceARN string) error {
+ _, err := p.ses.CreateTenantResourceAssociation(ctx, &sesv2.CreateTenantResourceAssociationInput{
+ TenantName: aws.String(tenant),
+ ResourceArn: aws.String(resourceARN),
+ })
+ if err != nil && !isAlreadyExists(err) {
+ return fmt.Errorf("mail: associate %s with tenant %q: %w", resourceARN, tenant, err)
+ }
+ return nil
+}
+
+// DisassociateTenantResource removes one association. It has to run before
+// DeleteEmailIdentity: SES refuses to delete an identity while an
+// association exists, which is also the cross-team contract for a borrowed
+// identity (design §7) — while CARLOS's association stands, the owner cannot
+// delete their own identity.
+func (p *awsProvisioner) DisassociateTenantResource(ctx context.Context, tenant, resourceARN string) error {
+ _, err := p.ses.DeleteTenantResourceAssociation(ctx, &sesv2.DeleteTenantResourceAssociationInput{
+ TenantName: aws.String(tenant),
+ ResourceArn: aws.String(resourceARN),
+ })
+ if err != nil && !isNotFound(err) {
+ return fmt.Errorf("mail: disassociate %s from tenant %q: %w", resourceARN, tenant, err)
+ }
+ return nil
+}
+
func (p *awsProvisioner) EnsureConfigSet(ctx context.Context, region, name, snsTopicARN string) error {
if err := p.checkRegion(region); err != nil {
return err
diff --git a/internal/mail/aws_test.go b/internal/mail/aws_test.go
index 0f28fa06f..ae10580bb 100644
--- a/internal/mail/aws_test.go
+++ b/internal/mail/aws_test.go
@@ -113,6 +113,14 @@ func (s *stubSES) PutEmailIdentityConfigurationSetAttributes(_ context.Context,
return &sesv2.PutEmailIdentityConfigurationSetAttributesOutput{}, nil
}
+func (s *stubSES) CreateTenantResourceAssociation(_ context.Context, in *sesv2.CreateTenantResourceAssociationInput, _ ...func(*sesv2.Options)) (*sesv2.CreateTenantResourceAssociationOutput, error) {
+ return &sesv2.CreateTenantResourceAssociationOutput{}, nil
+}
+
+func (s *stubSES) DeleteTenantResourceAssociation(_ context.Context, in *sesv2.DeleteTenantResourceAssociationInput, _ ...func(*sesv2.Options)) (*sesv2.DeleteTenantResourceAssociationOutput, error) {
+ return &sesv2.DeleteTenantResourceAssociationOutput{}, nil
+}
+
var _ sesAPI = (*stubSES)(nil)
// TestEnsureIdentityAlreadyExistsReadsBackTokensAndComposesTheARN pins the
diff --git a/internal/mail/domain.go b/internal/mail/domain.go
index 1ce9499e1..b41fe8d8f 100644
--- a/internal/mail/domain.go
+++ b/internal/mail/domain.go
@@ -167,16 +167,29 @@ func (r *DomainRecord) VerifiedConfigSetARNs() []string {
if st == nil || st.Status != StatusVerified || st.ConfigSet == "" || st.IdentityARN == "" {
continue
}
- parts := strings.Split(st.IdentityARN, ":")
- if len(parts) != 6 {
- continue
+ if arn, ok := ConfigSetARN(st.IdentityARN, st.ConfigSet); ok {
+ out = append(out, arn)
}
- out = append(out, strings.Join(parts[:5], ":")+":configuration-set/"+st.ConfigSet)
}
sort.Strings(out)
return out
}
+// ConfigSetARN composes a configuration set's ARN from an identity ARN in
+// the same account and region, which is the only place this package has
+// those two values. Reports false for an ARN it cannot parse, so a caller
+// never builds a resource string out of a malformed one.
+func ConfigSetARN(identityARN, configSet string) (string, bool) {
+ if identityARN == "" || configSet == "" {
+ return "", false
+ }
+ parts := strings.Split(identityARN, ":")
+ if len(parts) != 6 {
+ return "", false
+ }
+ return strings.Join(parts[:5], ":") + ":configuration-set/" + configSet, true
+}
+
// AddApp records an app as a referrer. Reports whether it changed anything.
func (r *DomainRecord) AddApp(app string) bool {
for _, a := range r.Apps {
diff --git a/internal/mail/memprovisioner.go b/internal/mail/memprovisioner.go
index 75bfb1795..5d1f2480f 100644
--- a/internal/mail/memprovisioner.go
+++ b/internal/mail/memprovisioner.go
@@ -48,6 +48,11 @@ type MemProvisioner struct {
// Identities is keyed "region|domain".
Identities map[string]*MemIdentity
Users map[string]*MemUser
+ // TenantResources is tenant -> resource ARN -> associated. An identity
+ // with an association cannot be deleted, mirroring SES, because a walk
+ // that tore down in the wrong order would pass against a permissive fake
+ // and fail on the first real teardown.
+ TenantResources map[string]map[string]bool
// FailNext, keyed by method name, is consumed one-shot. Consumption
// happens BEFORE any state change so a failed call leaves state
// untouched — a walk test needs the retry to start from a clean point.
@@ -61,12 +66,13 @@ type MemProvisioner struct {
func NewMemProvisioner(account string) *MemProvisioner {
return &MemProvisioner{
- account: account,
- Tenants: map[string]bool{},
- ConfigSets: map[string]string{},
- Identities: map[string]*MemIdentity{},
- Users: map[string]*MemUser{},
- FailNext: map[string]error{},
+ account: account,
+ Tenants: map[string]bool{},
+ ConfigSets: map[string]string{},
+ Identities: map[string]*MemIdentity{},
+ Users: map[string]*MemUser{},
+ TenantResources: map[string]map[string]bool{},
+ FailNext: map[string]error{},
StatusOnEnsure: Status{
DKIM: "PENDING", MailFrom: "PENDING", Sending: "PENDING",
},
@@ -194,10 +200,41 @@ func (p *MemProvisioner) DeleteIdentity(_ context.Context, region, domain string
if err := p.takeFailure("DeleteIdentity"); err != nil {
return err
}
+ got, ok := p.Identities[identityKey(region, domain)]
+ if ok {
+ for tenant, resources := range p.TenantResources {
+ if resources[got.ARN] {
+ return fmt.Errorf("mail: identity %s is still associated with tenant %s", domain, tenant)
+ }
+ }
+ }
delete(p.Identities, identityKey(region, domain))
return nil
}
+func (p *MemProvisioner) AssociateTenantResource(_ context.Context, tenant, resourceARN string) error {
+ p.mu.Lock()
+ defer p.mu.Unlock()
+ if err := p.takeFailure("AssociateTenantResource"); err != nil {
+ return err
+ }
+ if p.TenantResources[tenant] == nil {
+ p.TenantResources[tenant] = map[string]bool{}
+ }
+ p.TenantResources[tenant][resourceARN] = true
+ return nil
+}
+
+func (p *MemProvisioner) DisassociateTenantResource(_ context.Context, tenant, resourceARN string) error {
+ p.mu.Lock()
+ defer p.mu.Unlock()
+ if err := p.takeFailure("DisassociateTenantResource"); err != nil {
+ return err
+ }
+ delete(p.TenantResources[tenant], resourceARN)
+ return nil
+}
+
func (p *MemProvisioner) EnsureUser(_ context.Context, name, path string, identityARNs, configSetARNs, domains []string) (string, error) {
p.mu.Lock()
defer p.mu.Unlock()
diff --git a/internal/mail/memprovisioner_test.go b/internal/mail/memprovisioner_test.go
index 12ccdfbb9..db81baf13 100644
--- a/internal/mail/memprovisioner_test.go
+++ b/internal/mail/memprovisioner_test.go
@@ -131,3 +131,31 @@ func TestMemFailNextIsOneShot(t *testing.T) {
t.Fatalf("second call = %v, want nil", err)
}
}
+
+// The fake refuses to delete an identity that is still associated with a
+// tenant, because SES does. A walk that got the order wrong would pass
+// against a permissive fake and fail on the first real teardown.
+func TestMemProvisionerRefusesToDeleteAnAssociatedIdentity(t *testing.T) {
+ ctx := context.Background()
+ p := NewMemProvisioner("1")
+ id, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "mail-eu-west-1.x.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := p.AssociateTenantResource(ctx, "carlos-bag", id.ARN); err != nil {
+ t.Fatal(err)
+ }
+ if err := p.DeleteIdentity(ctx, "eu-west-1", "x.com"); err == nil {
+ t.Fatal("an associated identity was deleted")
+ }
+ if err := p.DisassociateTenantResource(ctx, "carlos-bag", id.ARN); err != nil {
+ t.Fatal(err)
+ }
+ // Disassociating twice is a success, like every other verb on this seam.
+ if err := p.DisassociateTenantResource(ctx, "carlos-bag", id.ARN); err != nil {
+ t.Fatal(err)
+ }
+ if err := p.DeleteIdentity(ctx, "eu-west-1", "x.com"); err != nil {
+ t.Fatal(err)
+ }
+}
diff --git a/internal/mail/provisioner.go b/internal/mail/provisioner.go
index 30dfaa5ee..1b2d0bf00 100644
--- a/internal/mail/provisioner.go
+++ b/internal/mail/provisioner.go
@@ -65,6 +65,18 @@ type Provisioner interface {
// once per (region, domain) — idempotent, like every verb here.
SetIdentityConfigSet(ctx context.Context, region, domain, configSet string) error
+ // AssociateTenantResource associates an identity, a configuration set or
+ // a template with a tenant. Associating does not modify the resource and
+ // one resource may belong to many tenants, so this is safe to call on an
+ // identity another product owns. It has exactly one side effect on the
+ // owner: an identity cannot be deleted while an association exists,
+ // which is why DisassociateTenantResource has to run before
+ // DeleteIdentity. Idempotent, like every verb here.
+ AssociateTenantResource(ctx context.Context, tenant, resourceARN string) error
+ // DisassociateTenantResource removes one association. "Already gone" is
+ // success.
+ DisassociateTenantResource(ctx context.Context, tenant, resourceARN string) error
+
// EnsureIdentity creates the domain identity, enables Easy DKIM and
// sets the custom MAIL FROM subdomain, all idempotently.
EnsureIdentity(ctx context.Context, region, domain, mailFrom string) (Identity, error)
diff --git a/internal/mail/walk.go b/internal/mail/walk.go
index a69f85dd9..e43b745cf 100644
--- a/internal/mail/walk.go
+++ b/internal/mail/walk.go
@@ -281,6 +281,30 @@ func EnsureDomain(ctx context.Context, s release.Store, p Provisioner, w dnsrec.
return nil, nil, err
}
+ // The account's tenant gets this identity, and its configuration set
+ // (design §7). Association does not modify the resource and one resource
+ // may belong to many tenants, so this changes nothing anyone can observe
+ // today. It is wired now because SES validates the association on every
+ // tenant-attributed send and step 3 sends that way.
+ //
+ // AFTER PutDomain, not beside EnsureIdentity, and that ordering is the
+ // whole of what keeps Teardown's orphan-claim sweep working. An
+ // association is the one thing that stops an identity being deleted, and
+ // the sweep (walk.go's orphaned-claims loop, below) deletes the identity
+ // behind a claim that has NO record — so it never runs the disassociate
+ // in Teardown's region loop. Associate before the record is durable and
+ // a crash between the two leaves an identity nothing can ever reap.
+ if err := p.AssociateTenantResource(ctx, tenant, ident.ARN); err != nil {
+ return nil, nil, fmt.Errorf("mail: associate identity %s with tenant %s: %w", domain, tenant, err)
+ }
+ if configSet != "" {
+ if setARN, ok := ConfigSetARN(ident.ARN, configSet); ok {
+ if err := p.AssociateTenantResource(ctx, tenant, setARN); err != nil {
+ return nil, nil, fmt.Errorf("mail: associate configuration set %s with tenant %s: %w", configSet, tenant, err)
+ }
+ }
+ }
+
// GRANT BEFORE BIND. Every app that sends on this domain — and every
// standalone credential those apps hold — gets its policy recomputed
// from the record just written, so the configuration-set ARN is in
@@ -1234,6 +1258,15 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, w dnsrec.Writ
}
}
}
+ // SES refuses to delete an identity while a tenant association
+ // exists, so the association goes first. The account's
+ // configuration set is NOT disassociated: it is per account and
+ // outlives any one app.
+ if st.IdentityARN != "" {
+ if err := p.DisassociateTenantResource(ctx, TenantName(acct), st.IdentityARN); err != nil {
+ return err
+ }
+ }
if err := p.DeleteIdentity(ctx, region, cur.Domain); err != nil {
return err
}
diff --git a/internal/mail/walk_domain_test.go b/internal/mail/walk_domain_test.go
index d261f8b87..a3f584a43 100644
--- a/internal/mail/walk_domain_test.go
+++ b/internal/mail/walk_domain_test.go
@@ -370,3 +370,56 @@ func TestEnsureDomainRepairsAPartialPublish(t *testing.T) {
t.Fatalf("after repair, published %d records, want 6 with no duplicates", len(inner.Records))
}
}
+
+// Step 2 associates each account's EXISTING tenant with its identities and
+// its configuration set (design §7, "Associations are wired in step 2, not
+// here"). Associating a resource does not modify it and changes no
+// behaviour on its own: no send carries X-SES-TENANT yet. It is done now so
+// that step 3 can rely on it, and because SES validates the association on
+// every tenant-attributed send.
+func TestEnsureDomainAssociatesTheIdentityAndTheSetWithTheAccountsTenant(t *testing.T) {
+ ctx := context.Background()
+ s, p := release.NewDirStore(t.TempDir()), NewMemProvisioner("1")
+ now := time.Date(2026, 9, 15, 12, 0, 0, 0, time.UTC)
+ if _, err := Declare(ctx, s, "bag", "harbourmail", "HARBOURMAIL_SMTP",
+ "noreply@harbourmail.bag.oncarlos.com", "owner@example.test", now); err != nil {
+ t.Fatal(err)
+ }
+ rec, _, err := EnsureDomain(ctx, s, p, dnsrec.NewMemWriter(), testDeployment(),
+ "bag", "harbourmail", "harbourmail.bag.oncarlos.com", now)
+ if err != nil {
+ t.Fatal(err)
+ }
+ tenant := TenantName("bag")
+ identity := rec.Regions["eu-west-1"].IdentityARN
+ set, ok := ConfigSetARN(identity, ConfigSetName("bag"))
+ if !ok {
+ t.Fatalf("no configuration-set ARN composed from %q", identity)
+ }
+ for _, arn := range []string{identity, set} {
+ if !p.TenantResources[tenant][arn] {
+ t.Errorf("%s is not associated with %s: %v", arn, tenant, p.TenantResources[tenant])
+ }
+ }
+ // A re-run associates the same pair again without error: every verb on
+ // this seam is idempotent, which is what makes the walk re-runnable.
+ if _, _, err := EnsureDomain(ctx, s, p, dnsrec.NewMemWriter(), testDeployment(),
+ "bag", "harbourmail", "harbourmail.bag.oncarlos.com", now); err != nil {
+ t.Fatalf("re-run: %v", err)
+ }
+
+ // An identity cannot be deleted while an association exists (design §3),
+ // so teardown removes the association first. MemProvisioner models that
+ // refusal, because without it the first real teardown would fail.
+ if err := Teardown(ctx, s, p, dnsrec.NewMemWriter(), testDeployment(), "bag", "harbourmail"); err != nil {
+ t.Fatalf("Teardown: %v", err)
+ }
+ if p.TenantResources[tenant][identity] {
+ t.Error("the identity association survived teardown, so the identity could never be deleted")
+ }
+ // The configuration set is per ACCOUNT and outlives any one app, so its
+ // association is never removed by an app's teardown.
+ if !p.TenantResources[tenant][set] {
+ t.Error("an app's teardown removed the account's configuration-set association")
+ }
+}