console: attach core review fixes — idempotent record conflict, two-sided rollback, channel rails
Review of the attach/detach core (task 3) found five defects and one design ruling to implement: - A pre-existing record at the host's key folded into a generic 500, which made the host permanently unattachable (the claim rolled back on every retry and detach 404'd). A conflict whose record reads back as this app's own is now an idempotent success keeping the claim; an unreadable one rolls the claim back and refuses 422 without naming a holder. - Rollback was one-sided: an ambiguous record write (a timeout after the object landed) released the claim and left the record, a route materializing for a host anyone could then claim. Both are unwound now, in detach's order. - channelFlag reached Record.Channel unvalidated — release.ValidChannel is the rail the instance-create door already uses. Existence stays deferred. - On the self-host backfill the record's channel now comes from the row that already serves the host (spec owner's ruling): #137's adoption converges the record's channel onto that live row, so any other value silently repoints a running operator route, and a two-channel app was refused as ambiguous for a domain whose channel was never in question. - Both verbs guard a nil Releases with the instance API's own 503.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 files changed,
+382
−35
internal/console/domainattach.go+129 −33internal/console/domainattach_test.go+253 −2
diff --git a/internal/console/domainattach.go b/internal/console/domainattach.go| index 26a40d2de..5249a5c91 100644 |
| --- a/internal/console/domainattach.go |
| +++ b/internal/console/domainattach.go |
| @@ -16,9 +16,12 @@ |
| // picks the failure worth having — a half-failed attach leaves at worst a |
| // claim with no record (inert: no route is materialized from a claim, and |
| // detach or a retry clears it), never a live route whose host is |
| -// unclaimed. The record write failing therefore rolls the claim back |
| -// best-effort and refuses; the reverse order would let two accounts each |
| -// declare a record for one host and the loser find out from the fleet. |
| +// unclaimed. A record write that fails ambiguously therefore unwinds both |
| +// best-effort and refuses, while a record ALREADY at the key (which, being |
| +// account+app scoped, can only be this app's own) makes the attach |
| +// idempotent rather than a permanent 500. The reverse write order would let |
| +// two accounts each declare a record for one host and the loser find out |
| +// from the fleet. |
| // |
| // audit + bumpSerial live in here rather than in the doors, so both doors |
| // get them, in the right place, for free: strictly after the write they |
| @@ -63,14 +66,18 @@ type attachResult struct { |
| // attachDomain claims host for acct's app and, when the app has no route to |
| // alias, declares the static route that becomes its first one. kindFlag and |
| // channelFlag may both be "" — the kind is then inferred (alias if there is |
| -// a route, static otherwise) and the channel is the app's sole shipped one. |
| +// a route, static otherwise) and the channel is the one the host's own |
| +// existing route follows, or failing that the app's sole shipped channel. |
| func (s *Server) attachDomain(ctx context.Context, acct *Account, app, host, kindFlag, channelFlag, principal string) (attachResult, *doorRefusal) { |
| + if ref := s.bucketRefusal(); ref != nil { |
| + return attachResult{}, ref |
| + } |
| // Policy first: shape and platform space are decided before any write, |
| // so a refused host never reaches a bucket key. |
| if msg := attachHostRefusal(host, s.cfg.AppsDomain); msg != "" { |
| return attachResult{}, &doorRefusal{http.StatusUnprocessableEntity, msg} |
| } |
| - route, err := s.readyRouteHost(acct.Sqid, app, host) |
| + route, selfChannel, err := s.readyRouteHost(acct.Sqid, app, host) |
| if err != nil { |
| log.Printf("console: attach %s: registry: %v", host, err) |
| return attachResult{}, &doorRefusal{http.StatusInternalServerError, |
| @@ -96,7 +103,7 @@ func (s *Server) attachDomain(ctx context.Context, acct *Account, app, host, kin |
| return attachResult{}, &doorRefusal{http.StatusUnprocessableEntity, |
| "an app with no running route can only attach a static domain — deploy first, then attach"} |
| } |
| - channel, ref := s.attachChannel(ctx, acct.Sqid, app, channelFlag) |
| + channel, ref := s.attachChannel(ctx, acct.Sqid, app, channelFlag, selfChannel) |
| if ref != nil { |
| return attachResult{}, ref |
| } |
| @@ -109,15 +116,38 @@ func (s *Server) attachDomain(ctx context.Context, acct *Account, app, host, kin |
| CreatedAt: claim.CreatedAt, CreatedBy: principal, |
| } |
| if err := provision.CreateRecord(ctx, s.cfg.Releases, rec); err != nil { |
| - // The claim landed and the record did not. Take the claim back so |
| - // the host stays attachable — best effort, because the bucket that |
| - // just refused a write may refuse this one too; a stranded claim is |
| - // inert (nothing materializes from a claim alone) and its own owner |
| - // can clear it with detach. |
| - log.Printf("console: attach %s: record write: %v", host, err) |
| - if derr := domainclaim.Delete(ctx, s.cfg.Releases, host); derr != nil { |
| - log.Printf("console: attach %s: claim rollback: %v (claim is stranded — detach clears it)", host, derr) |
| + // A record already at this key is NOT a failure to undo. The key is |
| + // account+app scoped, so a create that lost the race lost it to this |
| + // app's own record — a retry after a half-failed attach, or a record |
| + // written for a host whose claim had been released. GetRecord |
| + // re-reads it and refuses a body that disagrees with its own path, |
| + // so a successful read is proof the record describes this app and |
| + // account; the attach is then idempotent, the claim this call just |
| + // won stays, and the truth reported back is the record's channel, |
| + // not the one this call would have written. |
| + if errors.Is(err, release.ErrCASConflict) { |
| + existing, gerr := provision.GetRecord(ctx, s.cfg.Releases, acct.Sqid, app, host) |
| + if gerr == nil { |
| + s.audit(acct, app, principal, "domain-attach", host+" (first route, "+existing.Channel+", record already present)") |
| + s.bumpSerial(ctx) |
| + return attachResult{Claim: claim, FirstRoute: true, Channel: existing.Channel}, nil |
| + } |
| + // Unreadable: malformed, or a body that lies about its identity. |
| + // Not ours to delete and not safe to write over, so the claim |
| + // goes back and the member is told what stands in the way — the |
| + // fact of a record, and nothing about who wrote it. |
| + log.Printf("console: attach %s: existing record unreadable: %v", host, gerr) |
| + s.rollbackAttach(ctx, acct.Sqid, app, host, false) |
| + return attachResult{}, &doorRefusal{http.StatusUnprocessableEntity, |
| + "a route record already exists for this host — detach it or pick another hostname"} |
| } |
| + // Anything else is AMBIGUOUS: a timeout or a dropped connection can |
| + // come back as an error for a write that landed. Unwind both, in |
| + // detach's order (record, then claim), so the failure cannot leave a |
| + // record behind a released claim — a route materializing for a host |
| + // anyone may now claim. |
| + log.Printf("console: attach %s: record write: %v", host, err) |
| + s.rollbackAttach(ctx, acct.Sqid, app, host, true) |
| return attachResult{}, &doorRefusal{http.StatusInternalServerError, |
| "could not declare the route for this domain — nothing was attached, try again"} |
| } |
| @@ -135,6 +165,9 @@ func (s *Server) attachDomain(ctx context.Context, acct *Account, app, host, kin |
| // http.NotFound — a detach must not be the probe that confirms a host is |
| // claimed, or by whom. |
| func (s *Server) detachDomain(ctx context.Context, acct *Account, app, host, principal string) *doorRefusal { |
| + if ref := s.bucketRefusal(); ref != nil { |
| + return ref |
| + } |
| notAttached := &doorRefusal{http.StatusNotFound, "not attached to this app"} |
| // A host that is not even a hostname cannot be a claim key: refuse it |
| // here rather than handing it to a store as a key segment. |
| @@ -183,25 +216,29 @@ func (s *Server) detachDomain(ctx context.Context, acct *Account, app, host, pri |
| // without a registry (and every test that wants the first-route branch) has |
| // nothing to alias, and a missing DB must not be reported as a route. |
| // |
| -// attaching is the host being attached, and it is skipped as a candidate: |
| -// a claim whose route names its own host is a self-alias, which is not a |
| -// thing the box side can resolve. That case is real — an operator-added |
| -// custom-domain route (story.carlosframework.com, spec §7) that no claim |
| -// covers yet — and skipping it lands the attach on the first-route branch, |
| -// where the declared record describes the row that already exists and |
| -// #137's adopt-on-match transfers ownership with no box command. That is |
| -// the backfill, arriving through the member's own door. |
| -func (s *Server) readyRouteHost(sqid, app, attaching string) (string, error) { |
| +// attaching is the host being attached. It is never an alias candidate — a |
| +// claim whose route names its own host is a self-alias, which is not a thing |
| +// the box side can resolve — and its ready row is instead reported back as |
| +// selfChannel, the channel that row already follows. That case is real: an |
| +// operator-added custom-domain route (story.carlosframework.com, spec §7) |
| +// that no claim covers yet. Skipping it as an alias lands the attach on the |
| +// first-route branch, where the declared record describes the row that |
| +// already exists and #137's adopt-on-match transfers ownership with no box |
| +// command — the backfill, arriving through the member's own door — and |
| +// carrying its channel forward is what keeps that adoption from being a |
| +// silent repoint (provision.convergeOwned writes the record's channel onto |
| +// the row it adopts). |
| +func (s *Server) readyRouteHost(sqid, app, attaching string) (alias, selfChannel string, err error) { |
| if s.cfg.Registry == "" { |
| - return "", nil |
| + return "", "", nil |
| } |
| reg, err := openRegistry(s.cfg.Registry) |
| if err != nil { |
| - return "", err |
| + return "", "", err |
| } |
| routes, err := reg.List() // host order |
| if err != nil { |
| - return "", err |
| + return "", "", err |
| } |
| for _, rt := range routes { |
| if rt.App != app || release.RouteAccount(rt) != sqid { |
| @@ -210,23 +247,46 @@ func (s *Server) readyRouteHost(sqid, app, attaching string) (string, error) { |
| // A platform mint is not a source to alias — aliasing a mint is |
| // self-reference, the same rule firstReadyUpstream carries — and a |
| // route that is not ready has nothing to serve yet. |
| - if rt.Platform || rt.Status != "ready" || rt.Host == attaching { |
| + if rt.Platform || rt.Status != "ready" { |
| continue |
| } |
| - return rt.Host, nil |
| + if rt.Host == attaching { |
| + selfChannel = rt.Channel |
| + continue |
| + } |
| + if alias == "" { |
| + alias = rt.Host |
| + } |
| } |
| - return "", nil |
| + return alias, selfChannel, nil |
| } |
| // attachChannel resolves the channel a first-route record follows: the flag |
| -// when given, otherwise the app's sole shipped channel. Nothing shipped and |
| -// several channels are both refusals rather than guesses — a record pointed |
| -// at the wrong channel serves the wrong build from a domain the member just |
| +// when given, then the channel the host's own existing route already |
| +// follows, then the app's sole shipped channel. Nothing shipped and several |
| +// channels are both refusals rather than guesses — a record pointed at the |
| +// wrong channel serves the wrong build from a domain the member just |
| // published. |
| -func (s *Server) attachChannel(ctx context.Context, sqid, app, channelFlag string) (string, *doorRefusal) { |
| +// |
| +// selfChannel (the backfill case above) takes precedence over the listing |
| +// for two reasons: the record is about to be adopted onto that live row, so |
| +// naming any other channel would REPOINT a running route the member did not |
| +// ask to move, and an app with two channels would otherwise be refused as |
| +// ambiguous for a domain whose channel was never in question. |
| +func (s *Server) attachChannel(ctx context.Context, sqid, app, channelFlag, selfChannel string) (string, *doorRefusal) { |
| + // Shape only, never existence: a channel a member is about to ship is a |
| + // legitimate target, but a malformed one becomes a bucket key segment |
| + // and a route pinned to nothing (release.ValidChannel is the same rail |
| + // the instance-create door puts on its own channel field). |
| if channelFlag != "" { |
| + if !release.ValidChannel(channelFlag) { |
| + return "", &doorRefusal{http.StatusUnprocessableEntity, "invalid channel name"} |
| + } |
| return channelFlag, nil |
| } |
| + if selfChannel != "" && release.ValidChannel(selfChannel) { |
| + return selfChannel, nil |
| + } |
| prefix := release.ChannelsPrefix(sqid, app) |
| keys, err := s.cfg.Releases.List(ctx, prefix) |
| if err != nil { |
| @@ -255,6 +315,42 @@ func (s *Server) attachChannel(ctx context.Context, sqid, app, channelFlag strin |
| ") — say which one this domain serves"} |
| } |
| +// bucketRefusal is the "this console cannot answer at all" check both verbs |
| +// open with: a deployment with no bucket has nowhere to put a claim or a |
| +// record. Same 503 and the same sentence the instance API's gate uses |
| +// (apiinstances.go), so one deployment shape produces one answer wherever a |
| +// member meets it. |
| +func (s *Server) bucketRefusal() *doorRefusal { |
| + if s.cfg.Releases == nil { |
| + return &doorRefusal{http.StatusServiceUnavailable, |
| + "no deployment bucket on this host — the console is running without one"} |
| + } |
| + return nil |
| +} |
| + |
| +// rollbackAttach unwinds a half-written first-route attach, best effort: |
| +// the bucket that just refused a write may refuse these too, and a member |
| +// waiting on a refusal must not also wait on a retry loop. dropRecord asks |
| +// for the record to go first — set when the record write's outcome is |
| +// UNKNOWN (a timeout can report failure for an object that landed), and |
| +// clear when the record is known to be someone else's business. Delete is |
| +// idempotent, so removing a record that never landed costs one no-op. |
| +// |
| +// A rollback that itself fails leaves a claim with no record: inert (nothing |
| +// materializes from a claim alone), visible to its own owner in the app's |
| +// domain list, and clearable with detach. That is the failure this ordering |
| +// was chosen to have. |
| +func (s *Server) rollbackAttach(ctx context.Context, sqid, app, host string, dropRecord bool) { |
| + if dropRecord { |
| + if err := provision.DeleteRecord(ctx, s.cfg.Releases, sqid, app, host); err != nil { |
| + log.Printf("console: attach %s: record rollback: %v", host, err) |
| + } |
| + } |
| + if err := domainclaim.Delete(ctx, s.cfg.Releases, host); err != nil { |
| + log.Printf("console: attach %s: claim rollback: %v (claim is stranded — detach clears it)", host, err) |
| + } |
| +} |
| + |
| // claimRefusal maps a lost claim create onto the door's answer. The 409 is |
| // ONE string whoever holds the host — including the caller's own account: |
| // the message a prober gets may never depend on who the holder is. Seeing |
diff --git a/internal/console/domainattach_test.go b/internal/console/domainattach_test.go| index 57b12a1d1..5e3c8bc93 100644 |
| --- a/internal/console/domainattach_test.go |
| +++ b/internal/console/domainattach_test.go |
| @@ -70,12 +70,24 @@ func attachRegistry(t *testing.T, routes ...registry.Route) string { |
| // half-failed attach is reproduced without a broken bucket. |
| type opStore struct { |
| release.Store |
| - ops []string |
| - failPut string |
| + ops []string |
| + // failPut names a key whose Put fails before it lands. landThenFail |
| + // makes the same key LAND and then report failure — the ambiguous |
| + // outcome a timeout produces, and the one a one-sided rollback leaks a |
| + // record through. |
| + failPut string |
| + landThenFail bool |
| } |
| func (o *opStore) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) { |
| if o.failPut != "" && key == o.failPut { |
| + if o.landThenFail { |
| + o.ops = append(o.ops, "put-then-fail "+key) |
| + if _, err := o.Store.Put(ctx, key, body, ifMatch); err != nil { |
| + return "", err |
| + } |
| + return "", errors.New("opStore: timed out after the object landed") |
| + } |
| o.ops = append(o.ops, "put-fail "+key) |
| return "", errors.New("opStore: bucket unavailable") |
| } |
| @@ -108,6 +120,25 @@ func claimAbsent(t *testing.T, fx *consoleFixture, host string) { |
| } |
| } |
| +// auditedOnce asserts exactly one audit row for the action, naming host — |
| +// the record a member (and an incident) reads the door's history from. |
| +func auditedOnce(t *testing.T, fx *consoleFixture, acct *Account, app, action, host string) { |
| + t.Helper() |
| + events, err := fx.st.Events(acct.ID, app, 20) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + n := 0 |
| + for _, e := range events { |
| + if e.Action == action && strings.Contains(e.Detail, host) { |
| + n++ |
| + } |
| + } |
| + if n != 1 { |
| + t.Errorf("audit rows for %s %s = %d, want 1 (rows: %+v)", action, host, n, events) |
| + } |
| +} |
| + |
| func TestAttachAliasWhenAppHasReadyRoute(t *testing.T) { |
| fx, acct := attachFixture(t) |
| // Three candidate rows in host order: a platform mint and a route that |
| @@ -182,6 +213,52 @@ func TestAttachNeverAliasesAHostToItself(t *testing.T) { |
| } |
| } |
| +// TestAttachBackfillTakesChannelFromTheExistingRoute is the spec-owner's |
| +// ruling: on the self-host backfill the record's channel comes from the row |
| +// that already serves the host, not from the app's channel listing. Two |
| +// things turn on it — #137's adoption converges the record's channel onto |
| +// that live row, so any other value SILENTLY REPOINTS a running operator |
| +// route, and an app with several channels would otherwise be refused as |
| +// ambiguous for a domain whose channel was never in question. |
| +func TestAttachBackfillTakesChannelFromTheExistingRoute(t *testing.T) { |
| + fx, acct := attachFixture(t) |
| + shipVersion(t, fx, "story", "v1") |
| + seedChannel(t, fx, "story", "canary/live", "v1") |
| + seedChannel(t, fx, "story", "stable", "v1") // ambiguous, if the listing decided |
| + const host = "story.carlosframework.com" |
| + fx.srv.cfg.Registry = attachRegistry(t, |
| + registry.Route{Host: host, Kind: "static", App: "story", |
| + Account: fx.sqid, Channel: "canary/live", Status: "ready"}, |
| + ) |
| + res, ref := fx.srv.attachDomain(context.Background(), acct, "story", host, "", "", "paul@keymail.dev") |
| + if ref != nil { |
| + t.Fatalf("attach: refused %+v, want the backfill to go through on the row's own channel", ref) |
| + } |
| + if res.Claim.Route != "" || !res.FirstRoute || res.Channel != "canary/live" { |
| + t.Errorf("result = %+v, want a first-route attach on canary/live with an empty claim route", res) |
| + } |
| + rec, err := provision.GetRecord(context.Background(), fx.rel, fx.sqid, "story", host) |
| + if err != nil { |
| + t.Fatalf("record: %v", err) |
| + } |
| + if rec.Channel != "canary/live" { |
| + t.Errorf("record channel = %q, want canary/live — the channel the live route already follows", rec.Channel) |
| + } |
| + // An explicit flag still wins: repointing is allowed when it is asked for. |
| + const other = "docs.carlosframework.com" |
| + fx.srv.cfg.Registry = attachRegistry(t, |
| + registry.Route{Host: other, Kind: "static", App: "story", |
| + Account: fx.sqid, Channel: "canary/live", Status: "ready"}, |
| + ) |
| + res, ref = fx.srv.attachDomain(context.Background(), acct, "story", other, "", "stable", "paul@keymail.dev") |
| + if ref != nil { |
| + t.Fatalf("attach with -channel stable: refused %+v", ref) |
| + } |
| + if res.Channel != "stable" { |
| + t.Errorf("Channel = %q, want the explicitly named stable", res.Channel) |
| + } |
| +} |
| + |
| func TestAttachFirstRouteWritesClaimThenRecord(t *testing.T) { |
| fx, acct := attachFixture(t) |
| shipVersion(t, fx, "story", "v1") |
| @@ -223,6 +300,151 @@ func TestAttachFirstRouteWritesClaimThenRecord(t *testing.T) { |
| if claimPut < 0 || recPut < 0 || claimPut > recPut { |
| t.Errorf("write order %v: want the claim put before the record put (claim=%d record=%d)", ops.ops, claimPut, recPut) |
| } |
| + // The two things the doors get for free by this living in the core: the |
| + // audit row, and the serial bump that makes the fleet converge in |
| + // seconds instead of at the hourly floor. |
| + auditedOnce(t, fx, acct, "story", "domain-attach", host) |
| + if serial := ops.opIndex("put " + release.SerialKey); serial < recPut { |
| + t.Errorf("ops %v: want the serial bumped AFTER the writes it announces (serial=%d record=%d)", ops.ops, serial, recPut) |
| + } |
| +} |
| + |
| +// TestAttachIdempotentWhenRecordAlreadyExists is the retry after a partial |
| +// attach (and the claim-released-but-record-kept shape): the record key is |
| +// account+app scoped, so a create that loses the race there lost it to this |
| +// app's own record. Refusing would make the host permanently unattachable — |
| +// the claim would roll back on every retry and detach would 404. |
| +func TestAttachIdempotentWhenRecordAlreadyExists(t *testing.T) { |
| + fx, acct := attachFixture(t) |
| + shipVersion(t, fx, "story", "v1") |
| + seedChannel(t, fx, "story", "canary/live", "v1") |
| + const host = "story.carlosframework.com" |
| + // A record left behind by an earlier attach, on a DIFFERENT channel so |
| + // the answer proves which one is reported. |
| + if err := provision.CreateRecord(context.Background(), fx.rel, provision.Record{ |
| + Schema: 1, Host: host, App: "story", Account: fx.sqid, Kind: "static", Channel: "stable", |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| + res, ref := fx.srv.attachDomain(context.Background(), acct, "story", host, "", "", "paul@keymail.dev") |
| + if ref != nil { |
| + t.Fatalf("attach: refused %+v, want the idempotent success", ref) |
| + } |
| + if !res.FirstRoute || res.Channel != "stable" { |
| + t.Errorf("result = %+v, want FirstRoute with the EXISTING record's channel", res) |
| + } |
| + if _, _, err := domainclaim.Get(context.Background(), fx.rel, host); err != nil { |
| + t.Errorf("claim: %v — the claim this call won must survive the idempotent path", err) |
| + } |
| + // And the second call is a plain 409: the claim is now held. |
| + if _, ref := fx.srv.attachDomain(context.Background(), acct, "story", host, "", "", "paul@keymail.dev"); ref == nil || ref.Status != 409 { |
| + t.Errorf("second attach = %+v, want the neutral 409", ref) |
| + } |
| +} |
| + |
| +// TestAttachUnreadableExistingRecordRefuses422 is the other half: a record |
| +// at the key that cannot be read as this app's (malformed, or a body that |
| +// disagrees with its own path) is not ours to overwrite or delete. |
| +func TestAttachUnreadableExistingRecordRefuses422(t *testing.T) { |
| + fx, acct := attachFixture(t) |
| + shipVersion(t, fx, "story", "v1") |
| + seedChannel(t, fx, "story", "canary/live", "v1") |
| + const host = "story.carlosframework.com" |
| + if _, err := fx.rel.Put(context.Background(), |
| + provision.RecordKey(fx.sqid, "story", host), []byte("{not json"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, ref := fx.srv.attachDomain(context.Background(), acct, "story", host, "", "", "paul@keymail.dev") |
| + if ref == nil || ref.Status != 422 { |
| + t.Fatalf("refusal = %+v, want 422 for an unreadable record at the key", ref) |
| + } |
| + if strings.Contains(ref.Msg, fx.sqid) { |
| + t.Errorf("msg %q leaks identity — the member learns a record exists, nothing more", ref.Msg) |
| + } |
| + claimAbsent(t, fx, host) |
| +} |
| + |
| +// TestAttachAmbiguousRecordFailureUnwindsBoth: a write that LANDS and then |
| +// reports failure (a timeout) must not leave the record behind the claim it |
| +// rolls back — that record would materialize a route for a host anyone may |
| +// now claim. |
| +func TestAttachAmbiguousRecordFailureUnwindsBoth(t *testing.T) { |
| + fx, acct := attachFixture(t) |
| + shipVersion(t, fx, "story", "v1") |
| + seedChannel(t, fx, "story", "canary/live", "v1") |
| + const host = "story.carlosframework.com" |
| + ops := &opStore{Store: fx.rel, failPut: provision.RecordKey(fx.sqid, "story", host), landThenFail: true} |
| + fx.srv.cfg.Releases = ops |
| + |
| + _, ref := fx.srv.attachDomain(context.Background(), acct, "story", host, "", "", "paul@keymail.dev") |
| + if ref == nil || ref.Status != 500 { |
| + t.Fatalf("refusal = %+v, want 500 for an ambiguous record write", ref) |
| + } |
| + claimAbsent(t, fx, host) |
| + if _, err := provision.GetRecord(context.Background(), fx.rel, fx.sqid, "story", host); !errors.Is(err, os.ErrNotExist) { |
| + t.Errorf("record after the rollback: %v, want it gone — a record outliving its claim is the leak", err) |
| + } |
| + recDel := ops.opIndex("delete " + provision.RecordKey(fx.sqid, "story", host)) |
| + claimDel := ops.opIndex("delete " + domainclaim.Key(host)) |
| + if recDel < 0 || claimDel < 0 || recDel > claimDel { |
| + t.Errorf("ops %v: want the record dropped before the claim (record=%d claim=%d)", ops.ops, recDel, claimDel) |
| + } |
| +} |
| + |
| +// TestAttachClaimWriteFailureIs500 pins the non-CAS fallback: only a lost |
| +// race is "host unavailable". |
| +func TestAttachClaimWriteFailureIs500(t *testing.T) { |
| + fx, acct := attachFixture(t) |
| + shipVersion(t, fx, "story", "v1") |
| + seedChannel(t, fx, "story", "canary/live", "v1") |
| + const host = "story.carlosframework.com" |
| + fx.srv.cfg.Releases = &opStore{Store: fx.rel, failPut: domainclaim.Key(host)} |
| + |
| + _, ref := fx.srv.attachDomain(context.Background(), acct, "story", host, "", "", "paul@keymail.dev") |
| + if ref == nil || ref.Status != 500 { |
| + t.Fatalf("refusal = %+v, want 500 for a claim write that failed for reasons other than a lost race", ref) |
| + } |
| + if ref.Msg == "host unavailable" { |
| + t.Error("a broken bucket must not read as a taken host") |
| + } |
| + claimAbsent(t, fx, host) |
| +} |
| + |
| +func TestAttachWithNothingShippedRefused(t *testing.T) { |
| + fx, acct := attachFixture(t) |
| + const host = "story.carlosframework.com" |
| + _, ref := fx.srv.attachDomain(context.Background(), acct, "story", host, "", "", "paul@keymail.dev") |
| + if ref == nil || ref.Status != 422 || ref.Msg != "nothing shipped yet — carlos ship first" { |
| + t.Fatalf("refusal = %+v, want 422 \"nothing shipped yet — carlos ship first\"", ref) |
| + } |
| + claimAbsent(t, fx, host) |
| +} |
| + |
| +func TestAttachInvalidChannelFlagRefused(t *testing.T) { |
| + fx, acct := attachFixture(t) |
| + shipVersion(t, fx, "story", "v1") |
| + seedChannel(t, fx, "story", "canary/live", "v1") |
| + const host = "story.carlosframework.com" |
| + for _, bad := range []string{"../escape", "a/b/c", "canary live"} { |
| + _, ref := fx.srv.attachDomain(context.Background(), acct, "story", host, "", bad, "paul@keymail.dev") |
| + if ref == nil || ref.Status != 422 { |
| + t.Fatalf("channel %q: refusal = %+v, want 422", bad, ref) |
| + } |
| + claimAbsent(t, fx, host) |
| + } |
| +} |
| + |
| +// TestAttachWithoutBucketIs503: a console with no deployment bucket has |
| +// nowhere to put a claim, and says so rather than panicking on a nil store. |
| +func TestAttachWithoutBucketIs503(t *testing.T) { |
| + fx, acct := attachFixture(t) |
| + fx.srv.cfg.Releases = nil |
| + if _, ref := fx.srv.attachDomain(context.Background(), acct, "story", "story.carlosframework.com", "", "", "paul@keymail.dev"); ref == nil || ref.Status != 503 { |
| + t.Errorf("attach refusal = %+v, want 503", ref) |
| + } |
| + if ref := fx.srv.detachDomain(context.Background(), acct, "story", "story.carlosframework.com", "paul@keymail.dev"); ref == nil || ref.Status != 503 { |
| + t.Errorf("detach refusal = %+v, want 503", ref) |
| + } |
| } |
| // TestAttachFirstRouteRollsBackClaimOnRecordFailure is the half-failed |
| @@ -360,6 +582,35 @@ func TestDetachFirstRouteDeletesRecordThenClaim(t *testing.T) { |
| if recDel < 0 || claimDel < 0 || recDel > claimDel { |
| t.Errorf("ops %v: want the record deleted before the claim (record=%d claim=%d)", ops.ops, recDel, claimDel) |
| } |
| + auditedOnce(t, fx, acct, "story", "domain-detach", host) |
| + if serial := ops.opIndex("put " + release.SerialKey); serial < claimDel { |
| + t.Errorf("ops %v: want the serial bumped after the deletes it announces (serial=%d claim=%d)", ops.ops, serial, claimDel) |
| + } |
| +} |
| + |
| +// TestDetachAliasLeavesRecordsAlone: an alias claim (Route set) never had a |
| +// record of its own, and detaching it must not delete at that key — the app |
| +// may hold a record there for a route this claim only pointed at. |
| +func TestDetachAliasLeavesRecordsAlone(t *testing.T) { |
| + fx, acct := attachFixture(t) |
| + const host = "echo.example.com" |
| + fx.srv.cfg.Registry = attachRegistry(t, |
| + registry.Route{Host: "story." + fx.sqid + ".oncarlos.com", Kind: "static", App: "story", |
| + Account: fx.sqid, Channel: "canary/live", Status: "ready"}, |
| + ) |
| + res, ref := fx.srv.attachDomain(context.Background(), acct, "story", host, "", "", "paul@keymail.dev") |
| + if ref != nil || res.Claim.Route == "" { |
| + t.Fatalf("attach: %+v / %+v, want an alias claim", res, ref) |
| + } |
| + ops := &opStore{Store: fx.rel} |
| + fx.srv.cfg.Releases = ops |
| + if ref := fx.srv.detachDomain(context.Background(), acct, "story", host, "paul@keymail.dev"); ref != nil { |
| + t.Fatalf("detach: refused %+v", ref) |
| + } |
| + claimAbsent(t, fx, host) |
| + if i := ops.opIndex("delete " + provision.RecordKey(fx.sqid, "story", host)); i >= 0 { |
| + t.Errorf("ops %v: an alias detach deleted a record key it never wrote", ops.ops) |
| + } |
| } |
| func TestDetachForeignClaimIs404(t *testing.T) { |