amadan

carlos / platform Public

console: the instance create door refuses a host another app holds

Instance records are keyed per app, so two apps of one account could both
write the same host and the provisioning pass would silently keep one. The
door only knew apex labels (a label naming another claimed app). Ask the
same question of the host itself: a route at it, or a record at it under
another live or trashed app, refuses the create with 409.

Flattening canary hosts to <canary>-<app> (discussion 207) makes this
reachable: canary "a" of app "b-c" and canary "a-b" of app "c" are the same
label. Tests pin that, the flat one-label host being accepted, the
two-label host staying refused, and both orders of a canary against
another app's apex.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude c8ee0789e96ac430d04ef7b54360684c6cddad45 parent 8d8e4ba
3 files changed, +196 −0
  • internal/console/apex.go +43 −0
  • internal/console/apiinstances.go +18 −0
  • internal/console/apiinstancescanary_test.go +135 −0
diff --git a/internal/console/apex.go b/internal/console/apex.go
index a8a9d3e0a..eb977c525 100644
--- a/internal/console/apex.go
+++ b/internal/console/apex.go
@@ -80,6 +80,29 @@ func (s *Server) apexHolder(ctx context.Context, acct *Account, name string) (*a
if host == "" {
return nil, nil
}
+ return s.hostHolder(ctx, acct, host, name)
+}
+
+// hostHolder answers whether host is already held by an app of acct other
+// than own — nil when it is free, or when the only holder is own itself.
+// apexHolder is this question asked of an app's platform apex; the instance
+// create door asks it of whatever host a record is about to claim, because
+// instance records are keyed per app (provision.RecordKey) and nothing in
+// the key stops two apps of one account from both writing the same host. The
+// provisioning pass would then keep whichever record it listed first and
+// skip the other with a log line (provision/reconcile.go, "duplicate host"),
+// so the loser looks created and serves nothing, or serves the other app's
+// process under its own name.
+//
+// A flat canary host (<canary>-<app>.<domain>, discussion 207) is how that
+// became easy to reach: canary "a" of app "b-c" and canary "a-b" of app "c"
+// are the same label.
+//
+// Where a holder could be, and the fail-closed rule, are apexHolder's: 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.
+func (s *Server) hostHolder(ctx context.Context, acct *Account, host, own string) (*apexHold, error) {
+ name := own
if s.cfg.Registry != "" {
reg, err := openRegistry(s.cfg.Registry)
if err != nil {
@@ -157,6 +180,26 @@ func apexHeldSentence(host string, hold *apexHold) string {
"; remove that instance first, or choose another name"
}
+// hostHeldSentence is the instance create door's refusal for a host another
+// app of the account already holds (hostHolder), with the advice that fits
+// who holds it. It does not claim the host is anyone's apex: that is
+// apexHeldSentence's case, and the door's own apex check answers it first.
+func hostHeldSentence(host string, hold *apexHold) string {
+ switch {
+ case hold == nil:
+ return host + " is already held"
+ case hold.OtherAccount != "" && hold.Holder != "":
+ return host + " is already routed to " + hold.Holder + " in account " + hold.OtherAccount + "; that is an operator repair"
+ case hold.OtherAccount != "":
+ return host + " is already routed for account " + hold.OtherAccount + "; that is an operator repair"
+ case hold.Holder == "":
+ return host + " already has a route with no app; an operator has to remove it first"
+ case hold.Trashed:
+ return host + " is already held by " + hold.Holder + ", which is in the trash; choose another address"
+ }
+ return host + " is already held by " + hold.Holder + "; choose another address"
+}
+
// 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
diff --git a/internal/console/apiinstances.go b/internal/console/apiinstances.go
index 6b011d2f4..aecbbcf23 100644
--- a/internal/console/apiinstances.go
+++ b/internal/console/apiinstances.go
@@ -751,6 +751,24 @@ func (s *Server) handleAPIInstanceCreate(w http.ResponseWriter, r *http.Request)
}
}
}
+ // The sibling check above names apps; this one names HOSTS. Records are
+ // keyed per app, so another app of this account may already hold the very
+ // host this one is asking for without being called by its label: canary
+ // "a" of app "b-c" and canary "a-b" of app "c" are both a-b-c (the flat
+ // canary host, discussion 207). The provisioning pass would keep one
+ // record and skip the other, so the loser would look created and serve
+ // nothing. Refused here, for every host, on the same fail-closed terms as
+ // the apex checks; a host this app already holds falls through to the
+ // create's own "already exists".
+ switch hold, err := s.hostHolder(ctx, acct, host, app); {
+ case err != nil:
+ log.Printf("console: api instances create %s/%s: host holder check %s: %v", acct.Sqid, app, host, err)
+ jsonError(w, http.StatusInternalServerError, "could not check the host — try again")
+ return
+ case hold != nil:
+ jsonError(w, http.StatusConflict, hostHeldSentence(host, hold))
+ return
+ }
// A fleet-placed app (customer-fleets plan, `carlos apps place`) governs
// every one of its instances' Region uniformly — spec's "one flag on the
// app, all its instances follow" (appPlacement, fleets.go) — so when a
diff --git a/internal/console/apiinstancescanary_test.go b/internal/console/apiinstancescanary_test.go
new file mode 100644
index 000000000..b21d6e4ee
--- /dev/null
+++ b/internal/console/apiinstancescanary_test.go
@@ -0,0 +1,135 @@
+package console
+
+import (
+ "context"
+ "net/http"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/provision"
+ "github.com/carlosframework/platform/internal/release"
+)
+
+// canaryDoorFixture is a console with the platform apps domain set, and two
+// apps in the ops account ("bar" and "foo-bar") with instances enabled on
+// the account's platform subdomain, the way `carlos instances enable` pins
+// them. It returns the fixture, a member token and the manifest domain.
+func canaryDoorFixture(t *testing.T, code string, apps ...string) (*consoleFixture, string, string) {
+ t.Helper()
+ fx, member := instanceFixture(t, code)
+ fx.srv.cfg.AppsDomain = "oncarlos.test"
+ now := time.Now()
+ for _, a := range apps {
+ if _, err := fx.st.CreateAppIn(OpsAccountID, a, "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ }
+ for _, a := range append([]string{"titogo"}, apps...) {
+ if status, _, out := rawReq(t, http.MethodPut, enableURL(fx, "bab", a), member, `{}`); status != http.StatusOK {
+ t.Fatalf("enable %s = %d %s", a, status, out)
+ }
+ }
+ return fx, member, fx.sqid + ".oncarlos.test"
+}
+
+// The flat canary host is <canary>-<app>.<domain>: one label, so the create
+// door that insists on one label accepts it. This is the door half of
+// discussion 207: the host `carlos canary` builds must pass ValidHost.
+func TestAPIInstanceCreateAcceptsAFlatCanaryHost(t *testing.T) {
+ fx, member, domain := canaryDoorFixture(t, "code-canary-flat")
+ status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member,
+ `{"host":"new-queue-titogo.`+domain+`","channel":"canary/new-queue"}`)
+ if status != http.StatusCreated {
+ t.Fatalf("flat canary host create = %d %s, want 201", status, out)
+ }
+ // The two-label form stays refused: ValidHost is a tenancy boundary and
+ // is not loosened for canaries.
+ status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member,
+ `{"host":"other.titogo.`+domain+`","channel":"canary/other"}`)
+ if status != http.StatusBadRequest || !strings.Contains(out, "invalid host") {
+ t.Fatalf("two-label host create = %d %s, want 400 invalid host", status, out)
+ }
+}
+
+// A canary named "foo" on app "bar" lands on foo-bar.<domain>, which is also
+// the platform apex of an app called "foo-bar". Flattening makes that
+// reachable, so both orders are pinned: the canary cannot take a live or
+// trashed app's apex, and an app cannot be created over a canary that
+// already sits on its apex.
+func TestFlatCanaryHostCannotTakeAnotherAppsApex(t *testing.T) {
+ fx, member, domain := canaryDoorFixture(t, "code-canary-apex", "bar", "foo-bar")
+
+ // canary "foo" of app "bar": host foo-bar, app foo-bar's apex.
+ status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "bar"), member,
+ `{"host":"foo-bar.`+domain+`","channel":"canary/foo"}`)
+ if status != http.StatusConflict || !strings.Contains(out, "foo-bar") || !strings.Contains(out, "platform apex") {
+ t.Fatalf("canary onto a live app's apex = %d %s, want 409 naming the apex", status, out)
+ }
+ if err := fx.st.TrashAppRow(OpsAccountID, "foo-bar", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "bar"), member,
+ `{"host":"foo-bar.`+domain+`","channel":"canary/foo"}`)
+ if status != http.StatusConflict || !strings.Contains(out, "trash") {
+ t.Fatalf("canary onto a trashed app's apex = %d %s, want 409 naming the trash", status, out)
+ }
+ if err := fx.st.RestoreAppRow(OpsAccountID, "foo-bar"); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := provision.GetRecord(context.Background(), fx.rel, release.OpsAccount, "bar", "foo-bar."+domain); err == nil {
+ t.Fatal("a refused canary left a record behind")
+ }
+
+ // The other order: the canary exists while no app owns the name, and an
+ // app arrives under it afterwards.
+ status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "bar"), member,
+ `{"host":"qux-bar.`+domain+`","channel":"canary/qux"}`)
+ if status != http.StatusCreated {
+ t.Fatalf("canary on a free label = %d %s, want 201", status, out)
+ }
+ if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/"+fx.sqid+"/new", map[string]string{"name": "qux-bar"}, member); status != http.StatusConflict {
+ t.Fatalf("creating an app over a canary's host = %d %v, want 409", status, out)
+ }
+}
+
+// Two canaries of two apps can flatten to the same label: canary "a" of app
+// "b-c" and canary "a-b" of app "c" are both a-b-c. Instance records are
+// keyed per app, so nothing about the key stops the second one, and the
+// provisioning pass would quietly keep whichever record it lists first. The
+// door refuses a host another app of the account already holds, whatever
+// its channel, and in both directions.
+func TestFlatCanaryHostsOfTwoAppsCannotCollide(t *testing.T) {
+ fx, member, domain := canaryDoorFixture(t, "code-canary-collide", "b-c", "c")
+ const host = "a-b-c."
+
+ status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "b-c"), member,
+ `{"host":"`+host+domain+`","channel":"canary/a"}`)
+ if status != http.StatusCreated {
+ t.Fatalf("first canary = %d %s, want 201", status, out)
+ }
+ status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "c"), member,
+ `{"host":"`+host+domain+`","channel":"canary/a-b"}`)
+ if status != http.StatusConflict || !strings.Contains(out, "b-c") {
+ t.Fatalf("colliding canary of another app = %d %s, want 409 naming app b-c", status, out)
+ }
+ if _, err := provision.GetRecord(context.Background(), fx.rel, release.OpsAccount, "c", host+domain); err == nil {
+ t.Fatal("the refused canary left a record under app c")
+ }
+
+ // An ordinary instance of another app at a host a canary holds is the
+ // same collision, as is the reverse.
+ status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member,
+ `{"host":"`+host+domain+`"}`)
+ if status != http.StatusConflict {
+ t.Fatalf("ordinary instance on a canary's host = %d %s, want 409", status, out)
+ }
+
+ // The holder itself is told "already exists", as ever: this is not a
+ // sibling's host.
+ status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "b-c"), member,
+ `{"host":"`+host+domain+`","channel":"canary/a"}`)
+ if status != http.StatusConflict || !strings.Contains(out, "already exists") {
+ t.Fatalf("re-create by the holder = %d %s, want 409 already exists", status, out)
+ }
+}