amadan
instances set-environment/set-channel: say what a 404 really was (#264)
The record door answers no-record and not-your-app with the same 404, so both verbs told the owner of a box route with no instance record to check --account and --app. They now ask the app's route list, behind the same gate, and name the box route (with the box-side command), a platform canary (which nothing rebinds), or a host this console's box has no route for. --host is lowercased first. carlos route -h points remote users at the instances verbs. The routes door gains an additive platform_canary field so the CLI can tell canaries apart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Human
Prompt
Can you pick up carlos/platform#264
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
CLI-side diagnosis of the membership 404 on set-environment/set-channel via the routes door; carlos route -h pointer; additive platform_canary on the routes door. Copy approved by Paul (round 3).
Agent notes
Astra reviewed: 4 findings, all fixed (case, box-only route list, platform canaries, no root). The console's byte-identical 404 is deliberate and unchanged. Local root-package tests need TMPDIR outside $HOME or the config ancestor walk finds ~/.carlos/config.
6 files changed,
+207
−10
apimode.go+9 −1cli.go+14 −9instances_cli.go+54 −0instances_record_notfound_test.go+77 −0internal/console/apiroutes.go+7 −0internal/console/apiroutes_test.go+46 −0
CI passed — run details
diff --git a/apimode.go b/apimode.go| index c4ce9029d..2b72fd7d4 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -766,9 +766,14 @@ func resolvedAccount(sess *apiSession, raw, rawID *string) apiAcct { |
| // is a genuine server diagnosis, and apiErrorFromBody surfaces THAT text |
| // instead — never rewritten into this one. |
| func notFoundErr() error { |
| - return errors.New("not found: check --account and --app, and that your token's address is a member") |
| + return errMembershipNotFound |
| } |
| +// errMembershipNotFound is notFoundErr's one value, so a caller that can |
| +// narrow the riddle (recordNotFound, instances_cli.go) can recognise it with |
| +// errors.Is rather than by matching its text. |
| +var errMembershipNotFound = errors.New("not found: check --account and --app, and that your token's address is a member") |
| + |
| // hasJSONError reports whether a body carries the {"error": "..."} shape — |
| // the difference between a door's own diagnosis and the mux's byte-identical |
| // text/plain refusal. apiErrorFromBody asks it to decide which message to |
| @@ -3486,6 +3491,9 @@ type apiRouteRow struct { |
| Provisioned bool `json:"provisioned"` |
| Replication string `json:"replication"` |
| DBBytes *int64 `json:"db_bytes,omitempty"` // nil = unknown, printed as a dash |
| + // PlatformCanary is a route the domains reconciler owns whole; a console |
| + // that predates the field sends nothing and this reads false. |
| + PlatformCanary bool `json:"platform_canary,omitempty"` |
| } |
| // apiRoutePathRow is one path route of the same answer: a leading URL |
diff --git a/cli.go b/cli.go| index a4020850d..cf85415fd 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -587,7 +587,20 @@ func runRoute(args []string) { |
| "allow repointing to a channel that RESOLVES but holds a different version than the route currently serves "+ |
| "(never overrides an unresolvable target, a bad signature, or a missing pubkey)") |
| dryRun := fs.Bool("dry-run", false, "run the full safety-rail check and print the plan, but write nothing") |
| - fs.Usage = flagUsage(fs, "") |
| + const usage = "usage: carlos route --host <host> [--channel <channel>] [--environment <name>] [--tunnel <name>] [--registry <db>] [--allow-version-change] [--dry-run]\n" + |
| + " or: carlos route --app <app> [--channel <channel>] [--environment <name>] [--account <sqid|name|all>] [--account-id <sqid>] [--registry <db>] [--allow-version-change] [--dry-run]\n" + |
| + " (--account-id names ONE account by sqid and supersedes --account, \"all\" included; --tunnel is --host only)\n" + |
| + " or: carlos route --host <host> (--grant | --revoke) <port:n|host-role|doc-engine> [--registry <db>] [--dry-run]\n" + |
| + " or: carlos route --host <host> --hibernate[=false] [--registry <db>] [--dry-run]\n" + |
| + " or: carlos route --host <host> --backing <unit|\"\"> [--registry <db>] [--dry-run]\n" + |
| + " or: carlos route --host <host> --unit <name.service|-> [--registry <db>] [--dry-run]\n" + |
| + ` or: carlos route --host <host> --replication <stream|batched|daily|-> [--registry <db>] [--dry-run]` |
| + // -h names the remote verbs, because this one only ever finds the box |
| + // it runs on: someone holding just this verb could not tell that a |
| + // signed-in CLI changes a route's channel or environment from anywhere |
| + // (#264). |
| + fs.Usage = flagUsage(fs, usage+"\n\n"+ |
| + "This edits the routing table on the box it runs on, so it has to run there. To change a route's channel or environment from your own machine, sign in and use carlos instances set-channel or carlos instances set-environment.") |
| fs.Parse(args) |
| // --hibernate has to be TRI-state: a plain flag.Bool cannot tell "the |
| // operator did not mention hibernation" from "the operator asked for |
| @@ -621,14 +634,6 @@ func runRoute(args []string) { |
| } |
| }) |
| - const usage = "usage: carlos route --host <host> [--channel <channel>] [--environment <name>] [--tunnel <name>] [--registry <db>] [--allow-version-change] [--dry-run]\n" + |
| - " or: carlos route --app <app> [--channel <channel>] [--environment <name>] [--account <sqid|name|all>] [--account-id <sqid>] [--registry <db>] [--allow-version-change] [--dry-run]\n" + |
| - " (--account-id names ONE account by sqid and supersedes --account, \"all\" included; --tunnel is --host only)\n" + |
| - " or: carlos route --host <host> (--grant | --revoke) <port:n|host-role|doc-engine> [--registry <db>] [--dry-run]\n" + |
| - " or: carlos route --host <host> --hibernate[=false] [--registry <db>] [--dry-run]\n" + |
| - " or: carlos route --host <host> --backing <unit|\"\"> [--registry <db>] [--dry-run]\n" + |
| - " or: carlos route --host <host> --unit <name.service|-> [--registry <db>] [--dry-run]\n" + |
| - ` or: carlos route --host <host> --replication <stream|batched|daily|-> [--registry <db>] [--dry-run]` |
| if *host == "" && *app == "" { |
| log.Fatal(usage) |
| } |
diff --git a/instances_cli.go b/instances_cli.go| index 87a39de9c..706e6dcbc 100644 |
| --- a/instances_cli.go |
| +++ b/instances_cli.go |
| @@ -666,6 +666,9 @@ func runInstancesSetChannel(args []string) { |
| if *host == "" || *channel == "" { |
| log.Fatal(instancesRecordsUsage) |
| } |
| + // Records and routes are keyed lowercase, so a host typed with capitals |
| + // would miss both and read as a typo (#264). |
| + *host = strings.ToLower(strings.TrimSpace(*host)) |
| sess := requireAPISession("set-channel", *consoleFlag) |
| printUsing(stdout, sess) |
| acct := resolvedAccount(sess, acctRaw, acctIDRaw) |
| @@ -682,6 +685,10 @@ func runInstancesSetChannel(args []string) { |
| strings.TrimSpace(*host), strings.TrimSpace(*channel), prop.Needed, prop.ConsoleURL) |
| return |
| } |
| + ch := strings.TrimSpace(*channel) |
| + err = recordNotFound(context.Background(), sess, acct, app2, strings.TrimSpace(*host), err, func(h string) string { |
| + return fmt.Sprintf("%s is a box route with no instance record, so the console has nothing to move. until it is adopted as an instance, move it on its box: carlos route --host %s --channel %s", h, h, ch) |
| + }) |
| log.Fatalf("instances set-channel: %v", err) |
| } |
| fmt.Fprintf(stdout, "%s %s to %s\n", colorFor(stdout).ok("moved"), rec.Host, rec.Channel) |
| @@ -696,6 +703,42 @@ func runInstancesSetChannel(args []string) { |
| } |
| } |
| +// recordNotFound narrows the membership 404 a record write answers when its |
| +// host names no record (#264). The console answers "no such record" and "not |
| +// your app" with the same bytes on purpose (handleAPIInstanceUpdate), so the |
| +// write alone cannot tell them apart. The app's route list can: it sits |
| +// behind the same membership gate, so an answer from it proves the account, |
| +// app and membership right, and leaves the record as the thing missing. |
| +// |
| +// A host the list carries is a box route with no instance record, such as |
| +// one a tenant's own tooling writes into the box registry. Those are rebound |
| +// on the box until #261's adoption door gives them a |
| +// record, and boxRoute says how, except for a platform canary, which |
| +// `carlos route` refuses on both axes because the domains reconciler |
| +// rebuilds its row. A host the list does not carry is most likely a typo, |
| +// but the list is only the console's own box, so the message says no more |
| +// than that. When the list does not answer either, the account or app |
| +// really may be wrong, and the membership message stands. |
| +func recordNotFound(ctx context.Context, sess *apiSession, a apiAcct, app, host string, err error, boxRoute func(host string) string) error { |
| + if !errors.Is(err, errMembershipNotFound) { |
| + return err |
| + } |
| + rows, _, lerr := sess.routeList(ctx, a, app) |
| + if lerr != nil { |
| + return err |
| + } |
| + for _, r := range rows { |
| + if r.Host != host { |
| + continue |
| + } |
| + if r.PlatformCanary { |
| + return fmt.Errorf("%s is a canary route the platform mints for %s, and the platform sets its channel and environment. neither can be changed by hand", host, app) |
| + } |
| + return errors.New(boxRoute(host)) |
| + } |
| + return fmt.Errorf("%s has no instance named %s, and this console's box has no route by that name. your account and app are fine. carlos instances list --app %s shows its instances", app, host, app) |
| +} |
| + |
| // environmentSuffix renders a record's config binding for a one-line |
| // confirmation, and renders NOTHING for the default bundle. The empty case |
| // is the common one and naming it ("environment default") on every create |
| @@ -741,6 +784,8 @@ func runInstancesSetEnvironment(args []string) { |
| if *host == "" || !flagWasSet(fs, "environment") { |
| log.Fatal(instancesRecordsUsage) |
| } |
| + // Lowercased for set-channel's reason. |
| + *host = strings.ToLower(strings.TrimSpace(*host)) |
| env := strings.TrimSpace(*environment) |
| // Checked here as well as at the door, so a typo costs a refusal rather |
| // than a round trip. "default" is not a bundle name — it is the absence |
| @@ -761,6 +806,15 @@ func runInstancesSetEnvironment(args []string) { |
| strings.TrimSpace(*host), prop.Needed, prop.ConsoleURL) |
| return |
| } |
| + err = recordNotFound(context.Background(), sess, acct, app2, strings.TrimSpace(*host), err, func(h string) string { |
| + // The box verb spells "back to the default bundle" as "-", where |
| + // this one spells it as an empty value. |
| + boxEnv := env |
| + if boxEnv == "" { |
| + boxEnv = "-" |
| + } |
| + return fmt.Sprintf("%s is a box route with no instance record, so the console has nothing to rebind. until it is adopted as an instance, bind it on its box: carlos route --host %s --environment %s", h, h, boxEnv) |
| + }) |
| log.Fatalf("instances set-environment: %v", err) |
| } |
| if rec.Environment == "" { |
diff --git a/instances_record_notfound_test.go b/instances_record_notfound_test.go| new file mode 100644 |
| index 000000000..aec5a9f6a |
| --- /dev/null |
| +++ b/instances_record_notfound_test.go |
| @@ -0,0 +1,77 @@ |
| +package main |
| + |
| +import ( |
| + "context" |
| + "errors" |
| + "io" |
| + "net/http" |
| + "net/http/httptest" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// recordNotFound turns the record door's membership 404 into what actually |
| +// went wrong, using the app's route list as the witness (#264): a host the |
| +// list carries is a box route with no record, a host it does not carry is a |
| +// miss, and a list that will not answer either leaves the membership riddle |
| +// standing, since then the account or app really may be wrong. |
| +func TestRecordNotFoundNarrowsTheMembership404(t *testing.T) { |
| + routes := `{"routes":[{"host":"shop.example.com","kind":"instance","environment":"staging"},` + |
| + `{"host":"canary-ab12.shop.acme.oncarlos.test","kind":"instance","channel":"canary/ab12","platform_canary":true}]}` |
| + boxRoute := func(h string) string { return "BOX " + h } |
| + |
| + cases := []struct { |
| + name string |
| + host string |
| + routesOK bool |
| + in error |
| + want string // substring of the error; "" means the input error, unchanged |
| + }{ |
| + {"box route with no record", "shop.example.com", true, notFoundErr(), "BOX shop.example.com"}, |
| + {"platform canary is not sent to the box", "canary-ab12.shop.acme.oncarlos.test", true, notFoundErr(), "is a canary route the platform mints for shop"}, |
| + {"no such host", "shop.exmaple.com", true, notFoundErr(), "shop has no instance named shop.exmaple.com"}, |
| + {"route list refused too", "shop.example.com", false, notFoundErr(), ""}, |
| + {"another error passes through", "shop.example.com", true, errors.New("could not read the instance"), ""}, |
| + } |
| + for _, c := range cases { |
| + t.Run(c.name, func(t *testing.T) { |
| + listed := false |
| + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + if !strings.HasSuffix(r.URL.Path, "/routes") { |
| + t.Errorf("unexpected request %s", r.URL.Path) |
| + } |
| + listed = true |
| + if !c.routesOK { |
| + http.NotFound(w, r) |
| + return |
| + } |
| + io.WriteString(w, routes) |
| + })) |
| + defer ts.Close() |
| + |
| + err := recordNotFound(context.Background(), newTestSession(ts.URL), |
| + apiAcct{Seg: "acme"}, "shop", c.host, c.in, boxRoute) |
| + if c.want == "" { |
| + if err != c.in { |
| + t.Fatalf("err = %v, want the input error %v unchanged", err, c.in) |
| + } |
| + } else if err == nil || !strings.Contains(err.Error(), c.want) { |
| + t.Fatalf("err = %v, want it to contain %q", err, c.want) |
| + } |
| + if errors.Is(c.in, errMembershipNotFound) != listed { |
| + t.Fatalf("route list consulted = %v, want it only for the membership 404", listed) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// The membership message must survive as itself everywhere else: the |
| +// sentinel only changes how recordNotFound recognises it, not what it says. |
| +func TestNotFoundErrIsTheSentinel(t *testing.T) { |
| + if !errors.Is(apiErrorFromBody(http.StatusNotFound, []byte("404 page not found\n")), errMembershipNotFound) { |
| + t.Fatal("a text/plain 404 must come back as errMembershipNotFound") |
| + } |
| + if !strings.Contains(notFoundErr().Error(), "check --account and --app") { |
| + t.Fatalf("notFoundErr() = %q, want the membership message", notFoundErr()) |
| + } |
| +} |
diff --git a/internal/console/apiroutes.go b/internal/console/apiroutes.go| index 5dcd3f5ad..3c4905dc2 100644 |
| --- a/internal/console/apiroutes.go |
| +++ b/internal/console/apiroutes.go |
| @@ -71,6 +71,12 @@ type apiRoute struct { |
| // it — the same box-snapshot join, and the same staleness rule, as the |
| // instance list's db_bytes. Absent when unknown, never 0. |
| DBBytes *int64 `json:"db_bytes,omitempty"` |
| + // PlatformCanary marks a route the domains reconciler mints and rebuilds |
| + // whole (registry Platform && Canary), so neither its channel nor its |
| + // environment is anybody's to set, here or with `carlos route` on the |
| + // box. The CLI reads it to avoid recommending that command for one of |
| + // these after an instance write misses (#264). Additive, omitempty. |
| + PlatformCanary bool `json:"platform_canary,omitempty"` |
| } |
| // apiRoutePath is one path route: a leading URL segment on Host that is |
| @@ -157,6 +163,7 @@ func (s *Server) handleAPIRoutes(w http.ResponseWriter, r *http.Request) { |
| Health: health[rt.Host], Hibernate: rt.Hibernate, Unit: rt.Unit, |
| Backing: rt.Backing, Provisioned: rt.Provisioned, |
| Replication: rt.Replication, DBBytes: join.dbSize(rt.Host), |
| + PlatformCanary: rt.Platform && rt.Canary, |
| }) |
| } |
| // Sorted by host: registry order is not stable across passes, and a |
diff --git a/internal/console/apiroutes_test.go b/internal/console/apiroutes_test.go| index cdc036f45..4c8740738 100644 |
| --- a/internal/console/apiroutes_test.go |
| +++ b/internal/console/apiroutes_test.go |
| @@ -175,6 +175,52 @@ func TestAPIRoutesReportsTheReplicationRung(t *testing.T) { |
| // TestAPIRoutesListsOnlyThisAppsRoutes: the registry holds every tenant's |
| // rows, and the tenant boundary is the only thing standing between a |
| // caller and a neighbour's upstream addresses and unit names. |
| +// TestAPIRoutesMarksPlatformCanaries: a route the domains reconciler mints |
| +// and rebuilds (Platform && Canary) says so, and nothing else does, because |
| +// the CLI reads this to keep from recommending `carlos route` for a row that |
| +// verb refuses (#264). A Platform route that is not a canary stays |
| +// unmarked: `carlos route --host` may bind that one. |
| +func TestAPIRoutesMarksPlatformCanaries(t *testing.T) { |
| + fx, token := configFixture(t, "code-routes-canary") |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, "shop", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + reg, err := registry.Open(fx.regPath) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, rt := range []registry.Route{ |
| + {Host: "canary-ab12.shop.bab.oncarlos.test", Kind: "instance", Status: "ready", App: "shop", |
| + Account: "bab", Addr: "127.0.0.1:9005", Channel: "canary/ab12", Platform: true, Canary: true}, |
| + {Host: "shop.bab.oncarlos.test", Kind: "instance", Status: "ready", App: "shop", |
| + Account: "bab", Addr: "127.0.0.1:9006", Channel: "stable", Platform: true}, |
| + {Host: "shop.example.com", Kind: "instance", Status: "ready", App: "shop", |
| + Account: "bab", Addr: "127.0.0.1:9007", Channel: "stable"}, |
| + } { |
| + if err := reg.Upsert(rt); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + status, out := getJSON(t, routesURL(fx, "shop"), token) |
| + if status != http.StatusOK { |
| + t.Fatalf("GET routes = %d %v", status, out) |
| + } |
| + byHost := hostsOf(t, out) |
| + for host, want := range map[string]bool{ |
| + "canary-ab12.shop.bab.oncarlos.test": true, |
| + "shop.bab.oncarlos.test": false, |
| + "shop.example.com": false, |
| + } { |
| + row, ok := byHost[host] |
| + if !ok { |
| + t.Fatalf("routes = %v, want %s listed", out["routes"], host) |
| + } |
| + if got, _ := row["platform_canary"].(bool); got != want { |
| + t.Errorf("%s platform_canary = %v, want %v", host, row["platform_canary"], want) |
| + } |
| + } |
| +} |
| + |
| func TestAPIRoutesListsOnlyThisAppsRoutes(t *testing.T) { |
| fx, token := routesFixture(t, "code-routes-scope") |
| status, out := getJSON(t, routesURL(fx, "titogo"), token) |