amadan

carlos / platform Public

carlos add: name the route's account, never re-own an existing route (#175 Q7, G17)

Direct-mode carlos add defaulted --account to ops, so every route it wrote
was filed under the deployment operator, and console claims made as the
app's own account were refused against it. That is why no custom domain
could attach to Tito's controld hosts. The flag now has no default: the add
is refused unless --account or --account-id names the owner. A re-add that
names a different account than the existing row's owner is refused before
anything is written; changing owners belongs to adoption (P6).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Human

Prompt

Go ahead. (start P7 and the Q7 direct-add fix)

From this commit's own message. The prompt ledger has no entries for it — which is what a squashed, rebased or amended commit looks like, and what every commit looks like in a repo without a ledger.

Summary

Q7 ruling: carlos add requires an account and keeps an existing route's owner.
Agent notes
Breaking for callers relying on the ops default: Tito's titogo-route-sync, and platform-infrastructure's rehearse/smoke/rollout-status scripts. The binary already accepts --account, so callers can change first.
claude 85687649fff441d4899c7d3af5c1d9b7bb6f94ea parent 52f7114
4 files changed, +93 −26
  • cli.go +18 −1
  • cli_test.go +61 −11
  • e2e_test.go +11 −11
  • route_replication_test.go +3 −3

CI passed — run details

Intent

full ledger: carlos-add-account

diff --git a/cli.go b/cli.go
index ad82f5462..09c017214 100644
--- a/cli.go
+++ b/cli.go
@@ -69,7 +69,7 @@ func runAdd(args []string) {
fs := flag.NewFlagSet("add", flag.ExitOnError)
reg := fs.String("registry", defaultRegistry(), "registry database")
app := appFlag(fs, "app whose releases this route runs")
- acct := fs.String("account", "ops", "account (sqid or name) the app belongs to")
+ acct := fs.String("account", "", "account (sqid or name) the app belongs to. Required, unless --account-id names it")
acctID := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes --account")
slug := fs.String("slug", "", "root-domain short name (optional)")
kind := fs.String("kind", "instance", "instance | service | static")
@@ -90,6 +90,15 @@ func runAdd(args []string) {
if err := addRail(host, *kind); err != nil {
log.Fatalf("add %s: %v", host, err)
}
+ // The route's owner is named, never assumed (G17; instance-moves design
+ // §0, Q7). The old "ops" default filed every direct-mode add under the
+ // deployment operator, so console claims made as the app's own account
+ // were refused against the row: that is why no custom domain could
+ // attach to Tito's controld hosts. `--account ops` still says it,
+ // out loud.
+ if strings.TrimSpace(*acct) == "" && strings.TrimSpace(*acctID) == "" {
+ log.Fatalf("add %s: name the account the route belongs to with --account or --account-id", host)
+ }
if reservedSlugs()[*slug] {
log.Fatalf("slug %q is reserved", *slug)
}
@@ -195,6 +204,14 @@ func runAdd(args []string) {
if err != nil {
log.Fatal(err)
}
+ // A rewrite keeps the route's owner. Changing it moves which account's
+ // releases and claims the row answers to, and that transfer belongs to
+ // a door that names both sides (adoption, P6), not to a re-add whose
+ // account flag may simply be wrong.
+ if existing != nil && release.RouteAccount(*existing) != account {
+ log.Fatalf("add %s: this route belongs to account %s, not %s. carlos add does not change a route's owner",
+ host, release.RouteAccount(*existing), account)
+ }
route := registry.Route{
Host: host, Slug: *slug, Kind: *kind, Socket: *socket, Addr: *addr,
App: *app, Account: account,
diff --git a/cli_test.go b/cli_test.go
index a98f75b85..e9fc566f7 100644
--- a/cli_test.go
+++ b/cli_test.go
@@ -542,7 +542,7 @@ func TestRouteUnitStampsAnExistingRoute(t *testing.T) {
}
// The console as it exists on a box today: addr-backed, no unit.
- if out, err := run("add", "-registry", regPath, "-app", "console",
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "console",
"-addr", "127.0.0.1:8090", "console.carloku.com"); err != nil {
t.Fatalf("add console route: %v\n%s", err, out)
}
@@ -596,7 +596,7 @@ func TestRouteUnitStampsAnExistingRoute(t *testing.T) {
// It survives a re-`carlos add` of the same row (registry.Upsert's
// preservation): dropping it would silently restore issue #110.
- if out, err := run("add", "-registry", regPath, "-app", "console",
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "console",
"-addr", "127.0.0.1:8090", "console.carloku.com"); err != nil {
t.Fatalf("re-add console route: %v\n%s", err, out)
}
@@ -645,7 +645,7 @@ func TestRouteUnitStampsAnExistingRoute(t *testing.T) {
// mint a row that is unit-BACKED and unit-NAMED at once: the dispatch
// honours the name, so the carlos-app@<host> unit would never be cycled
// by adoption again — issue #110 recreated for an ordinary tenant.
- out, err = run("add", "-registry", regPath, "-app", "kass", "-backing", "unit",
+ out, err = run("add", "-account", "ops", "-registry", regPath, "-app", "kass", "-backing", "unit",
"-unit", "bespoke.service", "-db", filepath.Join(tmp, "t.db"), "pair.example.com")
if err == nil {
t.Fatalf("add --backing unit --unit bespoke.service: want non-zero exit, got 0\n%s", out)
@@ -653,7 +653,7 @@ func TestRouteUnitStampsAnExistingRoute(t *testing.T) {
if !strings.Contains(out, "carlos-app@") {
t.Errorf("add refusal = %q, want it to name the unit that already owns the process", out)
}
- if out, err := run("add", "-registry", regPath, "-app", "kass", "-backing", "unit",
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "kass", "-backing", "unit",
"-db", filepath.Join(tmp, "t.db"), "tenant.example.com"); err != nil {
t.Fatalf("add unit tenant: %v\n%s", err, out)
}
@@ -740,7 +740,7 @@ func TestAddPreservesUpstreamsOnReAdd(t *testing.T) {
}
const host = "kass.example.com"
- if out, err := run("add", "-registry", regPath, "-app", "kass",
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "kass",
"-addr", "127.0.0.1:9001", host); err != nil {
t.Fatalf("add: %v\n%s", err, out)
}
@@ -768,7 +768,7 @@ func TestAddPreservesUpstreamsOnReAdd(t *testing.T) {
// A re-add naming -addr again, once the row carries an established
// multi-upstream set, is refused rather than silently clearing it — the
// record owns that shape, not this box verb.
- out, err := run("add", "-registry", regPath, "-app", "kass",
+ out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "kass",
"-addr", "127.0.0.1:9001", host)
if err == nil {
t.Fatalf("re-add with --addr over an established Addrs set: want non-zero exit, got 0\n%s", out)
@@ -791,7 +791,7 @@ func TestAddPreservesUpstreamsOnReAdd(t *testing.T) {
// A re-add naming NO -addr at all must not blank Addrs/Health either —
// this box-local verb only carries what its own flags set, and the
// refusal above is -addr-specific, not a blanket lockout of the host.
- if out, err := run("add", "-registry", regPath, "-app", "kass", host); err != nil {
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "kass", host); err != nil {
t.Fatalf("re-add with no --addr: %v\n%s", err, out)
}
rt, err = reg.Get(host)
@@ -812,11 +812,11 @@ func TestAddPreservesUpstreamsOnReAdd(t *testing.T) {
// that has no established multi-upstream set is unaffected and still
// works exactly as it always has.
const solo = "solo.example.com"
- if out, err := run("add", "-registry", regPath, "-app", "kass",
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "kass",
"-addr", "127.0.0.1:9100", solo); err != nil {
t.Fatalf("add solo: %v\n%s", err, out)
}
- if out, err := run("add", "-registry", regPath, "-app", "kass",
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "kass",
"-addr", "127.0.0.1:9200", solo); err != nil {
t.Fatalf("re-add solo with --addr (no established Addrs set): %v\n%s", err, out)
}
@@ -1160,6 +1160,56 @@ func TestAddRefusesSidecarSuffixAndKind(t *testing.T) {
}
}
+// Direct-mode `carlos add` names the route's owner and never changes it
+// (G17; instance-moves design §0, Q7): no account is refused before any
+// write, and a re-add under another account leaves the row as it was.
+func TestAddNamesItsAccountAndKeepsTheOwner(t *testing.T) {
+ bin := buildCarlos(t)
+ tmp := t.TempDir()
+ scratch := t.TempDir()
+ regPath := filepath.Join(tmp, "registry.db")
+ run := func(args ...string) (string, error) {
+ t.Helper()
+ cmd := exec.Command(bin, args...)
+ cmd.Env = carlosTestEnv(t)
+ cmd.Dir = scratch
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+ const host = "elatromme.go.example.com"
+
+ out, err := run("add", "-registry", regPath, "-app", "titogo", "-addr", "127.0.0.1:8444", host)
+ if err == nil || !strings.Contains(out, "--account") {
+ t.Fatalf("add with no account = %v\n%s\nwant a refusal naming --account", err, out)
+ }
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer reg.Close()
+ if rt, gerr := reg.Get(host); gerr != nil || rt != nil {
+ t.Fatalf("a refused add still wrote a row: %+v, %v", rt, gerr)
+ }
+
+ if out, err := run("add", "-registry", regPath, "-account-id", "bae", "-app", "titogo", "-addr", "127.0.0.1:8444", host); err != nil {
+ t.Fatalf("add -account-id bae: %v\n%s", err, out)
+ }
+ out, err = run("add", "-registry", regPath, "-account", "ops", "-app", "titogo", "-addr", "127.0.0.1:8445", host)
+ if err == nil || !strings.Contains(out, "belongs to account bae") {
+ t.Fatalf("re-add under ops = %v\n%s\nwant a refusal naming the owner", err, out)
+ }
+ rt, err := reg.Get(host)
+ if err != nil || rt == nil {
+ t.Fatalf("get: %+v, %v", rt, err)
+ }
+ if rt.Account != "bae" || rt.Addr != "127.0.0.1:8444" {
+ t.Fatalf("row after the refused re-add = account %q addr %q, want bae and 127.0.0.1:8444 untouched", rt.Account, rt.Addr)
+ }
+ if out, err := run("add", "-registry", regPath, "-account-id", "bae", "-app", "titogo", "-addr", "127.0.0.1:8445", host); err != nil {
+ t.Fatalf("re-add under the owner: %v\n%s", err, out)
+ }
+}
+
// TestAddRefusesSidecarSuffixWiredIntoRunAdd drives `carlos add` as a
// subprocess: addRail's own test proves the predicate, this proves runAdd
// actually calls it, before any registry write.
@@ -1179,7 +1229,7 @@ func TestAddRefusesSidecarSuffixWiredIntoRunAdd(t *testing.T) {
}
host := "bot.eleven.bdm" + provision.SidecarSuffix
- out, err := run("add", "-registry", regPath, "-app", "eleven", host)
+ out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "eleven", host)
if err == nil {
t.Fatalf("carlos add %s: want non-zero exit, got 0\n%s", host, out)
}
@@ -1195,7 +1245,7 @@ func TestAddRefusesSidecarSuffixWiredIntoRunAdd(t *testing.T) {
t.Fatalf("a refused add still wrote a row: %+v, %v", rt, gerr)
}
- out, err = run("add", "-registry", regPath, "-app", "eleven", "-kind", "sidecar", "app.example.com")
+ out, err = run("add", "-account", "ops", "-registry", regPath, "-app", "eleven", "-kind", "sidecar", "app.example.com")
if err == nil {
t.Fatalf("carlos add -kind sidecar: want non-zero exit, got 0\n%s", out)
}
diff --git a/e2e_test.go b/e2e_test.go
index abe518b90..fb8c6721d 100644
--- a/e2e_test.go
+++ b/e2e_test.go
@@ -1306,7 +1306,7 @@ func TestAddUnitBackingRequiresDBForHibernate(t *testing.T) {
// No -db: refused, non-zero exit, no row written.
// pick() walks cwd ancestors for .carlos/config, so the subprocess must not run from the real checkout.
- cmd := exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass",
+ cmd := exec.Command(carlosBin, "add", "-account", "ops", "-registry", regPath, "-app", "kass",
"-backing", "unit", "-hibernate", "no-db.example.com")
cmd.Env = append(os.Environ(), "HOME="+tmp, "CARLOS_CONSOLE=")
cmd.Dir = tmp
@@ -1321,7 +1321,7 @@ func TestAddUnitBackingRequiresDBForHibernate(t *testing.T) {
// With -db: succeeds, and the route's DBPath is exactly what was given
// (ops.FleetDBs reads this verbatim — no default synthesized here).
dbPath := filepath.Join(tmp, "unit-state", "app.db")
- cmd = exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass",
+ cmd = exec.Command(carlosBin, "add", "-account", "ops", "-registry", regPath, "-app", "kass",
"-backing", "unit", "-hibernate", "-db", dbPath, "with-db.example.com")
cmd.Env = append(os.Environ(), "HOME="+tmp, "CARLOS_CONSOLE=")
cmd.Dir = tmp
@@ -1393,7 +1393,7 @@ func TestAddRefusesRelativeDBOnExecRoutes(t *testing.T) {
}
// The vitogo add, verbatim shape: exec backing (the default), relative -db.
- out, err := run("add", "-registry", regPath, "-app", "vitogo",
+ out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "vitogo",
"-kind", "instance", "-db", "vito.db", "-hibernate",
"-channel", "canary/vitogo", "vito.example.com")
if err == nil {
@@ -1420,7 +1420,7 @@ func TestAddRefusesRelativeDBOnExecRoutes(t *testing.T) {
// A unit tenant keeps the contract: StateDirectory= is real there, a
// relative -db is the documented convention, and this guard must not
// take it away.
- out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ out, err = run("add", "-account", "ops", "-registry", regPath, "-app", "vitogo",
"-kind", "instance", "-backing", "unit", "-db", "vito.db",
"unit.example.com")
if err != nil {
@@ -1428,7 +1428,7 @@ func TestAddRefusesRelativeDBOnExecRoutes(t *testing.T) {
}
// An absolute -db on an exec route is untouched by the guard.
- out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ out, err = run("add", "-account", "ops", "-registry", regPath, "-app", "vitogo",
"-kind", "instance", "-db", "/data/vito.example.com.db",
"vito.example.com")
if err != nil {
@@ -1463,7 +1463,7 @@ func TestAddAccountIDRefusesAName(t *testing.T) {
}
regPath := filepath.Join(tmp, "registry.db")
- out, err := exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass",
+ out, err := exec.Command(carlosBin, "add", "-account", "ops", "-registry", regPath, "-app", "kass",
"-account-id", "ops", "phantom.example.com").CombinedOutput()
if err == nil {
t.Fatalf("add --account-id ops: want non-zero exit, got 0\n%s", out)
@@ -1476,7 +1476,7 @@ func TestAddAccountIDRefusesAName(t *testing.T) {
// A name that is merely sqid-SHAPED is refused the same way when it
// carries an i, l or o — the letters no id can ever encode to.
- out, err = exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass",
+ out, err = exec.Command(carlosBin, "add", "-account", "ops", "-registry", regPath, "-app", "kass",
"-account-id", "oil", "phantom2.example.com").CombinedOutput()
if err == nil {
t.Fatalf("add --account-id oil: want non-zero exit, got 0\n%s", out)
@@ -1488,7 +1488,7 @@ func TestAddAccountIDRefusesAName(t *testing.T) {
// A real sqid is accepted and stamped verbatim — never resolved against
// the registry, which is the point of the flag: this one has no accounts
// in it at all.
- out, err = exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass",
+ out, err = exec.Command(carlosBin, "add", "-account", "ops", "-registry", regPath, "-app", "kass",
"-account-id", "bah", "real.example.com").CombinedOutput()
if err != nil {
t.Fatalf("add --account-id bah: %v\n%s", err, out)
@@ -1545,7 +1545,7 @@ func TestAddOnExistingRouteSaysUpdated(t *testing.T) {
return string(out), err
}
- out, err := run("add", "-registry", regPath, "-app", "vitogo",
+ out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "vitogo",
"-db", "/data/first.db", "vito.example.com")
if err != nil {
t.Fatalf("first add: %v\n%s", err, out)
@@ -1554,7 +1554,7 @@ func TestAddOnExistingRouteSaysUpdated(t *testing.T) {
t.Errorf("first add output = %q, want %q", out, "added vito.example.com")
}
- out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ out, err = run("add", "-account", "ops", "-registry", regPath, "-app", "vitogo",
"-db", "/data/second.db", "vito.example.com")
if err != nil {
t.Fatalf("re-add: %v\n%s", err, out)
@@ -3445,7 +3445,7 @@ func TestAddOnABoundHostPreservesItsEnvironment(t *testing.T) {
t.Fatalf("route --environment demo: %v\n%s", err, out)
}
- if out, err := f.run(f.fullEnv(), "add", "-registry", f.regPath,
+ if out, err := f.run(f.fullEnv(), "add", "-account", "ops", "-registry", f.regPath,
"-app", "tito", "demo.tito.example.com"); err != nil {
t.Fatalf("re-add: %v\n%s", err, out)
}
diff --git a/route_replication_test.go b/route_replication_test.go
index ec6fc74e0..b325caf36 100644
--- a/route_replication_test.go
+++ b/route_replication_test.go
@@ -146,7 +146,7 @@ func TestRouteReplicationSetsAndClearsTheRung(t *testing.T) {
// A hibernating instance as one exists on a box today: a large database,
// low write volume, on the every-second default because nothing could say
// otherwise.
- if out, err := run("add", "-registry", regPath, "-app", "seapointish",
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "seapointish",
"-socket", filepath.Join(tmp, "s.sock"), "-hibernate", "seapointish.example.com"); err != nil {
t.Fatalf("add route: %v\n%s", err, out)
}
@@ -234,7 +234,7 @@ func TestRouteReplicationRefusesCombinationWithOtherAxes(t *testing.T) {
out, err := cmd.CombinedOutput()
return string(out), err
}
- if out, err := run("add", "-registry", regPath, "-app", "a",
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "a",
"-socket", filepath.Join(tmp, "a.sock"), "-hibernate", "a.example.com"); err != nil {
t.Fatalf("add: %v\n%s", err, out)
}
@@ -295,7 +295,7 @@ func TestRouteHibernateOffClearsTheRung(t *testing.T) {
}
host := "seapointish.example.com"
- if out, err := run("add", "-registry", regPath, "-app", "seapointish",
+ if out, err := run("add", "-account", "ops", "-registry", regPath, "-app", "seapointish",
"-socket", filepath.Join(tmp, "s.sock"), "-hibernate", host); err != nil {
t.Fatalf("add route: %v\n%s", err, out)
}