Apex holds: an app-less or other-account route holds it too
An apex is the name's own only when the route's app is the name and its account, if it names one, is the apex's; anything else, including a route with no app, holds the host (the domains pass defers to it all the same). The startup report applies the same test. apexHolder returns who holds the host — app, trashed, other account — so the doors word the refusal from one answer instead of asking twice, with advice that fits (a trashed holder is restored-and-removed or purged). Release-store failures report as such; database ones as a failed check. Rename re-checks the new name's apex after committing, the move check has a deadline, and the instances door re-reads the sibling label after writing its record and takes the record back if the name arrived in between. Docs for apps and instances say what is refused.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
9 files changed,
+229
−73
docs/site/cli.md+12 −0internal/console/apex.go+102 −28internal/console/apex_test.go+49 −10internal/console/apicli.go+2 −3internal/console/apicli_test.go+1 −1internal/console/apiinstances.go+19 −0internal/console/apps_claim_test.go+4 −4internal/console/appsettings.go+16 −8internal/console/server.go+24 −19
CI passed — run details
diff --git a/docs/site/cli.md b/docs/site/cli.md| index 618e6eab3..e9029225a 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -81,6 +81,13 @@ uses, so a name that form would refuse is refused here too. |
| carlos apps create --app hello |
| ``` |
| +A name is also refused when its platform apex, `<app>.<account>.<apps |
| +domain>`, is already taken: by another app's instance, even one in the Trash, |
| +or by any route there at all. The new app would otherwise never get an address |
| +of its own. Renaming an app, or moving it to another account, is checked the |
| +same way. The error names whatever holds the address. Remove that instance |
| +first, or pick another name. |
| + |
| `carlos apps delete` moves an app to the Trash. Every route drops, custom |
| domains included, the name stays reserved, and you have thirty days to |
| `carlos apps restore` it. Restoring brings back releases, channels and the |
| @@ -466,6 +473,11 @@ create anything. After that, `create`, `list`, `delete`, `set-upstreams`, |
| the box's reconcile pass turns a record into a live route. A password in |
| front of an instance's hostname is `carlos gate`. |
| +An instance may sit at any free label under the account's platform domain, |
| +but not at another app's platform apex. That is refused while the other app |
| +is live and while it is in the Trash, since restoring it would bring the |
| +clash back. |
| + |
| ```sh |
| carlos instances enable --app hello |
| carlos instances list --app hello |
diff --git a/internal/console/apex.go b/internal/console/apex.go| index 74e833156..a8a9d3e0a 100644 |
| --- a/internal/console/apex.go |
| +++ b/internal/console/apex.go |
| @@ -33,67 +33,127 @@ func (s *Server) apexHost(sqid, app string) string { |
| return app + "." + sqid + "." + domain |
| } |
| -// apexHolder names the app, other than name, that already holds the platform |
| -// apex name would have in acct — "" when nobody does. It looks where a holder |
| -// could be: a route at the host (whatever put it there), and an instance |
| -// record at the host under any other app of the account, live or trashed, |
| -// which the provisioning reconciler would turn into a route. |
| +// apexHold is who already holds an apex: the app a route or an instance |
| +// record belongs to — "" for a route that names no app (a status page, a bare |
| +// proxy, anything hand-added) — whether that app is in the trash, and the |
| +// account a route says it serves when that is not the apex's own. |
| +type apexHold struct { |
| + Holder string |
| + Trashed bool |
| + OtherAccount string |
| +} |
| + |
| +// apexErr says which store an apexHolder failure came from, so a door can |
| +// tell a member the truth about what could not be checked. |
| +type apexErr struct { |
| + bucket bool |
| + err error |
| +} |
| + |
| +func (e *apexErr) Error() string { return e.err.Error() } |
| +func (e *apexErr) Unwrap() error { return e.err } |
| + |
| +// apexBucketFailure reports whether err came from the release store rather |
| +// than from this box's registry or database. |
| +func apexBucketFailure(err error) bool { |
| + var ae *apexErr |
| + return errors.As(err, &ae) && ae.bucket |
| +} |
| + |
| +// apexHolder answers whether the platform apex name would have in acct is |
| +// already held by something that is not name's own — nil when it is free. |
| +// It looks where a holder could be: a route at the host, whatever put it |
| +// there, and an instance record at it under any other app of the account, |
| +// live or trashed, which the provisioning reconciler would turn into a route. |
| +// |
| +// A route is name's own only when it says so on both counts: its app is name |
| +// and its account, if it names one, is acct. A route with no app at all holds |
| +// the host exactly as firmly as one with an app — the domains pass defers to |
| +// it all the same. |
| // |
| -// It fails CLOSED: an error is an error, never "free". |
| -func (s *Server) apexHolder(ctx context.Context, acct *Account, name string) (string, error) { |
| +// The route half reads THIS console box's registry, as appHasRoutes and |
| +// routeAccountFor do; a route hand-added on another box is invisible to it, |
| +// while a reconciler-minted one is covered by its record. It fails CLOSED: |
| +// an error is an error, never "free". |
| +func (s *Server) apexHolder(ctx context.Context, acct *Account, name string) (*apexHold, error) { |
| host := s.apexHost(acct.Sqid, name) |
| if host == "" { |
| - return "", nil |
| + return nil, nil |
| } |
| if s.cfg.Registry != "" { |
| reg, err := openRegistry(s.cfg.Registry) |
| if err != nil { |
| - return "", err |
| + return nil, &apexErr{err: err} |
| } |
| rt, err := reg.Get(host) |
| if err != nil { |
| - return "", err |
| + return nil, &apexErr{err: err} |
| } |
| - if rt != nil && rt.App != name { |
| - return rt.App, nil |
| + if rt != nil && (rt.App != name || (rt.Account != "" && rt.Account != acct.Sqid)) { |
| + hold := &apexHold{Holder: rt.App} |
| + if rt.Account != "" && rt.Account != acct.Sqid { |
| + hold.OtherAccount = rt.Account |
| + } |
| + return hold, nil |
| } |
| } |
| if s.cfg.Releases == nil { |
| - return "", nil |
| + return nil, nil |
| } |
| live, err := s.store.AppsIn(acct.ID) |
| if err != nil { |
| - return "", err |
| + return nil, &apexErr{err: err} |
| } |
| trashed, err := s.store.TrashedAppsIn(acct.ID) |
| if err != nil { |
| - return "", err |
| + return nil, &apexErr{err: err} |
| + } |
| + type candidate struct { |
| + app string |
| + trashed bool |
| + } |
| + var apps []candidate |
| + for _, a := range live { |
| + apps = append(apps, candidate{app: a}) |
| } |
| - apps := live |
| for _, t := range trashed { |
| - apps = append(apps, t.App) |
| + apps = append(apps, candidate{app: t.App, trashed: true}) |
| } |
| - for _, other := range apps { |
| - if other == name { |
| + for _, c := range apps { |
| + if c.app == name { |
| continue |
| } |
| - rec, err := provision.GetRecord(ctx, s.cfg.Releases, acct.Sqid, other, host) |
| + rec, err := provision.GetRecord(ctx, s.cfg.Releases, acct.Sqid, c.app, host) |
| if errors.Is(err, os.ErrNotExist) { |
| continue |
| } |
| if err != nil { |
| - return "", err |
| + return nil, &apexErr{bucket: true, err: err} |
| } |
| if rec != nil { |
| - return other, nil |
| + return &apexHold{Holder: c.app, Trashed: c.trashed}, nil |
| } |
| } |
| - return "", nil |
| + return nil, nil |
| } |
| -// apexHeldSentence is the refusal, in one voice for every door that gives it. |
| -func apexHeldSentence(host, holder string) string { |
| - return host + " — this app's platform address — is already an instance of " + holder + |
| +// apexHeldSentence is the refusal, in one voice for every door that gives |
| +// it, with the advice that fits who holds the host. |
| +func apexHeldSentence(host string, hold *apexHold) string { |
| + switch { |
| + case hold == nil: |
| + return host + " — this app's platform address — is already held" |
| + case hold.OtherAccount != "" && hold.Holder != "": |
| + return host + " — this app's platform address — is already routed to " + hold.Holder + " in account " + hold.OtherAccount + "; that is an operator repair" |
| + case hold.OtherAccount != "": |
| + return host + " — this app's platform address — is already routed for account " + hold.OtherAccount + "; that is an operator repair" |
| + case hold.Holder == "": |
| + return host + " — this app's platform address — already has a route with no app; an operator has to remove it first" |
| + case hold.Trashed: |
| + return host + " — this app's platform address — is already an instance of " + hold.Holder + |
| + ", which is in the trash; restore " + hold.Holder + " and remove that instance, or purge it, first" |
| + } |
| + return host + " — this app's platform address — is already an instance of " + hold.Holder + |
| "; remove that instance first, or choose another name" |
| } |
| @@ -132,7 +192,13 @@ func (s *Server) apexSquats(ctx context.Context) []string { |
| continue |
| } |
| label, sqid, ok := strings.Cut(rest, ".") |
| - if !ok || strings.Contains(sqid, ".") || label == rt.App { |
| + if !ok || strings.Contains(sqid, ".") { |
| + continue |
| + } |
| + // The same test apexHolder applies: a route is the label's own only |
| + // when its app is the label and its account, if it names one, is the |
| + // apex's. |
| + if rt.App == label && (rt.Account == "" || rt.Account == sqid) { |
| continue |
| } |
| acct, err := s.store.AccountBySqid(sqid) |
| @@ -147,7 +213,15 @@ func (s *Server) apexSquats(ctx context.Context) []string { |
| if trashed { |
| state = "in the trash" |
| } |
| - out = append(out, rt.Host+" is app "+label+"'s platform apex ("+label+" is "+state+"), but the route belongs to "+rt.App+" in account "+rt.Account) |
| + owner := rt.App |
| + if owner == "" { |
| + owner = "no app" |
| + } |
| + inAcct := rt.Account |
| + if inAcct == "" { |
| + inAcct = "no account" |
| + } |
| + out = append(out, rt.Host+" is app "+label+"'s platform apex ("+label+" is "+state+"), but the route belongs to "+owner+" ("+inAcct+")") |
| } |
| return out |
| } |
diff --git a/internal/console/apex_test.go b/internal/console/apex_test.go| index 517cd8b46..c8223859a 100644 |
| --- a/internal/console/apex_test.go |
| +++ b/internal/console/apex_test.go |
| @@ -46,6 +46,41 @@ func TestANameWhoseApexASiblingHoldsIsNotFree(t *testing.T) { |
| t.Fatalf("creating beta over titogo's route = %d %v, want 409", status, out) |
| } |
| + // A route that names no app holds the host just as firmly: the domains |
| + // pass defers to it all the same. |
| + fx.addRoute(t, registry.Route{Host: "delta." + domain, Account: fx.sqid, Kind: "proxy"}) |
| + status, out = postJSON(t, newApp, map[string]string{"name": "delta"}, member) |
| + if status != http.StatusConflict { |
| + t.Fatalf("creating delta over an app-less route = %d %v, want 409", status, out) |
| + } |
| + if msg, _ := out["error"].(string); !strings.Contains(msg, "no app") { |
| + t.Errorf("the refusal should say the route has no app: %q", msg) |
| + } |
| + |
| + // Nor is a route that wears the name but serves another account the |
| + // name's own. |
| + fx.addRoute(t, registry.Route{Host: "epsilon." + domain, App: "epsilon", Account: "zzzz", Kind: "instance"}) |
| + status, out = postJSON(t, newApp, map[string]string{"name": "epsilon"}, member) |
| + if status != http.StatusConflict { |
| + t.Fatalf("creating epsilon over another account's route = %d %v, want 409", status, out) |
| + } |
| + if msg, _ := out["error"].(string); !strings.Contains(msg, "zzzz") { |
| + t.Errorf("the refusal should name the account the route serves: %q", msg) |
| + } |
| + |
| + // Trashing the sibling does not free the name: its record survives, and |
| + // restoring it would bring the instance back. |
| + if err := fx.st.TrashAppRow(OpsAccountID, "titogo", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + status, out = postJSON(t, newApp, map[string]string{"name": "acme"}, member) |
| + if status != http.StatusConflict { |
| + t.Fatalf("creating acme over a trashed titogo's instance = %d %v, want 409", status, out) |
| + } |
| + if msg, _ := out["error"].(string); !strings.Contains(msg, "trash") { |
| + t.Errorf("the refusal should say titogo is in the trash: %q", msg) |
| + } |
| + |
| // A name nobody holds is still just a name. |
| if status, out := postJSON(t, newApp, map[string]string{"name": "gamma"}, member); status != http.StatusOK { |
| t.Fatalf("creating a free name = %d %v, want 200", status, out) |
| @@ -101,13 +136,14 @@ func TestMoveIntoAnAccountWhereTheApexIsHeldIsRefused(t *testing.T) { |
| } |
| // At boot the console names every route already sitting at another app's |
| -// apex — live or trashed — and nothing else: an app's own apex, a label no |
| -// app wears, and hosts outside the apps domain are not squats. |
| +// apex — live or trashed, with another app, no app, or another account on |
| +// the route — and nothing else: an app's own apex, a label no app wears, and |
| +// hosts outside the apps domain are not squats. |
| func TestTheStartupReportNamesSquatsAndNothingElse(t *testing.T) { |
| fx := newConsoleFixture(t) |
| fx.srv.cfg.AppsDomain = "oncarlos.test" |
| now := time.Now() |
| - for _, app := range []string{"acme", "parked"} { |
| + for _, app := range []string{"acme", "parked", "blank", "elsewhere"} { |
| if _, err := fx.st.CreateAppIn(OpsAccountID, app, "paul@keymail.dev", now); err != nil { |
| t.Fatal(err) |
| } |
| @@ -117,20 +153,23 @@ func TestTheStartupReportNamesSquatsAndNothingElse(t *testing.T) { |
| } |
| d := fx.sqid + ".oncarlos.test" |
| for _, rt := range []registry.Route{ |
| - {Host: "acme." + d, App: "titogo", Account: fx.sqid, Kind: "instance"}, // squat on a live app |
| - {Host: "parked." + d, App: "titogo", Account: fx.sqid, Kind: "instance"}, // squat on a trashed one |
| - {Host: "titogo." + d, App: "titogo", Account: fx.sqid, Kind: "instance"}, // its own apex |
| - {Host: "nobody." + d, App: "titogo", Account: fx.sqid, Kind: "instance"}, // no app by that name |
| + {Host: "acme." + d, App: "titogo", Account: fx.sqid, Kind: "instance"}, // squat on a live app |
| + {Host: "parked." + d, App: "titogo", Account: fx.sqid, Kind: "instance"}, // squat on a trashed one |
| + {Host: "blank." + d, Account: fx.sqid, Kind: "proxy"}, // squat with no app |
| + {Host: "elsewhere." + d, App: "elsewhere", Account: "zzzz", Kind: "proxy"}, // right name, wrong account |
| + {Host: "titogo." + d, App: "titogo", Account: fx.sqid, Kind: "instance"}, // its own apex |
| + {Host: "nobody." + d, App: "titogo", Account: fx.sqid, Kind: "instance"}, // no app by that name |
| {Host: "acme.example.com", App: "titogo", Account: fx.sqid, Kind: "instance"}, |
| } { |
| fx.addRoute(t, rt) |
| } |
| got := fx.srv.apexSquats(context.Background()) |
| - if len(got) != 2 { |
| - t.Fatalf("report = %q, want the two squats", got) |
| + if len(got) != 4 { |
| + t.Fatalf("report = %q, want the four squats", got) |
| } |
| joined := strings.Join(got, "\n") |
| - for _, want := range []string{"acme." + d, "parked." + d, "in the trash", "belongs to titogo"} { |
| + for _, want := range []string{"acme." + d, "parked." + d, "in the trash", "belongs to titogo", |
| + "blank." + d, "belongs to no app", "elsewhere." + d, "(zzzz)"} { |
| if !strings.Contains(joined, want) { |
| t.Errorf("report missing %q:\n%s", want, joined) |
| } |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 16d711558..805bea56d 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -412,7 +412,7 @@ func (s *Server) handleAPICreateApp(w http.ResponseWriter, r *http.Request) { |
| // account role, and handing a CI credential the power to adopt release |
| // history is exactly the widening svcrole exists to prevent. |
| ownerClaim := p.Kind == PrincipalUser && acct.Role == RoleOwner |
| - outcome, placed := s.claimApp(r.Context(), acct, name, addr, place, ownerClaim) |
| + outcome, placed, held := s.claimApp(r.Context(), acct, name, addr, place, ownerClaim) |
| switch outcome { |
| case claimInvalid: |
| jsonError(w, http.StatusBadRequest, |
| @@ -422,8 +422,7 @@ func (s *Server) handleAPICreateApp(w http.ResponseWriter, r *http.Request) { |
| case claimTaken: |
| jsonError(w, http.StatusConflict, "that name is taken") |
| case claimApexHeld: |
| - holder, _ := s.apexHolder(r.Context(), acct, name) |
| - jsonError(w, http.StatusConflict, "that name is not free: "+apexHeldSentence(s.apexHost(acct.Sqid, name), holder)) |
| + jsonError(w, http.StatusConflict, "that name is not free: "+apexHeldSentence(s.apexHost(acct.Sqid, name), held)) |
| case claimCheckFailed, claimCreateFailed: |
| jsonError(w, http.StatusInternalServerError, "could not create the app — try again") |
| case claimStoreFailed: |
diff --git a/internal/console/apicli_test.go b/internal/console/apicli_test.go| index ff1a2a567..3b61d6e36 100644 |
| --- a/internal/console/apicli_test.go |
| +++ b/internal/console/apicli_test.go |
| @@ -2501,7 +2501,7 @@ func TestClaimAppPlacementWriteFailureDoesNotUndoClaim(t *testing.T) { |
| } |
| acct := &Account{ID: OpsAccountID, Sqid: fx.sqid} |
| - outcome, placed := fx.srv.claimApp(context.Background(), acct, "titogo", "paul@keymail.dev", "fleet/pi", false) |
| + outcome, placed, _ := fx.srv.claimApp(context.Background(), acct, "titogo", "paul@keymail.dev", "fleet/pi", false) |
| if outcome != claimOK { |
| t.Fatalf("claimApp outcome = %v, want claimOK — the claim itself must still succeed", outcome) |
| } |
diff --git a/internal/console/apiinstances.go b/internal/console/apiinstances.go| index 73d22b141..97072532e 100644 |
| --- a/internal/console/apiinstances.go |
| +++ b/internal/console/apiinstances.go |
| @@ -628,8 +628,12 @@ func (s *Server) handleAPIInstanceCreate(w http.ResponseWriter, r *http.Request) |
| // pinned to the account's platform subdomain, or to a domain of the |
| // app's own? |
| platformNamespace := s.cfg.AppsDomain != "" && m.Domain == acct.Sqid+"."+s.cfg.AppsDomain |
| + // siblingLabel is set when this host sits at a label that is not the |
| + // app's own — the case re-checked after the record is written, below. |
| + siblingLabel := "" |
| if platformNamespace { |
| if label := strings.TrimSuffix(host, "."+m.Domain); label != app { |
| + siblingLabel = label |
| live, trashed, err := s.store.AppNameState(acct.ID, label) |
| if err != nil { |
| log.Printf("console: api instances create sibling check %s/%s: %v", acct.Sqid, label, err) |
| @@ -812,6 +816,21 @@ func (s *Server) handleAPIInstanceCreate(w http.ResponseWriter, r *http.Request) |
| jsonError(w, http.StatusBadGateway, "could not create the instance — try again") |
| return |
| } |
| + // The sibling check above and this write are two stores, so an app can be |
| + // created under the label between them (claimApp asked apexHolder before |
| + // the record existed). Re-read after writing, and on a hit take the record |
| + // back: the name that arrived owns its apex. The same after-the-commit |
| + // re-check a rename makes (appsettings.go). |
| + if siblingLabel != "" { |
| + if live, trashed, err := s.store.AppNameState(acct.ID, siblingLabel); err == nil && (live || trashed) { |
| + if derr := provision.DeleteRecord(ctx, s.cfg.Releases, acct.Sqid, app, host); derr != nil { |
| + log.Printf("console: api instances create %s/%s/%s: %s was claimed mid-create and UNDO FAILED: %v", acct.Sqid, app, host, siblingLabel, derr) |
| + } |
| + jsonError(w, http.StatusConflict, |
| + host+" is app "+siblingLabel+"'s platform apex — it was created while this instance was being written; the instance was not kept") |
| + return |
| + } |
| + } |
| // DNS seam (spec §10): on a single-region deployment nothing writes |
| // DNS — the app's wildcard already resolves every instance host to the |
| // edge. Multi-region wants <slug>.<domain> CNAME -> <region>.<domain> |
diff --git a/internal/console/apps_claim_test.go b/internal/console/apps_claim_test.go| index 3d55f3513..c62d01316 100644 |
| --- a/internal/console/apps_claim_test.go |
| +++ b/internal/console/apps_claim_test.go |
| @@ -47,7 +47,7 @@ func TestOwnerClaimsAppWhoseObjectsExist(t *testing.T) { |
| fx := newConsoleFixture(t) |
| seedBucketApp(t, fx, "bab", "console") |
| acct := opsAccount(t, fx) |
| - outcome, _ := fx.srv.claimApp(context.Background(), acct, "console", "paul@keymail.dev", "", true) |
| + outcome, _, _ := fx.srv.claimApp(context.Background(), acct, "console", "paul@keymail.dev", "", true) |
| if outcome != claimOK { |
| t.Fatalf("owner claim over existing objects = %v, want claimOK", outcome) |
| } |
| @@ -63,7 +63,7 @@ func TestNonOwnerStillRefusedOverExistingObjects(t *testing.T) { |
| fx := newConsoleFixture(t) |
| seedBucketApp(t, fx, "bab", "console") |
| acct := opsAccount(t, fx) |
| - outcome, _ := fx.srv.claimApp(context.Background(), acct, "console", "member@keymail.dev", "", false) |
| + outcome, _, _ := fx.srv.claimApp(context.Background(), acct, "console", "member@keymail.dev", "", false) |
| if outcome != claimTaken { |
| t.Fatalf("non-owner claim over existing objects = %v, want claimTaken", outcome) |
| } |
| @@ -79,7 +79,7 @@ func TestLegacyPrefixStillBlocksAnOwner(t *testing.T) { |
| t.Fatal(err) |
| } |
| acct := opsAccount(t, fx) |
| - outcome, _ := fx.srv.claimApp(context.Background(), acct, "oldsite", "paul@keymail.dev", "", true) |
| + outcome, _, _ := fx.srv.claimApp(context.Background(), acct, "oldsite", "paul@keymail.dev", "", true) |
| if outcome != claimTaken { |
| t.Fatalf("owner claim over a LEGACY prefix = %v, want claimTaken — those objects are owed a migrate", outcome) |
| } |
| @@ -94,7 +94,7 @@ func TestGrantStillBlocksAnOwner(t *testing.T) { |
| t.Fatal(err) |
| } |
| acct := opsAccount(t, fx) |
| - outcome, _ := fx.srv.claimApp(context.Background(), acct, "derp", "paul@keymail.dev", "", true) |
| + outcome, _, _ := fx.srv.claimApp(context.Background(), acct, "derp", "paul@keymail.dev", "", true) |
| if outcome != claimTaken { |
| t.Fatalf("owner claim over a GRANTED name = %v, want claimTaken", outcome) |
| } |
diff --git a/internal/console/appsettings.go b/internal/console/appsettings.go| index 6ea0059f7..907f244da 100644 |
| --- a/internal/console/appsettings.go |
| +++ b/internal/console/appsettings.go |
| @@ -645,12 +645,12 @@ func (s *Server) handleAppRename(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| // The new name's platform apex must not already be another app's (apex.go). |
| - switch holder, err := s.apexHolder(ctx, acct, to); { |
| + switch hold, err := s.apexHolder(ctx, acct, to); { |
| case err != nil: |
| back("err", "Could not check "+to+"'s platform address — nothing was renamed. Try again.") |
| return |
| - case holder != "": |
| - back("err", "Cannot rename to "+to+": "+apexHeldSentence(s.apexHost(acct.Sqid, to), holder)+".") |
| + case hold != nil: |
| + back("err", "Cannot rename to "+to+": "+apexHeldSentence(s.apexHost(acct.Sqid, to), hold)+".") |
| return |
| } |
| if !confirmed(r) { |
| @@ -689,13 +689,18 @@ func (s *Server) handleAppRename(w http.ResponseWriter, r *http.Request) { |
| if wired, err := s.appHasRoutes(app); err == nil && wired { |
| stale = true |
| } |
| + // And the new name's apex, which an instance racing this rename can take |
| + // between the check above and the commit (apex.go). |
| + if hold, err := s.apexHolder(ctx, acct, to); err == nil && hold != nil { |
| + stale = true |
| + } |
| if stale { |
| if err := s.store.RenameAppIn(acct.ID, to, app); err != nil { |
| log.Printf("console: rename %s/%s -> %s: mid-rename ship detected and UNDO FAILED: %v — console record and platform state now disagree", acct.Sqid, app, to, err) |
| - back("err", "A ship or route landed under "+app+" while renaming, and undoing hit an error — the record may disagree with the platform. Re-check this page.") |
| + back("err", "Something landed under "+app+" or at "+to+"'s address while renaming, and undoing hit an error — the record may disagree with the platform. Re-check this page.") |
| return |
| } |
| - back("err", "A ship or route landed under "+app+" while renaming — the rename was undone; the name stays.") |
| + back("err", "A ship, a route, or an instance at "+to+"'s address landed while renaming — the rename was undone; the name stays.") |
| return |
| } |
| s.audit(acct, to, addr, "rename", "renamed "+app+" to "+to) |
| @@ -1133,12 +1138,15 @@ func (s *Server) handleAppMoveAccount(w http.ResponseWriter, r *http.Request) { |
| } |
| // In the destination, this app's platform apex must not already be |
| // another app's (apex.go). |
| - switch holder, err := s.apexHolder(r.Context(), dest, app); { |
| + apexCtx, apexCancel := context.WithTimeout(r.Context(), 10*time.Second) |
| + hold, err := s.apexHolder(apexCtx, dest, app) |
| + apexCancel() |
| + switch { |
| case err != nil: |
| back("err", "Could not check "+app+"'s platform address in "+dest.Name+" — nothing was moved. Try again.") |
| return |
| - case holder != "": |
| - back("err", "Cannot move "+app+" to "+dest.Name+": "+apexHeldSentence(s.apexHost(dest.Sqid, app), holder)+".") |
| + case hold != nil: |
| + back("err", "Cannot move "+app+" to "+dest.Name+": "+apexHeldSentence(s.apexHost(dest.Sqid, app), hold)+".") |
| return |
| } |
| if drole, err := s.store.Role(dest.ID, addr); err != nil || drole == "" { |
diff --git a/internal/console/server.go b/internal/console/server.go| index 58844a340..8a7666c78 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -3428,31 +3428,34 @@ func ownerExempt(prefix, ownPrefix string) bool { return prefix == ownPrefix } |
| // all, and an account-scoped one legitimately reaches this function, so |
| // without an explicit pin the exemption would hand every CI credential the |
| // same power. |
| -func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placement string, ownerClaim bool) (outcome claimOutcome, placed string) { |
| +// |
| +// held is set only with claimApexHeld: who holds the name's platform apex, so |
| +// a door can say so without asking again. |
| +func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placement string, ownerClaim bool) (outcome claimOutcome, placed string, held *apexHold) { |
| if !release.ValidApp(name) { |
| - return claimInvalid, "" |
| + return claimInvalid, "", nil |
| } |
| // "new" is the literal path segment GET /a/{sqid}/apps/new resolves to — |
| // Go 1.22's mux prefers it over apps/{app}, so an app named "new" could |
| // never be viewed. Reserved before any store write, same as an invalid name. |
| if name == "new" { |
| - return claimReserved, "" |
| + return claimReserved, "", nil |
| } |
| // Taken means taken IN THIS ACCOUNT — app names are unique per account, |
| // never globally, so another account's "blog" is not a conflict. |
| if has, err := s.store.HasAppIn(acct.ID, name); err != nil { |
| - return claimCheckFailed, "" |
| + return claimCheckFailed, "", nil |
| } else if has { |
| - return claimTaken, "" |
| + return claimTaken, "", nil |
| } |
| // The pre-accounts checks still apply to the OPS account, where every |
| // CLI-granted and pre-accounts app lives: a name with grants or bucket |
| // objects there stays claim-proof. |
| if acct.ID == OpsAccountID { |
| if has, err := s.store.HasOwners(name); err != nil { |
| - return claimCheckFailed, "" |
| + return claimCheckFailed, "", nil |
| } else if has { |
| - return claimTaken, "" |
| + return claimTaken, "", nil |
| } |
| } |
| // The account's own prefix, plus — for ops only — the legacy unqualified |
| @@ -3476,15 +3479,18 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem |
| // A name whose platform apex another app of the account already holds is |
| // not free either, whatever the tables say: the new app would never get |
| // its own URL (apex.go). |
| - if holder, err := s.apexHolder(ctx, acct, name); err != nil { |
| - return claimStoreFailed, "" |
| - } else if holder != "" { |
| - return claimApexHeld, "" |
| + if hold, err := s.apexHolder(ctx, acct, name); err != nil { |
| + if apexBucketFailure(err) { |
| + return claimStoreFailed, "", nil |
| + } |
| + return claimCheckFailed, "", nil |
| + } else if hold != nil { |
| + return claimApexHeld, "", hold |
| } |
| for _, prefix := range prefixes { |
| keys, err := s.cfg.Releases.List(ctx, prefix) |
| if err != nil { |
| - return claimStoreFailed, "" |
| + return claimStoreFailed, "", nil |
| } |
| if len(keys) == 0 { |
| continue |
| @@ -3505,14 +3511,14 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem |
| if ownerClaim && ownerExempt(prefix, ownPrefix) { |
| continue |
| } |
| - return claimTaken, "" |
| + return claimTaken, "", nil |
| } |
| created, err := s.store.CreateAppIn(acct.ID, name, addr, time.Now()) |
| if err != nil { |
| - return claimCreateFailed, "" |
| + return claimCreateFailed, "", nil |
| } |
| if !created { // lost a claim race after the checks |
| - return claimTaken, "" |
| + return claimTaken, "", nil |
| } |
| if placement != "" { |
| if err := s.store.SetAppPlacement(acct.ID, name, placement); err != nil { |
| @@ -3540,7 +3546,7 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem |
| if _, err := release.SetPipeline(ctx, s.cfg.Releases, s.cfg.Key, acct.Sqid, name, release.DefaultPipelineChannels(), addr); err != nil { |
| log.Printf("console: claim %s/%s: set default pipeline: %v", acct.Sqid, name, err) |
| } |
| - return claimOK, placed |
| + return claimOK, placed, nil |
| } |
| // handleNewAppForm is the create-app page; the claim itself stays on |
| @@ -3585,7 +3591,7 @@ func (s *Server) handleNewApp(w http.ResponseWriter, r *http.Request) { |
| // The browser door never places at create time, so the second return |
| // (what actually stuck) is always "" here and there is nothing to echo |
| // it to — this form has no placement concept at all. |
| - outcome, _ := s.claimApp(r.Context(), acct, name, addr, "", role == RoleOwner) |
| + outcome, _, held := s.claimApp(r.Context(), acct, name, addr, "", role == RoleOwner) |
| switch outcome { |
| case claimInvalid: |
| fail("Invalid app name — lowercase letters, digits, and dashes, starting with a letter.") |
| @@ -3594,8 +3600,7 @@ func (s *Server) handleNewApp(w http.ResponseWriter, r *http.Request) { |
| case claimTaken: |
| fail("That name is taken.") |
| case claimApexHeld: |
| - holder, _ := s.apexHolder(r.Context(), acct, name) |
| - fail("That name is not free: " + apexHeldSentence(s.apexHost(acct.Sqid, name), holder) + ".") |
| + fail("That name is not free: " + apexHeldSentence(s.apexHost(acct.Sqid, name), held) + ".") |
| case claimCheckFailed: |
| fail("Could not check that name. Please try again.") |
| case claimStoreFailed: |