amadan

carlos / platform Public

registry: routes learn which systemd unit owns their process

Addr-backed routes — the consoles, `-addr 127.0.0.1:8090` under a bespoke
console.service — are owned by neither of the two things the adopt pass's
restart dispatch can cycle, so promote → adopt stamped a new Version (and
therefore X-Carlos-Version) behind a process still serving the old build
(issue #110). The new `unit` column is where a route names the service that
owns it.

Additive in the ALTER list, so an existing registry.db upgrades in place and
every row already on every box reads "" — today's behaviour exactly. Upsert
preserves a non-empty value for the tunnel/caps reason with the caps stakes:
a re-`carlos add` on the console's row must not silently drop the one stamp
that makes adoption cycle it. SetUnit validates (SetBacking does not, because
backing has two legal values the doors compare against; a unit name is free
text handed to systemctl months later, and a typo would surface as a restart
that quietly did nothing — the very bug this closes).

Deliberately not in Fingerprint(): unit changes who restarts the process, not
what the edge serves.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 5d5fd39c084f6965c56d962b596c0e95db267218 parent 7a3d8c1
2 files changed, +338 −15
  • internal/registry/registry.go +114 −14
  • internal/registry/registry_test.go +224 −1
diff --git a/internal/registry/registry.go b/internal/registry/registry.go
index 24192db6d..aa87db238 100644
--- a/internal/registry/registry.go
+++ b/internal/registry/registry.go
@@ -107,6 +107,27 @@ type Route struct {
// DynamicUser, StateDirectory) that the activator drives via systemctl.
Backing string
+ // Unit is the systemd unit that owns this route's process when it is
+ // neither an exec child nor a carlos-app@ tenant — an operator-run
+ // bespoke service (console.service is the case that forced the column:
+ // an addr-backed route, `-addr 127.0.0.1:8090`, whose process no
+ // backing-dispatched restart could reach). Empty for everything else,
+ // which is every route on every box today.
+ //
+ // The adopt pass's restart dispatch tries this unit FIRST when it is set
+ // (internal/host's makeRestartRoute): before the change, adoption
+ // repointed the symlink, stamped Version — and therefore the edge's
+ // X-Carlos-Version header — and cycled nothing, so the old process went
+ // on serving a build the header already claimed was live (issue #110).
+ //
+ // Written only by SetUnit / `carlos add -unit` / `carlos route -unit`,
+ // each of which validates the name (ValidUnitName); Upsert preserves a
+ // non-empty existing value — see Environment's comment for why.
+ //
+ // Deliberately NOT in Fingerprint(): unit changes who restarts the
+ // process, not what the edge serves.
+ Unit string
+
// Platform marks a route the domains reconcile pass minted
// (<app>.<sqid>.<apps-domain>). Only platform routes are ever
// auto-removed by reconciliation; custom domains are operator/owner
@@ -181,6 +202,32 @@ func ValidTunnelName(name string) bool {
return tunnelNameRe.MatchString(name)
}
+// unitNameRe: a systemd unit name that must END in .service — the only unit
+// type this column means anything for (a route's process). The character
+// class is systemd's own alphabet for the part before the suffix, including
+// the '@' of a template instance, so `console.service` and
+// `titogo-canary@demo.service` both pass and a typo does not.
+//
+// Lives here rather than in the CLI for the same reason ValidTunnelName does:
+// `unit` is this package's column, and this is the shape SetUnit enforces on
+// every write, not merely the shape one door happens to check.
+var unitNameRe = regexp.MustCompile(`^[A-Za-z0-9@_.\-]+\.service$`)
+
+// ValidUnitName reports whether name is usable as a route's Unit binding.
+// Unlike ValidTunnelName, "" is VALID here: it is the column's default and
+// its only "no unit" value, so every caller that can clear a binding
+// (`carlos route -unit -`, SetUnit(host, "")) passes it through this same
+// gate rather than around it.
+//
+// The rail is about a typo, not an injection: these strings reach systemctl
+// as one argv element of an exec.Command, so there is no shell to escape.
+// What there IS, without the rail, is `console.servcie` written silently to
+// the column and discovered only as a restart that quietly did nothing —
+// exactly the failure the column exists to end.
+func ValidUnitName(name string) bool {
+ return name == "" || unitNameRe.MatchString(name)
+}
+
func Open(path string) (*Registry, error) {
d, err := sql.Open("sqlite", path)
if err != nil {
@@ -257,6 +304,7 @@ CREATE TABLE IF NOT EXISTS routes (
`ALTER TABLE routes ADD COLUMN tunnel TEXT NOT NULL DEFAULT ''`,
`ALTER TABLE routes ADD COLUMN provisioned INTEGER NOT NULL DEFAULT 0`,
`ALTER TABLE routes ADD COLUMN seed TEXT NOT NULL DEFAULT ''`,
+ `ALTER TABLE routes ADD COLUMN unit TEXT NOT NULL DEFAULT ''`,
} {
if _, err := r.sql.Exec(stmt); err != nil && !strings.Contains(err.Error(), "duplicate column") {
return err
@@ -420,7 +468,7 @@ func (r *Registry) Accounts() ([]Account, error) {
}
// routeCols is the column list every route read and the one route write
-// share — 29 names, in this exact order. scanRoute's destinations, Upsert's
+// share — 32 names, in this exact order. scanRoute's destinations, Upsert's
// placeholder row and Upsert's argument list all count from here: adding a
// column means adding it in FOUR places, and a miscount is a silent column
// shift rather than an error (TestUpsertRoundTripsAllColumns is what catches
@@ -429,7 +477,7 @@ const routeCols = `host, slug, kind, socket, addr, status, last_error, db_path,
version, prev_version, app, pool, region, channel, hibernate, canary,
replication, next_due, backing, status_named, platform, dns_status,
dns_checked_at, cert_expires_at, account, environment, caps, tunnel,
-provisioned, seed, created_at`
+provisioned, seed, unit, created_at`
func scanRoute(scan func(...any) error) (Route, error) {
var t Route
@@ -438,7 +486,7 @@ func scanRoute(scan func(...any) error) (Route, error) {
&t.LastError, &t.DBPath, &t.Version, &t.PrevVersion, &t.App, &t.Pool,
&t.Region, &t.Channel, &hib, &can, &t.Replication, &t.NextDue, &t.Backing,
&sn, &plat, &t.DNSStatus, &t.DNSCheckedAt, &t.CertExpiresAt, &t.Account,
- &t.Environment, &t.Caps, &t.Tunnel, &prov, &t.Seed, &t.CreatedAt)
+ &t.Environment, &t.Caps, &t.Tunnel, &prov, &t.Seed, &t.Unit, &t.CreatedAt)
t.Hibernate = hib != 0
t.Canary = can != 0
t.StatusNamed = sn != 0
@@ -499,10 +547,10 @@ func (r *Registry) Delete(host string) error {
return err
}
-// Upsert inserts or updates a route by host. Five columns are deliberately
-// not plain overwrites — `environment`, `caps`, `tunnel`, `provisioned` and
-// `seed` all keep the existing row's value when the incoming Route carries
-// the zero one.
+// Upsert inserts or updates a route by host. Six columns are deliberately
+// not plain overwrites — `environment`, `caps`, `tunnel`, `provisioned`,
+// `seed` and `unit` all keep the existing row's value when the incoming
+// Route carries the zero one.
//
// The reason is the same for all five, and it is structural: a full-row
// Upsert caller (`carlos add`, the domains reconciler's two mint paths, the
@@ -520,11 +568,17 @@ func (r *Registry) Delete(host string) error {
// over that route FOREVER — deleting the bucket record could no longer
// retire it, and only a human noticing would.
//
+// `unit` is the same shape as `tunnel` with the blast radius of `caps`: a
+// re-`carlos add` on the console's row would drop the stamp that is the only
+// thing making adoption cycle that process, and nothing would report it —
+// the next promote would stamp X-Carlos-Version onto a console still serving
+// the old build, which is issue #110 returning silently.
+//
// An explicit non-zero value still wins in every case; clearing keeps
// exactly one door per column, none of which goes through Upsert:
// SetEnvironment (`carlos route -environment -`), SetCaps (`carlos route
-// -revoke`), SetTunnel (`carlos route -tunnel -`), SetSeed, and for
-// `provisioned` the row's own deletion.
+// -revoke`), SetTunnel (`carlos route -tunnel -`), SetUnit (`carlos route
+// -unit -`), SetSeed, and for `provisioned` the row's own deletion.
func (r *Registry) Upsert(t Route) error {
if t.CreatedAt == 0 {
t.CreatedAt = time.Now().Unix()
@@ -543,7 +597,7 @@ func (r *Registry) Upsert(t Route) error {
}
_, err := r.sql.Exec(`
INSERT INTO routes (`+routeCols+`)
-VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
+VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
ON CONFLICT(host) DO UPDATE SET
slug=excluded.slug, kind=excluded.kind, socket=excluded.socket,
addr=excluded.addr, status=excluded.status, last_error=excluded.last_error,
@@ -560,13 +614,14 @@ ON CONFLICT(host) DO UPDATE SET
caps=CASE WHEN excluded.caps = '' THEN routes.caps ELSE excluded.caps END,
tunnel=CASE WHEN excluded.tunnel = '' THEN routes.tunnel ELSE excluded.tunnel END,
provisioned=CASE WHEN excluded.provisioned = 0 THEN routes.provisioned ELSE excluded.provisioned END,
- seed=CASE WHEN excluded.seed = '' THEN routes.seed ELSE excluded.seed END`,
+ seed=CASE WHEN excluded.seed = '' THEN routes.seed ELSE excluded.seed END,
+ unit=CASE WHEN excluded.unit = '' THEN routes.unit ELSE excluded.unit END`,
t.Host, t.Slug, t.Kind, t.Socket, t.Addr, t.Status, t.LastError,
t.DBPath, t.Version, t.PrevVersion, t.App, t.Pool, t.Region, t.Channel,
b2i(t.Hibernate), b2i(t.Canary), t.Replication, t.NextDue, t.Backing,
b2i(t.StatusNamed), b2i(t.Platform), t.DNSStatus, t.DNSCheckedAt,
t.CertExpiresAt, t.Account, t.Environment, t.Caps, t.Tunnel,
- b2i(t.Provisioned), t.Seed, t.CreatedAt)
+ b2i(t.Provisioned), t.Seed, t.Unit, t.CreatedAt)
return err
}
@@ -728,6 +783,49 @@ func (r *Registry) SetBacking(host, backing string) error {
return nil
}
+// SetUnit stamps the systemd unit that owns a route's process — the door
+// issue #110 needed and did not have: a console is an addr-backed route under
+// an operator-run `console.service`, which the adopt pass's restart dispatch
+// could reach by neither of its two arms (Bounce owns no such process, and
+// there is no carlos-app@<host> unit to try-restart), so promote → adopt
+// stamped a new Version behind a process still running the old build.
+//
+// Narrow UPDATE with an error on an unmatched host, the SetBacking contract
+// and for the same reason: a human types the hostname into this one directly
+// (`carlos route -unit console.service` against a live registry is how the
+// production consoles get theirs), and a silent no-op on a typo would be
+// worse than a refusal.
+//
+// Deliberately NOT a compare-and-swap, again following SetBacking: unit is
+// not a set an operator merges into (SetCaps's case) and not a value a pass
+// rewrites hourly (SetDBPath's) — it is one operator naming the unit that
+// owns one process.
+//
+// It DOES validate, which SetBacking does not, and the difference is the
+// point: backing has two legal values and the doors compare against both, so
+// a bad value cannot survive the flag. A unit name is free text an operator
+// types once, on the row whose whole purpose is to be handed to systemctl
+// months later — a typo would be discovered as a restart that silently did
+// nothing, i.e. as the very bug this column closes. "" is accepted, because
+// clearing the binding is a legitimate write (see ValidUnitName).
+func (r *Registry) SetUnit(host, unit string) error {
+ if !ValidUnitName(unit) {
+ return fmt.Errorf("unit %q: want a systemd service unit name (e.g. console.service) or \"\" to clear", unit)
+ }
+ res, err := r.sql.Exec(`UPDATE routes SET unit = ? WHERE host = ?`, unit, host)
+ if err != nil {
+ return err
+ }
+ n, err := res.RowsAffected()
+ if err != nil {
+ return err
+ }
+ if n == 0 {
+ return fmt.Errorf("no route for host %q", host)
+ }
+ return nil
+}
+
// SetChannels repoints every host in hosts to channel as ONE transaction —
// every row moves or none do. `carlos route -app` uses this for its bulk
// form: the caller runs every route's safety-rail check to completion FIRST
@@ -991,8 +1089,10 @@ WHERE kind = 'instance' AND next_due > 0 AND next_due <= ? ORDER BY host`, now)
// two DIFFERENT values of the SAME length (channel canary/1 -> canary/2, a
// git short-sha version bump) change the string exactly like any other
// write. `grants` stays deliberately excluded (registry.go:93's rule: grants
-// change unit shape, not routing). Row count is folded into the result so an
-// empty table and a never-opened one still differ from a populated one.
+// change unit shape, not routing), and so does `unit` (it changes who
+// restarts the process, not what the edge serves). Row count is folded into
+// the result so an empty table and a never-opened one still differ from a
+// populated one.
// Still a fresh connection per poll — it must see other processes' commits
// (same WAL-snapshot quirk as List).
//
diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go
index 82ff11a3a..d027979ad 100644
--- a/internal/registry/registry_test.go
+++ b/internal/registry/registry_test.go
@@ -73,7 +73,8 @@ func TestUpsertRoundTripsAllColumns(t *testing.T) {
App: "kass", Pool: "shared", Region: "ie", Channel: "stable",
Environment: "demo", Caps: "host-role,port:25",
Hibernate: true, Canary: true, Replication: "batched",
- NextDue: 4200, Backing: BackingUnit, CreatedAt: 1700000000,
+ NextDue: 4200, Backing: BackingUnit, Unit: "console.service",
+ CreatedAt: 1700000000,
}
if err := reg.Upsert(want); err != nil {
t.Fatal(err)
@@ -1376,6 +1377,228 @@ func TestValidTunnelName(t *testing.T) {
}
}
+// A registry file written BEFORE the unit column existed must still open, its
+// rows must read as "no unit" (which is what every route on every box is
+// today), and the new column must be writable on that same migrated row. The
+// pre-change file is simulated the way TestPreCapsRegistryOpensAndReadsUngranted
+// simulates its own: drop the column from a fresh registry and write a row
+// through raw SQL, which is the shape an older binary left behind.
+func TestPreUnitRegistryOpensAndReadsNoUnit(t *testing.T) {
+ path := filepath.Join(t.TempDir(), "registry.db")
+ reg, err := Open(path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.Close(); err != nil {
+ t.Fatal(err)
+ }
+
+ old, err := sql.Open("sqlite", path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := old.Exec(`ALTER TABLE routes DROP COLUMN unit`); err != nil {
+ t.Fatalf("simulate a pre-unit registry: %v", err)
+ }
+ if _, err := old.Exec(`INSERT INTO routes (host, kind, addr, status, created_at)
+VALUES ('console.carloku.com', 'instance', '127.0.0.1:8090', 'ready', 1700000000)`); err != nil {
+ t.Fatal(err)
+ }
+ if err := old.Close(); err != nil {
+ t.Fatal(err)
+ }
+
+ reg, err = Open(path)
+ if err != nil {
+ t.Fatalf("a pre-unit registry must still open: %v", err)
+ }
+ defer reg.Close()
+ got, err := reg.Get("console.carloku.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got == nil {
+ t.Fatal("the pre-existing row vanished across the migration")
+ }
+ if got.Unit != "" {
+ t.Fatalf("Unit = %q on a row written before the column existed, want %q", got.Unit, "")
+ }
+ if got.Host != "console.carloku.com" || got.Kind != "instance" ||
+ got.Addr != "127.0.0.1:8090" || got.CreatedAt != 1700000000 {
+ t.Fatalf("the migrated row shifted columns: %+v", *got)
+ }
+ // The stamp the live consoles need, on exactly that migrated row.
+ if err := reg.SetUnit("console.carloku.com", "console.service"); err != nil {
+ t.Fatal(err)
+ }
+ got, err = reg.Get("console.carloku.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "console.service" {
+ t.Fatalf("Unit = %q after SetUnit on a migrated row", got.Unit)
+ }
+ // And clearing it again has a door, so a mis-stamp is reversible.
+ if err := reg.SetUnit("console.carloku.com", ""); err != nil {
+ t.Fatal(err)
+ }
+ got, err = reg.Get("console.carloku.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "" {
+ t.Fatalf("SetUnit clear failed: %q", got.Unit)
+ }
+}
+
+// SetUnit is narrow (it touches no other column) and refuses an unknown host
+// — the SetBacking contract, for the same reason: a human types the hostname
+// in directly.
+func TestSetUnitIsNarrowAndErrorsOnAnUnknownHost(t *testing.T) {
+ reg := openTestRegistry(t)
+ base := Route{
+ Host: "console.example.com", Kind: "instance", App: "console",
+ Addr: "127.0.0.1:8090", Status: "ready", Version: "abc1234", Channel: "canary/console",
+ }
+ if err := reg.Upsert(base); err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.SetUnit("console.example.com", "console.service"); err != nil {
+ t.Fatal(err)
+ }
+ got, err := reg.Get("console.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "console.service" {
+ t.Fatalf("Unit = %q, want console.service", got.Unit)
+ }
+ if got.Version != "abc1234" || got.Channel != "canary/console" || got.Addr != "127.0.0.1:8090" {
+ t.Fatalf("SetUnit touched another column: %+v", *got)
+ }
+ if err := reg.SetUnit("nope.example.com", "console.service"); err == nil {
+ t.Fatal("SetUnit on an unknown host: got nil error, want one")
+ }
+}
+
+// A typo'd unit must fail at WRITE time, loudly, rather than at restart time
+// silently — the whole reason the column exists is that a restart which
+// reaches nothing looks exactly like a restart that worked.
+func TestSetUnitRefusesAMalformedName(t *testing.T) {
+ reg := openTestRegistry(t)
+ if err := reg.Upsert(Route{Host: "console.example.com", Kind: "instance", Status: "ready"}); err != nil {
+ t.Fatal(err)
+ }
+ for _, bad := range []string{"console.servcie", "console", "console.timer", "console service", "console.service extra"} {
+ if err := reg.SetUnit("console.example.com", bad); err == nil {
+ t.Fatalf("SetUnit(%q) accepted; want a refusal", bad)
+ }
+ }
+ got, err := reg.Get("console.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "" {
+ t.Fatalf("a refused SetUnit still wrote %q", got.Unit)
+ }
+}
+
+func TestValidUnitName(t *testing.T) {
+ valid := []string{
+ "", // the column's default: no unit
+ "console.service", //
+ "keymail-relay.service", // hyphens
+ "titogo-canary@demo.service", // template instance
+ "carlos_app.service", // underscore
+ "a.b.c.service", // dots
+ }
+ invalid := []string{
+ "console", // no suffix
+ "console.servcie", // the typo this rail is for
+ "carlos-adopt.timer", // a unit, but not one that owns a process
+ "console.socket", //
+ "has space.service", // space
+ "../../etc/x.service", // path traversal
+ "console.service; reboot", // shell-shaped, and refused as a shape
+ ".service", // suffix alone
+ }
+ for _, n := range valid {
+ if !ValidUnitName(n) {
+ t.Errorf("ValidUnitName(%q) = false, want true", n)
+ }
+ }
+ for _, n := range invalid {
+ if ValidUnitName(n) {
+ t.Errorf("ValidUnitName(%q) = true, want false", n)
+ }
+ }
+}
+
+// Same rationale as TestUpsertPreservesTunnel, with the stakes of the caps
+// column: a full-row Upsert caller (`carlos add`, a reconciler's mint) knows
+// nothing about which unit owns a route's process, and dropping the stamp
+// would silently restore issue #110 on the one row it matters for.
+func TestUpsertPreservesUnit(t *testing.T) {
+ reg := openTestRegistry(t)
+ base := Route{Host: "console.example.com", Kind: "instance", App: "console", Status: "ready"}
+ if err := reg.Upsert(base); err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.SetUnit("console.example.com", "console.service"); err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.Upsert(base); err != nil {
+ t.Fatal(err)
+ }
+ got, err := reg.Get("console.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "console.service" {
+ t.Fatalf("Upsert clobbered Unit: %q", got.Unit)
+ }
+ // Preservation is a floor, not a freeze: an explicit value still wins.
+ base.Unit = "console-next.service"
+ if err := reg.Upsert(base); err != nil {
+ t.Fatal(err)
+ }
+ got, err = reg.Get("console.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "console-next.service" {
+ t.Fatalf("Unit = %q, want an explicit Upsert value to win", got.Unit)
+ }
+}
+
+// unit changes who RESTARTS the process, not what the edge serves — so the
+// edge must not reload for one. Same rule and same test shape as
+// TestFingerprintIgnoresAGrant, and this pins the exclusion so a later
+// "hash every column" tidy-up has to argue with a test.
+func TestFingerprintIgnoresAUnitStamp(t *testing.T) {
+ reg := openTestRegistry(t)
+ if err := reg.Upsert(Route{
+ Host: "console.example.com", Kind: "instance", App: "console",
+ Addr: "127.0.0.1:8090", Status: "ready",
+ }); err != nil {
+ t.Fatal(err)
+ }
+ before, err := reg.Fingerprint()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.SetUnit("console.example.com", "console.service"); err != nil {
+ t.Fatal(err)
+ }
+ after, err := reg.Fingerprint()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if after != before {
+ t.Fatalf("Fingerprint moved on a unit stamp (%s -> %s) — the edge would reload for a change that does not touch routing", before, after)
+ }
+}
+
func TestValidTunnelNameLengthBoundary(t *testing.T) {
ok32 := "01234567890123456789012345678901" // 32 chars
if len(ok32) != 32 {