amadan

carlos / platform Public

Merge branch 'instances-trashed-apex' into main

An app's platform apex is its own, whichever arrives first (discussion 153)
claude-code pushed by james@keymail.dev 520fcc046f4ad797d33e63c751a1a5c42fac0dc5 parent a430d4dparent d506a0c
12 files changed, +571 −23
  • cmd/console/main.go +3 −0
  • docs/site/cli.md +12 −0
  • internal/console/apex.go +227 −0
  • internal/console/apex_test.go +177 −0
  • internal/console/apicli.go +3 −1
  • internal/console/apicli_test.go +1 −1
  • internal/console/apiinstances.go +36 −2
  • internal/console/apiinstancesenable_test.go +34 −0
  • internal/console/apps_claim_test.go +4 −4
  • internal/console/appsettings.go +29 −2
  • internal/console/server.go +30 −13
  • internal/console/store.go +15 −0

CI passed — run details

diff --git a/cmd/console/main.go b/cmd/console/main.go
index f70be5037..6edc35e4d 100644
--- a/cmd/console/main.go
+++ b/cmd/console/main.go
@@ -355,6 +355,9 @@ func serve() {
// SES outcomes an app asked to be told about. Same placement rule as
// every pass above; with nothing queued, a pass is two indexed queries.
go srv.StartMailEventPush(context.Background())
+ // Once, at boot: log any route already sitting at another app's platform
+ // apex (internal/console/apex.go). Reports; repairs nothing.
+ go srv.ReportApexSquats(context.Background())
addr := envOr("CONSOLE_ADDR", "127.0.0.1:8090")
ln, err := net.Listen("tcp", addr)
if err != nil {
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
new file mode 100644
index 000000000..a8a9d3e0a
--- /dev/null
+++ b/internal/console/apex.go
@@ -0,0 +1,227 @@
+package console
+
+// An app's platform apex is <app>.<sqid>.<AppsDomain>, and it is that app's.
+// Two things can put somebody else there. The instances create door is one:
+// a sibling registering an instance at another app's apex, which that door
+// refuses (live or trashed — apiinstances.go). The other is the name arriving
+// AFTER the instance: an app created, renamed or moved to a name whose apex a
+// sibling already registered while the name was free. Nothing mints a route
+// over a host that already has one (the domains pass defers to byHost for
+// ever), so the new app would never have its own URL — and `carlos email
+// events push` would post its recipients' addresses to the sibling. This file
+// is the check those doors share, and a startup report for anything that got
+// there before the check existed.
+
+import (
+ "context"
+ "errors"
+ "log"
+ "os"
+ "strings"
+
+ "github.com/carlosframework/platform/internal/provision"
+)
+
+// apexHost is the platform apex app would have in the account with this
+// sqid, or "" on a deployment with no apps domain (there is no apex space to
+// collide in).
+func (s *Server) apexHost(sqid, app string) string {
+ domain := strings.ToLower(strings.TrimSpace(s.cfg.AppsDomain))
+ if domain == "" {
+ return ""
+ }
+ return app + "." + sqid + "." + domain
+}
+
+// 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.
+//
+// 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, nil
+ }
+ if s.cfg.Registry != "" {
+ reg, err := openRegistry(s.cfg.Registry)
+ if err != nil {
+ return nil, &apexErr{err: err}
+ }
+ rt, err := reg.Get(host)
+ if err != nil {
+ return nil, &apexErr{err: err}
+ }
+ 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, nil
+ }
+ live, err := s.store.AppsIn(acct.ID)
+ if err != nil {
+ return nil, &apexErr{err: err}
+ }
+ trashed, err := s.store.TrashedAppsIn(acct.ID)
+ if err != nil {
+ 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})
+ }
+ for _, t := range trashed {
+ apps = append(apps, candidate{app: t.App, trashed: true})
+ }
+ for _, c := range apps {
+ if c.app == name {
+ continue
+ }
+ rec, err := provision.GetRecord(ctx, s.cfg.Releases, acct.Sqid, c.app, host)
+ if errors.Is(err, os.ErrNotExist) {
+ continue
+ }
+ if err != nil {
+ return nil, &apexErr{bucket: true, err: err}
+ }
+ if rec != nil {
+ return &apexHold{Holder: c.app, Trashed: c.trashed}, nil
+ }
+ }
+ return nil, nil
+}
+
+// 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"
+}
+
+// ReportApexSquats logs, once, every route that sits at an app's platform apex
+// but belongs to a different app — the state the checks above now prevent,
+// left over from before they existed. It reports and repairs nothing: which
+// app is in the wrong place is a person's call, and moving a live route is
+// not something to do quietly at boot. Started from cmd/console, never from
+// NewServer.
+func (s *Server) ReportApexSquats(ctx context.Context) {
+ for _, line := range s.apexSquats(ctx) {
+ log.Printf("console: apex squat: %s", line)
+ }
+}
+
+// apexSquats is ReportApexSquats' finding, one line per squat.
+func (s *Server) apexSquats(ctx context.Context) []string {
+ domain := strings.ToLower(strings.TrimSpace(s.cfg.AppsDomain))
+ if domain == "" || s.cfg.Registry == "" {
+ return nil
+ }
+ reg, err := openRegistry(s.cfg.Registry)
+ if err != nil {
+ log.Printf("console: apex squat check: open registry: %v", err)
+ return nil
+ }
+ routes, err := reg.List()
+ if err != nil {
+ log.Printf("console: apex squat check: list routes: %v", err)
+ return nil
+ }
+ var out []string
+ for _, rt := range routes {
+ rest, ok := strings.CutSuffix(rt.Host, "."+domain)
+ if !ok {
+ continue
+ }
+ label, sqid, ok := strings.Cut(rest, ".")
+ 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)
+ if err != nil || acct == nil {
+ continue
+ }
+ live, trashed, err := s.store.AppNameState(acct.ID, label)
+ if err != nil || (!live && !trashed) {
+ continue
+ }
+ state := "live"
+ if trashed {
+ state = "in the trash"
+ }
+ 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
new file mode 100644
index 000000000..c8223859a
--- /dev/null
+++ b/internal/console/apex_test.go
@@ -0,0 +1,177 @@
+package console
+
+import (
+ "context"
+ "net/http"
+ "net/url"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/registry"
+)
+
+// A name whose platform apex a sibling already holds is not free. The
+// instance door refuses a sibling's apex once the sibling exists; this is the
+// other order — the instance first, while the name was free, and the app after
+// it — which would leave the new app with no URL of its own for ever.
+func TestANameWhoseApexASiblingHoldsIsNotFree(t *testing.T) {
+ fx, member := instanceFixture(t, "code-apex-claim")
+ fx.srv.cfg.AppsDomain = "oncarlos.test"
+ domain := fx.sqid + ".oncarlos.test"
+ if status, _, out := rawReq(t, http.MethodPut, enableURL(fx, fx.sqid, "titogo"), member, `{}`); status != http.StatusOK {
+ t.Fatalf("enable = %d %s", status, out)
+ }
+ // While no app is called acme, titogo may take acme's label.
+ if status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, fx.sqid, "titogo"), member, `{"host":"acme.`+domain+`"}`); status != http.StatusCreated {
+ t.Fatalf("instance at a free label = %d %s", status, out)
+ }
+ newApp := fx.ts.URL + "/api/cli/apps/" + fx.sqid + "/new"
+
+ // An instance record is enough: the reconciler would make it a route.
+ status, out := postJSON(t, newApp, map[string]string{"name": "acme"}, member)
+ if status != http.StatusConflict {
+ t.Fatalf("creating acme over titogo's instance = %d %v, want 409", status, out)
+ }
+ if msg, _ := out["error"].(string); !strings.Contains(msg, "acme."+domain) || !strings.Contains(msg, "titogo") {
+ t.Errorf("the refusal should name the address and who holds it: %q", msg)
+ }
+ if has, _ := fx.st.HasAppIn(OpsAccountID, "acme"); has {
+ t.Error("acme was created")
+ }
+
+ // So is a route on its own, whatever put it there.
+ fx.addRoute(t, registry.Route{Host: "beta." + domain, App: "titogo", Account: fx.sqid, Kind: "instance"})
+ if status, out := postJSON(t, newApp, map[string]string{"name": "beta"}, member); status != http.StatusConflict {
+ 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)
+ }
+}
+
+// Renaming onto a name whose apex a sibling holds is the same arrival by
+// another door, and is refused the same way.
+func TestRenameOntoAHeldApexIsRefused(t *testing.T) {
+ fx := newConsoleFixture(t)
+ fx.srv.cfg.AppsDomain = "oncarlos.test"
+ freshApp(t, fx, "sprout")
+ fx.addRoute(t, registry.Route{Host: "sapling." + fx.sqid + ".oncarlos.test", App: "kass", Account: fx.sqid, Kind: "instance"})
+ c := signInPaul(t, fx)
+ csrf := sessionToken(t, fx, c)
+ form := url.Values{"csrf": {csrf}, "to": {"sapling"}, "confirmed": {"1"}}
+ if status, loc, body := postForm(t, c, fx.u("/apps/sprout/rename"), form); strings.Contains(body, "Rename sprout to sapling?") {
+ t.Fatalf("offered the rename onto a held apex = %d -> %q", status, loc)
+ }
+ if has, _ := fx.st.HasAppIn(OpsAccountID, "sapling"); has {
+ t.Error("sprout was renamed onto an apex kass holds")
+ }
+ if has, _ := fx.st.HasAppIn(OpsAccountID, "sprout"); !has {
+ t.Error("sprout should be untouched")
+ }
+}
+
+// Moving an app into an account where another app holds its apex there is
+// the third arrival, and is refused before the card.
+func TestMoveIntoAnAccountWhereTheApexIsHeldIsRefused(t *testing.T) {
+ fx := newConsoleFixture(t)
+ fx.srv.cfg.AppsDomain = "oncarlos.test"
+ const destID = 4242
+ if err := fx.st.CreateAccount(destID, "cdef", "Second", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ stampKassRoutes(t, fx, "cdef")
+ fx.addRoute(t, registry.Route{Host: "kass.cdef.oncarlos.test", App: "other", Account: "cdef", Kind: "instance"})
+ c := signInPaul(t, fx)
+ csrf := sessionToken(t, fx, c)
+ // Not confirmed: without the check this renders the move card, so the
+ // card's absence is the refusal.
+ status, _, body := postForm(t, c, fx.u("/apps/kass/move"), url.Values{"csrf": {csrf}})
+ if strings.Contains(body, "Move kass to Second?") {
+ t.Fatalf("offered the move into an account where the apex is held = %d", status)
+ }
+ if has, _ := fx.st.HasAppIn(destID, "kass"); has {
+ t.Error("kass was moved")
+ }
+ if has, _ := fx.st.HasAppIn(OpsAccountID, "kass"); !has {
+ t.Error("kass should still be in its own account")
+ }
+}
+
+// At boot the console names every route already sitting at another app's
+// 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", "blank", "elsewhere"} {
+ if _, err := fx.st.CreateAppIn(OpsAccountID, app, "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ }
+ if err := fx.st.TrashAppRow(OpsAccountID, "parked", now); err != nil {
+ t.Fatal(err)
+ }
+ 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: "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) != 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",
+ "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 fb43a2099..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,
@@ -421,6 +421,8 @@ func (s *Server) handleAPICreateApp(w http.ResponseWriter, r *http.Request) {
jsonError(w, http.StatusConflict, "that name is reserved")
case claimTaken:
jsonError(w, http.StatusConflict, "that name is taken")
+ case claimApexHeld:
+ 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 017f47a61..97072532e 100644
--- a/internal/console/apiinstances.go
+++ b/internal/console/apiinstances.go
@@ -613,23 +613,42 @@ func (s *Server) handleAPIInstanceCreate(w http.ResponseWriter, r *http.Request)
// to that app. Custom-domain manifests never enter apex space (ValidHost
// pinned the suffix above), so this check cannot touch them.
//
+ // "Claimed" includes an app in the TRASH. A trashed app's name is
+ // reserved until purge — CreateAppIn and RenameAppIn both refuse it — and
+ // so is its apex: were the check live apps only, a sibling could take the
+ // apex while the app sat in the trash and keep it across a restore, with
+ // the domains pass deferring to the sibling's route for ever. That is
+ // also where `carlos email events push` sends the restored app's
+ // recipients' addresses (discussion 153). Only a purge frees the apex.
+ // The name arriving after the instance — a create, rename or move — is
+ // the other way in, and apex.go closes it.
+ //
// The same question answers a second one further down (the steering
// default), so it is asked once, here, and named: is this manifest
// 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 {
- claimed, err := s.store.HasAppIn(acct.ID, label)
+ 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)
jsonError(w, http.StatusInternalServerError, "could not check the host — try again")
return
}
- if claimed {
+ switch {
+ case live:
jsonError(w, http.StatusConflict,
host+" is app "+label+"'s platform apex — create it as an instance of "+label+" instead")
return
+ case trashed:
+ jsonError(w, http.StatusConflict,
+ host+" is app "+label+"'s platform apex, and "+label+" is in the trash — its apex stays reserved until it is purged")
+ return
}
}
}
@@ -797,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/apiinstancesenable_test.go b/internal/console/apiinstancesenable_test.go
index 7271aee5b..2a9c6857a 100644
--- a/internal/console/apiinstancesenable_test.go
+++ b/internal/console/apiinstancesenable_test.go
@@ -207,6 +207,40 @@ func TestAPIInstanceCreateSiblingApexRefused(t *testing.T) {
t.Fatalf("app-token sibling apex create = %d %s, want 409", status, out)
}
+ // A sibling in the TRASH still owns its apex: the name is reserved until
+ // purge, and an apex taken while it is trashed would be kept across a
+ // restore (discussion 153). Only a purge frees it.
+ if err := fx.st.TrashAppRow(OpsAccountID, "marketing", now); err != nil {
+ t.Fatal(err)
+ }
+ status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), appTok, `{"host":"marketing.`+domain+`"}`)
+ if status != http.StatusConflict || !strings.Contains(out, "trash") {
+ t.Fatalf("trashed sibling apex create = %d %s, want 409 naming the trash", status, out)
+ }
+ if err := fx.st.RestoreAppRow(OpsAccountID, "marketing"); err != nil {
+ t.Fatal(err)
+ }
+ if status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member,
+ `{"host":"marketing.`+domain+`"}`); status != http.StatusConflict {
+ t.Fatalf("restored sibling apex create = %d %s, want 409", status, out)
+ }
+ if err := fx.st.TrashAppRow(OpsAccountID, "marketing", now); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.PurgeAppRow(OpsAccountID, "marketing"); err != nil {
+ t.Fatal(err)
+ }
+ if status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member,
+ `{"host":"marketing.`+domain+`"}`); status != http.StatusCreated {
+ t.Fatalf("a purged sibling's label create = %d %s, want 201 — a purge frees the apex", status, out)
+ }
+ // ...and having taken it, titogo holds it: an app created under that
+ // name afterwards would never get its own URL, so the name is not free
+ // (apex.go) until titogo's instance there is gone.
+ if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/"+fx.sqid+"/new", map[string]string{"name": "marketing"}, member); status != http.StatusConflict {
+ t.Fatalf("re-creating marketing under titogo's instance = %d %v, want 409", status, out)
+ }
+
// The app's own apex and an unclaimed label are exactly what the door
// is for.
if status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member,
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 5a38a41af..907f244da 100644
--- a/internal/console/appsettings.go
+++ b/internal/console/appsettings.go
@@ -644,6 +644,15 @@ func (s *Server) handleAppRename(w http.ResponseWriter, r *http.Request) {
back("err", "A host still routes this name — remove its routes first.")
return
}
+ // The new name's platform apex must not already be another app's (apex.go).
+ 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 hold != nil:
+ back("err", "Cannot rename to "+to+": "+apexHeldSentence(s.apexHost(acct.Sqid, to), hold)+".")
+ return
+ }
if !confirmed(r) {
cv := &ConfirmView{
Title: "Rename " + app + " to " + to + "?",
@@ -680,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)
@@ -1122,6 +1136,19 @@ func (s *Server) handleAppMoveAccount(w http.ResponseWriter, r *http.Request) {
back("err", "The routes name account "+target+", which this console does not know — that is an operator repair.")
return
}
+ // In the destination, this app's platform apex must not already be
+ // another app's (apex.go).
+ 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 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 == "" {
back("err", "You are not a member of "+dest.Name+" ("+target+"), so moving the record there would strand the app for you. Ask an owner of that account, or an operator, to finish the move.")
return
diff --git a/internal/console/server.go b/internal/console/server.go
index 7efe304fc..8a7666c78 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -3332,6 +3332,7 @@ const (
claimCheckFailed // a database lookup failed
claimStoreFailed // the release-store listing failed
claimCreateFailed // the insert itself failed
+ claimApexHeld // another app already holds the name's platform apex (apex.go)
)
// normalizeAppName is the one normalization both claim doors apply before
@@ -3427,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
@@ -3472,10 +3476,21 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem
// and the database checks above stay outside it, as they always were.
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()
+ // 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 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
@@ -3496,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 {
@@ -3531,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
@@ -3576,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.")
@@ -3584,6 +3599,8 @@ func (s *Server) handleNewApp(w http.ResponseWriter, r *http.Request) {
fail("That name is reserved.")
case claimTaken:
fail("That name is taken.")
+ case claimApexHeld:
+ 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:
diff --git a/internal/console/store.go b/internal/console/store.go
index fd47e4e41..eca0917ce 100644
--- a/internal/console/store.go
+++ b/internal/console/store.go
@@ -1468,6 +1468,21 @@ func (s *Store) TrashedAppsIn(accountID int64) ([]TrashedApp, error) {
return out, rows.Err()
}
+// AppNameState reports, in one read, whether an account holds app live, in
+// the trash, or not at all — for a caller that must tell the three apart
+// without a restore or a trash landing between two separate questions.
+func (s *Store) AppNameState(accountID int64, app string) (live, trashed bool, err error) {
+ var deletedAt int64
+ err = s.db.QueryRow(`SELECT deleted_at FROM apps WHERE account_id = ? AND app = ?`, accountID, app).Scan(&deletedAt)
+ if errors.Is(err, sql.ErrNoRows) {
+ return false, false, nil
+ }
+ if err != nil {
+ return false, false, err
+ }
+ return deletedAt == 0, deletedAt != 0, nil
+}
+
// HasTrashedAppIn reports whether an account holds a TRASHED row for app —
// the check the claim-once door uses to explain a refusal ("that name is in
// the trash") rather than just saying no.