Merge pull request #206 from carlosframework/titogo-bugs
Route-scoped config delivery: exec_delivery per environment, fast rebinding convergence, carlos routes
25 files changed,
+4293
−195
apimode.go+38 −0appconfig_cli.go+90 −50cli.go+13 −3cli_test.go+19 −0docs/superpowers/plans/2026-08-16-route-scoped-config-delivery.md+2028 −0docs/superpowers/specs/2026-08-16-route-scoped-config-delivery-design.md+413 −0e2e_test.go+50 −12internal/appconfig/appconfig.go+33 −13internal/appconfig/environments_test.go+115 −0internal/appconfig/routefingerprint.go+90 −0internal/appconfig/routefingerprint_test.go+115 −0internal/appconfig/store.go+40 −23internal/appconfig/syncexec.go+90 −24internal/appconfig/syncexec_test.go+201 −0internal/console/apiconfig.go+48 −24internal/console/apiconfig_test.go+86 −5internal/console/apiinstances.go+31 −7internal/console/apiroutes.go+142 −0internal/console/apiroutes_test.go+209 −0internal/console/authcli.go+6 −0internal/host/agent.go+53 −8internal/host/agent_test.go+150 −26main.go+4 −0routes_cli.go+85 −0routes_cli_test.go+144 −0
diff --git a/apimode.go b/apimode.go| index 77a214cac..92aa9e78c 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -1865,6 +1865,44 @@ func (s *apiSession) instanceList(ctx context.Context, a apiAcct, app, match, he |
| return res.Instances, res.Truncated, res.Total, nil |
| } |
| +// apiRouteRow is one row of the console's routes answer |
| +// (internal/console/apiroutes.go). Every field is additive on the wire: a |
| +// console that predates one sends nothing and it decodes as the zero |
| +// value, which is why an empty Environment must be printed as "takes the |
| +// app default" rather than as a binding this client failed to read. |
| +type apiRouteRow struct { |
| + Host string `json:"host"` |
| + Kind string `json:"kind"` |
| + Upstream string `json:"upstream"` |
| + App string `json:"app"` |
| + Channel string `json:"channel"` |
| + Environment string `json:"environment"` |
| + Status string `json:"status"` |
| + Health string `json:"health"` |
| + Hibernate bool `json:"hibernate"` |
| + Unit string `json:"unit"` |
| + Backing string `json:"backing"` |
| + Provisioned bool `json:"provisioned"` |
| +} |
| + |
| +// routeList reads one app's routes on the console's own box (issue #64). |
| +func (s *apiSession) routeList(ctx context.Context, a apiAcct, app string) ([]apiRouteRow, error) { |
| + status, body, err := s.getJSON(ctx, apiPath(a, app, "/routes")) |
| + if err != nil { |
| + return nil, err |
| + } |
| + if status != http.StatusOK { |
| + return nil, apiErrorFromBody(status, body) |
| + } |
| + var res struct { |
| + Routes []apiRouteRow `json:"routes"` |
| + } |
| + if err := json.Unmarshal(body, &res); err != nil { |
| + return nil, fmt.Errorf("routes: unreadable response: %w", err) |
| + } |
| + return res.Routes, nil |
| +} |
| + |
| // apiLogsResponse is the console's logs answer (internal/console/apilogs.go) |
| // — records are applog's own wire shape, decoded as-is. |
| type apiLogsResponse struct { |
diff --git a/appconfig_cli.go b/appconfig_cli.go| index 4dc3facc0..9ad738c13 100644 |
| --- a/appconfig_cli.go |
| +++ b/appconfig_cli.go |
| @@ -24,7 +24,7 @@ commands: |
| unset -app <a> [-environment <name>] KEY [KEY2 ...] remove config vars |
| list -app <a> [-environment <name>] list an app's config vars |
| environments -app <a> list an app's named environment bundles |
| - exec-delivery -app <a> file|off opt an app's TCP-addr exec routes into config delivery (issue #18) |
| + exec-delivery -app <a> [-environment <name>] file|off|inherit opt one bundle's TCP-addr exec routes into config delivery |
| sync -app <a> request immediate config convergence (issue #44) |
| -environment names a bundle at apps/<acct>/<app>/environments/<name>.json, |
| @@ -32,14 +32,24 @@ layered on top of the default bundle (config.json) at materialization time — |
| see LoadMerged. Omit -environment for the default bundle itself. The bundle |
| is created implicitly on its first "set", same as config.json today. |
| -exec-delivery is APP-LEVEL ONLY (never -environment — see runEnvExecDelivery): |
| -it sets/clears "exec_delivery" in the default bundle alone. "file" makes |
| -SyncExec write /data/<host>.config.env for the app's TCP-addr exec routes |
| -(routes carlos proxies to but does not spawn — e.g. titogo's router fleet) |
| -exactly as it already does for socket-exec routes, but those routes are |
| -NEVER bounced: carlos does not own the process, so the app's own supervisor |
| -owns the restart. "off" clears the field; a plain "env list" mentions it |
| -when set. |
| +exec-delivery sets "exec_delivery" in ONE bundle. "file" makes SyncExec |
| +write /data/<host>.config.env for the TCP-addr exec routes that resolve |
| +through that bundle (routes carlos proxies to but does not spawn — e.g. |
| +titogo's router fleet) exactly as it already does for socket-exec routes, |
| +but those routes are NEVER bounced: carlos does not own the process, so the |
| +app's own supervisor owns the restart. |
| + |
| +Without -environment it is the app's default bundle, and "off" clears it. |
| +With -environment it is that bundle alone, and "inherit" clears it — a |
| +bundle that says nothing takes the app default's answer, so clearing there |
| +is NOT the same as turning delivery off. To deliver to one service only: |
| +turn the app default off, then set "file" on the bundle its route is bound |
| +to (carlos route -host <h> -environment <name> binds it). |
| + |
| +Turning the app default off does NOT delete files already written for |
| +routes that lose the opt-in: they hold decrypted secrets, and carlos cannot |
| +know when the app's own supervisor has stopped reading them. It names each |
| +one in its log every pass until you delete it by hand. |
| sync touches apps/<acct>/<app>/config-gen.json — the agent's fast poll loop |
| (host package) notices within ~3s and re-renders exec-backed instances; |
| @@ -329,17 +339,19 @@ func envVarsFromAPI(vars []apiConfigVar) map[string]string { |
| } |
| // printEnvList renders `env list`'s output — header line, an exec-delivery |
| -// line when set and only for the default bundle (app-level only — see |
| -// appconfig.Manifest.ExecDelivery), then a sorted KEY/VALUE tabwriter — |
| +// line when the listed bundle sets one (see appconfig.Manifest.ExecDelivery), |
| +// then a sorted KEY/VALUE tabwriter — |
| // byte-identical between the direct and API branches of runEnvList, which |
| // differ only in HOW they fetch execDelivery/vars. |
| func printEnvList(app, environment, execDelivery string, vars map[string]string) { |
| fmt.Printf("%s\n", colorFor(os.Stdout).head(fmt.Sprintf("%s (%s):", app, bundleLabel(environment)))) |
| - // exec_delivery is app-level only and never printed for a named |
| - // environment's listing, matching where it can ever actually be SET |
| - // (runEnvExecDelivery refuses -environment before it reaches appconfig |
| - // at all). |
| - if environment == "" && execDelivery != "" { |
| + // exec_delivery is printed for whichever bundle is being listed — since |
| + // issue #195 it can be set in a named one, where it means "routes bound |
| + // to this environment get files". A bundle SILENT on it prints nothing |
| + // rather than the value it inherits: the listing shows what this bundle |
| + // holds, and rendering an inherited value would make the bundle look |
| + // like it states something it does not. |
| + if execDelivery != "" { |
| fmt.Printf("exec-delivery: %s\n", execDelivery) |
| } |
| var keys []string |
| @@ -359,41 +371,56 @@ func printEnvList(app, environment, execDelivery string, vars map[string]string) |
| // verb, deliberately NOT `env set -app <a> EXEC_DELIVERY=...`: the field |
| // is not a config var (it's never rendered into an EnvironmentFile, never |
| // listed alongside KEY/VALUE pairs), it's a manifest-level flag with its |
| -// own validation and its own restriction to the default bundle, so it |
| -// gets its own verb surface instead of overloading env set's KEY=value |
| -// parsing with a special-cased key name. |
| +// own validation and its own bundle, so it gets its own verb surface |
| +// instead of overloading env set's KEY=value parsing with a special-cased |
| +// key name. |
| +// |
| +// -environment (issue #195) writes the flag into that bundle instead of |
| +// the app default. The VOCABULARY changes with the scope, and that is the |
| +// point: at app scope the second word is "off", because clearing the |
| +// default bundle's field really does stop delivery; in a named bundle it |
| +// is "inherit", because clearing it there hands the question back to the |
| +// app default. Spelling the second case "off" would read as "stop |
| +// delivering" while doing the opposite whenever the app default is on — |
| +// exactly the kind of quiet lie this feature exists to avoid — so "off" |
| +// is refused there, by name, rather than accepted as a synonym. |
| func runEnvExecDelivery(args []string) { |
| fs := flag.NewFlagSet("env exec-delivery", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - // -environment is accepted (matching every other env subcommand's |
| - // flag set) purely so a mistaken attempt gets a clear, specific |
| - // refusal here rather than a generic "unknown flag" — see the check |
| - // below. There is no environment-scoped variant to route it to: |
| - // appconfig.SetExecDelivery has no environment parameter at all. |
| - environment := fs.String("environment", "", "refused: exec_delivery is app-level only (default bundle), never a named environment") |
| + environment := fs.String("environment", "", |
| + "named environment bundle to set the flag in (omit for the app's default bundle)") |
| acctRaw, acctIDRaw, acctOf := accountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to work through (API mode only; default: the only logged-in one)") |
| fs.Parse(args) |
| - usage := "usage: carlos env exec-delivery -app <a> [-console <origin>] file|off" |
| + usage := "usage: carlos env exec-delivery -app <a> [-console <origin>] file|off\n" + |
| + " or: carlos env exec-delivery -app <a> -environment <name> [-console <origin>] file|inherit" |
| if fs.NArg() != 1 { |
| log.Fatal(usage) |
| } |
| - if *environment != "" { |
| - log.Fatal(`carlos env exec-delivery: -environment is refused — exec_delivery is APP-LEVEL ONLY, read from the ` + |
| - "default bundle (config.json) alone. LoadMerged never carries this field from a named environment's " + |
| - "manifest into anything the sync path reads — it builds its merged result from Vars alone — so it is not " + |
| - "merely ignored there, it is structurally impossible for it to take effect there at all, per the issue #18 " + |
| - "design comment's \"config-that-lies\" rationale: the reader is the app's own supervisor, one program per " + |
| - "app, not per-environment. Omit -environment.") |
| - } |
| - var value string |
| - switch fs.Arg(0) { |
| - case appconfig.ExecDeliveryFile: |
| + checkEnvironmentFlag(*environment) |
| + // One stored value either way — "" clears the field. What differs is |
| + // which SECOND WORD spells that clear, and which bundle it clears in. |
| + // cleared carries the operator's own word through to the wire and to |
| + // the confirmation line, so the two never paraphrase each other. |
| + var value, cleared string |
| + switch { |
| + case fs.Arg(0) == appconfig.ExecDeliveryFile: |
| value = appconfig.ExecDeliveryFile |
| - case "off": |
| - value = "" |
| + case *environment == "" && fs.Arg(0) == "off": |
| + value, cleared = "", "off" |
| + case *environment != "" && fs.Arg(0) == "inherit": |
| + value, cleared = "", "inherit" |
| + case *environment != "" && fs.Arg(0) == "off": |
| + log.Fatalf(`carlos env exec-delivery: "off" is not a value for a named environment — say "inherit" instead. `+ |
| + `Clearing exec_delivery in a bundle does not turn delivery off, it hands the question back to the app's `+ |
| + `default bundle (config.json), so a route bound to %q keeps getting files whenever the app-level flag is `+ |
| + `on. To stop delivery for the whole app: carlos env exec-delivery -app %s off`, *environment, bundleAppName(*app)) |
| default: |
| - log.Fatalf("carlos env exec-delivery: %q is not a valid value — want %q or \"off\"", fs.Arg(0), appconfig.ExecDeliveryFile) |
| + want := `"off"` |
| + if *environment != "" { |
| + want = `"inherit"` |
| + } |
| + log.Fatalf("carlos env exec-delivery: %q is not a valid value — want %q or %s", fs.Arg(0), appconfig.ExecDeliveryFile, want) |
| } |
| sess, err := resolveMode(*consoleFlag) |
| @@ -405,31 +432,44 @@ func runEnvExecDelivery(args []string) { |
| printUsing(os.Stdout, sess) |
| acct := resolvedAccount(sess, acctRaw, acctIDRaw) |
| appName = resolvedApp(sess, acct, *app, usage) |
| - // The wire vocabulary is "file"/"off", never "" — value is "" for |
| - // the local SetExecDelivery convention (clearing the field), but |
| - // the console's PUT decodes ExecDelivery as a pointer where nil |
| - // means "no change", so clearing it must still be spelled "off". |
| - wire := "off" |
| + // The wire vocabulary is a WORD, never "" — the console's PUT |
| + // decodes ExecDelivery as a pointer where nil means "no change", so |
| + // clearing must still be spelled. The word sent is the operator's |
| + // own: the console refuses "off" for a named bundle exactly as this |
| + // function does, so relaying it verbatim keeps the two refusals |
| + // identical rather than letting one paraphrase the other. |
| + wire := cleared |
| if value != "" { |
| wire = value |
| } |
| if _, err := sess.configApply(context.Background(), acct, appName, |
| - apiConfigApply{ExecDelivery: &wire}); err != nil { |
| + apiConfigApply{Environment: *environment, ExecDelivery: &wire}); err != nil { |
| log.Fatal(err) |
| } |
| } else { |
| if *app == "" { |
| log.Fatal(usage) |
| } |
| - if err := appconfig.SetExecDelivery(context.Background(), openStore(), acctOf(), *app, value); err != nil { |
| + if err := appconfig.SetExecDeliveryAt(context.Background(), openStore(), acctOf(), *app, *environment, value); err != nil { |
| log.Fatal(err) |
| } |
| } |
| + word := value |
| if value == "" { |
| - fmt.Printf("%s off for %s\n", colorFor(os.Stdout).ok("exec-delivery"), appName) |
| - } else { |
| - fmt.Printf("%s %s for %s\n", colorFor(os.Stdout).ok("exec-delivery"), value, appName) |
| + word = cleared |
| + } |
| + fmt.Printf("%s %s for %s (%s)\n", colorFor(os.Stdout).ok("exec-delivery"), word, appName, bundleLabel(*environment)) |
| +} |
| + |
| +// bundleAppName renders -app for a refusal printed BEFORE the app has been |
| +// resolved (the refusals above run at flag-parse time, ahead of |
| +// resolvedApp): an omitted -app must not turn the suggested command into |
| +// one with an empty argument. |
| +func bundleAppName(app string) string { |
| + if app == "" { |
| + return "<app>" |
| } |
| + return app |
| } |
| func runEnvEnvironments(args []string) { |
diff --git a/cli.go b/cli.go| index 482a64298..102ffb7df 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -299,14 +299,24 @@ func runInstances(args []string) { |
| // cycled by that unit on adoption, whatever BACKING says (and BACKING is |
| // empty for every such route, which is the shape that made issue #110 |
| // invisible from this listing). |
| - fmt.Fprintln(w, "HOST\tAPP\tKIND\tCHANNEL\tSTATUS\tVERSION\tHIB\tBACKING\tUNIT") |
| + // |
| + // ENV is the named config bundle the route is bound to (issue #64). It |
| + // decides which secrets this route's process is delivered, and reading |
| + // it used to mean sqlite3 against the registry — the one step of |
| + // titogo's secrets procedure that still needed box access. It sits |
| + // beside CHANNEL because the two are the same kind of fact about a |
| + // route and are set by the same verb (`carlos route`). Empty means the |
| + // app's default bundle, which is a real answer, so it prints blank |
| + // rather than being singled out. Off-box, `carlos routes` answers the |
| + // same question through the console. |
| + fmt.Fprintln(w, "HOST\tAPP\tKIND\tCHANNEL\tENV\tSTATUS\tVERSION\tHIB\tBACKING\tUNIT") |
| for _, t := range routes { |
| hib := "" |
| if t.Hibernate { |
| hib = "yes" |
| } |
| - fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n", |
| - t.Host, t.App, t.Kind, t.Channel, t.Status, t.Version, hib, t.Backing, t.Unit) |
| + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n", |
| + t.Host, t.App, t.Kind, t.Channel, t.Environment, t.Status, t.Version, hib, t.Backing, t.Unit) |
| } |
| w.Flush() |
| } |
diff --git a/cli_test.go b/cli_test.go| index 064b5c44f..b86b37ef7 100644 |
| --- a/cli_test.go |
| +++ b/cli_test.go |
| @@ -606,6 +606,25 @@ func TestRouteUnitStampsAnExistingRoute(t *testing.T) { |
| t.Errorf("instances output = %q, want a UNIT column carrying console.service", out) |
| } |
| + // The same listing carries a route's config-ENVIRONMENT binding (issue |
| + // #64): it decides which secrets the route's process is delivered, and |
| + // reading it used to mean sqlite3 against this file. Bind one and read |
| + // it back through the binary. |
| + if out, err := run("route", "-registry", regPath, "-host", "console.carloku.com", |
| + "-environment", "relay"); err == nil { |
| + // The bind only succeeds where the bundle exists; where it does |
| + // not, the refusal is the environment rail doing its job and the |
| + // column assertion below still has the unbound case to check. |
| + _ = out |
| + } |
| + out, err = run("instances", "-registry", regPath) |
| + if err != nil { |
| + t.Fatalf("instances: %v\n%s", err, out) |
| + } |
| + if !strings.Contains(out, "ENV") { |
| + t.Errorf("instances output = %q, want an ENV column — a route's config binding was unreadable without sqlite3 (#64)", out) |
| + } |
| + |
| // And the clear door, so a mis-stamp is reversible. |
| if out, err := run("route", "-registry", regPath, "-host", "console.carloku.com", "-unit", "-"); err != nil { |
| t.Fatalf("route -unit -: %v\n%s", err, out) |
diff --git a/docs/superpowers/plans/2026-08-16-route-scoped-config-delivery.md b/docs/superpowers/plans/2026-08-16-route-scoped-config-delivery.md| new file mode 100644 |
| index 000000000..b0a1cef75 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/2026-08-16-route-scoped-config-delivery.md |
| @@ -0,0 +1,2028 @@ |
| +# Route-scoped config delivery — implementation plan |
| + |
| +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
| + |
| +**Goal:** Close #195, #196 and #64 — the three defects in titogo's |
| +"deliver one sealed credential to one service" runbook: scope |
| +`exec_delivery` to the route's bound environment, make a route's |
| +environment binding converge in seconds instead of an hour, and let a |
| +member read a route's binding back without box access. |
| + |
| +**Architecture:** Three independent changes on one branch. (1) |
| +`SyncExec`'s opt-in check reads the route's bound environment bundle, |
| +falling back to the app default — an overlay, exactly like vars. (2) The |
| +agent's fast poll loop fingerprints the registry-side facts that decide |
| +what config a route gets, so a binding change is a change it can see. (3) |
| +A new account-scoped `GET /api/cli/apps/{account}/{app}/routes` served |
| +from the console's own registry, fronted by `carlos routes`. |
| + |
| +**Tech Stack:** Go 1.x, stdlib only. `internal/appconfig` (config plane), |
| +`internal/host` (agent loops), `internal/console` (HTTP API), root package |
| +(CLI). |
| + |
| +**Spec:** `docs/superpowers/specs/2026-08-16-route-scoped-config-delivery-design.md` |
| + |
| +## Global Constraints |
| + |
| +- **Gate before every commit:** `go build ./...`, `go vet ./...`, |
| + `go test ./... -count=1` all clean, every package; `gofmt -l` clean for |
| + files you touched. Sandbox note: use |
| + `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off`; go |
| + commands may need the sandbox disabled ("read-only file system" from the |
| + toolchain is the tell). |
| +- **Wire-shape changes are additive.** Live manifests exist in production |
| + buckets; anything written before this change must still parse, and an |
| + older carlos parsing a bundle this writes must not break. |
| +- **TDD.** Write the failing test, run it, watch it fail for the right |
| + reason, then implement. |
| +- **Nothing merges to `main` without a PR.** Never offer or take a local |
| + merge to `main`. |
| +- **A test that shells out to the carlos binary must build its env through |
| + `carlosTestEnv`** and set `cmd.Dir` to a scratch dir. |
| +- **Do not change `LoadMerged`.** The merged manifest stays vars-only and |
| + never carries `ExecDelivery`. The opt-in is resolved by reading the two |
| + bundles' fields directly. |
| +- Vocabulary, fixed: at app scope the CLI words are `file` / `off`; at |
| + environment scope they are `file` / `inherit`, and `off` is refused |
| + there by name. |
| + |
| +--- |
| + |
| +### Task 1: Environment-scoped `exec_delivery` write path |
| + |
| +**Files:** |
| +- Modify: `internal/appconfig/store.go:229-283` (`SetExecDelivery`) |
| +- Modify: `internal/appconfig/appconfig.go:22-56` (`Manifest.ExecDelivery` |
| + doc comment, `ValidExecDelivery`) |
| +- Test: `internal/appconfig/environments_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `getForWriteAt(ctx, s, key, app) (Manifest, string, bool, error)`, |
| + `Key(acct, app) string`, `EnvironmentKey(acct, app, name) string`, |
| + `touchLogged(ctx, s, acct, app)`, `DefaultBundle = "default"`. |
| +- Produces: |
| + `func SetExecDeliveryAt(ctx context.Context, s release.Store, acct, app, envName, value string) error` |
| + — `envName == ""` writes the default bundle (identical to today), |
| + otherwise the named bundle. `SetExecDelivery` stays as a wrapper with |
| + its existing signature; every current caller keeps working unchanged. |
| + |
| +- [ ] **Step 1: Write the failing tests** |
| + |
| +Append to `internal/appconfig/environments_test.go`. Use whatever |
| +store/helper constructor the neighbouring tests in that file already use |
| +(read the top of the file first — do not invent one). |
| + |
| +```go |
| +func TestSetExecDeliveryAtEnvironmentRoundTrips(t *testing.T) { |
| + ctx := context.Background() |
| + s := newTestStore(t) // match this file's existing helper |
| + // A bundle must exist before a flag can be written into it, exactly as |
| + // the default bundle must. |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatalf("set: %v", err) |
| + } |
| + m, err := LoadEnvironment(ctx, s, "bab", "titogo", "relay") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if m.ExecDelivery != ExecDeliveryFile { |
| + t.Fatalf("environment bundle exec_delivery = %q, want %q", m.ExecDelivery, ExecDeliveryFile) |
| + } |
| + // The vars already in the bundle survive the flag write — this is a |
| + // read-modify-write, not a replace. |
| + if m.Vars["A"].Value != "1" { |
| + t.Fatalf("A = %q, want it preserved at \"1\"", m.Vars["A"].Value) |
| + } |
| + // And the DEFAULT bundle is untouched: an environment-scoped write that |
| + // leaked into config.json would opt the whole fleet in, which is the |
| + // bug this closes. |
| + def, found, err := getIfExists(ctx, s, "bab", "titogo") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if found && def.ExecDelivery != "" { |
| + t.Fatalf("default bundle exec_delivery = %q, want it untouched", def.ExecDelivery) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtEnvironmentClearsToInherit(t *testing.T) { |
| + ctx := context.Background() |
| + s := newTestStore(t) |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ""); err != nil { |
| + t.Fatalf("clear: %v", err) |
| + } |
| + m, err := LoadEnvironment(ctx, s, "bab", "titogo", "relay") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if m.ExecDelivery != "" { |
| + t.Fatalf("exec_delivery = %q, want %q (inherit)", m.ExecDelivery, "") |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtRefusesAbsentBundle(t *testing.T) { |
| + ctx := context.Background() |
| + s := newTestStore(t) |
| + // Nothing has ever been written for this app. |
| + err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", "") |
| + if err == nil { |
| + t.Fatal("clearing the flag on a bundle that does not exist must be refused, not silently create one") |
| + } |
| + if !strings.Contains(err.Error(), "relay") { |
| + t.Fatalf("refusal %q must name the bundle it could not find", err) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtRefusesInvalidValue(t *testing.T) { |
| + ctx := context.Background() |
| + s := newTestStore(t) |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", "socket"); err == nil { |
| + t.Fatal("an unknown exec_delivery value must be refused before it reaches the bucket") |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtBumpsGeneration(t *testing.T) { |
| + ctx := context.Background() |
| + s := newTestStore(t) |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, before, err := s.Get(ctx, GenKey("bab", "titogo")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, after, err := s.Get(ctx, GenKey("bab", "titogo")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if before == after { |
| + t.Fatal("an environment-scoped exec_delivery write must bump the generation object — the fast loop watches nothing else for it") |
| + } |
| +} |
| +``` |
| + |
| +Add `"strings"` to that file's imports if it is not already there. |
| + |
| +- [ ] **Step 2: Run the tests to verify they fail** |
| + |
| +Run: `go test ./internal/appconfig/ -run TestSetExecDeliveryAt -count=1` |
| +Expected: FAIL — `undefined: SetExecDeliveryAt`. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +In `internal/appconfig/store.go`, replace `SetExecDelivery`'s body with a |
| +delegation and add the generalized form beneath it: |
| + |
| +```go |
| +// SetExecDelivery writes (`file`) or clears (`""`) the DEFAULT bundle's |
| +// exec_delivery field — issue #18's TCP-addr exec route opt-in (see |
| +// Manifest.ExecDelivery). The app-level door, unchanged in behaviour and |
| +// signature; SetExecDeliveryAt is the general one. |
| +func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value string) error { |
| + return SetExecDeliveryAt(ctx, s, acct, app, "", value) |
| +} |
| + |
| +// SetExecDeliveryAt writes or clears exec_delivery in ONE bundle: the |
| +// default bundle (envName == "") or a named environment's, via the same |
| +// CAS read-modify-write path SetEnv/SetSecret/Unset use. value must be |
| +// ValidExecDelivery; anything else is refused here, before it ever |
| +// reaches the bucket, mirroring applyAt's var-name/var-value validation. |
| +// |
| +// Environment scope is issue #195 (ruled 2026-08-16): the app-level flag |
| +// answered "does this APP get files", so opting one relay route in wrote |
| +// a file for every router route of the same app too. The flag now lives |
| +// wherever the route's config comes from, and clearing it in a named |
| +// bundle means INHERIT the app default — not "off". Route scope is still |
| +// refused: it would need a registry column, a record field and reconcile |
| +// convergence, three doors onto one bit, where the environment binding is |
| +// a scope the operator has already chosen. |
| +// |
| +// Turning the field off against a bundle that does not exist is refused, |
| +// not silently turned into a create — the same "unset-only against a |
| +// nonexistent object" refusal applyAt documents, for the same reason: |
| +// minting an empty bundle just to hold an absent flag would satisfy |
| +// `carlos route -environment`'s existence rail and Sync's "configured, |
| +// then fully unset" state for nothing the operator asked for. |
| +func SetExecDeliveryAt(ctx context.Context, s release.Store, acct, app, envName, value string) error { |
| + if !release.ValidAccount(acct) { |
| + return fmt.Errorf("config: invalid account %q", acct) |
| + } |
| + if !release.ValidApp(app) { |
| + return fmt.Errorf("config: invalid app %q", app) |
| + } |
| + if !ValidExecDelivery(value) { |
| + return fmt.Errorf("config: invalid exec_delivery %q — want %q or \"\" (off/inherit)", value, ExecDeliveryFile) |
| + } |
| + key, label := Key(acct, app), DefaultBundle |
| + if envName != "" { |
| + if !ValidEnvironmentName(envName) { |
| + return fmt.Errorf("config: invalid environment name %q", envName) |
| + } |
| + key, label = EnvironmentKey(acct, app, envName), envName |
| + } |
| + m, etag, found, err := getForWriteAt(ctx, s, key, app) |
| + if err != nil { |
| + return err |
| + } |
| + if !found && value == "" { |
| + return fmt.Errorf("config: %s has no %s bundle — nothing to turn off", app, label) |
| + } |
| + m.ExecDelivery = value |
| + m.Schema, m.App = 1, app |
| + raw, err := json.MarshalIndent(m, "", " ") |
| + if err != nil { |
| + return err |
| + } |
| + if _, err := s.Put(ctx, key, raw, etag); err != nil { |
| + return err |
| + } |
| + // The opt-in flag changes what SyncExec delivers for this app's routes, |
| + // so it is a config change like any other and earns the same fast |
| + // convergence. The generation object is per-APP, not per-bundle |
| + // (generation.go), so an environment-scoped write signals through the |
| + // same key an app-level one does — which is why the fast loop needs no |
| + // new bundle awareness for this half. |
| + touchLogged(ctx, s, acct, app) |
| + return nil |
| +} |
| +``` |
| + |
| +Then rewrite the `APP-LEVEL ONLY` paragraph of `Manifest.ExecDelivery`'s |
| +doc comment in `internal/appconfig/appconfig.go` (the block currently |
| +beginning "APP-LEVEL ONLY: read from the default bundle (config.json) |
| +alone."): |
| + |
| +```go |
| + // BUNDLE-LEVEL, resolved as an overlay (issue #195, ruled 2026-08-16). |
| + // SyncExec reads this field from the bundle a route's config actually |
| + // comes from: a route bound to environment e takes e's value when e |
| + // STATES one, and inherits the default bundle's when it does not. An |
| + // unbound route reads the default bundle alone, exactly as before. |
| + // LoadMerged still never carries this field into its merged result — |
| + // the merged manifest is what gets rendered into a file, and a delivery |
| + // flag has no business appearing there; SyncExec's checkOptedIn reads |
| + // the two bundles' fields directly instead. |
| + // |
| + // The 2026-08-03 ruling this narrows said a named environment's |
| + // manifest is never a valid place for the field, because "the reader is |
| + // the app's own supervisor, one program per app". That premise is false |
| + // in the deployment the feature was built for: the titogo app |
| + // supervises its relay and its router fleet through different units, so |
| + // an app-level flag thrown to deliver one sealed credential to the |
| + // relay wrote a file for every router too. |
| + // |
| + // STILL NOT PER-ROUTE. A per-route flag would need a registry column |
| + // and — since a Provisioned route's Channel and Environment belong to |
| + // its instance record — a record field and reconcile convergence too: |
| + // three doors onto one bit, and the "config-that-lies" hazard the |
| + // original ruling named is real at that granularity, where routes |
| + // sharing one bundle would disagree about whether it is delivered. |
| + // The environment binding is one door onto a scope that already exists. |
| + // |
| + // Additive to schema 1 (schema is not bumped for this field), so any |
| + // other value is refused at SET time (SetExecDeliveryAt, |
| + // ValidExecDelivery) and, since an older binary must still be able to |
| + // parse a manifest a newer one wrote, ignored — loudly, once per pass — |
| + // at SYNC time (see SyncExec's checkOptedIn) rather than causing a hard |
| + // failure. An older binary reading a bundle this one wrote ignores the |
| + // field entirely (it consults the default bundle alone), which is the |
| + // pre-change behaviour, not a fault. |
| +``` |
| + |
| +- [ ] **Step 4: Run the tests to verify they pass** |
| + |
| +Run: `go test ./internal/appconfig/ -count=1` |
| +Expected: PASS, including every pre-existing test — `SetExecDelivery`'s |
| +behaviour must be byte-identical. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add internal/appconfig/store.go internal/appconfig/appconfig.go internal/appconfig/environments_test.go |
| +git commit -m "appconfig: exec_delivery can be set on a named environment bundle (#195)" |
| +``` |
| + |
| +--- |
| + |
| +### Task 2: `SyncExec` resolves the opt-in per environment |
| + |
| +**Files:** |
| +- Modify: `internal/appconfig/syncexec.go:116-192` (the `optedInRes` / |
| + `optedIn` / `checkOptedIn` / `inScope` block) and `:374-387` (the |
| + stale-secrets notice loop) |
| +- Test: `internal/appconfig/syncexec_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `SetExecDeliveryAt` (Task 1) in tests only; |
| + `getIfExistsAt(ctx, s, key) (*Manifest, bool, error)`, |
| + `EnvironmentKey(acct, app, name) string`. |
| +- Produces: no exported change. `SyncExec`'s signature is untouched. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Append to `internal/appconfig/syncexec_test.go`. Read the top of that file |
| +first and reuse its existing fixture helpers (store constructor, registry |
| +constructor, route builder, key generation) rather than inventing new |
| +ones — the names below are the shape, not necessarily the spelling. |
| + |
| +```go |
| +// TestSyncExecOptInResolvesPerEnvironment is issue #195's matrix: which |
| +// hosts get a <host>.config.env, for each combination of the app-level |
| +// flag and the bound bundle's own. Two TCP-addr exec routes of ONE app, |
| +// one bound to "relay" and one unbound — titogo's exact shape. |
| +func TestSyncExecOptInResolvesPerEnvironment(t *testing.T) { |
| + for _, tc := range []struct { |
| + name string |
| + appLevel string // "" | ExecDeliveryFile |
| + envLevel string // "absent" | "silent" | "file" | "inherit" | "bogus" |
| + wantRelayFile bool |
| + wantUnboundFile bool |
| + }{ |
| + {"both off", "", "silent", false, false}, |
| + {"app on, env silent — inherit on", ExecDeliveryFile, "silent", true, true}, |
| + {"app off, env on — the narrow case", "", "file", true, false}, |
| + {"app on, env on", ExecDeliveryFile, "file", true, true}, |
| + {"app off, env bundle absent", "", "absent", false, false}, |
| + {"app on, env bundle absent — inherit on", ExecDeliveryFile, "absent", true, true}, |
| + {"app on, env states an unknown value — that statement decides", ExecDeliveryFile, "bogus", false, true}, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + ctx := context.Background() |
| + s := newTestStore(t) |
| + dir := t.TempDir() |
| + // The default bundle always exists and always has a var, so the |
| + // only thing under test is WHETHER a file is written. |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"SHARED": "1"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if tc.appLevel != "" { |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", tc.appLevel); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if tc.envLevel != "absent" { |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"SEALED": {Kind: "env", Value: "2"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + switch tc.envLevel { |
| + case "file": |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatal(err) |
| + } |
| + case "bogus": |
| + // Written past the setter's validation on purpose: this |
| + // is what an OLDER binary sees when a NEWER one invents |
| + // a value, and it must not brick the pass. |
| + writeRawManifest(t, ctx, s, EnvironmentKey("bab", "titogo", "relay"), |
| + &Manifest{Schema: 1, App: "titogo", ExecDelivery: "socket", |
| + Vars: map[string]Var{"SEALED": {Kind: "env", Value: "2"}}}) |
| + } |
| + } |
| + reg := newTestRegistry(t) |
| + mustAddRoute(t, reg, Route{Host: "relay.example", Kind: "instance", App: "titogo", |
| + Account: "bab", Addr: "127.0.0.1:9001", Environment: "relay"}) |
| + mustAddRoute(t, reg, Route{Host: "router.example", Kind: "instance", App: "titogo", |
| + Account: "bab", Addr: "127.0.0.1:9002"}) |
| + |
| + if err := SyncExec(ctx, reg, s, nil, dir, nil); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + assertConfigFile(t, dir, "relay.example", tc.wantRelayFile) |
| + assertConfigFile(t, dir, "router.example", tc.wantUnboundFile) |
| + }) |
| + } |
| +} |
| + |
| +// assertConfigFile asserts a route's <host>.config.env exists (or not). |
| +func assertConfigFile(t *testing.T, dir, host string, want bool) { |
| + t.Helper() |
| + _, err := os.Stat(filepath.Join(dir, host+configEnvSuffix)) |
| + switch { |
| + case want && err != nil: |
| + t.Fatalf("%s: want a config file, got %v", host, err) |
| + case !want && err == nil: |
| + t.Fatalf("%s: want NO config file, one was written", host) |
| + } |
| +} |
| + |
| +// TestSyncExecEnvBundleFetchErrorLeavesFileAlone: an unreadable |
| +// environment bundle is not a determination that the route is un-opted — |
| +// an existing file must survive it byte-identical. |
| +func TestSyncExecEnvBundleFetchErrorLeavesFileAlone(t *testing.T) { |
| + // Build the opted-in, converged state first, snapshot the file's bytes, |
| + // then make the environment bundle's key fail on Get and run one more |
| + // pass. Use whatever failing-store wrapper this file already has for |
| + // the default-bundle fetch-failure test; extend it to the environment |
| + // key rather than writing a second wrapper. |
| +} |
| +``` |
| + |
| +If the file has no raw-manifest writer, add one beside the test: |
| + |
| +```go |
| +// writeRawManifest puts a manifest at key without going through a setter — |
| +// the only way to stage a value a current binary would refuse to write. |
| +func writeRawManifest(t *testing.T, ctx context.Context, s release.Store, key string, m *Manifest) { |
| + t.Helper() |
| + raw, err := json.MarshalIndent(m, "", " ") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := s.Put(ctx, key, raw, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| +``` |
| + |
| +(Check the store's `Put` etag convention for an unconditional write in the |
| +existing tests and match it — `""` here is a placeholder for whatever this |
| +repo's "no precondition" spelling is.) |
| + |
| +- [ ] **Step 2: Run the test to verify it fails** |
| + |
| +Run: `go test ./internal/appconfig/ -run TestSyncExecOptInResolvesPerEnvironment -count=1` |
| +Expected: FAIL on the "app off, env on" row — no file is written for |
| +`relay.example`, because the opt-in is still read from the default bundle. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +In `internal/appconfig/syncexec.go`, replace the `optedIn` map, |
| +`checkOptedIn` closure and `inScope` closure with: |
| + |
| +```go |
| + // optKey keys the opt-in decision on the bundle a route's config |
| + // actually resolves through, not on the app alone (issue #195). Two |
| + // routes of one app bound to different environments can now differ, so |
| + // the cache — and the once-per-pass logs below — must too. |
| + type optKey struct{ acct, app, env string } |
| + optedIn := map[optKey]optedInRes{} |
| + // envBundles caches the named-bundle read the same way loadDefault |
| + // caches the default's: a box with many routes on one (app, |
| + // environment) pair costs one fetch, and the logs below fire at most |
| + // once per pair per pass rather than once per route. |
| + type envRes struct { |
| + m *Manifest |
| + found bool |
| + err error |
| + } |
| + envBundles := map[optKey]envRes{} |
| + loadEnvBundle := func(acct, app, env string) envRes { |
| + k := optKey{acct, app, env} |
| + if e, done := envBundles[k]; done { |
| + return e |
| + } |
| + m, found, err := getIfExistsAt(ctx, s, EnvironmentKey(acct, app, env)) |
| + if err != nil { |
| + err = fmt.Errorf("%s/%s (environment %q): config fetch failed: %w", acct, app, env, err) |
| + } |
| + e := envRes{m: m, found: found, err: err} |
| + envBundles[k] = e |
| + return e |
| + } |
| + checkOptedIn := func(acct, app, env string) optedInRes { |
| + k := optKey{acct, app, env} |
| + if c, done := optedIn[k]; done { |
| + return c |
| + } |
| + var c optedInRes |
| + // The route's OWN bundle first, and only when it STATES something: |
| + // a named bundle that is silent on the field inherits the app |
| + // default, exactly as a key it does not set inherits the default |
| + // bundle's value. Absence is inheritance, never "off" — an |
| + // operator who has never mentioned exec_delivery in a bundle has |
| + // not asked for delivery to stop. |
| + decided := false |
| + if env != "" { |
| + eb := loadEnvBundle(acct, app, env) |
| + switch { |
| + case eb.err != nil: |
| + // NOT a determination — same contract as the default |
| + // bundle's fetch-failure branch below: say so loudly, leave |
| + // every file alone, retry next pass. |
| + fmt.Printf(" %s/%s: exec_delivery unresolved for environment %q (config fetch failed), leaving this bundle's TCP-addr route file(s) as-is: %v\n", acct, app, env, eb.err) |
| + c, decided = optedInRes{v: false, known: false}, true |
| + case eb.found && eb.m.ExecDelivery == ExecDeliveryFile: |
| + c, decided = optedInRes{v: true, known: true}, true |
| + case eb.found && eb.m.ExecDelivery != "": |
| + // A value THIS binary doesn't understand, in a bundle that |
| + // does state one: a statement, so it decides — treated as |
| + // not opted in, loudly, once per bundle per pass, never a |
| + // hard failure (additive discipline, CLAUDE.md). |
| + fmt.Printf(" %s/%s: config: exec_delivery %q in environment %q is not a value this binary understands — ignoring it (not opted in); upgrade carlos if this is expected\n", acct, app, eb.m.ExecDelivery, env) |
| + c, decided = optedInRes{v: false, known: true}, true |
| + } |
| + // found-but-silent, and absent, both fall through to the app |
| + // default. A bundle bound but missing is a defect the route's |
| + // own resolution reports (LoadMerged refuses it), and its file |
| + // is left as-is there — the opt-in verdict never reaches a |
| + // write in that case. |
| + } |
| + if !decided { |
| + def := loadDefault(acct, app) |
| + switch { |
| + case def.err != nil: |
| + // NOT a determination: this app may well be opted in and this |
| + // box just failed to see it this pass — indistinguishable here |
| + // from any other route's ordinary fetch-failure blip. Silence |
| + // would mean an opted-in app's delivery can stall with zero |
| + // trace in the log, on the exact path this feature exists to |
| + // light up, so say so loudly — the same "unresolved, left |
| + // as-is" contract the resolution loop below already gives a |
| + // bound-group fetch failure. |
| + // |
| + // It is safe to leave any existing file exactly as it is |
| + // regardless of how long this persists, because reapConfigEnv |
| + // keys on ROUTE EXISTENCE, never on scope (its live map is |
| + // built from every route's Host, full stop — see its own doc |
| + // comment). A route that drops out of scope for one pass, for |
| + // ANY reason, is simply not visited by the resolution loop, |
| + // the materialization loop, or reap; its file is untouched by |
| + // construction. Next pass retries from scratch. |
| + fmt.Printf(" %s/%s: exec_delivery unresolved (config fetch failed), leaving this app's TCP-addr route file(s) as-is: %v\n", acct, app, def.err) |
| + c = optedInRes{v: false, known: false} |
| + case !def.found: |
| + c = optedInRes{v: false, known: true} |
| + case def.m.ExecDelivery == ExecDeliveryFile: |
| + c = optedInRes{v: true, known: true} |
| + case def.m.ExecDelivery == "": |
| + c = optedInRes{v: false, known: true} |
| + default: |
| + fmt.Printf(" %s/%s: config: exec_delivery %q in the default bundle is not a value this binary understands — ignoring it (not opted in); upgrade carlos if this is expected\n", acct, app, def.m.ExecDelivery) |
| + c = optedInRes{v: false, known: true} |
| + } |
| + } |
| + optedIn[k] = c |
| + return c |
| + } |
| + // inScope is the single door both loops below use to decide whether a |
| + // route is in scope this pass, and — for the addr shape — that it must |
| + // never be bounced. One function so the two loops structurally cannot |
| + // diverge on either question. |
| + inScope := func(rt registry.Route) (ok, addrShape bool) { |
| + if execSyncable(rt) { |
| + return true, false |
| + } |
| + if execAddrCandidate(rt) && checkOptedIn(release.RouteAccount(rt), rt.App, rt.Environment).v { |
| + return true, true |
| + } |
| + return false, false |
| + } |
| +``` |
| + |
| +Update the `optedInRes` doc comment above it to say the verdict is per |
| +`(account, app, environment)`, not per app. |
| + |
| +Then, in the stale-secrets notice loop near the end of `SyncExec`, change |
| +the one call site: |
| + |
| +```go |
| + c := checkOptedIn(release.RouteAccount(rt), rt.App, rt.Environment) |
| +``` |
| + |
| +Finally update the package doc at the top of the file: the sentence |
| +beginning "A third eligibility case (issue #18): a TCP-addr exec route |
| +[...] whose app has opted in via its default bundle's additive |
| +`"exec_delivery": "file"` manifest field" becomes "whose BOUND BUNDLE has |
| +opted in — the route's named environment when it states a value, the |
| +app's default bundle otherwise (issue #195)". |
| + |
| +- [ ] **Step 4: Run the tests to verify they pass** |
| + |
| +Run: `go test ./internal/appconfig/ -count=1` |
| +Expected: PASS, all rows of the matrix and every pre-existing SyncExec |
| +test — the unbound-route behaviour must be unchanged. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add internal/appconfig/syncexec.go internal/appconfig/syncexec_test.go |
| +git commit -m "appconfig: SyncExec resolves exec_delivery from the route's bound bundle (#195)" |
| +``` |
| + |
| +--- |
| + |
| +### Task 3: `ConfigRouteFingerprints` |
| + |
| +**Files:** |
| +- Create: `internal/appconfig/routefingerprint.go` |
| +- Test: `internal/appconfig/routefingerprint_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `execSyncable(rt)`, `execAddrCandidate(rt)` (syncexec.go), |
| + `syncable(rt)` (sync.go), `release.RouteAccount(rt)`, |
| + `registry.BackingUnit`. |
| +- Produces: |
| + `func ConfigRouteFingerprints(routes []registry.Route) (exec, unit string)` |
| + — two hex digests, one per delivery half. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `internal/appconfig/routefingerprint_test.go`: |
| + |
| +```go |
| +package appconfig |
| + |
| +import ( |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +func execRoute(host, env string) registry.Route { |
| + return registry.Route{Host: host, Kind: "instance", App: "titogo", |
| + Account: "bab", Addr: "127.0.0.1:9001", Environment: env} |
| +} |
| + |
| +func unitRoute(host, env string) registry.Route { |
| + return registry.Route{Host: host, Kind: "instance", App: "keymail", |
| + Account: "bae", Socket: "/run/x.sock", Backing: registry.BackingUnit, |
| + Environment: env} |
| +} |
| + |
| +func TestConfigRouteFingerprintsMoveOnRebinding(t *testing.T) { |
| + before := []registry.Route{execRoute("relay.example", "")} |
| + after := []registry.Route{execRoute("relay.example", "relay")} |
| + be, bu := ConfigRouteFingerprints(before) |
| + ae, au := ConfigRouteFingerprints(after) |
| + if be == ae { |
| + t.Fatal("binding an exec route to an environment must move the exec digest — this is issue #196") |
| + } |
| + if bu != au { |
| + t.Fatal("an exec route's rebinding must not move the UNIT digest — that would ask for a root pass for nothing") |
| + } |
| +} |
| + |
| +func TestConfigRouteFingerprintsUnitHalfIsIndependent(t *testing.T) { |
| + before := []registry.Route{unitRoute("keymail.dev", "")} |
| + after := []registry.Route{unitRoute("keymail.dev", "staging")} |
| + be, bu := ConfigRouteFingerprints(before) |
| + ae, au := ConfigRouteFingerprints(after) |
| + if bu == au { |
| + t.Fatal("binding a unit-backed route to an environment must move the unit digest") |
| + } |
| + if be != ae { |
| + t.Fatal("a unit route's rebinding must not move the exec digest") |
| + } |
| +} |
| + |
| +func TestConfigRouteFingerprintsIgnoreOrderAndIrrelevantColumns(t *testing.T) { |
| + a := execRoute("a.example", "") |
| + b := execRoute("b.example", "relay") |
| + e1, _ := ConfigRouteFingerprints([]registry.Route{a, b}) |
| + e2, _ := ConfigRouteFingerprints([]registry.Route{b, a}) |
| + if e1 != e2 { |
| + t.Fatal("registry order must not move the digest — it is not stable across passes") |
| + } |
| + c := execRoute("a.example", "") |
| + c.Version = "deadbeef" |
| + c.LastError = "whatever" |
| + e3, _ := ConfigRouteFingerprints([]registry.Route{c, b}) |
| + if e1 != e3 { |
| + t.Fatal("a column that cannot change what config a route gets must not move the digest — every version stamp would trigger a pass") |
| + } |
| +} |
| + |
| +func TestConfigRouteFingerprintsMoveWhenARouteLeavesScope(t *testing.T) { |
| + both := []registry.Route{execRoute("a.example", ""), execRoute("b.example", "")} |
| + one := []registry.Route{execRoute("a.example", "")} |
| + e1, _ := ConfigRouteFingerprints(both) |
| + e2, _ := ConfigRouteFingerprints(one) |
| + if e1 == e2 { |
| + t.Fatal("a route leaving the registry must move the digest — its file needs reaping") |
| + } |
| + // Leaving scope by status, not by deletion, counts the same way. |
| + failed := []registry.Route{execRoute("a.example", ""), execRoute("b.example", "")} |
| + failed[1].Status = "failed" |
| + e3, _ := ConfigRouteFingerprints(failed) |
| + if e1 == e3 { |
| + t.Fatal("a route falling out of the eligibility predicate must move the digest") |
| + } |
| +} |
| + |
| +func TestConfigRouteFingerprintsIgnoreOutOfScopeRoutes(t *testing.T) { |
| + base := []registry.Route{execRoute("a.example", "")} |
| + withStatic := append([]registry.Route{}, base...) |
| + withStatic = append(withStatic, registry.Route{Host: "site.example", Kind: "static", App: "site", Account: "bab"}) |
| + e1, u1 := ConfigRouteFingerprints(base) |
| + e2, u2 := ConfigRouteFingerprints(withStatic) |
| + if e1 != e2 || u1 != u2 { |
| + t.Fatal("a route neither half can deliver config to must not move either digest") |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run the test to verify it fails** |
| + |
| +Run: `go test ./internal/appconfig/ -run TestConfigRouteFingerprints -count=1` |
| +Expected: FAIL — `undefined: ConfigRouteFingerprints`. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +Create `internal/appconfig/routefingerprint.go`: |
| + |
| +```go |
| +// Route-shape fingerprints: the registry-side half of "has this box's |
| +// config delivery got anything to do". |
| +// |
| +// The agent's fast loop (host package's configGenLoop) decides whether to |
| +// converge by reading each config-carrying app's generation object out of |
| +// the bucket. That catches every BUCKET-side change — a var set, a secret |
| +// rotated, an exec_delivery flag flipped — and nothing at all on the |
| +// REGISTRY side. But a route's Environment is a registry column, and it |
| +// decides which bundle the route's config comes from, so binding a route |
| +// to an environment changes what that route should be served while moving |
| +// no etag anywhere. Delivery then waits for the hourly configSyncLoop: |
| +// issue #196, an hour of a live route serving the wrong merge, whose only |
| +// symptom is the app saying a credential is missing. |
| +// |
| +// These digests give that loop something to watch. They live here, beside |
| +// ExecConfigApps and UnitConfigApps and composed from the SAME eligibility |
| +// predicates those two use, for the reason both of those give for living |
| +// where they do: a fingerprint that disagreed with the sweep about which |
| +// routes matter would either burn passes forever or miss changes silently. |
| +package appconfig |
| + |
| +import ( |
| + "crypto/sha256" |
| + "encoding/hex" |
| + "sort" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// ConfigRouteFingerprints returns one digest per delivery half over the |
| +// registry-side facts that decide what config a route gets: which routes |
| +// are in scope at all, and — for each — the (account, app, environment) |
| +// triple its manifest resolves from. |
| +// |
| +// Set MEMBERSHIP is what carries scope changes: a route that gains |
| +// Status: "failed", loses its app stamp, or is deleted outright simply |
| +// stops contributing a row, which moves the digest exactly as a rebinding |
| +// does. That is deliberate — those transitions all change what should be |
| +// on disk (a file to remove, a file to stop updating), and the pass they |
| +// trigger resolves each case properly. |
| +// |
| +// The OPT-IN is deliberately not here, for the reason ExecConfigApps gives |
| +// for omitting it too: reading it costs a bucket fetch per app per tick, |
| +// which is exactly what the fast loop exists to avoid. It does not need to |
| +// be: exec_delivery is a bundle write, and every bundle write bumps the |
| +// app's generation object (SetExecDeliveryAt), which the loop already |
| +// watches. |
| +// |
| +// Two digests, not one, because the two halves are delivered by different |
| +// processes: the exec half by the agent itself (SyncExec), the unit half |
| +// by a root pass the agent can only REQUEST. A shared digest would ask for |
| +// a root config run every time an exec route was rebound. |
| +func ConfigRouteFingerprints(routes []registry.Route) (exec, unit string) { |
| + var execRows, unitRows []string |
| + for _, rt := range routes { |
| + row := rt.Host + "|" + release.RouteAccount(rt) + "|" + rt.App + "|" + rt.Environment |
| + if execSyncable(rt) || execAddrCandidate(rt) { |
| + execRows = append(execRows, row) |
| + } |
| + if syncable(rt) && rt.Backing == registry.BackingUnit { |
| + unitRows = append(unitRows, row) |
| + } |
| + } |
| + return digestRows(execRows), digestRows(unitRows) |
| +} |
| + |
| +// digestRows sorts and hashes. Sorted because registry order is not stable |
| +// across passes and an order-sensitive digest would fire constantly; |
| +// NUL-separated because a hostname cannot contain one, so no two distinct |
| +// row sets can concatenate to the same bytes. |
| +func digestRows(rows []string) string { |
| + sort.Strings(rows) |
| + h := sha256.New() |
| + for _, r := range rows { |
| + h.Write([]byte(r)) |
| + h.Write([]byte{0}) |
| + } |
| + return hex.EncodeToString(h.Sum(nil)) |
| +} |
| +``` |
| + |
| +Add `"github.com/carlosframework/platform/internal/release"` to the |
| +imports (the row uses `release.RouteAccount`). |
| + |
| +- [ ] **Step 4: Run the tests to verify they pass** |
| + |
| +Run: `go test ./internal/appconfig/ -count=1` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add internal/appconfig/routefingerprint.go internal/appconfig/routefingerprint_test.go |
| +git commit -m "appconfig: fingerprint the registry-side facts that decide a route's config (#196)" |
| +``` |
| + |
| +--- |
| + |
| +### Task 4: The fast loop watches the fingerprints |
| + |
| +**Files:** |
| +- Modify: `internal/host/agent.go:571-670` (`genCache`, `genPollPass`) and |
| + `:792-850` (`configGenLoop`) |
| +- Test: `internal/host/agent_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `appconfig.ConfigRouteFingerprints(routes) (exec, unit string)` |
| + (Task 3). |
| +- Produces: `type genCache struct` with `etags map[[2]string]string` and |
| + unexported digest fields; `func newGenCache() *genCache`; |
| + `genPollPass(ctx, routes, store, c *genCache) (execChanged, unitChanged bool, readErrs []error)`. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Append to `internal/host/agent_test.go`: |
| + |
| +```go |
| +// TestGenPollPassSyncsOnRouteRebinding is issue #196: a route bound to an |
| +// environment must converge from the FAST loop, not an hour later. The |
| +// bucket does not move at all here — only the registry column does. |
| +func TestGenPollPassSyncsOnRouteRebinding(t *testing.T) { |
| + ctx := context.Background() |
| + s := newGenStore(t) // match this file's existing helper |
| + routes := []registry.Route{execRouteFixture("relay.example", "")} |
| + cache := newGenCache() |
| + genPollPass(ctx, routes, s, cache) // first sight: silent |
| + |
| + rebound := []registry.Route{execRouteFixture("relay.example", "relay")} |
| + execChanged, unitChanged, errs := genPollPass(ctx, rebound, s, cache) |
| + if len(errs) > 0 { |
| + t.Fatalf("unexpected read errors: %v", errs) |
| + } |
| + if !execChanged { |
| + t.Fatal("binding a route to an environment must make the exec half sync — this is the hour titogo lost") |
| + } |
| + if unitChanged { |
| + t.Fatal("an exec route's rebinding must not request a root config pass") |
| + } |
| + // And the pass AFTER it is quiet again: a digest that re-fired every |
| + // tick would run SyncExec forever. |
| + if execChanged, _, _ := genPollPass(ctx, rebound, s, cache); execChanged { |
| + t.Fatal("the pass after a rebinding must be silent again") |
| + } |
| +} |
| + |
| +// TestGenPollPassFirstSightOfRouteShapeIsSilent: a box coming up has never |
| +// seen the digest before, and must not treat that as a change — every |
| +// instance on the box would sync at once on boot. |
| +func TestGenPollPassFirstSightOfRouteShapeIsSilent(t *testing.T) { |
| + ctx := context.Background() |
| + s := newGenStore(t) |
| + routes := []registry.Route{execRouteFixture("relay.example", "relay")} |
| + execChanged, unitChanged, errs := genPollPass(ctx, routes, s, newGenCache()) |
| + if len(errs) > 0 { |
| + t.Fatalf("unexpected read errors: %v", errs) |
| + } |
| + if execChanged || unitChanged { |
| + t.Fatal("the first pass must record the shape silently — configSyncLoop's first pass owns boot convergence") |
| + } |
| +} |
| +``` |
| + |
| +Add an `execRouteFixture` helper beside them if the file has no equivalent: |
| + |
| +```go |
| +func execRouteFixture(host, env string) registry.Route { |
| + return registry.Route{Host: host, Kind: "instance", App: "titogo", |
| + Account: "bab", Addr: "127.0.0.1:9001", Environment: env} |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run the test to verify it fails** |
| + |
| +Run: `go test ./internal/host/ -run TestGenPollPass -count=1` |
| +Expected: FAIL — `undefined: newGenCache` (and, once that compiles, a |
| +`execChanged == false` failure on the rebinding). |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +In `internal/host/agent.go`, add above `genPollPass`: |
| + |
| +```go |
| +// genCache is everything configGenLoop remembers between ticks: one |
| +// generation etag per config-carrying app, and the previous tick's |
| +// route-shape digests. One object rather than two parameters because the |
| +// two are remembered for exactly one reason — deciding what "changed" |
| +// means — and a caller holding one without the other would silently lose |
| +// half the loop's sensitivity. |
| +type genCache struct { |
| + etags map[[2]string]string |
| + // exec/unit are appconfig.ConfigRouteFingerprints' two digests as of |
| + // the last pass that computed them; shapeSeen is false until the first |
| + // such pass. The bool is not redundant with an empty string: a box with |
| + // no config-carrying routes at all has a real, stable digest of the |
| + // empty set, and treating that as "never seen" would make its first |
| + // route look like a change on the tick after boot rather than on the |
| + // tick it appeared. |
| + exec, unit string |
| + shapeSeen bool |
| +} |
| + |
| +func newGenCache() *genCache { return &genCache{etags: map[[2]string]string{}} } |
| +``` |
| + |
| +Change `genPollPass`'s signature and body. The signature becomes: |
| + |
| +```go |
| +func genPollPass(ctx context.Context, routes []registry.Route, store release.Store, c *genCache) (execChanged, unitChanged bool, readErrs []error) { |
| +``` |
| + |
| +Inside, replace every `cache[...]` with `c.etags[...]`, and add the shape |
| +comparison at the top of the function, before the etag loop: |
| + |
| +```go |
| + // The registry-side half (issue #196). Cheap — no bucket read at all — |
| + // and checked first so a rebinding converges on the same tick a var |
| + // change would, rather than waiting for one. |
| + execFP, unitFP := appconfig.ConfigRouteFingerprints(routes) |
| + if c.shapeSeen { |
| + execChanged = execChanged || execFP != c.exec |
| + unitChanged = unitChanged || unitFP != c.unit |
| + } |
| + c.exec, c.unit, c.shapeSeen = execFP, unitFP, true |
| +``` |
| + |
| +Extend `genPollPass`'s doc comment with a bullet in the "Semantics" list: |
| + |
| +```go |
| +// - The ROUTE SHAPE is watched too, not just the bucket: a route's |
| +// Environment binding lives in the box's own registry and moves no |
| +// etag anywhere, so without this the fast loop cannot see a rebinding |
| +// at all and delivery waits out configSyncLoop's hourly floor (issue |
| +// #196 — ten minutes of a live route serving the wrong merge, whose |
| +// only symptom is the app reporting a missing credential). First sight |
| +// of the shape is silent for the same reason first sight of an app is. |
| +// Covers BOTH doors onto the column: `carlos route -environment` on |
| +// the box, and the provisioning reconciler converging Environment from |
| +// an instance record — which is the sanctioned door for a Provisioned |
| +// row, and which a fix in `carlos route` would have missed entirely. |
| +``` |
| + |
| +In `configGenLoop`, change the cache construction and the call: |
| + |
| +```go |
| + // One cache for the life of the process: what "changed" means is |
| + // exactly "differs from the last tick that could read it". |
| + cache := newGenCache() |
| +``` |
| + |
| +(the `genPollPass(ctx, routes, store, cache)` call site needs no edit). |
| + |
| +Then fix the existing tests mechanically — these three substitutions over |
| +`internal/host/agent_test.go`: |
| + |
| +- `cache := map[[2]string]string{}` → `cache := newGenCache()` |
| +- `cache[[2]string{` → `cache.etags[[2]string{` |
| +- `_, ok := cache[` → `_, ok := cache.etags[` |
| + |
| +- [ ] **Step 4: Run the tests to verify they pass** |
| + |
| +Run: `go test ./internal/host/ -count=1` |
| +Expected: PASS, including all 26 pre-existing `genPollPass` assertions. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add internal/host/agent.go internal/host/agent_test.go |
| +git commit -m "host: the fast config loop notices a route's environment rebinding (#196)" |
| +``` |
| + |
| +--- |
| + |
| +### Task 5: `carlos env exec-delivery -environment` |
| + |
| +**Files:** |
| +- Modify: `appconfig_cli.go:358-435` (`runEnvExecDelivery`), `:331-356` |
| + (`printEnvList`), `:20-58` (`envUsage`) |
| +- Test: `appconfig_cli_test.go` (or the file in the root package that |
| + already tests `env` subcommands — check `e2e_test.go` for the |
| + established pattern first) |
| + |
| +**Interfaces:** |
| +- Consumes: `appconfig.SetExecDeliveryAt(ctx, store, acct, app, envName, value)` |
| + (Task 1); `apiConfigApply{Environment string, ExecDelivery *string}` |
| + (existing wire struct in `apimode.go`). |
| +- Produces: no exported change. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +The refusals are the behaviour worth pinning. Follow the file's existing |
| +convention for driving a CLI verb — if these tests shell out to the built |
| +binary, they MUST use `carlosTestEnv` and set `cmd.Dir` to a scratch dir |
| +(CLAUDE.md; the fallback is the operator's real login and console). |
| + |
| +```go |
| +func TestEnvExecDeliveryRefusesOffAtEnvironmentScope(t *testing.T) { |
| + out, err := runCarlos(t, "env", "exec-delivery", "-app", "titogo", "-environment", "relay", "off") |
| + if err == nil { |
| + t.Fatal(`"off" must be refused for a named environment — clearing the field there means inherit`) |
| + } |
| + if !strings.Contains(out, "inherit") { |
| + t.Fatalf("the refusal must name the spelling that IS accepted; got %q", out) |
| + } |
| +} |
| + |
| +func TestEnvExecDeliveryRefusesUnknownValueAtEnvironmentScope(t *testing.T) { |
| + out, err := runCarlos(t, "env", "exec-delivery", "-app", "titogo", "-environment", "relay", "socket") |
| + if err == nil { |
| + t.Fatal("an unknown value must be refused") |
| + } |
| + if !strings.Contains(out, "file") || !strings.Contains(out, "inherit") { |
| + t.Fatalf("the refusal must name both accepted spellings; got %q", out) |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run the tests to verify they fail** |
| + |
| +Run: `go test . -run TestEnvExecDelivery -count=1` |
| +Expected: FAIL — `-environment` is currently refused outright, so the |
| +first test fails on the message and the second passes for the wrong |
| +reason. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +Rewrite `runEnvExecDelivery`: |
| + |
| +```go |
| +// runEnvExecDelivery is `carlos env exec-delivery` — issue #18's opt-in |
| +// verb, deliberately NOT `env set -app <a> EXEC_DELIVERY=...`: the field |
| +// is not a config var (it's never rendered into an EnvironmentFile, never |
| +// listed alongside KEY/VALUE pairs), it's a manifest-level flag with its |
| +// own validation and its own bundle, so it gets its own verb surface |
| +// instead of overloading env set's KEY=value parsing with a special-cased |
| +// key name. |
| +// |
| +// -environment (issue #195) writes the flag into that bundle instead of |
| +// the app default. The vocabulary changes with the scope, and that is the |
| +// point: at app scope the second word is "off", because clearing the |
| +// default bundle's field really does stop delivery; in a named bundle it |
| +// is "inherit", because clearing it there hands the question back to the |
| +// app default. Spelling the second case "off" would read as "stop |
| +// delivering" while doing the opposite whenever the app default is on, |
| +// which is precisely the kind of quiet lie this whole feature exists to |
| +// avoid — so "off" is refused there, by name, rather than accepted as a |
| +// synonym. |
| +func runEnvExecDelivery(args []string) { |
| + fs := flag.NewFlagSet("env exec-delivery", flag.ExitOnError) |
| + app := fs.String("app", "", "app name") |
| + environment := fs.String("environment", "", |
| + "named environment bundle to set the flag in (omit for the app's default bundle)") |
| + acctRaw, acctIDRaw, acctOf := accountFlags(fs) |
| + consoleFlag := fs.String("console", "", "console origin to work through (API mode only; default: the only logged-in one)") |
| + fs.Parse(args) |
| + usage := "usage: carlos env exec-delivery -app <a> [-console <origin>] file|off\n" + |
| + " or: carlos env exec-delivery -app <a> -environment <name> [-console <origin>] file|inherit" |
| + if fs.NArg() != 1 { |
| + log.Fatal(usage) |
| + } |
| + checkEnvironmentFlag(*environment) |
| + // The wire and store vocabulary is one value either way — "" clears the |
| + // field. What differs is which SECOND WORD spells the clear, and which |
| + // bundle it clears in. |
| + var value, cleared string |
| + switch { |
| + case fs.Arg(0) == appconfig.ExecDeliveryFile: |
| + value = appconfig.ExecDeliveryFile |
| + case *environment == "" && fs.Arg(0) == "off": |
| + value, cleared = "", "off" |
| + case *environment != "" && fs.Arg(0) == "inherit": |
| + value, cleared = "", "inherit" |
| + case *environment != "" && fs.Arg(0) == "off": |
| + log.Fatalf(`carlos env exec-delivery: %q is not a value for a named environment — say "inherit" instead. `+ |
| + `Clearing exec_delivery in a bundle does not turn delivery off, it hands the question back to the app's `+ |
| + `default bundle (config.json), so a route bound to %q keeps getting files whenever the app-level flag is on. `+ |
| + `To stop delivery for the whole app: carlos env exec-delivery -app %s off`, |
| + "off", *environment, *app) |
| + default: |
| + want := `"off"` |
| + if *environment != "" { |
| + want = `"inherit"` |
| + } |
| + log.Fatalf("carlos env exec-delivery: %q is not a valid value — want %q or %s", fs.Arg(0), appconfig.ExecDeliveryFile, want) |
| + } |
| + |
| + sess, err := resolveMode(*consoleFlag) |
| + if err != nil { |
| + log.Fatal(err) |
| + } |
| + appName := *app |
| + if sess != nil { |
| + printUsing(os.Stdout, sess) |
| + acct := resolvedAccount(sess, acctRaw, acctIDRaw) |
| + appName = resolvedApp(sess, acct, *app, usage) |
| + // The console's PUT decodes ExecDelivery as a pointer where nil |
| + // means "no change", so a clear must still be spelled as a word. |
| + // The word is the scope's own: the server refuses "off" for a named |
| + // bundle exactly as this function does, so relaying the operator's |
| + // own vocabulary keeps the two refusals identical. |
| + wire := cleared |
| + if value != "" { |
| + wire = value |
| + } |
| + if _, err := sess.configApply(context.Background(), acct, appName, |
| + apiConfigApply{Environment: *environment, ExecDelivery: &wire}); err != nil { |
| + log.Fatal(err) |
| + } |
| + } else { |
| + if *app == "" { |
| + log.Fatal(usage) |
| + } |
| + if err := appconfig.SetExecDeliveryAt(context.Background(), openStore(), acctOf(), *app, *environment, value); err != nil { |
| + log.Fatal(err) |
| + } |
| + } |
| + if value == "" { |
| + fmt.Printf("%s %s for %s (%s)\n", colorFor(os.Stdout).ok("exec-delivery"), cleared, appName, bundleLabel(*environment)) |
| + } else { |
| + fmt.Printf("%s %s for %s (%s)\n", colorFor(os.Stdout).ok("exec-delivery"), value, appName, bundleLabel(*environment)) |
| + } |
| +} |
| +``` |
| + |
| +In `printEnvList`, drop the environment suppression: |
| + |
| +```go |
| + // exec_delivery is printed for whichever bundle is being listed — it can |
| + // now be set in a named one (issue #195). A bundle that is SILENT on it |
| + // prints nothing, rather than printing the value it inherits: the |
| + // listing shows what this bundle holds, and rendering an inherited |
| + // value would make the bundle look like it holds a statement it does |
| + // not, which is exactly the confusion that makes the app-level flag |
| + // hard to reason about in the first place. |
| + if execDelivery != "" { |
| + fmt.Printf("exec-delivery: %s\n", execDelivery) |
| + } |
| +``` |
| + |
| +Update `envUsage`: the command line becomes |
| + |
| +``` |
| + exec-delivery -app <a> [-environment <name>] file|off|inherit opt an app's (or one bundle's) TCP-addr exec routes into config delivery |
| +``` |
| + |
| +and replace the "exec-delivery is APP-LEVEL ONLY" paragraph with: |
| + |
| +``` |
| +exec-delivery sets "exec_delivery" in one bundle. "file" makes SyncExec |
| +write /data/<host>.config.env for the TCP-addr exec routes that resolve |
| +through that bundle (routes carlos proxies to but does not spawn — e.g. |
| +titogo's router fleet) exactly as it already does for socket-exec routes, |
| +but those routes are NEVER bounced: carlos does not own the process, so |
| +the app's own supervisor owns the restart. |
| + |
| +Without -environment it is the app default, and "off" clears it. With |
| +-environment it is that bundle alone, and "inherit" clears it — a bundle |
| +that says nothing takes the app default's answer, so clearing there is not |
| +the same as turning delivery off. To deliver to ONE service: turn the app |
| +default off, and set "file" on the bundle its route is bound to. |
| + |
| +Turning the app default off does NOT delete files already written for |
| +routes that lose the opt-in — they hold decrypted secrets and carlos |
| +cannot know when the app's own supervisor has stopped reading them. It |
| +names each one in its log every pass until you delete it by hand. |
| +``` |
| + |
| +- [ ] **Step 4: Run the tests to verify they pass** |
| + |
| +Run: `go test . -count=1` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add appconfig_cli.go appconfig_cli_test.go |
| +git commit -m "cli: carlos env exec-delivery -environment (#195)" |
| +``` |
| + |
| +--- |
| + |
| +### Task 6: The console's config door accepts environment scope |
| + |
| +**Files:** |
| +- Modify: `internal/console/apiconfig.go:230-241` (GET's exec_delivery |
| + emission), `:369-388` (POST's exec_delivery validation), `:427-441` |
| + (POST's second CAS write) |
| +- Test: `internal/console/apiconfig_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `appconfig.SetExecDeliveryAt` (Task 1). |
| +- Produces: no Go-level export change. Wire: `exec_delivery` accepts |
| + `"file"` and `"inherit"` when `environment` is non-empty, and `"file"` / |
| + `"off"` when it is empty; GET emits the named bundle's own value. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Append to `internal/console/apiconfig_test.go`, following that file's |
| +existing request helpers: |
| + |
| +```go |
| +func TestAPIConfigPutExecDeliveryAtEnvironmentScope(t *testing.T) { |
| + // PUT {"environment":"relay","exec_delivery":"file"} must land in the |
| + // NAMED bundle and leave the default bundle's field empty. |
| +} |
| + |
| +func TestAPIConfigPutRefusesOffAtEnvironmentScope(t *testing.T) { |
| + // PUT {"environment":"relay","exec_delivery":"off"} → 400, and the |
| + // message must name "inherit". |
| +} |
| + |
| +func TestAPIConfigPutAcceptsInheritAtEnvironmentScope(t *testing.T) { |
| + // PUT {"environment":"relay","exec_delivery":"inherit"} clears the |
| + // named bundle's field; a following GET omits exec_delivery. |
| +} |
| + |
| +func TestAPIConfigPutRefusesInheritAtAppScope(t *testing.T) { |
| + // PUT {"exec_delivery":"inherit"} with no environment → 400: there is |
| + // nothing above the default bundle to inherit FROM, so accepting the |
| + // word would silently mean "off". |
| +} |
| + |
| +func TestAPIConfigGetEmitsNamedBundleExecDelivery(t *testing.T) { |
| + // After the environment-scoped set, GET ?environment=relay carries |
| + // exec_delivery; GET with no environment does not. |
| +} |
| +``` |
| + |
| +Write each body out against the file's own helpers — the comments state |
| +the assertion, the code must make it. |
| + |
| +- [ ] **Step 2: Run the tests to verify they fail** |
| + |
| +Run: `go test ./internal/console/ -run TestAPIConfig -count=1` |
| +Expected: FAIL — the environment-scoped PUT is currently a 400 ("app-level |
| +only") and GET suppresses the field for a named bundle. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +In `handleAPIConfigGet`, replace the emission condition: |
| + |
| +```go |
| + // exec_delivery is emitted for whichever bundle was asked for: since |
| + // issue #195 it can be set in a named one, where it means "routes bound |
| + // to this environment get files". Omitted when the bundle is silent on |
| + // it rather than reported as the inherited value — the answer describes |
| + // this bundle, and an inherited value would read as a statement the |
| + // bundle does not make. |
| + if m.ExecDelivery != "" { |
| + out["exec_delivery"] = m.ExecDelivery |
| + } |
| +``` |
| + |
| +In `handleAPIConfigPut`, replace the `req.ExecDelivery` validation block: |
| + |
| +```go |
| + execDelivery := "" |
| + if req.ExecDelivery != nil { |
| + // The wire's clearing word is the SCOPE's own, and the two are not |
| + // interchangeable: clearing the default bundle's flag stops |
| + // delivery, while clearing a named bundle's hands the question back |
| + // to the default. Accepting "off" for a named bundle would let a |
| + // caller ask to stop delivery and get the opposite whenever the app |
| + // default is on. Both refusals are worded here, once, so the CLI |
| + // and any other client read the same sentence. |
| + switch *req.ExecDelivery { |
| + case appconfig.ExecDeliveryFile: |
| + execDelivery = appconfig.ExecDeliveryFile |
| + case "off": |
| + if envName != "" { |
| + jsonError(w, http.StatusBadRequest, |
| + "exec_delivery \"off\" is app-level — in a named environment, clearing the flag means "+ |
| + "\"inherit\" (routes bound to it fall back to the app's default bundle), which is not the "+ |
| + "same as turning delivery off. Send \"inherit\" for this bundle, or send \"off\" with no "+ |
| + "environment to turn the app's default off.") |
| + return |
| + } |
| + execDelivery = "" |
| + case "inherit": |
| + if envName == "" { |
| + jsonError(w, http.StatusBadRequest, |
| + "exec_delivery \"inherit\" needs an environment — the default bundle has nothing above it to "+ |
| + "inherit from. Send \"off\" to clear it.") |
| + return |
| + } |
| + execDelivery = "" |
| + default: |
| + want := `"off"` |
| + if envName != "" { |
| + want = `"inherit"` |
| + } |
| + jsonError(w, http.StatusBadRequest, |
| + fmt.Sprintf("invalid exec_delivery %q — want %q or %s", *req.ExecDelivery, appconfig.ExecDeliveryFile, want)) |
| + return |
| + } |
| + } |
| +``` |
| + |
| +And the second CAS write: |
| + |
| +```go |
| + if req.ExecDelivery != nil { |
| + // A SECOND CAS write, and deliberately not folded into the one |
| + // above: it targets one field of one bundle through a different |
| + // setter, and the two cannot be one transaction. The audit row for |
| + // the set/unset half is written above, before this runs — if this |
| + // fails, the trail still says truthfully what landed. |
| + if err := appconfig.SetExecDeliveryAt(ctx, s.cfg.Releases, acct.Sqid, app, envName, execDelivery); err != nil { |
| + s.configStoreError(w, fmt.Sprintf("api config exec-delivery %s/%s (%s)", acct.Sqid, app, bundle), err) |
| + return |
| + } |
| + label := *req.ExecDelivery |
| + s.audit(acct, app, addr, "exec-delivery", "exec-delivery "+label+" ("+bundle+") (cli)") |
| + s.bumpSerial(ctx) |
| + } |
| +``` |
| + |
| +- [ ] **Step 4: Run the tests to verify they pass** |
| + |
| +Run: `go test ./internal/console/ -count=1` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add internal/console/apiconfig.go internal/console/apiconfig_test.go |
| +git commit -m "console: config API accepts environment-scoped exec_delivery (#195)" |
| +``` |
| + |
| +--- |
| + |
| +### Task 7: `GET /api/cli/apps/{account}/{app}/routes` |
| + |
| +**Files:** |
| +- Create: `internal/console/apiroutes.go` |
| +- Modify: `internal/console/authcli.go` (registration, beside the config |
| + family at :294-296), `internal/console/apiinstances.go:214-296` |
| + (extract `healthByHost` from `instanceHealth`) |
| +- Test: `internal/console/apiroutes_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `s.apiAccountApp(w, r) (string, *Account, string, bool)`, |
| + `openRegistry(path) (*registry.Registry, error)`, |
| + `s.OwnerAppMetrics(ctx, hosts []string, unitByHost map[string]string) ([]InstanceMetrics, error)`, |
| + `healthWord(kind, status string, hibernate, reporting, expectReadings bool) string`, |
| + `expectsReadings(provisioned bool, unit, backing string) bool`, |
| + `release.RouteAccount(rt) string`. |
| +- Produces: `func (s *Server) handleAPIRoutes(w http.ResponseWriter, r *http.Request)`; |
| + `func (s *Server) healthByHost(ctx context.Context, routeFor map[string]registry.Route) map[string]string`. |
| + Wire: `{"routes":[{"host","kind","upstream","app","channel","environment","status","health","hibernate","unit","backing","provisioned"}]}`. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `internal/console/apiroutes_test.go`. Copy the gate assertions from |
| +`apiinstances_test.go` verbatim in shape — the 404 discipline is the |
| +security boundary and must be pinned identically here. |
| + |
| +```go |
| +func TestAPIRoutesRequiresMembership(t *testing.T) { |
| + // A signed-in caller who is not a member of the account gets the |
| + // family's byte-identical 404 — not a 403, not an empty list. |
| +} |
| + |
| +func TestAPIRoutesRefusesAnAppTokenOutright(t *testing.T) { |
| + // This door is member-only: an app token that CAN reach |
| + // /instances must not reach /routes. Assert the 404 (or whatever |
| + // apiAccountApp answers a non-person bearer) rather than a 200. |
| +} |
| + |
| +func TestAPIRoutesListsOnlyThisAppsRoutes(t *testing.T) { |
| + // A registry holding: this app's route, another app's route in the |
| + // SAME account, and a route stamped to another account entirely. |
| + // Only the first appears. |
| +} |
| + |
| +func TestAPIRoutesReportsTheEnvironmentBinding(t *testing.T) { |
| + // The whole point of issue #64: a route with Environment "relay" |
| + // reports "relay", and an unbound one reports "". |
| +} |
| + |
| +func TestAPIRoutesSaysSoWhenTheConsoleServesNoRegistry(t *testing.T) { |
| + // cfg.Registry == "": a 503 naming the situation, never an empty |
| + // list — "you have no routes" would be a lie. |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run the tests to verify they fail** |
| + |
| +Run: `go test ./internal/console/ -run TestAPIRoutes -count=1` |
| +Expected: FAIL — `undefined: handleAPIRoutes`, and the endpoint 404s |
| +because nothing is registered. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +First, extract the health computation from `instanceHealth` so one |
| +function computes the word. In `internal/console/apiinstances.go`, replace |
| +`instanceHealth`'s final block (from `am, err := s.OwnerAppMetrics(...)` to |
| +its `return out`) with a call, and add the extracted function: |
| + |
| +```go |
| + for h, word := range s.healthByHost(ctx, routeFor) { |
| + out[h] = word |
| + } |
| + return out |
| +} |
| + |
| +// healthByHost is the metrics half of a health answer, shared by |
| +// instanceHealth (records joined to routes) and handleAPIRoutes (routes |
| +// directly), so the two — and the Running tab, which uses the same |
| +// healthWord — can never disagree about an instance's state. |
| +// |
| +// routeFor arrives ALREADY SCOPED to the caller's own account and app. |
| +// This function performs no tenant check of its own and must not be |
| +// handed an unfiltered registry listing: OwnerAppMetrics' allowlist is |
| +// exactly the hosts and stamped units it is given, so a foreign unit name |
| +// reaching unitByHost below would widen the metrics view past the |
| +// caller's own app. |
| +func (s *Server) healthByHost(ctx context.Context, routeFor map[string]registry.Route) map[string]string { |
| + out := make(map[string]string, len(routeFor)) |
| + hosts := make([]string, 0, len(routeFor)) |
| + unitByHost := map[string]string{} |
| + for h, rt := range routeFor { |
| + hosts = append(hosts, h) |
| + if rt.Unit != "" { |
| + unitByHost[h] = rt.Unit |
| + } |
| + } |
| + am, err := s.OwnerAppMetrics(ctx, hosts, unitByHost) |
| + if err != nil { |
| + // An unreadable sweep is not knowledge that these instances are |
| + // down: every host reads as not-reporting below, which healthWord |
| + // turns into "No readings" or "Not responding" per expectsReadings — |
| + // the same answer the page gives when the sweep is unavailable. |
| + log.Printf("console: health by host: metrics: %v", err) |
| + am = nil |
| + } |
| + rep := make(map[string]bool, len(am)) |
| + for _, m := range am { |
| + if m.Reporting() { |
| + rep[m.Host] = true |
| + } |
| + } |
| + for h, rt := range routeFor { |
| + status := rt.Status |
| + if status == "" { |
| + status = "ready" // registry contract: empty status means ready |
| + } |
| + out[h] = healthWord(rt.Kind, status, rt.Hibernate, rep[h], |
| + expectsReadings(rt.Provisioned, rt.Unit, rt.Backing)) |
| + } |
| + return out |
| +} |
| +``` |
| + |
| +Keep `instanceHealth`'s tenant-boundary doc comment and its `routeFor` |
| +construction exactly where they are — only the metrics half moves. |
| + |
| +Create `internal/console/apiroutes.go`: |
| + |
| +```go |
| +// The routes read (`carlos routes`, issue #64): what routes this app |
| +// actually has on this deployment, and — the reason the issue was |
| +// reopened — what named config environment each one is bound to. |
| +// |
| +// Before this, a route's environment binding could be read exactly one |
| +// way: sqlite3 against /data/registry.db on the box. `carlos route` is a |
| +// setter, `carlos apps` is console-side, and the instance-record API |
| +// answers for RECORDS, so a hand-added route (titogo's relay, the one that |
| +// needed the binding) had nothing to read at all. That made verifying the |
| +// scope of a sealed-credential delivery the one step of the whole |
| +// procedure needing box access, in a flow whose selling point is that a |
| +// developer needs no AWS anything — and it is why issue #196 took an hour |
| +// to see rather than a minute. |
| +// |
| +// GATE: apiAccountApp, the member-proving lookup, NOT the instance family's |
| +// instanceGate. An app token is a machine credential minted for one |
| +// instance to register itself; this answer describes every route of the |
| +// app — upstream addresses, owning units, environment bindings — which is |
| +// an operator's view, not a workload's. The narrower gate is the whole |
| +// difference, so do not "unify" it with the instance family's. |
| +// |
| +// SCOPE: the console reads its OWN box's registry, so in a multi-box fleet |
| +// the other boxes' routes do not appear here. That is the same blind spot |
| +// instanceHealth has today; internal/fleetapi is the eventual fix (#145) |
| +// and is unmerged. The CLI says so in its own help rather than letting a |
| +// short list read as a complete one. |
| +package console |
| + |
| +import ( |
| + "context" |
| + "log" |
| + "net/http" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// routeListTimeout bounds the metrics join, the only part of this handler |
| +// that can be slow: the registry is a local file. |
| +const routeListTimeout = 15 * time.Second |
| + |
| +// apiRoute is one row of the answer. Every field is drawn from the |
| +// registry row EXCEPT health, which is the same word instanceHealth and |
| +// the Running tab compute (healthByHost). |
| +type apiRoute struct { |
| + Host string `json:"host"` |
| + Kind string `json:"kind"` |
| + Upstream string `json:"upstream"` |
| + App string `json:"app"` |
| + Channel string `json:"channel"` |
| + Environment string `json:"environment"` |
| + Status string `json:"status"` |
| + Health string `json:"health"` |
| + Hibernate bool `json:"hibernate"` |
| + Unit string `json:"unit,omitempty"` |
| + Backing string `json:"backing,omitempty"` |
| + Provisioned bool `json:"provisioned"` |
| +} |
| + |
| +// routeUpstream renders the one cell that answers "where does this route |
| +// send traffic": a socket wins over addresses (registry.Route's own |
| +// precedence), a multi-upstream set is shown whole rather than reduced to |
| +// its first element, and a tunnel-backed route says so instead of |
| +// rendering an empty cell that would read as a broken row. |
| +func routeUpstream(rt registry.Route) string { |
| + switch { |
| + case rt.Socket != "": |
| + return rt.Socket |
| + case rt.Addrs != "": |
| + return rt.Addrs |
| + case rt.Addr != "": |
| + return rt.Addr |
| + case rt.Tunnel != "": |
| + return "tunnel:" + rt.Tunnel |
| + } |
| + return "" |
| +} |
| + |
| +func (s *Server) handleAPIRoutes(w http.ResponseWriter, r *http.Request) { |
| + _, acct, app, ok := s.apiAccountApp(w, r) |
| + if !ok { |
| + return |
| + } |
| + if s.cfg.Registry == "" { |
| + // Never an empty list: a console with no registry cannot know any |
| + // route's state, and "[]" would read as "this app has no routes". |
| + jsonError(w, http.StatusServiceUnavailable, |
| + "this console serves no route registry, so it cannot say what routes exist") |
| + return |
| + } |
| + reg, err := openRegistry(s.cfg.Registry) |
| + if err != nil { |
| + log.Printf("console: api routes: registry %s: %v", s.cfg.Registry, err) |
| + jsonError(w, http.StatusBadGateway, "could not read the route registry — try again") |
| + return |
| + } |
| + all, err := reg.List() |
| + if err != nil { |
| + log.Printf("console: api routes: registry list: %v", err) |
| + jsonError(w, http.StatusBadGateway, "could not read the route registry — try again") |
| + return |
| + } |
| + // The tenant boundary, and the only one: the registry read above |
| + // returned every route on this box, other accounts' included. A row |
| + // enters routeFor only if it is stamped to THIS app and resolves to |
| + // THIS account — release.RouteAccount, so an unstamped account column |
| + // resolves the same way every other read of it resolves rather than |
| + // being silently excluded or silently included. |
| + routeFor := map[string]registry.Route{} |
| + for _, rt := range all { |
| + if rt.App != app || release.RouteAccount(rt) != acct.Sqid { |
| + continue |
| + } |
| + routeFor[rt.Host] = rt |
| + } |
| + ctx, cancel := context.WithTimeout(r.Context(), routeListTimeout) |
| + defer cancel() |
| + health := s.healthByHost(ctx, routeFor) |
| + rows := make([]apiRoute, 0, len(routeFor)) |
| + for _, rt := range routeFor { |
| + rows = append(rows, apiRoute{ |
| + Host: rt.Host, Kind: rt.Kind, Upstream: routeUpstream(rt), App: rt.App, |
| + Channel: rt.Channel, Environment: rt.Environment, Status: rt.Status, |
| + Health: health[rt.Host], Hibernate: rt.Hibernate, Unit: rt.Unit, |
| + Backing: rt.Backing, Provisioned: rt.Provisioned, |
| + }) |
| + } |
| + // Sorted by host: registry order is not stable across passes, and a |
| + // listing that reshuffles between calls is hard to read a diff out of. |
| + sortRoutesByHost(rows) |
| + writeJSON(w, http.StatusOK, map[string]any{"routes": rows}) |
| +} |
| + |
| +func sortRoutesByHost(rows []apiRoute) { |
| + sort.Slice(rows, func(i, j int) bool { return rows[i].Host < rows[j].Host }) |
| +} |
| +``` |
| + |
| +Add `"sort"` to the import block. |
| + |
| +Register it in `internal/console/authcli.go`, immediately after the config |
| +family's three lines: |
| + |
| +```go |
| + // The routes read (apiroutes.go, issue #64) — the member-only door onto |
| + // this box's registry rows for one app, so a route's environment |
| + // binding can be read back without a box shell. NOT instanceGate: see |
| + // that file's package comment for why an app token must not reach it. |
| + s.mux.HandleFunc("GET /api/cli/apps/{account}/{app}/routes", s.handleAPIRoutes) |
| +``` |
| + |
| +- [ ] **Step 4: Run the tests to verify they pass** |
| + |
| +Run: `go test ./internal/console/ -count=1` |
| +Expected: PASS, including every pre-existing `instanceHealth` test — the |
| +extraction must not change a single health word. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add internal/console/apiroutes.go internal/console/apiroutes_test.go internal/console/authcli.go internal/console/apiinstances.go |
| +git commit -m "console: GET /api/cli/apps/{account}/{app}/routes (#64)" |
| +``` |
| + |
| +--- |
| + |
| +### Task 8: `carlos routes` |
| + |
| +**Files:** |
| +- Create: `routes_cli.go` |
| +- Modify: `apimode.go` (add the client method beside `instanceList` at |
| + :1838), `cli.go` (verb dispatch — find the switch that routes |
| + `"instances"` to `runInstances` and add `"routes"` beside it), and the |
| + top-level usage/help text that lists verbs |
| +- Test: `routes_cli_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `apiPath(a apiAcct, app, tail string) string`, |
| + `s.getJSON(ctx, path) (int, []byte, error)`, |
| + `apiErrorFromBody(status, body) error`, `requireAPISession(verb, |
| + consoleFlag) *apiSession`, `resolvedAccount`, `resolvedApp`, |
| + `newTable(stdout)`, `printUsing`, `mutedf`. |
| +- Produces: |
| + `type apiRouteRow struct` (client mirror of the wire shape); |
| + `func (s *apiSession) routeList(ctx context.Context, a apiAcct, app string) ([]apiRouteRow, error)`; |
| + `func runRoutes(args []string)`. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `routes_cli_test.go`. Follow the root package's existing |
| +CLI-against-a-fake-console pattern (read `instances_cli_test.go` first and |
| +reuse its fake-console harness rather than building a second one). |
| + |
| +```go |
| +func TestRoutesListsTheEnvironmentColumn(t *testing.T) { |
| + // A fake console answering {"routes":[{"host":"connect.go.tito.io", |
| + // "environment":"relay",...}]} must produce a table whose row carries |
| + // both the host and "relay" — this is the read issue #64 exists for. |
| +} |
| + |
| +func TestRoutesRefusesDirectStoreMode(t *testing.T) { |
| + // With CARLOS_DEPLOYMENT_BUCKET set and no session, the verb refuses by |
| + // name: the registry lives on a box, and the box-side listing is |
| + // `carlos instances`. Assert the refusal names that command. |
| +} |
| + |
| +func TestRoutesSaysWhenTheAppHasNoRoutes(t *testing.T) { |
| + // An empty list prints a line saying so, not an empty table — and the |
| + // line mentions that only this deployment's own box is covered. |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run the tests to verify they fail** |
| + |
| +Run: `go test . -run TestRoutes -count=1` |
| +Expected: FAIL — `undefined: runRoutes`. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +Add to `apimode.go`, beneath `instanceList`: |
| + |
| +```go |
| +// apiRouteRow is one row of the console's routes answer |
| +// (internal/console/apiroutes.go). Every field is additive on the wire: a |
| +// console that predates one sends nothing and it decodes as the zero |
| +// value, which is why the table below prints "—" for an empty ENVIRONMENT |
| +// rather than treating absence as a binding. |
| +type apiRouteRow struct { |
| + Host string `json:"host"` |
| + Kind string `json:"kind"` |
| + Upstream string `json:"upstream"` |
| + App string `json:"app"` |
| + Channel string `json:"channel"` |
| + Environment string `json:"environment"` |
| + Status string `json:"status"` |
| + Health string `json:"health"` |
| + Hibernate bool `json:"hibernate"` |
| + Unit string `json:"unit"` |
| + Backing string `json:"backing"` |
| + Provisioned bool `json:"provisioned"` |
| +} |
| + |
| +// routeList reads one app's routes on the console's own box (issue #64). |
| +func (s *apiSession) routeList(ctx context.Context, a apiAcct, app string) ([]apiRouteRow, error) { |
| + status, body, err := s.getJSON(ctx, apiPath(a, app, "/routes")) |
| + if err != nil { |
| + return nil, err |
| + } |
| + if status != http.StatusOK { |
| + return nil, apiErrorFromBody(status, body) |
| + } |
| + var res struct { |
| + Routes []apiRouteRow `json:"routes"` |
| + } |
| + if err := json.Unmarshal(body, &res); err != nil { |
| + return nil, fmt.Errorf("routes: unreadable response: %w", err) |
| + } |
| + return res.Routes, nil |
| +} |
| +``` |
| + |
| +Create `routes_cli.go`: |
| + |
| +```go |
| +// `carlos routes` — what routes this app has, and what each one is bound |
| +// to (issue #64). |
| +// |
| +// API MODE ONLY, and that is the point of the verb existing at all. The |
| +// facts it prints live in a box's registry, and reading them used to mean |
| +// sqlite3 on the box: `carlos route` is a setter, `carlos apps` answers |
| +// from the console's app list, and `carlos instances list` answers for |
| +// instance RECORDS, so a hand-added route had no reader anywhere. Since a |
| +// route's ENVIRONMENT binding is what decides which sealed secrets it |
| +// gets, "which routes is this credential delivered to" was a question only |
| +// answerable with box access — the one step of titogo's whole secrets |
| +// procedure that needed it. |
| +// |
| +// The box-side listing is `carlos instances` (cli.go), unchanged: an |
| +// operator standing on the box already has the registry in front of them, |
| +// and a second box-local verb answering the same question would be two |
| +// commands to keep in step for no gain. |
| +package main |
| + |
| +import ( |
| + "context" |
| + "flag" |
| + "fmt" |
| + "log" |
| + "os" |
| + "strings" |
| +) |
| + |
| +const routesUsage = `usage: carlos routes -app <a> [-account <sqid|name>] [-match <substr>] [-console <origin>] |
| + |
| +Lists the routes this app has on the console's deployment: where each one |
| +sends traffic, which release channel it follows, and which named config |
| +environment it is bound to (the binding that decides which secrets it |
| +gets — see carlos env, carlos secrets). |
| + |
| +Signed-in only (carlos auth login). On a box, the local listing is |
| +"carlos instances".` |
| + |
| +func runRoutes(args []string) { |
| + fs := flag.NewFlagSet("routes", flag.ExitOnError) |
| + app := fs.String("app", "", "app whose routes to list") |
| + match := fs.String("match", "", "only hosts containing this substring") |
| + consoleFlag := fs.String("console", "", "console to list through (default: the only logged-in one)") |
| + acctRaw := fs.String("account", "", "account (sqid or name) the app belongs to (default: your only account, or the configured/chosen one)") |
| + acctIDRaw := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account") |
| + fs.Parse(args) |
| + sess := requireAPISession("routes", *consoleFlag) |
| + acct := resolvedAccount(sess, acctRaw, acctIDRaw) |
| + app2 := resolvedApp(sess, acct, *app, routesUsage) |
| + rows, err := sess.routeList(context.Background(), acct, app2) |
| + if err != nil { |
| + log.Fatalf("routes: %v", err) |
| + } |
| + shown := 0 |
| + w := newTable(stdout) |
| + fmt.Fprintln(w, "HOST\tKIND\tUPSTREAM\tCHANNEL\tENVIRONMENT\tSTATUS\tHEALTH") |
| + for _, r := range rows { |
| + if *match != "" && !strings.Contains(r.Host, *match) { |
| + continue |
| + } |
| + shown++ |
| + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\n", |
| + r.Host, dashIfEmpty(r.Kind), dashIfEmpty(r.Upstream), dashIfEmpty(r.Channel), |
| + // An empty ENVIRONMENT is a real answer — the route takes the |
| + // app's default bundle — so it prints as a dash rather than a |
| + // blank cell an operator would read as missing data. |
| + dashIfEmpty(r.Environment), dashIfEmpty(r.Status), dashIfEmpty(r.Health)) |
| + } |
| + w.Flush() |
| + if shown == 0 { |
| + mutedf(stdout, "no routes for %s on this deployment's box", app2) |
| + } |
| + // Said every time, not only when the list is empty: this console reads |
| + // its OWN box's registry, so on a multi-box fleet a short answer is not |
| + // a complete one, and an operator must not read it as one (#145). |
| + mutedf(stdout, "(routes on this deployment's own box; other boxes in a fleet are not listed)") |
| +} |
| + |
| +// dashIfEmpty renders an empty cell as "—" so a column that is legitimately |
| +// blank reads as an answer rather than as missing data. |
| +func dashIfEmpty(s string) string { |
| + if s == "" { |
| + return "—" |
| + } |
| + return s |
| +} |
| +``` |
| + |
| +If `dashIfEmpty` (or an equivalent) already exists in the root package, |
| +use that one and delete this copy — check before adding. |
| + |
| +Register the verb in `cli.go`'s dispatch switch beside `"instances"`, and |
| +add it to the top-level usage text: |
| + |
| +``` |
| + routes list an app's routes and what each is bound to |
| +``` |
| + |
| +- [ ] **Step 4: Run the tests to verify they pass** |
| + |
| +Run: `go test . -count=1` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add routes_cli.go routes_cli_test.go apimode.go cli.go |
| +git commit -m "cli: carlos routes (#64)" |
| +``` |
| + |
| +--- |
| + |
| +### Task 9: The box-side listing shows the binding, and the docs catch up |
| + |
| +**Files:** |
| +- Modify: `cli.go:284-310` (the bare `carlos instances` table) |
| +- Modify: `README.md` (the command list, if it enumerates verbs — check |
| + first) |
| +- Test: `cli_test.go` (or wherever the bare listing is already tested) |
| + |
| +**Interfaces:** |
| +- Consumes: nothing new. |
| +- Produces: nothing new. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +```go |
| +func TestInstancesListingShowsTheEnvironmentBinding(t *testing.T) { |
| + // A registry with one route bound to "relay": the bare `carlos |
| + // instances` table must carry an ENV column and the value, so an |
| + // operator standing on the box can read the binding without sqlite3. |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run the test to verify it fails** |
| + |
| +Run: `go test . -run TestInstancesListing -count=1` |
| +Expected: FAIL — no ENV column. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +In `cli.go`'s `runInstances` bare-listing branch: |
| + |
| +```go |
| + // BACKING and UNIT are the two halves of one answer — who owns this |
| + // route's process — so they print side by side: a route with a UNIT is |
| + // cycled by that unit on adoption, whatever BACKING says (and BACKING is |
| + // empty for every such route, which is the shape that made issue #110 |
| + // invisible from this listing). |
| + // |
| + // ENV is the named config bundle the route is bound to (issue #64): it |
| + // decides which secrets this route's process is delivered, and reading |
| + // it used to mean sqlite3 against the registry. Empty means the app's |
| + // default bundle, which is a real answer, so it prints blank rather |
| + // than being omitted. |
| + fmt.Fprintln(w, "HOST\tAPP\tKIND\tCHANNEL\tENV\tSTATUS\tVERSION\tHIB\tBACKING\tUNIT") |
| + for _, t := range routes { |
| + hib := "" |
| + if t.Hibernate { |
| + hib = "yes" |
| + } |
| + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n", |
| + t.Host, t.App, t.Kind, t.Channel, t.Environment, t.Status, t.Version, hib, t.Backing, t.Unit) |
| + } |
| +``` |
| + |
| +- [ ] **Step 4: Run the tests to verify they pass** |
| + |
| +Run: `go test ./... -count=1` |
| +Expected: PASS, every package. |
| + |
| +- [ ] **Step 5: Full gate, then commit** |
| + |
| +```bash |
| +go build ./... && go vet ./... && go test ./... -count=1 |
| +gofmt -l $(git diff --name-only main... | grep '\.go$') |
| +git add -A |
| +git commit -m "cli: carlos instances shows a route's environment binding (#64)" |
| +``` |
| + |
| +--- |
| + |
| +### Task 10: Branch close-out |
| + |
| +**Files:** |
| +- Modify: the spec (`docs/superpowers/specs/2026-08-16-route-scoped-config-delivery-design.md`) |
| + only if implementation diverged from it — the spec must describe what |
| + was built, not what was planned. |
| + |
| +- [ ] **Step 1: Run the full gate one more time from a clean tree** |
| + |
| +```bash |
| +go build ./... && go vet ./... && go test ./... -count=1 |
| +GOOS=darwin go build ./... |
| +``` |
| + |
| +The `GOOS=darwin` compile is not optional: `internal/host` links into the |
| +CLI, which people run on Macs, and CI runs this. |
| + |
| +- [ ] **Step 2: Reconcile the spec with what was built** |
| + |
| +Read the spec section by section against the diff. Fix any place the |
| +implementation went a different way. Commit any edits. |
| + |
| +- [ ] **Step 3: Push the branch and open the PR** |
| + |
| +```bash |
| +git push -u origin titogo-bugs |
| +gh pr create --title "Route-scoped config delivery: exec_delivery per environment, fast rebinding convergence, carlos routes" --body "$(cat <<'EOF' |
| +Closes #195, #196, #64 — the three defects in titogo's "deliver one sealed |
| +credential to one service" runbook. |
| + |
| +**#195** `exec_delivery` is read from the bundle a route's config actually |
| +resolves through: the route's named environment when it states a value, |
| +the app's default bundle otherwise. Additive — every deployment on an |
| +app-level `file` today delivers to exactly the same routes. Reverses half |
| +of the 2026-08-03 issue #18 ruling (ruled 2026-08-16); route scope is |
| +still refused, and the comment says why. |
| + |
| +**#196** The agent's fast config loop fingerprints the registry-side facts |
| +that decide what config a route gets, so binding a route to an environment |
| +converges in ~3s instead of waiting out the hourly floor. Covers both |
| +doors onto the column — `carlos route -environment` and the reconciler |
| +converging `Environment` from an instance record — where the fix suggested |
| +in the issue would have missed the second. |
| + |
| +**#64** `carlos routes` over a new member-only |
| +`GET /api/cli/apps/{account}/{app}/routes`, so a route's environment |
| +binding can be read back without a box shell. Works for hand-added routes |
| +with no instance record, which is the case that made this urgent. The bare |
| +box-side `carlos instances` listing gains an ENV column too. |
| + |
| +Known limitation, stated in the code and the CLI's own help: the console |
| +reads its own box's registry, so other boxes in a fleet are not listed |
| +(#145). |
| + |
| +Spec: `docs/superpowers/specs/2026-08-16-route-scoped-config-delivery-design.md` |
| + |
| +🤖 Generated with [Claude Code](https://claude.com/claude-code) |
| + |
| +https://claude.ai/code/session_015gyhxeoXGBW4WCFRnMqBue |
| +EOF |
| +)" |
| +``` |
| + |
| +- [ ] **Step 4: Post the migration note on #195** |
| + |
| +titogo's migration has a manual step the issue does not mention. Comment |
| +on #195 with it: turning the app-level flag off leaves the router routes' |
| +`*.config.env` files in place (they hold decrypted secrets and carlos |
| +cannot know when the app's supervisor stopped reading them), naming them |
| +in the log every pass until deleted by hand. |
| + |
| +--- |
| + |
| +## Deploy (after the PR merges — a human's action, not part of this plan) |
| + |
| +Ship the platform binary to both boxes, promote, adopt, restart the |
| +consoles for the API half. The agent half takes effect on the agent's next |
| +start; the `exec_delivery` half on the next pass after that. Verify with |
| +`carlos routes -app titogo` from off-box, which is the whole point of #64. |
diff --git a/docs/superpowers/specs/2026-08-16-route-scoped-config-delivery-design.md b/docs/superpowers/specs/2026-08-16-route-scoped-config-delivery-design.md| new file mode 100644 |
| index 000000000..67c44164e |
| --- /dev/null |
| +++ b/docs/superpowers/specs/2026-08-16-route-scoped-config-delivery-design.md |
| @@ -0,0 +1,413 @@ |
| +# Route-scoped config delivery — design |
| + |
| +2026-08-16. Closes #195, #196, #64. |
| + |
| +Three issues filed from titogo, all from one runbook: deliver one sealed |
| +credential to one service without hand-writing a file on the box. The |
| +procedure is `carlos secrets set -environment relay`, then `carlos route |
| +-host connect.go.tito.io -environment relay`, then `carlos env |
| +exec-delivery -app titogo file`. Each of the three steps has a defect, and |
| +they compose into an hour lost to a symptom that reads as a bad credential. |
| + |
| +| # | Defect | Fix | |
| +|---|--------|-----| |
| +| #195 | `exec_delivery` is app-level, so opting one route in opts the whole fleet in | Read the flag from the route's bound environment bundle, inheriting the app default | |
| +| #196 | Binding a route to an environment doesn't re-materialize its config for up to an hour | Fingerprint the in-scope route set in the agent's fast loop | |
| +| #64 | A route's environment binding cannot be read back off-box | `carlos routes`, over a new account-scoped CLI API endpoint | |
| + |
| +The three share a branch and nothing else: no file is touched by more than |
| +one of them. |
| + |
| +## 1. Environment-scoped `exec_delivery` (#195) |
| + |
| +### 1.1 What the flag does today |
| + |
| +`SyncExec` (`internal/appconfig/syncexec.go`) materializes a route's merged |
| +config into `<dataDir>/<host>.config.env`. Two route shapes reach it: |
| + |
| +- **socket-exec** (`execSyncable`): the activator spawns the process, so |
| + carlos owns it. In scope unconditionally, and bounced when the rendered |
| + file changes. |
| +- **TCP-addr exec** (`execAddrCandidate`, issue #18): carlos proxies to |
| + `127.0.0.1:<port>` but neither spawns nor owns the process — titogo's |
| + router fleet. In scope only if the app opted in, and **never** bounced; |
| + the file carries `execAddrConfigHeader` saying so, and the app's own |
| + supervisor owns noticing it. |
| + |
| +`Manifest.ExecDelivery` (`"exec_delivery": "file"`) is that opt-in. It is |
| +read from the app's default bundle, `apps/<acct>/<app>/config.json`, alone. |
| + |
| +The flag decides **whether a file is written for a route**. It has never |
| +had anything to do with the file's *contents*: those come from |
| +`LoadMerged`, the default bundle overlaid by whatever environment the route |
| +is bound to, and that half has been per-route since it shipped. |
| + |
| +### 1.2 The defect |
| + |
| +Because the flag is read from one app-level object, it answers "does this |
| +**app** get files" — one answer for every TCP-addr exec route of the app on |
| +the box. titogo needed sealed Apple Wallet material delivered to |
| +`connect.go.tito.io` alone; turning the flag on wrote a file for every |
| +router route too. Harmless in practice (nothing reads them) but it is a |
| +fleet-wide switch thrown to deliver one service's credential, and it has to |
| +be explained every time someone reads the runbook. |
| + |
| +### 1.3 The ruling this reverses |
| + |
| +`appconfig.go:35-50` records the issue #18 design comment (2026-08-03): |
| + |
| +> APP-LEVEL ONLY [...] a named environment's manifest is never a valid |
| +> place for this field [...] The reader is the app's own supervisor, one |
| +> program per app, not per-route or per-environment: per-route or |
| +> per-environment opt-in would let half a fleet read config the other half |
| +> silently ignores, the "config-that-lies" failure mode. |
| + |
| +The premise — one supervisor program per app — is false in the deployment |
| +that motivated the feature. The `titogo` app supervises the relay and the |
| +router fleet through different units. Splitting delivery along the |
| +environment binding is therefore not "half a fleet ignoring config": it is |
| +the same axis the operator already chose when they bound the route, and the |
| +same axis the secrets themselves are scoped on. |
| + |
| +**Ruled 2026-08-16 (Paul):** environment scope is allowed; **route** scope |
| +is still refused. A per-route flag would need a registry column and — per |
| +the 2026-08-10 rule that a `Provisioned` route's Channel and Environment |
| +belong to its instance record — a record field and reconcile convergence |
| +too, three doors onto one bit. The environment bundle is one door onto a |
| +scope that already exists. |
| + |
| +### 1.4 Resolution |
| + |
| +`checkOptedIn` is keyed on `(account, app, environment)` instead of |
| +`(account, app)`. For a route bound to environment `e`: |
| + |
| +| Bundle state | Verdict | |
| +|--------------|---------| |
| +| route bound to no environment | app default bundle's value (today's behaviour, unchanged) | |
| +| `e.json` carries `exec_delivery` | that value decides | |
| +| `e.json` is silent on it | **inherit** the app default | |
| +| `e.json` absent, or unreadable | inherit / unresolved, as §1.6 | |
| + |
| +Inheritance is what keeps this additive: every deployment carrying an |
| +app-level `file` today delivers to exactly the same routes after the |
| +upgrade. An operator narrows by turning the app-level flag off and setting |
| +`file` on the one bundle. |
| + |
| +`LoadMerged` is **not** changed: the merged manifest stays vars-only and |
| +never carries `ExecDelivery` forward. The opt-in is resolved by |
| +`checkOptedIn` reading the two bundles' fields directly, not by merging |
| +them. Two reasons: the merged manifest is what gets *rendered into a file*, |
| +and a delivery flag has no business appearing there; and `LoadMerged` is |
| +shared with the unit path (`sync.go`), which has no opt-in concept at all. |
| + |
| +### 1.5 CLI |
| + |
| +``` |
| +carlos env exec-delivery -app <a> file|off (unchanged) |
| +carlos env exec-delivery -app <a> -environment <e> file|inherit (new) |
| +``` |
| + |
| +At environment scope the spellings are `file` and `inherit`, and `off` is |
| +**refused there by name**, with a message pointing at the app-level form. |
| +In a bundle, clearing the field means "inherit"; a verb that spelled that |
| +`off` would read as "stop delivering" while doing the opposite. |
| + |
| +An explicit per-environment override-to-off has no caller today and is not |
| +built. Leaving it out keeps the value space free for a `"none"` sentinel |
| +later, which an older binary already handles correctly: its unknown-value |
| +branch logs once and falls back to not-opted-in, which is what `none` would |
| +mean. |
| + |
| +`carlos env list -app <a> -environment <e>` prints the bundle's own |
| +`exec-delivery:` line when the field is set, and says nothing when it is |
| +not (inherit is the absence of a statement, and printing the inherited |
| +value would make the bundle look like it holds one). `printEnvList` |
| +already receives the field for a named bundle and suppresses it; the |
| +suppression is what goes. |
| + |
| +### 1.5.1 Console door |
| + |
| +`carlos env exec-delivery` has an API mode, so the console's config door |
| +moves with it (`internal/console/apiconfig.go`): |
| + |
| +- **GET** currently emits `exec_delivery` only when `envName == ""`. It |
| + emits the named bundle's own field too, still omitting it when unset. |
| +- **POST** currently refuses `exec_delivery` outright for a named bundle |
| + ("exec_delivery is app-level only"). It accepts `"file"` and a new |
| + `"inherit"` at environment scope, and refuses `"off"` there by name — |
| + the same refusal the CLI gives, worded once on the server so the |
| + operator reads the server's sentence rather than a paraphrase. The |
| + **mirror refusal** is enforced too: `"inherit"` with no environment is a |
| + 400, because the default bundle has nothing above it to inherit from and |
| + silently treating the word as `"off"` would be the same class of quiet |
| + lie in the other direction. |
| + |
| +The audit row records the caller's **own word** (`file` / `off` / |
| +`inherit`) and the bundle it applied to. Both clearing words store `""`, |
| +so a row rebuilt from the stored value would lose which scope was acted |
| +on. |
| + |
| +The two-CAS-writes shape of an apply-plus-exec_delivery request is |
| +unchanged; the second write becomes `SetExecDeliveryAt` against the |
| +bundle's own key. |
| + |
| +### 1.6 Failure modes |
| + |
| +Unchanged in kind from today, extended to the second bundle: |
| + |
| +- **Env bundle fetch error** → `known: false`, the "unresolved, leaving |
| + this app's TCP-addr route file(s) as-is" branch. Not a determination; the |
| + file is untouched and the next pass retries. |
| +- **Env bundle absent while a route is bound to it** → inherit the app |
| + default for the opt-in question. The route's *resolution* errors anyway |
| + (`LoadMerged` refuses a bound-but-absent bundle) and its file is left |
| + as-is, so the opt-in verdict never reaches a write in this case. |
| +- **Unrecognized value in an env bundle** → same additive discipline as the |
| + default bundle: log once per `(acct, app, env)` per pass, treat as not |
| + opted in. |
| + |
| +The once-per-pass logging discipline moves from per-`(acct, app)` to |
| +per-`(acct, app, env)`, so a box with many routes on one app still logs |
| +once per distinct bundle it consulted, never once per route. |
| + |
| +### 1.7 Migration cost, stated |
| + |
| +Turning the app-level flag off does **not** delete the router routes' |
| +`*.config.env` files. That is the standing ruling at `syncexec.go:343-387`: |
| +an un-opted route's file is frozen, warned about once per pass, and never |
| +removed, because deleting it would silently change what the app's own |
| +supervisor layers in at a moment carlos cannot announce. titogo's migration |
| +therefore ends with a hand deletion of the router files once their |
| +supervisor has stopped reading them, and a per-pass log line until then. |
| +Existing behaviour; it lands on them because of this change, so the issue |
| +and the runbook say so. |
| + |
| +### 1.8 Fast loop |
| + |
| +`ExecConfigApps` is unaffected: it is the route-*shape* half only and |
| +deliberately does not consult the opt-in (consulting it would cost a bucket |
| +read per app per tick, which is what the fast loop exists to avoid). The |
| +generation object is per-app and already covers every bundle |
| +(`applyEnvironment` touches it), so an environment bundle write already |
| +signals correctly. |
| + |
| +## 2. The fast loop watches the registry (#196) |
| + |
| +### 2.1 The defect |
| + |
| +`genPollPass` (`internal/host/agent.go:625`) decides "did anything change" |
| +from bucket generation etags alone. A route's `environment` lives in the |
| +box's own registry, so a binding change moves no etag and the fast loop |
| +cannot see it. Delivery waits for `configSyncLoop`'s hourly floor. |
| + |
| +Reproduced on `carlos-tito-ie` 2026-08-15: route bound at 06:11, file still |
| +holding the default bundle at 06:18. The nudge that fixed it was re-writing |
| +three identical secret values — which bumped `config-gen.json` and gave the |
| +fast loop the etag movement it was waiting for. |
| + |
| +The natural order of the documented flow is exactly the order that fails: |
| +seal the values, then bind the route. Everything then *looks* right — the |
| +bundle exists, the binding is right, `carlos secrets list` shows the keys — |
| +and the process reads a file with none of them. The only symptom is the |
| +app's own startup line saying the credential is missing, which reads as a |
| +bad credential rather than an undelivered one. |
| + |
| +### 2.2 Why not bump the generation from `carlos route` |
| + |
| +The issue proposes it, and it is the wrong door. Since 2026-08-10 a |
| +`Provisioned` route's Environment belongs to its **instance record**, and |
| +the reconciler converges it from `apps/<sqid>/<app>/instances/<host>.json` |
| +on every pass — `carlos route -environment` refuses such a row outright. |
| +So the sanctioned way to change a route's environment does not go through |
| +`carlos route` at all, and a fix there would miss it. It would also need |
| +bucket write credentials in a box-local verb that has never had them. |
| + |
| +The agent-side fix covers both doors, needs no new credential, and catches |
| +any third door added later. |
| + |
| +### 2.3 Fix |
| + |
| +New in `internal/appconfig`, beside `ExecConfigApps` and `UnitConfigApps` |
| +so the eligibility predicates cannot drift from the sweeps that use them: |
| + |
| +```go |
| +// ConfigRouteFingerprints returns one digest per delivery half over the |
| +// registry-side facts that decide what config a route gets. |
| +func ConfigRouteFingerprints(routes []registry.Route) (exec, unit string) |
| +``` |
| + |
| +For each route passing that half's predicate (`execSyncable || |
| +execAddrCandidate` for exec; `syncable && Backing == BackingUnit` for |
| +unit), contribute `host|account|app|environment`; sort; hash. |
| + |
| +Set membership carries scope changes for free: a route entering or leaving |
| +scope moves the digest exactly as a rebinding does, so a route that becomes |
| +`ready`, gains a `Unit`, or is deleted also gets a prompt pass. |
| + |
| +`genPollPass` keeps the two previous digests alongside its etag cache and |
| +ORs a movement into that half's `changed` flag. First sight records |
| +silently, matching the existing boot rule — a box coming up must not bounce |
| +every instance it has because it has never seen the digest before; |
| +`configSyncLoop`'s ~1min first pass owns boot convergence. |
| + |
| +The opt-in is deliberately **not** in the fingerprint, for the reason |
| +`ExecConfigApps` gives: it costs a bucket read per app per tick. An |
| +`exec_delivery` change is a bundle write, which bumps the generation object |
| +already. |
| + |
| +Result: convergence goes from ≤1 hour to ~3s, through both doors. A |
| +spurious digest movement costs one pass that writes nothing — `SyncExec` |
| +and `Sync` both compare rendered bytes and act only on a real change, so it |
| +can never cost a restart. |
| + |
| +### 2.4 A consequence, recorded during implementation |
| + |
| +Set membership carrying scope changes means a route **leaving and |
| +re-entering** the registry now moves the digest, so the tick that re-adds |
| +it is no longer silent. Two existing tests asserted that silence |
| +(`TestGenPollPassDropsVanishedApps`, `TestGenPollPassPrunesUnitAppsToo`), |
| +and their assertion moved to the **settled tick after** the re-add. |
| + |
| +The ruling those tests encode is unchanged and still pinned directly: a |
| +vanished app's etag cache entry is dropped, so a re-add is a first sight |
| +rather than a comparison against a remembered etag. What is no longer |
| +separately observable from those tests is the etag half's *silence*, since |
| +the shape movement covers it either way. Firing there is correct on its own |
| +terms — a route entering or leaving scope changes what belongs on disk, a |
| +file to write or one to reap — and the pass it triggers resolves each case |
| +properly rather than acting on the digest. |
| + |
| +`genPollPass`'s signature took a `*genCache` (etags plus the two digests |
| +plus a `shapeSeen` bool) rather than growing a second parameter. The bool is |
| +not redundant with an empty digest: a box with no config-carrying routes has |
| +a real, stable digest of the empty set. |
| + |
| +## 3. `carlos routes` (#64) |
| + |
| +### 3.1 The defect |
| + |
| +After binding a route to an environment there is no way to confirm it. |
| +`carlos route` is a setter, `carlos apps` is console-side, and the console |
| +API exposes channels but not routes — so verifying the scope of a |
| +credential delivery means `sqlite3 /data/registry.db` on the box. That is |
| +the one step in titogo's whole secrets procedure that still needs box |
| +access, in a flow whose selling point is that a developer needs no AWS |
| +anything. It is also why #196 took an hour to see. |
| + |
| +### 3.2 API |
| + |
| +``` |
| +GET /api/cli/apps/{account}/{app}/routes |
| +``` |
| + |
| +Registered in the existing CLI-API family (`internal/console/authcli.go`) |
| +so it inherits `apiAccountApp`'s membership proof and the family's |
| +byte-identical 404 verbatim: an unknown account, someone else's account, an |
| +unclaimed app, and an app token probing a foreign path are one answer. |
| + |
| +Served from the console's own registry — the same open-and-list |
| +`instanceHealth` already does — filtered to |
| +`rt.App == app && release.RouteAccount(rt) == acct`. That is the tenant |
| +boundary the reconciler itself uses, and the account fallback is |
| +`release.RouteAccount`'s, the same resolution every other read of a route's |
| +account performs. |
| + |
| +Response rows carry: `host`, `kind`, `upstream` (socket, then the |
| +multi-upstream set, then the legacy addr, then `tunnel:<name>`), `app`, |
| +`channel`, `environment`, `status`, `health`, `hibernate`, `unit`, |
| +`backing`, `provisioned`. Sorted by host, because registry order is not |
| +stable across passes. |
| + |
| +`health` is `healthWord`, computed the same way the Running tab and |
| +`carlos instances list` compute it, so the three cannot disagree. To make |
| +that structural rather than a promise, the metrics half of |
| +`instanceHealth` is **extracted** as `healthByHost(ctx, routeFor)` and both |
| +doors call it. The tenant scoping stays in each caller, beside the reason |
| +that caller's rows are the caller's to see; `healthByHost` does no check of |
| +its own and carries a comment saying it must never be handed an unfiltered |
| +registry listing (`OwnerAppMetrics`' allowlist is exactly the hosts and |
| +units it is given). |
| + |
| +A console with no `Registry` configured cannot know any route's state; it |
| +answers with an explicit "this console serves no registry" rather than an |
| +empty list, which would read as "you have no routes". |
| + |
| +### 3.3 CLI |
| + |
| +``` |
| +carlos routes -app <a> [-account <sqid|name>] [-match <substr>] [-console <origin>] |
| +``` |
| + |
| +API mode only, like the instance record verbs: a routes listing is a |
| +console-authorized read, and the box-local operator equivalent already |
| +exists (§3.4). Columns: |
| + |
| +``` |
| +HOST KIND UPSTREAM CHANNEL ENVIRONMENT STATUS HEALTH |
| +``` |
| + |
| +This works for hand-added routes with no instance record — which is exactly |
| +`connect.go.tito.io`, so adding an ENVIRONMENT column to `carlos instances |
| +list` (record-backed) would not have closed this issue. |
| + |
| +### 3.4 Box side |
| + |
| +The bare `carlos instances` listing (`cli.go:260`) is already #64's |
| +"registry mode": host, app, kind, channel, status, version, hibernate, |
| +backing, unit. It gains one column, `ENV`. A second box-local verb |
| +answering the same question is not built. |
| + |
| +### 3.5 Limitation, stated not papered over |
| + |
| +The console reads *its own box's* registry, so in a multi-box fleet the |
| +other boxes' routes do not appear. This is the same blind spot |
| +`instanceHealth` has today. `internal/fleetapi` is the eventual fix and is |
| +unmerged (#145). The API doc comment says so, and the CLI prints the |
| +caveat as a line **on every call**, not only when the list is empty — a |
| +short answer on a fleet is not a complete one, and the reader has no way to |
| +tell from the rows themselves. |
| + |
| +## 4. Testing |
| + |
| +TDD throughout, against the suites that already own these paths. |
| + |
| +- `internal/appconfig/syncexec_test.go` — the §1.4 resolution matrix as a |
| + table: unbound/bound × default on/off × bundle on/inherit/absent/unknown, |
| + asserting which hosts get a file. Plus: the file's *contents* are |
| + unchanged by the flag's scope, and a bundle fetch error leaves an |
| + existing file byte-identical. |
| +- `internal/appconfig/environments_test.go` — `SetExecDelivery` at |
| + environment scope round-trips, refuses `off`, refuses an invalid value, |
| + and bumps the generation object. |
| +- `appconfig_cli_test.go` / `e2e_test.go` — `-environment` + `off` is |
| + refused by name; `-environment` + `inherit` clears the field. |
| +- `internal/console/apiconfig_test.go` — GET emits a named bundle's own |
| + `exec_delivery` and omits it when unset; POST accepts `file`/`inherit` |
| + at environment scope and refuses `off` there with the server's own |
| + sentence. |
| +- A new fingerprint test beside `genPollPass`'s existing tests: a rebinding |
| + moves the right half's digest and not the other's; first sight is silent; |
| + a route leaving scope moves the digest; an unrelated column (Version) |
| + does not. |
| +- `internal/console/apiinstances_test.go` gate patterns applied to the new |
| + endpoint: non-member gets the 404, an app token bound elsewhere gets the |
| + 404, a member sees only their own app's rows, a foreign-stamped row on |
| + the same box never appears. |
| +- `carlosTestEnv` + a scratch `cmd.Dir` for anything shelling out to the |
| + binary (CLAUDE.md's rule; the fallback is the operator's real login). |
| + |
| +Gate before any commit lands: `go build ./...`, `go vet ./...`, |
| +`go test ./... -count=1`, `gofmt -l` clean on touched files. |
| + |
| +## 5. Deploy |
| + |
| +Ordinary: PR, merge, ship the platform binary to both boxes, promote, |
| +adopt, restart the consoles for the API half. The agent half (§2) takes |
| +effect on the next agent restart; the `exec_delivery` half (§1) takes |
| +effect on the next pass after that. |
| + |
| +No wire-shape migration: `exec_delivery` in an environment bundle is |
| +additive to schema 1, an older binary parsing such a bundle ignores the |
| +field entirely (it reads the flag from the default bundle only), and a |
| +newer binary reading an older deployment sees no environment-scoped field |
| +and inherits — which is today's behaviour exactly. |
diff --git a/e2e_test.go b/e2e_test.go| index 27a7fa66e..9e63d7cb3 100644 |
| --- a/e2e_test.go |
| +++ b/e2e_test.go |
| @@ -1988,10 +1988,11 @@ func TestEnvironmentBundlesCLI(t *testing.T) { |
| } |
| } |
| -// TestExecDeliveryCLI exercises `carlos env exec-delivery` (issue #18): |
| -// the app-level opt-in that lets SyncExec materialize |
| -// /data/<host>.config.env for TCP-addr exec routes — round-trip, unknown |
| -// value refusal, and named-environment placement refusal, following |
| +// TestExecDeliveryCLI exercises `carlos env exec-delivery` (issues #18, |
| +// #195): the opt-in that lets SyncExec materialize /data/<host>.config.env |
| +// for TCP-addr exec routes — round-trip at app scope, round-trip at |
| +// environment scope, unknown-value refusal, and the two scope-specific |
| +// clearing words refusing each other's ("off" vs "inherit"), following |
| // TestEnvCLI's build-a-real-binary pattern. |
| func TestExecDeliveryCLI(t *testing.T) { |
| goBin, err := exec.LookPath("go") |
| @@ -2074,15 +2075,52 @@ func TestExecDeliveryCLI(t *testing.T) { |
| t.Fatalf("env exec-delivery on = %q, want a refusal naming the bad value", msg) |
| } |
| - // Named-environment placement is refused before it ever reaches the |
| - // store: -environment is app-level only for this verb. |
| - msg = runFails("env", "exec-delivery", "-account", "bab", "-app", "titogo", "-environment", "demo", "file") |
| - if !strings.Contains(strings.ToLower(msg), "app-level") { |
| - t.Fatalf("env exec-delivery -environment demo file = %q, want a refusal explaining it's app-level only", msg) |
| - } |
| store := release.NewDirStore(storeDir) |
| - if _, _, err := store.Get(context.Background(), "apps/bab/titogo/environments/demo.json"); err == nil { |
| - t.Fatal("env exec-delivery -environment demo wrote a named environment bundle despite the refusal") |
| + |
| + // Named-environment placement (issue #195): the flag lands in THAT |
| + // bundle, and a plain (default-bundle) listing does not mention it — |
| + // the whole point being that opting one bundle in must not read as |
| + // opting the app in. |
| + out = run("env", "exec-delivery", "-account", "bab", "-app", "titogo", "-environment", "demo", "file") |
| + if !strings.Contains(out, "demo") || !strings.Contains(out, "file") { |
| + t.Fatalf("env exec-delivery -environment demo file = %q, want a confirmation naming the bundle and the value", out) |
| + } |
| + out = run("env", "list", "-account", "bab", "-app", "titogo", "-environment", "demo") |
| + if !regexp.MustCompile(`(?m)^exec-delivery:\s*file$`).MatchString(out) { |
| + t.Fatalf("env list -environment demo = %q, want an exec-delivery: file line", out) |
| + } |
| + out = run("env", "list", "-account", "bab", "-app", "titogo") |
| + if strings.Contains(out, "exec-delivery") { |
| + t.Fatalf("default-bundle env list = %q, must not report a NAMED bundle's flag", out) |
| + } |
| + |
| + // "off" is refused for a named bundle, by name: clearing there means |
| + // inherit, and a verb that spelled it "off" would read as "stop |
| + // delivering" while doing the opposite. |
| + msg = runFails("env", "exec-delivery", "-account", "bab", "-app", "titogo", "-environment", "demo", "off") |
| + if !strings.Contains(msg, "inherit") { |
| + t.Fatalf("env exec-delivery -environment demo off = %q, want a refusal naming \"inherit\"", msg) |
| + } |
| + out = run("env", "list", "-account", "bab", "-app", "titogo", "-environment", "demo") |
| + if !strings.Contains(out, "exec-delivery") { |
| + t.Fatalf("the refused \"off\" cleared the bundle's flag anyway: %q", out) |
| + } |
| + |
| + // "inherit" clears it. |
| + out = run("env", "exec-delivery", "-account", "bab", "-app", "titogo", "-environment", "demo", "inherit") |
| + if !strings.Contains(out, "inherit") { |
| + t.Fatalf("env exec-delivery -environment demo inherit = %q, want a confirmation", out) |
| + } |
| + out = run("env", "list", "-account", "bab", "-app", "titogo", "-environment", "demo") |
| + if strings.Contains(out, "exec-delivery") { |
| + t.Fatalf("env list -environment demo after inherit = %q, must no longer mention exec-delivery", out) |
| + } |
| + |
| + // And "inherit" is refused at APP scope, for the mirror-image reason: |
| + // the default bundle has nothing above it to inherit from. |
| + msg = runFails("env", "exec-delivery", "-account", "bab", "-app", "titogo", "inherit") |
| + if !strings.Contains(msg, "off") { |
| + t.Fatalf("env exec-delivery inherit (no environment) = %q, want a refusal naming \"off\"", msg) |
| } |
| // Turning off against an app that was never configured at all is |
diff --git a/internal/appconfig/appconfig.go b/internal/appconfig/appconfig.go| index 846396278..7d2521c2a 100644 |
| --- a/internal/appconfig/appconfig.go |
| +++ b/internal/appconfig/appconfig.go |
| @@ -33,20 +33,40 @@ type Manifest struct { |
| // does not own the process (see syncexec.go). |
| // |
| // Additive to schema 1 (schema is not bumped for this field), so any |
| - // other value is refused at SET time (SetExecDelivery, ValidExecDelivery) |
| - // and, since an older binary must still be able to parse a manifest a |
| - // newer one wrote, ignored — loudly, once per pass — at SYNC time |
| - // (see SyncExec's checkOptedIn) rather than causing a hard failure. |
| + // other value is refused at SET time (SetExecDeliveryAt, |
| + // ValidExecDelivery) and, since an older binary must still be able to |
| + // parse a manifest a newer one wrote, ignored — loudly, once per pass — |
| + // at SYNC time (see SyncExec's checkOptedIn) rather than causing a hard |
| + // failure. An older binary reading a NAMED bundle this one wrote the |
| + // field into ignores it entirely (it consults the default bundle |
| + // alone), which is the pre-#195 behaviour, not a fault. |
| // |
| - // APP-LEVEL ONLY: read from the default bundle (config.json) alone. |
| - // SetExecDelivery has no environment-scoped variant, unlike apply's |
| - // applyAt generalization — a named environment's manifest is never a |
| - // valid place for this field, and LoadMerged's merged result never |
| - // carries it forward. The reader is the app's own supervisor, one |
| - // program per app, not per-route or per-environment: per-route or |
| - // per-environment opt-in would let half a fleet read config the other |
| - // half silently ignores, the "config-that-lies" failure mode (issue |
| - // #18 design comment, 2026-08-03). |
| + // BUNDLE-LEVEL, resolved as an overlay (issue #195, ruled 2026-08-16). |
| + // SyncExec reads this field from the bundle a route's config actually |
| + // comes from: a route bound to environment e takes e's value when e |
| + // STATES one, and inherits the default bundle's when it does not. An |
| + // unbound route reads the default bundle alone, exactly as before. |
| + // LoadMerged still never carries this field into its merged result — |
| + // the merged manifest is what gets rendered into a file, and a delivery |
| + // flag has no business appearing there; checkOptedIn reads the two |
| + // bundles' fields directly instead. |
| + // |
| + // The 2026-08-03 ruling this narrows said a named environment's |
| + // manifest is never a valid place for the field, because "the reader is |
| + // the app's own supervisor, one program per app". That premise is false |
| + // in the deployment the feature was built for: the titogo app |
| + // supervises its relay and its router fleet through different units, so |
| + // the app-level flag thrown to deliver one sealed credential to the |
| + // relay wrote a file for every router too. |
| + // |
| + // STILL NOT PER-ROUTE. A per-route flag would need a registry column |
| + // and — since a Provisioned route's Channel and Environment belong to |
| + // its instance record — a record field and reconcile convergence too: |
| + // three doors onto one bit. The "config-that-lies" hazard the original |
| + // ruling named is real at that granularity, where two routes resolving |
| + // through the SAME bundle could disagree about whether it is delivered. |
| + // The environment binding is one door onto a scope that already exists, |
| + // and is the same axis the secrets themselves are scoped on. |
| ExecDelivery string `json:"exec_delivery,omitempty"` |
| } |
diff --git a/internal/appconfig/environments_test.go b/internal/appconfig/environments_test.go| index 24ef81079..d82b85e56 100644 |
| --- a/internal/appconfig/environments_test.go |
| +++ b/internal/appconfig/environments_test.go |
| @@ -331,3 +331,118 @@ func TestUnsetEnvironmentReportsActualRemovals(t *testing.T) { |
| t.Fatalf("B = %+v, want it untouched", m.Vars["B"]) |
| } |
| } |
| + |
| +// --- issue #195: exec_delivery at environment scope --- |
| + |
| +func TestSetExecDeliveryAtEnvironmentRoundTrips(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + // A bundle must exist before a flag can be written into it, exactly as |
| + // the default bundle must. |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatalf("set: %v", err) |
| + } |
| + m, err := LoadEnvironment(ctx, s, "bab", "titogo", "relay") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if m.ExecDelivery != ExecDeliveryFile { |
| + t.Fatalf("environment bundle exec_delivery = %q, want %q", m.ExecDelivery, ExecDeliveryFile) |
| + } |
| + // The vars already in the bundle survive the flag write — read-modify- |
| + // write, not replace. |
| + if m.Vars["A"].Value != "1" { |
| + t.Fatalf("A = %q, want it preserved at \"1\"", m.Vars["A"].Value) |
| + } |
| + // And the DEFAULT bundle is untouched: an environment-scoped write that |
| + // leaked into config.json would opt the whole fleet in, which is the |
| + // bug this closes. |
| + def, found, err := getIfExists(ctx, s, "bab", "titogo") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if found && def.ExecDelivery != "" { |
| + t.Fatalf("default bundle exec_delivery = %q, want it untouched", def.ExecDelivery) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtEnvironmentClearsToInherit(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ""); err != nil { |
| + t.Fatalf("clear: %v", err) |
| + } |
| + m, err := LoadEnvironment(ctx, s, "bab", "titogo", "relay") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if m.ExecDelivery != "" { |
| + t.Fatalf("exec_delivery = %q, want it cleared to inherit", m.ExecDelivery) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtRefusesAbsentBundle(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", "") |
| + if err == nil { |
| + t.Fatal("clearing the flag on a bundle that does not exist must be refused, not silently create one") |
| + } |
| + if !strings.Contains(err.Error(), "relay") { |
| + t.Fatalf("refusal %q must name the bundle it could not find", err) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtRefusesInvalidValue(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", "socket"); err == nil { |
| + t.Fatal("an unknown exec_delivery value must be refused before it reaches the bucket") |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtRefusesInvalidEnvironmentName(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "Relay", ExecDeliveryFile); err == nil { |
| + t.Fatal("an invalid environment name must be refused before any bucket write") |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryAtBumpsGeneration(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"A": {Kind: "env", Value: "1"}}, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, before, err := s.Get(ctx, GenKey("bab", "titogo")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, after, err := s.Get(ctx, GenKey("bab", "titogo")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if before == after { |
| + t.Fatal("an environment-scoped exec_delivery write must bump the generation object — the fast loop watches nothing else for it") |
| + } |
| +} |
diff --git a/internal/appconfig/routefingerprint.go b/internal/appconfig/routefingerprint.go| new file mode 100644 |
| index 000000000..10cd89e27 |
| --- /dev/null |
| +++ b/internal/appconfig/routefingerprint.go |
| @@ -0,0 +1,90 @@ |
| +// Route-shape fingerprints: the registry-side half of "has this box's |
| +// config delivery got anything to do". |
| +// |
| +// The agent's fast loop (host package's configGenLoop) decides whether to |
| +// converge by reading each config-carrying app's generation object out of |
| +// the bucket. That catches every BUCKET-side change — a var set, a secret |
| +// rotated, an exec_delivery flag flipped — and nothing at all on the |
| +// REGISTRY side. But a route's Environment is a registry column, and it |
| +// decides which bundle the route's config comes from, so binding a route |
| +// to an environment changes what that route should be served while moving |
| +// no etag anywhere. Delivery then waits out configSyncLoop's hourly floor: |
| +// issue #196, reproduced on carlos-tito-ie 2026-08-15 as ten minutes of a |
| +// live route serving the wrong merge, whose only symptom was the app |
| +// reporting a credential missing — which reads as a BAD credential rather |
| +// than an undelivered one. The nudge that fixed it was re-writing three |
| +// identical secret values, purely to bump the generation object. |
| +// |
| +// These digests give that loop something to watch. They live here, beside |
| +// ExecConfigApps and UnitConfigApps and composed from the SAME eligibility |
| +// predicates those two use, for the reason both of those give for living |
| +// where they do: a fingerprint that disagreed with the sweep about which |
| +// routes matter would either burn passes forever or miss changes silently. |
| +package appconfig |
| + |
| +import ( |
| + "crypto/sha256" |
| + "encoding/hex" |
| + "sort" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// ConfigRouteFingerprints returns one digest per delivery half over the |
| +// registry-side facts that decide what config a route gets: which routes |
| +// are in scope at all, and — for each — the (account, app, environment) |
| +// triple its manifest resolves from. |
| +// |
| +// Set MEMBERSHIP is what carries scope changes: a route that gains |
| +// Status: "failed", loses its app stamp, or is deleted outright simply |
| +// stops contributing a row, which moves the digest exactly as a rebinding |
| +// does. That is deliberate — those transitions all change what should be |
| +// on disk (a file to remove, a file to stop updating), and the pass a |
| +// movement triggers resolves each case properly rather than acting on the |
| +// digest itself. |
| +// |
| +// The OPT-IN is deliberately NOT here, for the reason ExecConfigApps gives |
| +// for omitting it too: reading it costs a bucket fetch per app per tick, |
| +// which is exactly what the fast loop exists to avoid. It does not need to |
| +// be — exec_delivery is a bundle write, and every bundle write bumps the |
| +// app's generation object (SetExecDeliveryAt), which the loop already |
| +// watches. |
| +// |
| +// Two digests, not one, because the two halves are delivered by different |
| +// processes: the exec half by the agent itself (SyncExec), the unit half |
| +// by a root pass the agent can only REQUEST. A shared digest would ask for |
| +// a root config run every time an exec route was rebound. |
| +func ConfigRouteFingerprints(routes []registry.Route) (exec, unit string) { |
| + var execRows, unitRows []string |
| + for _, rt := range routes { |
| + // Exactly the inputs the two sweeps resolve a route's manifest |
| + // from: SyncExec's groupKey and Sync's are both |
| + // (RouteAccount, App, Environment), and Host is what names the |
| + // file. Nothing else on the row can change which bytes belong in |
| + // it, so nothing else belongs in the digest — a Version stamp |
| + // here would fire a pass on every adopt. |
| + row := rt.Host + "|" + release.RouteAccount(rt) + "|" + rt.App + "|" + rt.Environment |
| + if execSyncable(rt) || execAddrCandidate(rt) { |
| + execRows = append(execRows, row) |
| + } |
| + if syncable(rt) && rt.Backing == registry.BackingUnit { |
| + unitRows = append(unitRows, row) |
| + } |
| + } |
| + return digestRows(execRows), digestRows(unitRows) |
| +} |
| + |
| +// digestRows sorts and hashes. Sorted because registry order is not stable |
| +// across passes and an order-sensitive digest would fire constantly; |
| +// NUL-separated because none of the four fields can contain one, so no two |
| +// distinct row sets can concatenate to the same bytes. |
| +func digestRows(rows []string) string { |
| + sort.Strings(rows) |
| + h := sha256.New() |
| + for _, r := range rows { |
| + h.Write([]byte(r)) |
| + h.Write([]byte{0}) |
| + } |
| + return hex.EncodeToString(h.Sum(nil)) |
| +} |
diff --git a/internal/appconfig/routefingerprint_test.go b/internal/appconfig/routefingerprint_test.go| new file mode 100644 |
| index 000000000..da060b2c3 |
| --- /dev/null |
| +++ b/internal/appconfig/routefingerprint_test.go |
| @@ -0,0 +1,115 @@ |
| +package appconfig |
| + |
| +import ( |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// fpExecRoute is the TCP-addr exec shape (issue #18): carlos proxies to it |
| +// but neither spawns nor owns the process. |
| +func fpExecRoute(host, env string) registry.Route { |
| + return registry.Route{Host: host, Kind: "instance", App: "titogo", |
| + Account: "bab", Status: "ready", Addr: "127.0.0.1:9001", Environment: env} |
| +} |
| + |
| +// fpUnitRoute is the unit-backed shape the ROOT pass delivers config for. |
| +func fpUnitRoute(host, env string) registry.Route { |
| + return registry.Route{Host: host, Kind: "instance", App: "keymail", |
| + Account: "bae", Status: "ready", Addr: "127.0.0.1:9100", |
| + Backing: registry.BackingUnit, Environment: env} |
| +} |
| + |
| +func TestConfigRouteFingerprintsMoveOnRebinding(t *testing.T) { |
| + before := []registry.Route{fpExecRoute("connect.go.tito.io", "")} |
| + after := []registry.Route{fpExecRoute("connect.go.tito.io", "relay")} |
| + be, bu := ConfigRouteFingerprints(before) |
| + ae, au := ConfigRouteFingerprints(after) |
| + if be == ae { |
| + t.Fatal("binding an exec route to an environment must move the exec digest — this is the hour issue #196 cost") |
| + } |
| + if bu != au { |
| + t.Fatal("an exec route's rebinding must not move the UNIT digest — that would ask for a root config pass for nothing") |
| + } |
| +} |
| + |
| +func TestConfigRouteFingerprintsUnitHalfIsIndependent(t *testing.T) { |
| + before := []registry.Route{fpUnitRoute("keymail.dev", "")} |
| + after := []registry.Route{fpUnitRoute("keymail.dev", "staging")} |
| + be, bu := ConfigRouteFingerprints(before) |
| + ae, au := ConfigRouteFingerprints(after) |
| + if bu == au { |
| + t.Fatal("binding a unit-backed route to an environment must move the unit digest — the root pass resolves per environment too") |
| + } |
| + if be != ae { |
| + t.Fatal("a unit route's rebinding must not move the exec digest") |
| + } |
| +} |
| + |
| +func TestConfigRouteFingerprintsIgnoreOrderAndIrrelevantColumns(t *testing.T) { |
| + a := fpExecRoute("a.example", "") |
| + b := fpExecRoute("b.example", "relay") |
| + e1, _ := ConfigRouteFingerprints([]registry.Route{a, b}) |
| + e2, _ := ConfigRouteFingerprints([]registry.Route{b, a}) |
| + if e1 != e2 { |
| + t.Fatal("registry order must not move the digest — it is not stable across passes") |
| + } |
| + // A column that cannot change which bytes belong in a route's config |
| + // file must not move the digest, or every adopt would fire a pass. |
| + c := fpExecRoute("a.example", "") |
| + c.Version = "deadbeef" |
| + c.PrevVersion = "cafebabe" |
| + c.LastError = "whatever" |
| + c.Hibernate = true |
| + e3, _ := ConfigRouteFingerprints([]registry.Route{c, b}) |
| + if e1 != e3 { |
| + t.Fatalf("an irrelevant column moved the digest (%s -> %s)", e1, e3) |
| + } |
| +} |
| + |
| +func TestConfigRouteFingerprintsMoveWhenARouteLeavesScope(t *testing.T) { |
| + both := []registry.Route{fpExecRoute("a.example", ""), fpExecRoute("b.example", "")} |
| + one := []registry.Route{fpExecRoute("a.example", "")} |
| + e1, _ := ConfigRouteFingerprints(both) |
| + e2, _ := ConfigRouteFingerprints(one) |
| + if e1 == e2 { |
| + t.Fatal("a route leaving the registry must move the digest — its file needs reaping") |
| + } |
| + // Leaving scope by STATUS, not by deletion, counts exactly the same: |
| + // the eligibility predicate is what set membership is drawn from. |
| + failed := []registry.Route{fpExecRoute("a.example", ""), fpExecRoute("b.example", "")} |
| + failed[1].Status = "failed" |
| + e3, _ := ConfigRouteFingerprints(failed) |
| + if e1 == e3 { |
| + t.Fatal("a route falling out of the eligibility predicate must move the digest") |
| + } |
| +} |
| + |
| +func TestConfigRouteFingerprintsIgnoreOutOfScopeRoutes(t *testing.T) { |
| + base := []registry.Route{fpExecRoute("a.example", "")} |
| + withOthers := append([]registry.Route{}, base...) |
| + withOthers = append(withOthers, |
| + // A static route: no config is delivered to it by either half. |
| + registry.Route{Host: "site.example", Kind: "static", App: "site", Account: "bab"}, |
| + // A platform-minted row: excluded by execRouteBase and by |
| + // release.Adoptable alike. |
| + registry.Route{Host: "edge.example", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:9500", Platform: true}, |
| + ) |
| + e1, u1 := ConfigRouteFingerprints(base) |
| + e2, u2 := ConfigRouteFingerprints(withOthers) |
| + if e1 != e2 || u1 != u2 { |
| + t.Fatal("a route neither half can deliver config to must not move either digest") |
| + } |
| +} |
| + |
| +func TestConfigRouteFingerprintsEmptySetIsStable(t *testing.T) { |
| + e1, u1 := ConfigRouteFingerprints(nil) |
| + e2, u2 := ConfigRouteFingerprints([]registry.Route{}) |
| + if e1 != e2 || u1 != u2 { |
| + t.Fatal("nil and empty must digest identically — a box with no routes must not look like it changed every tick") |
| + } |
| + if e1 == "" || u1 == "" { |
| + t.Fatal("the empty set must have a real digest, not the empty string: the fast loop tells 'never computed' from 'computed, and empty' by its own bool, not by this value") |
| + } |
| +} |
diff --git a/internal/appconfig/store.go b/internal/appconfig/store.go| index 34be45bb4..de97f2217 100644 |
| --- a/internal/appconfig/store.go |
| +++ b/internal/appconfig/store.go |
| @@ -226,28 +226,36 @@ func Unset(ctx context.Context, s release.Store, acct, app string, keys []string |
| return apply(ctx, s, acct, app, DefaultBundle, nil, keys) |
| } |
| -// SetExecDelivery writes (`file`) or clears (`""`) the default bundle's |
| +// SetExecDelivery writes (`file`) or clears (`""`) the DEFAULT bundle's |
| // exec_delivery field — issue #18's TCP-addr exec route opt-in (see |
| -// Manifest.ExecDelivery) — via the same CAS read-modify-write path |
| -// SetEnv/SetSecret/Unset use. value must be ValidExecDelivery; anything |
| -// else is refused here, before it ever reaches the bucket, mirroring |
| -// applyAt's var-name/var-value validation. |
| +// Manifest.ExecDelivery). The app-level door, unchanged in behaviour and |
| +// signature; SetExecDeliveryAt is the general one. |
| +func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value string) error { |
| + return SetExecDeliveryAt(ctx, s, acct, app, "", value) |
| +} |
| + |
| +// SetExecDeliveryAt writes or clears exec_delivery in ONE bundle: the |
| +// default bundle (envName == "") or a named environment's, via the same |
| +// CAS read-modify-write path SetEnv/SetSecret/Unset use. value must be |
| +// ValidExecDelivery; anything else is refused here, before it ever reaches |
| +// the bucket, mirroring applyAt's var-name/var-value validation. |
| // |
| -// APP-LEVEL ONLY, structurally: unlike apply's applyAt generalization, |
| -// there is no environment-scoped variant of this function — it always |
| -// writes Key(acct, app), never EnvironmentKey(acct, app, name). A named |
| -// environment's manifest is never a valid place for this field (Manifest. |
| -// ExecDelivery's doc comment has the "config-that-lies" rationale); the |
| -// CLI's -environment flag is refused before it can reach here at all |
| -// (appconfig_cli.go's runEnvExecDelivery). |
| +// Environment scope is issue #195 (ruled 2026-08-16). The app-level flag |
| +// answered "does this APP get files", so opting one relay route in wrote a |
| +// file for every router route of the same app too — a fleet-wide switch |
| +// thrown to deliver one service's credential. The flag now lives wherever |
| +// the route's config comes from, and clearing it in a named bundle means |
| +// INHERIT the app default, never "off": a bundle that says nothing has not |
| +// asked for delivery to stop. Route scope is still refused — see |
| +// Manifest.ExecDelivery. |
| // |
| -// Turning the field off (value == "") against an app with no default |
| -// bundle at all is refused, not silently turned into a create — the same |
| -// "unset-only against a nonexistent object" refusal applyAt documents, for |
| -// the same reason: minting an empty bundle just to hold an absent flag |
| -// would satisfy `carlos route -environment`'s existence rail and Sync's |
| -// "configured, then fully unset" state for nothing the operator asked for. |
| -func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value string) error { |
| +// Turning the field off against a bundle that does not exist is refused, |
| +// not silently turned into a create — the same "unset-only against a |
| +// nonexistent object" refusal applyAt documents, for the same reason: |
| +// minting an empty bundle just to hold an absent flag would satisfy |
| +// `carlos route -environment`'s existence rail and Sync's "configured, |
| +// then fully unset" state for nothing the operator asked for. |
| +func SetExecDeliveryAt(ctx context.Context, s release.Store, acct, app, envName, value string) error { |
| if !release.ValidAccount(acct) { |
| return fmt.Errorf("config: invalid account %q", acct) |
| } |
| @@ -255,15 +263,21 @@ func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value stri |
| return fmt.Errorf("config: invalid app %q", app) |
| } |
| if !ValidExecDelivery(value) { |
| - return fmt.Errorf("config: invalid exec_delivery %q — want %q or \"\" (off)", value, ExecDeliveryFile) |
| + return fmt.Errorf("config: invalid exec_delivery %q — want %q or \"\" (off/inherit)", value, ExecDeliveryFile) |
| + } |
| + key, label := Key(acct, app), "default" |
| + if envName != "" { |
| + if !ValidEnvironmentName(envName) { |
| + return fmt.Errorf("config: invalid environment name %q", envName) |
| + } |
| + key, label = EnvironmentKey(acct, app, envName), envName |
| } |
| - key := Key(acct, app) |
| m, etag, found, err := getForWriteAt(ctx, s, key, app) |
| if err != nil { |
| return err |
| } |
| if !found && value == "" { |
| - return fmt.Errorf("config: %s has no default bundle — nothing to turn off", app) |
| + return fmt.Errorf("config: %s has no %s bundle — nothing to turn off", app, label) |
| } |
| m.ExecDelivery = value |
| m.Schema, m.App = 1, app |
| @@ -277,7 +291,10 @@ func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value stri |
| // The opt-in flag changes what SyncExec delivers for this app's routes, |
| // so it is a config change like any other and earns the same fast |
| // convergence — see apply's comment for why the touch cannot fail this |
| - // call. |
| + // call. The generation object is per-APP, not per-bundle |
| + // (generation.go), so an environment-scoped write signals through the |
| + // same key an app-level one does — which is why the fast loop needs no |
| + // new bundle awareness for this half. |
| touchLogged(ctx, s, acct, app) |
| return nil |
| } |
diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go| index 306f6d2f9..2c7f9a381 100644 |
| --- a/internal/appconfig/syncexec.go |
| +++ b/internal/appconfig/syncexec.go |
| @@ -9,16 +9,21 @@ |
| // 2026-08-02-exec-config-delivery-design.md. |
| // |
| // A third eligibility case (issue #18): a TCP-addr exec route (Backing == |
| -// "", Socket == "", Addr != "") whose app has opted in via its default |
| -// bundle's additive "exec_delivery": "file" manifest field |
| -// (Manifest.ExecDelivery) gets the identical file, materialized with the |
| -// same layering guarantees, environments-awareness, reap discipline, and |
| -// mass-removal/path-traversal guards as a socket-exec route — but is |
| -// NEVER bounced (see syncExecHost and execAddrCandidate): carlos routes |
| -// to these processes without spawning or owning them, so the restart half |
| -// belongs to the app's own supervisor. An un-opted TCP-addr route is |
| -// skipped exactly as before this case existed — see execAddrCandidate and |
| -// SyncExec's checkOptedIn closure. |
| +// "", Socket == "", Addr != "") whose BOUND BUNDLE has opted in via the |
| +// additive "exec_delivery": "file" manifest field (Manifest.ExecDelivery) |
| +// gets the identical file, materialized with the same layering guarantees, |
| +// environments-awareness, reap discipline, and mass-removal/path-traversal |
| +// guards as a socket-exec route — but is NEVER bounced (see syncExecHost |
| +// and execAddrCandidate): carlos routes to these processes without |
| +// spawning or owning them, so the restart half belongs to the app's own |
| +// supervisor. An un-opted TCP-addr route is skipped exactly as before this |
| +// case existed — see execAddrCandidate and SyncExec's checkOptedIn closure. |
| +// |
| +// "Bound bundle" is issue #195: the flag is read from the route's named |
| +// environment when that bundle STATES a value, and from the app's default |
| +// bundle otherwise. Before that it was app-level only, so opting one |
| +// relay route in wrote a file for every other TCP-addr route of the same |
| +// app — a fleet-wide switch thrown to deliver one service's credential. |
| // |
| // Resolution mirrors sync.go's exactly — the group is (Account, App, |
| // Environment) and a route's bound environment is an overlay on the app's |
| @@ -119,20 +124,81 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv |
| v bool |
| known bool |
| } |
| - // optedIn caches the app-level exec_delivery decision so a box with |
| - // many TCP-addr routes on one app doesn't cost more than one manifest |
| - // read for it (loadDefault's own cache already dedupes the fetch; this |
| - // dedupes the value-interpretation, including the two logs below, |
| - // which must fire at most once per (acct, app) per pass, not once per |
| - // route). |
| - optedIn := map[appKey]optedInRes{} |
| - checkOptedIn := func(acct, app string) optedInRes { |
| - ak := appKey{acct, app} |
| - if c, done := optedIn[ak]; done { |
| + // optKey keys the opt-in decision on the BUNDLE a route's config |
| + // actually resolves through, not on the app alone (issue #195). Two |
| + // routes of one app bound to different environments can now differ, so |
| + // the cache — and the once-per-pass logs below — must too. |
| + type optKey struct{ acct, app, env string } |
| + // optedIn caches the exec_delivery decision so a box with many TCP-addr |
| + // routes on one bundle doesn't cost more than one manifest read for it |
| + // (loadDefault/loadEnvBundle already dedupe the fetches; this dedupes |
| + // the value-interpretation, including the logs below, which must fire |
| + // at most once per (acct, app, env) per pass, not once per route). |
| + optedIn := map[optKey]optedInRes{} |
| + // envBundles caches the named-bundle read the same way loadDefault |
| + // caches the default's, and for the same reason. |
| + type envRes struct { |
| + m *Manifest |
| + found bool |
| + err error |
| + } |
| + envBundles := map[optKey]envRes{} |
| + loadEnvBundle := func(acct, app, env string) envRes { |
| + k := optKey{acct, app, env} |
| + if e, done := envBundles[k]; done { |
| + return e |
| + } |
| + m, found, err := getIfExistsAt(ctx, s, EnvironmentKey(acct, app, env)) |
| + if err != nil { |
| + err = fmt.Errorf("%s/%s (environment %q): config fetch failed: %w", acct, app, env, err) |
| + } |
| + e := envRes{m: m, found: found, err: err} |
| + envBundles[k] = e |
| + return e |
| + } |
| + checkOptedIn := func(acct, app, env string) optedInRes { |
| + k := optKey{acct, app, env} |
| + if c, done := optedIn[k]; done { |
| return c |
| } |
| - def := loadDefault(acct, app) |
| var c optedInRes |
| + // The route's OWN bundle first, and only when it STATES something: |
| + // a named bundle silent on the field inherits the app default, |
| + // exactly as a key it does not set inherits the default bundle's |
| + // value. Absence is inheritance, never "off" — an operator who has |
| + // never mentioned exec_delivery in a bundle has not asked for |
| + // delivery to stop. |
| + decided := false |
| + if env != "" { |
| + eb := loadEnvBundle(acct, app, env) |
| + switch { |
| + case eb.err != nil: |
| + // NOT a determination — the same contract the default |
| + // bundle's fetch-failure branch below gives: say so loudly, |
| + // leave every file alone, retry next pass. |
| + fmt.Printf(" %s/%s: exec_delivery unresolved for environment %q (config fetch failed), leaving this bundle's TCP-addr route file(s) as-is: %v\n", acct, app, env, eb.err) |
| + c, decided = optedInRes{v: false, known: false}, true |
| + case eb.found && eb.m.ExecDelivery == ExecDeliveryFile: |
| + c, decided = optedInRes{v: true, known: true}, true |
| + case eb.found && eb.m.ExecDelivery != "": |
| + // A value THIS binary doesn't understand, in a bundle that |
| + // does state one: a statement, so it decides — treated as |
| + // not opted in, loudly, once per bundle per pass, never a |
| + // hard failure (additive discipline, CLAUDE.md). |
| + fmt.Printf(" %s/%s: config: exec_delivery %q in environment %q is not a value this binary understands — ignoring it (not opted in); upgrade carlos if this is expected\n", acct, app, eb.m.ExecDelivery, env) |
| + c, decided = optedInRes{v: false, known: true}, true |
| + } |
| + // found-but-silent, and absent, both fall through to the app |
| + // default. A bundle bound but MISSING is a defect the route's |
| + // own resolution reports (LoadMerged refuses it) and its file is |
| + // left as-is there — the verdict reached here never gets to a |
| + // write in that case. |
| + } |
| + if decided { |
| + optedIn[k] = c |
| + return c |
| + } |
| + def := loadDefault(acct, app) |
| switch { |
| case def.err != nil: |
| // NOT a determination: this app may well be opted in and this |
| @@ -172,7 +238,7 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv |
| fmt.Printf(" %s/%s: config: exec_delivery %q in the default bundle is not a value this binary understands — ignoring it (not opted in); upgrade carlos if this is expected\n", acct, app, def.m.ExecDelivery) |
| c = optedInRes{v: false, known: true} |
| } |
| - optedIn[ak] = c |
| + optedIn[k] = c |
| return c |
| } |
| // inScope is the single door both loops below use to decide whether a |
| @@ -185,7 +251,7 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv |
| if execSyncable(rt) { |
| return true, false |
| } |
| - if execAddrCandidate(rt) && checkOptedIn(release.RouteAccount(rt), rt.App).v { |
| + if execAddrCandidate(rt) && checkOptedIn(release.RouteAccount(rt), rt.App, rt.Environment).v { |
| return true, true |
| } |
| return false, false |
| @@ -375,7 +441,7 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv |
| if !execAddrCandidate(rt) { |
| continue |
| } |
| - c := checkOptedIn(release.RouteAccount(rt), rt.App) |
| + c := checkOptedIn(release.RouteAccount(rt), rt.App, rt.Environment) |
| if c.v || !c.known { |
| continue |
| } |
diff --git a/internal/appconfig/syncexec_test.go b/internal/appconfig/syncexec_test.go| index 53d5519d9..9daa680cf 100644 |
| --- a/internal/appconfig/syncexec_test.go |
| +++ b/internal/appconfig/syncexec_test.go |
| @@ -1628,3 +1628,204 @@ func TestUnitConfigAppsNoRoutes(t *testing.T) { |
| t.Fatalf("UnitConfigApps(nil) = %v, want empty", got) |
| } |
| } |
| + |
| +// --- issue #195: the opt-in resolves from the route's bound bundle --- |
| + |
| +// TestSyncExecOptInResolvesPerEnvironment is issue #195's matrix: which |
| +// hosts get a <host>.config.env, for each combination of the app-level |
| +// flag and the bound bundle's own. Two TCP-addr exec routes of ONE app, |
| +// one bound to "relay" and one unbound — titogo's exact shape, where |
| +// turning the app-level flag on to deliver the relay's sealed Apple Wallet |
| +// material wrote a file for every router route too. |
| +func TestSyncExecOptInResolvesPerEnvironment(t *testing.T) { |
| + for _, tc := range []struct { |
| + name string |
| + appLevel string // "" (never set) | ExecDeliveryFile |
| + envLevel string // absent | silent | file | bogus |
| + wantRelayFile bool |
| + wantUnboundFile bool |
| + }{ |
| + {"both off", "", "silent", false, false}, |
| + {"app on, env silent — inherit on", ExecDeliveryFile, "silent", true, true}, |
| + {"app off, env on — the narrow case #195 exists for", "", "file", true, false}, |
| + {"app on, env on", ExecDeliveryFile, "file", true, true}, |
| + {"app off, env bundle absent", "", "absent", false, false}, |
| + // Bound to a bundle that does not exist: the opt-in inherits "on" |
| + // from the app default, but the route never reaches a write anyway |
| + // — LoadMerged refuses a bound-but-absent bundle, so the group |
| + // resolves to an ERROR and the file is left exactly as it is. The |
| + // unbound router is untouched by that and still gets its file. |
| + {"app on, env bundle absent — resolution errors before delivery", ExecDeliveryFile, "absent", false, true}, |
| + {"app on, env states a value this binary doesn't know — that statement decides", ExecDeliveryFile, "bogus", false, true}, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + // The default bundle always exists and always has a var, so the |
| + // only thing under test is WHETHER a file is written. |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"SHARED": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if tc.appLevel != "" { |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", tc.appLevel); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + } |
| + switch tc.envLevel { |
| + case "absent": |
| + // Nothing written: the route is bound to a bundle that does |
| + // not exist. |
| + case "bogus": |
| + // Written past the setter's validation on purpose, and |
| + // without SaveEnvironment first (a create, not an update): |
| + // this is what an OLDER binary sees when a NEWER one |
| + // invents a value, and it must not brick the pass. |
| + raw := []byte(`{"schema":1,"app":"titogo","vars":{"SEALED":{"kind":"env","value":"2"}},"exec_delivery":"grpc"}`) |
| + if _, err := s.Put(ctx, EnvironmentKey("bab", "titogo", "relay"), raw, ""); err != nil { |
| + t.Fatalf("Put: %v", err) |
| + } |
| + default: |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"SEALED": {Kind: "env", Value: "2"}}, nil); err != nil { |
| + t.Fatalf("SaveEnvironment: %v", err) |
| + } |
| + if tc.envLevel == "file" { |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDeliveryAt: %v", err) |
| + } |
| + } |
| + } |
| + for _, rt := range []registry.Route{ |
| + {Host: "connect.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:9001", Environment: "relay"}, |
| + {Host: "router-a.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:9002"}, |
| + } { |
| + if err := reg.Upsert(rt); err != nil { |
| + t.Fatalf("Upsert %s: %v", rt.Host, err) |
| + } |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + assertConfigFile(t, dd, "connect.go.tito.io", tc.wantRelayFile) |
| + assertConfigFile(t, dd, "router-a.go.tito.io", tc.wantUnboundFile) |
| + }) |
| + } |
| +} |
| + |
| +// assertConfigFile asserts a route's <host>.config.env exists (or not). |
| +func assertConfigFile(t *testing.T, dir, host string, want bool) { |
| + t.Helper() |
| + _, err := os.Stat(filepath.Join(dir, host+configEnvSuffix)) |
| + switch { |
| + case want && err != nil: |
| + t.Fatalf("%s: want a config file, got %v", host, err) |
| + case !want && err == nil: |
| + t.Fatalf("%s: want NO config file, one was written", host) |
| + } |
| +} |
| + |
| +// TestSyncExecEnvScopedOptInDeliversTheMergedBundle: the flag decides |
| +// WHETHER a file is written, never what is in it. A route opted in through |
| +// its own bundle still gets the default bundle overlaid by that bundle — |
| +// the same merge LoadMerged has always produced. |
| +func TestSyncExecEnvScopedOptInDeliversTheMergedBundle(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"SHARED": "1", "OVERRIDDEN": "default"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", map[string]Var{ |
| + "OVERRIDDEN": {Kind: "env", Value: "relay"}, |
| + "WALLET": {Kind: "env", Value: "sealed"}, |
| + }, nil); err != nil { |
| + t.Fatalf("SaveEnvironment: %v", err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDeliveryAt: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "connect.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:9001", Environment: "relay", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + got, err := os.ReadFile(filepath.Join(dd, "connect.go.tito.io"+configEnvSuffix)) |
| + if err != nil { |
| + t.Fatalf("read config file: %v", err) |
| + } |
| + want := execAddrConfigHeader + "OVERRIDDEN=relay\nSHARED=1\nWALLET=sealed\n" |
| + if string(got) != want { |
| + t.Fatalf("config env = %q, want %q (the merged bundle, unchanged by where the flag lives)", got, want) |
| + } |
| + // exec_delivery itself must never be rendered into the file: it is a |
| + // delivery flag, not a config var. |
| + if strings.Contains(string(got), "exec_delivery") || strings.Contains(string(got), "EXEC_DELIVERY") { |
| + t.Fatalf("the delivery flag leaked into the rendered file: %q", got) |
| + } |
| +} |
| + |
| +// TestSyncExecEnvBundleFetchErrorLeavesFileAlone: an unreadable |
| +// environment bundle is NOT a determination that the route is un-opted. |
| +// The route's existing file must survive it byte-identical — the same |
| +// contract a default-bundle fetch failure already has, extended to the |
| +// second bundle the opt-in now consults. |
| +func TestSyncExecEnvBundleFetchErrorLeavesFileAlone(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"SHARED": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "relay", |
| + map[string]Var{"WALLET": {Kind: "env", Value: "sealed"}}, nil); err != nil { |
| + t.Fatalf("SaveEnvironment: %v", err) |
| + } |
| + if err := SetExecDeliveryAt(ctx, s, "bab", "titogo", "relay", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDeliveryAt: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "connect.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:9001", Environment: "relay", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + path := filepath.Join(dd, "connect.go.tito.io"+configEnvSuffix) |
| + before, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("first pass wrote no file: %v", err) |
| + } |
| + |
| + blip := &failingKeyStore{Store: s, failKey: EnvironmentKey("bab", "titogo", "relay"), |
| + err: errors.New("503 SlowDown")} |
| + out := captureStdout(t, func() { |
| + if err := SyncExec(ctx, reg, blip, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec (blip): %v", err) |
| + } |
| + }) |
| + after, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("a bundle fetch blip removed a live route's config file: %v", err) |
| + } |
| + if !bytes.Equal(before, after) { |
| + t.Fatalf("config file changed on a fetch blip: %q -> %q", before, after) |
| + } |
| + if !strings.Contains(out, "unresolved") || !strings.Contains(out, "relay") { |
| + t.Fatalf("a fetch blip against the bound bundle must say so loudly and name the environment; got %q", out) |
| + } |
| +} |
diff --git a/internal/console/apiconfig.go b/internal/console/apiconfig.go| index c1cd87c26..31ec0179b 100644 |
| --- a/internal/console/apiconfig.go |
| +++ b/internal/console/apiconfig.go |
| @@ -232,11 +232,13 @@ func (s *Server) handleAPIConfigGet(w http.ResponseWriter, r *http.Request) { |
| "environments": envs, |
| "vars": vars, |
| } |
| - // exec_delivery is APP-LEVEL: it lives in the default bundle alone and |
| - // LoadMerged never carries it out of a named one (Manifest.ExecDelivery's |
| - // doc comment has the "config-that-lies" rationale). Reporting it while |
| - // answering for a named bundle would suggest it could be set there. |
| - if envName == "" && m.ExecDelivery != "" { |
| + // exec_delivery is emitted for whichever bundle was asked for: since |
| + // issue #195 it can be set in a named one, where it means "the TCP-addr |
| + // exec routes bound to this environment get files". Omitted when the |
| + // bundle is silent on it, rather than reported as the value it |
| + // INHERITS — the answer describes this bundle, and an inherited value |
| + // would read as a statement the bundle does not make. |
| + if m.ExecDelivery != "" { |
| out["exec_delivery"] = m.ExecDelivery |
| } |
| // Public material by design — it only encrypts. Serving it is what makes |
| @@ -368,22 +370,44 @@ func (s *Server) handleAPIConfigPut(w http.ResponseWriter, r *http.Request) { |
| // a harmless no-op into an error. Same division appconfig.applyAt makes. |
| execDelivery := "" |
| if req.ExecDelivery != nil { |
| + // The wire's clearing word is the SCOPE's own, and the two are not |
| + // interchangeable (issue #195): clearing the default bundle's flag |
| + // stops delivery, while clearing a named bundle's hands the |
| + // question back to the default. Accepting "off" for a named bundle |
| + // would let a caller ask to stop delivery and get the opposite |
| + // whenever the app default is on. Both refusals are worded here, |
| + // once, so the CLI and any other client read the same sentence |
| + // rather than a paraphrase of it. |
| switch *req.ExecDelivery { |
| case appconfig.ExecDeliveryFile: |
| execDelivery = appconfig.ExecDeliveryFile |
| case "off": |
| + if envName != "" { |
| + jsonError(w, http.StatusBadRequest, |
| + "exec_delivery \"off\" is app-level — in a named environment, clearing the flag means \"inherit\" "+ |
| + "(routes bound to it fall back to the app's default bundle), which is not the same as turning "+ |
| + "delivery off. Send \"inherit\" for this bundle, or send \"off\" with no environment to turn "+ |
| + "the app's default off.") |
| + return |
| + } |
| // The wire's "off" is appconfig's "" — the vocabulary changes at |
| // this boundary and nowhere else. |
| execDelivery = "" |
| + case "inherit": |
| + if envName == "" { |
| + jsonError(w, http.StatusBadRequest, |
| + "exec_delivery \"inherit\" needs an environment — the default bundle has nothing above it to "+ |
| + "inherit from. Send \"off\" to clear it.") |
| + return |
| + } |
| + execDelivery = "" |
| default: |
| + want := `"off"` |
| + if envName != "" { |
| + want = `"inherit"` |
| + } |
| jsonError(w, http.StatusBadRequest, |
| - fmt.Sprintf("invalid exec_delivery %q — want %q or \"off\"", *req.ExecDelivery, appconfig.ExecDeliveryFile)) |
| - return |
| - } |
| - if envName != "" { |
| - jsonError(w, http.StatusBadRequest, |
| - "exec_delivery is app-level only — it lives in the default bundle (config.json) alone, "+ |
| - "never in a named environment, so send it without an environment") |
| + fmt.Sprintf("invalid exec_delivery %q — want %q or %s", *req.ExecDelivery, appconfig.ExecDeliveryFile, want)) |
| return |
| } |
| } |
| @@ -425,20 +449,20 @@ func (s *Server) handleAPIConfigPut(w http.ResponseWriter, r *http.Request) { |
| s.bumpSerial(ctx) |
| } |
| if req.ExecDelivery != nil { |
| - // A SECOND CAS write, and deliberately not folded into the one above: |
| - // SetExecDelivery is app-level and has no environment-scoped variant |
| - // at all (store.go). The two cannot be one transaction, so the audit |
| - // row for the set/unset half is written above, before this runs — if |
| - // this fails, the trail still says truthfully what landed. |
| - if err := appconfig.SetExecDelivery(ctx, s.cfg.Releases, acct.Sqid, app, execDelivery); err != nil { |
| - s.configStoreError(w, fmt.Sprintf("api config exec-delivery %s/%s", acct.Sqid, app), err) |
| + // A SECOND CAS write, and deliberately not folded into the one |
| + // above: it writes one FIELD of the bundle through a different |
| + // setter, where the block above writes vars. The two cannot be one |
| + // transaction, so the audit row for the set/unset half is written |
| + // above, before this runs — if this fails, the trail still says |
| + // truthfully what landed. |
| + if err := appconfig.SetExecDeliveryAt(ctx, s.cfg.Releases, acct.Sqid, app, envName, execDelivery); err != nil { |
| + s.configStoreError(w, fmt.Sprintf("api config exec-delivery %s/%s (%s)", acct.Sqid, app, bundle), err) |
| return |
| } |
| - label := "off" |
| - if execDelivery != "" { |
| - label = execDelivery |
| - } |
| - s.audit(acct, app, addr, "exec-delivery", "exec-delivery "+label+" (cli)") |
| + // The caller's own word, not one reconstructed from the stored |
| + // value: "off" and "inherit" both store "", and a trail that |
| + // recorded them identically would lose which scope was acted on. |
| + s.audit(acct, app, addr, "exec-delivery", "exec-delivery "+*req.ExecDelivery+" ("+bundle+") (cli)") |
| s.bumpSerial(ctx) |
| } |
| writeJSON(w, http.StatusOK, map[string]any{"result": "ok", "removed": removed}) |
diff --git a/internal/console/apiconfig_test.go b/internal/console/apiconfig_test.go| index db95ca691..5d54d35f0 100644 |
| --- a/internal/console/apiconfig_test.go |
| +++ b/internal/console/apiconfig_test.go |
| @@ -496,8 +496,8 @@ func TestAPIConfigNamedEnvironment(t *testing.T) { |
| if envs, _ := out["environments"].([]any); len(envs) != 1 || envs[0] != "staging" { |
| t.Fatalf("environments on the default bundle = %v, want [staging]", out["environments"]) |
| } |
| - // The exec_delivery field is app-level: even asking for a named bundle, |
| - // it is never reported from one. |
| + // No exec_delivery anywhere: nothing set it, and the field is omitted |
| + // when a bundle is silent on it rather than reported as empty. |
| if _, present := out["exec_delivery"]; present { |
| t.Fatalf("exec_delivery = %v, want absent", out["exec_delivery"]) |
| } |
| @@ -585,10 +585,18 @@ func TestAPIConfigPutRefusals(t *testing.T) { |
| "set": map[string]any{"A": map[string]any{"kind": "secret", "envelope": envelope, "value": "hunter2"}}, |
| }, http.StatusBadRequest, "value"}, |
| {"bad exec_delivery value", map[string]any{"exec_delivery": "x"}, http.StatusBadRequest, "exec_delivery"}, |
| - {"exec_delivery with an environment", map[string]any{ |
| + // Issue #195: "file" IS accepted with an environment now (see |
| + // TestAPIConfigExecDeliveryAtEnvironmentScope). What each scope |
| + // refuses is the OTHER scope's clearing word, because the two mean |
| + // different things: "off" stops delivery, "inherit" hands the |
| + // question back to the app default. |
| + {"exec_delivery off with an environment", map[string]any{ |
| "environment": "staging", |
| - "exec_delivery": "file", |
| - }, http.StatusBadRequest, "app-level"}, |
| + "exec_delivery": "off", |
| + }, http.StatusBadRequest, "inherit"}, |
| + {"exec_delivery inherit without an environment", map[string]any{ |
| + "exec_delivery": "inherit", |
| + }, http.StatusBadRequest, "needs an environment"}, |
| } |
| for _, c := range cases { |
| t.Run(c.name, func(t *testing.T) { |
| @@ -902,3 +910,76 @@ func TestAPIConfigPutLargeEnvelope(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// TestAPIConfigExecDeliveryAtEnvironmentScope is issue #195 through the |
| +// door: the flag lands in the NAMED bundle, the default bundle stays |
| +// silent, and "inherit" clears it. The point of the whole change is that |
| +// opting one bundle in must be readable as exactly that — so a GET for the |
| +// default bundle must not report a named bundle's flag, and vice versa. |
| +func TestAPIConfigExecDeliveryAtEnvironmentScope(t *testing.T) { |
| + fx, token := configFixture(t, "code-config-execdelivery-env") |
| + |
| + status, out := putJSON(t, configURL(fx, "titogo"), map[string]any{ |
| + "environment": "relay", |
| + "set": map[string]any{"WALLET": map[string]any{"kind": "env", "value": "sealed"}}, |
| + "exec_delivery": "file", |
| + }, token) |
| + if status != http.StatusOK { |
| + t.Fatalf("PUT environment=relay exec_delivery=file = %d %v", status, out) |
| + } |
| + |
| + status, out = getJSON(t, configURL(fx, "titogo")+"?environment=relay", token) |
| + if status != http.StatusOK { |
| + t.Fatalf("GET relay = %d %v", status, out) |
| + } |
| + if str(t, out, "exec_delivery") != appconfig.ExecDeliveryFile { |
| + t.Fatalf("relay exec_delivery = %v, want %q", out["exec_delivery"], appconfig.ExecDeliveryFile) |
| + } |
| + |
| + // The default bundle never learned about it — the fleet-wide switch |
| + // issue #195 is about was exactly this leaking upward. |
| + status, out = getJSON(t, configURL(fx, "titogo"), token) |
| + if status != http.StatusOK { |
| + t.Fatalf("GET default = %d %v", status, out) |
| + } |
| + if _, present := out["exec_delivery"]; present { |
| + t.Fatalf("default bundle exec_delivery = %v, want absent — an environment-scoped opt-in must not read as an app-level one", out["exec_delivery"]) |
| + } |
| + |
| + // "inherit" clears the named bundle's field without touching its vars. |
| + status, out = putJSON(t, configURL(fx, "titogo"), map[string]any{ |
| + "environment": "relay", |
| + "exec_delivery": "inherit", |
| + }, token) |
| + if status != http.StatusOK { |
| + t.Fatalf("PUT environment=relay exec_delivery=inherit = %d %v", status, out) |
| + } |
| + status, out = getJSON(t, configURL(fx, "titogo")+"?environment=relay", token) |
| + if status != http.StatusOK { |
| + t.Fatalf("GET relay = %d %v", status, out) |
| + } |
| + if _, present := out["exec_delivery"]; present { |
| + t.Fatalf("relay exec_delivery = %v after inherit, want the field absent", out["exec_delivery"]) |
| + } |
| + if v := varsByKey(t, out)["WALLET"]; str(t, v, "value") != "sealed" { |
| + t.Fatalf("WALLET = %v after inherit, want it untouched", v) |
| + } |
| + |
| + // The audit trail records the caller's own word and the bundle it |
| + // applied to: "off" and "inherit" both store "", so a row built from |
| + // the stored value would lose which scope was acted on. |
| + events, err := fx.st.Events(OpsAccountID, "titogo", 20) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var joined string |
| + for _, e := range events { |
| + joined += e.Detail + "\n" |
| + } |
| + if !strings.Contains(joined, "exec-delivery file (relay)") { |
| + t.Fatalf("audit trail = %q, want a row naming the value and the bundle", joined) |
| + } |
| + if !strings.Contains(joined, "exec-delivery inherit (relay)") { |
| + t.Fatalf("audit trail = %q, want the clear recorded as \"inherit\", not as \"off\"", joined) |
| + } |
| +} |
diff --git a/internal/console/apiinstances.go b/internal/console/apiinstances.go| index 98f5243cb..ab7202f7e 100644 |
| --- a/internal/console/apiinstances.go |
| +++ b/internal/console/apiinstances.go |
| @@ -265,13 +265,41 @@ func (s *Server) instanceHealth(ctx context.Context, recs []provision.Record) ma |
| unitByHost[rec.Host] = rt.Unit |
| } |
| } |
| + for h, word := range s.healthByHost(ctx, routeFor) { |
| + out[h] = word |
| + } |
| + return out |
| +} |
| + |
| +// healthByHost is the metrics half of a health answer, shared by |
| +// instanceHealth (records joined to routes) and handleAPIRoutes (routes |
| +// directly, issue #64) so the two — and the Running tab, which uses the |
| +// same healthWord — can never disagree about an instance's state. |
| +// |
| +// routeFor arrives ALREADY SCOPED to the caller's own account and app. |
| +// This function performs no tenant check of its own and must never be |
| +// handed an unfiltered registry listing: OwnerAppMetrics' allowlist is |
| +// exactly the hosts and stamped units it is given, so a foreign unit name |
| +// reaching unitByHost below would widen the metrics view past the caller's |
| +// own app. The scoping lives in each caller, beside the reason that |
| +// caller's rows are the caller's to see. |
| +func (s *Server) healthByHost(ctx context.Context, routeFor map[string]registry.Route) map[string]string { |
| + out := make(map[string]string, len(routeFor)) |
| + hosts := make([]string, 0, len(routeFor)) |
| + unitByHost := map[string]string{} |
| + for h, rt := range routeFor { |
| + hosts = append(hosts, h) |
| + if rt.Unit != "" { |
| + unitByHost[h] = rt.Unit |
| + } |
| + } |
| am, err := s.OwnerAppMetrics(ctx, hosts, unitByHost) |
| if err != nil { |
| // An unreadable sweep is not knowledge that these instances are |
| // down: every host reads as not-reporting below, which healthWord |
| // turns into "No readings" or "Not responding" per expectsReadings — |
| // the same answer the page gives when the sweep is unavailable. |
| - log.Printf("console: api instances health: metrics: %v", err) |
| + log.Printf("console: health by host: metrics: %v", err) |
| am = nil |
| } |
| rep := make(map[string]bool, len(am)) |
| @@ -280,16 +308,12 @@ func (s *Server) instanceHealth(ctx context.Context, recs []provision.Record) ma |
| rep[m.Host] = true |
| } |
| } |
| - for _, rec := range recs { |
| - rt, ok := routeFor[rec.Host] |
| - if !ok { |
| - continue |
| - } |
| + for h, rt := range routeFor { |
| status := rt.Status |
| if status == "" { |
| status = "ready" // registry contract: empty status means ready |
| } |
| - out[rec.Host] = healthWord(rt.Kind, status, rt.Hibernate, rep[rec.Host], |
| + out[h] = healthWord(rt.Kind, status, rt.Hibernate, rep[h], |
| expectsReadings(rt.Provisioned, rt.Unit, rt.Backing)) |
| } |
| return out |
diff --git a/internal/console/apiroutes.go b/internal/console/apiroutes.go| new file mode 100644 |
| index 000000000..489397f96 |
| --- /dev/null |
| +++ b/internal/console/apiroutes.go |
| @@ -0,0 +1,142 @@ |
| +// The routes read (`carlos routes`, issue #64): what routes this app |
| +// actually has on this deployment, and — the reason the issue was reopened |
| +// — what named config environment each one is bound to. |
| +// |
| +// Before this, a route's environment binding could be read exactly one |
| +// way: sqlite3 against /data/registry.db on the box. `carlos route` is a |
| +// setter, `carlos apps` answers from the console's app list, and the |
| +// instance API answers for RECORDS — so a hand-added route (titogo's |
| +// relay, the one that needed the binding) had no reader anywhere. Since |
| +// the binding is what decides which sealed secrets a route is delivered, |
| +// "which routes is this credential going to" was a question only |
| +// answerable with box access: the one step of titogo's whole secrets |
| +// procedure that needed it, in a flow whose selling point is that a |
| +// developer needs no AWS anything. It is also why issue #196 took an hour |
| +// to see rather than a minute. |
| +// |
| +// GATE: apiAccountApp, the member-proving lookup — NOT the instance |
| +// family's instanceGate. An app token is a machine credential minted for |
| +// one instance to register itself; this answer describes every route of |
| +// the app, with upstream addresses, owning units and environment |
| +// bindings, which is an operator's view rather than a workload's. The |
| +// narrower gate is the whole difference between the two doors; do not |
| +// "unify" them. |
| +// |
| +// SCOPE: the console reads its OWN box's registry, so in a multi-box fleet |
| +// the other boxes' routes do not appear here. That is the same blind spot |
| +// instanceHealth has today; internal/fleetapi is the eventual fix (#145) |
| +// and is unmerged. The CLI says so in its own output rather than letting a |
| +// short list read as a complete one. |
| +package console |
| + |
| +import ( |
| + "context" |
| + "log" |
| + "net/http" |
| + "sort" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// routeListTimeout bounds the metrics join, the only part of this handler |
| +// that can be slow — the registry is a local file this process already |
| +// holds open. |
| +const routeListTimeout = 15 * time.Second |
| + |
| +// apiRoute is one row of the answer. Every field is drawn from the |
| +// registry row EXCEPT Health, which is the same word instanceHealth and |
| +// the Running tab compute (healthByHost). |
| +type apiRoute struct { |
| + Host string `json:"host"` |
| + Kind string `json:"kind"` |
| + Upstream string `json:"upstream"` |
| + App string `json:"app"` |
| + Channel string `json:"channel"` |
| + Environment string `json:"environment"` |
| + Status string `json:"status"` |
| + Health string `json:"health"` |
| + Hibernate bool `json:"hibernate"` |
| + Unit string `json:"unit,omitempty"` |
| + Backing string `json:"backing,omitempty"` |
| + Provisioned bool `json:"provisioned"` |
| +} |
| + |
| +// routeUpstream renders the one cell answering "where does this route send |
| +// traffic": a socket wins over addresses (registry.Route's own |
| +// precedence), a multi-upstream set is shown whole rather than reduced to |
| +// its first element, and a tunnel-backed route says so instead of |
| +// rendering an empty cell a reader would take for a broken row. |
| +func routeUpstream(rt registry.Route) string { |
| + switch { |
| + case rt.Socket != "": |
| + return rt.Socket |
| + case rt.Addrs != "": |
| + return rt.Addrs |
| + case rt.Addr != "": |
| + return rt.Addr |
| + case rt.Tunnel != "": |
| + return "tunnel:" + rt.Tunnel |
| + } |
| + return "" |
| +} |
| + |
| +func (s *Server) handleAPIRoutes(w http.ResponseWriter, r *http.Request) { |
| + _, acct, app, ok := s.apiAccountApp(w, r) |
| + if !ok { |
| + return |
| + } |
| + if s.cfg.Registry == "" { |
| + // Never an empty list: a console with no registry cannot know any |
| + // route's state, and "[]" would read as "this app has no routes", |
| + // which is a different and wrong answer. (Opening "" would also |
| + // hand sqlite a throwaway temp DB and say the same thing, having |
| + // created a file for it — see instanceHealth's twin guard.) |
| + jsonError(w, http.StatusServiceUnavailable, |
| + "this console serves no route registry, so it cannot say what routes exist") |
| + return |
| + } |
| + reg, err := openRegistry(s.cfg.Registry) |
| + if err != nil { |
| + log.Printf("console: api routes: registry %s: %v", s.cfg.Registry, err) |
| + jsonError(w, http.StatusBadGateway, "could not read the route registry — try again") |
| + return |
| + } |
| + all, err := reg.List() |
| + if err != nil { |
| + log.Printf("console: api routes: registry list: %v", err) |
| + jsonError(w, http.StatusBadGateway, "could not read the route registry — try again") |
| + return |
| + } |
| + // The tenant boundary, and the only one: the read above returned every |
| + // route on this box, other accounts' included. A row enters routeFor |
| + // only if it is stamped to THIS app and resolves to THIS account. |
| + // release.RouteAccount, not a bare column compare, so an unstamped |
| + // account resolves the way every other read of that column resolves — |
| + // a second, private notion of "whose route is this" is exactly how a |
| + // listing ends up disagreeing with the reconciler about ownership. |
| + routeFor := map[string]registry.Route{} |
| + for _, rt := range all { |
| + if rt.App != app || release.RouteAccount(rt) != acct.Sqid { |
| + continue |
| + } |
| + routeFor[rt.Host] = rt |
| + } |
| + ctx, cancel := context.WithTimeout(r.Context(), routeListTimeout) |
| + defer cancel() |
| + health := s.healthByHost(ctx, routeFor) |
| + rows := make([]apiRoute, 0, len(routeFor)) |
| + for _, rt := range routeFor { |
| + rows = append(rows, apiRoute{ |
| + Host: rt.Host, Kind: rt.Kind, Upstream: routeUpstream(rt), App: rt.App, |
| + Channel: rt.Channel, Environment: rt.Environment, Status: rt.Status, |
| + Health: health[rt.Host], Hibernate: rt.Hibernate, Unit: rt.Unit, |
| + Backing: rt.Backing, Provisioned: rt.Provisioned, |
| + }) |
| + } |
| + // Sorted by host: registry order is not stable across passes, and a |
| + // listing that reshuffles between calls is one nobody can diff. |
| + sort.Slice(rows, func(i, j int) bool { return rows[i].Host < rows[j].Host }) |
| + writeJSON(w, http.StatusOK, map[string]any{"routes": rows}) |
| +} |
diff --git a/internal/console/apiroutes_test.go b/internal/console/apiroutes_test.go| new file mode 100644 |
| index 000000000..576c9e02c |
| --- /dev/null |
| +++ b/internal/console/apiroutes_test.go |
| @@ -0,0 +1,209 @@ |
| +package console |
| + |
| +// The routes read (apiroutes.go, issue #64). The properties that matter, in |
| +// the order they'd hurt if lost: the gate is the family's byte-identical |
| +// 404 and an app token cannot reach this door at all; a caller sees only |
| +// their own app's rows out of a registry holding every tenant's; and the |
| +// environment binding — the whole reason the issue was reopened — comes |
| +// back. |
| + |
| +import ( |
| + "net/http" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// routesURL is the routes door for one app in the fixture's account. |
| +func routesURL(fx *consoleFixture, app string) string { |
| + return fx.ts.URL + "/api/cli/apps/bab/" + app + "/routes" |
| +} |
| + |
| +// routesFixture is a console with titogo claimed by the ops account, a |
| +// member token for it, and a registry holding a spread of rows: two of |
| +// titogo's (one bound to an environment, one not), one belonging to |
| +// another app in the SAME account, and one stamped to a different account |
| +// entirely. |
| +func routesFixture(t *testing.T, code string) (*consoleFixture, string) { |
| + t.Helper() |
| + fx, token := configFixture(t, code) |
| + reg, err := registry.Open(fx.regPath) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, rt := range []registry.Route{ |
| + // The hand-added relay: no instance record anywhere, which is |
| + // exactly why `carlos instances list` could not answer for it. |
| + {Host: "connect.go.tito.io", Kind: "instance", Status: "ready", App: "titogo", |
| + Account: "bab", Addr: "127.0.0.1:9001", Channel: "canary/relay", Environment: "relay"}, |
| + {Host: "router-a.go.tito.io", Kind: "instance", Status: "ready", App: "titogo", |
| + Account: "bab", Addr: "127.0.0.1:9002", Channel: "stable"}, |
| + {Host: "neighbour.example.com", Kind: "instance", Status: "ready", App: "kass", |
| + Account: "bab", Addr: "127.0.0.1:9003"}, |
| + {Host: "foreign.example.com", Kind: "instance", Status: "ready", App: "titogo", |
| + Account: "zzzz", Addr: "127.0.0.1:9004", Environment: "someone-elses"}, |
| + } { |
| + if err := reg.Upsert(rt); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + return fx, token |
| +} |
| + |
| +// hostsOf indexes a routes response by host. |
| +func hostsOf(t *testing.T, out map[string]any) map[string]map[string]any { |
| + t.Helper() |
| + rows, ok := out["routes"].([]any) |
| + if !ok { |
| + t.Fatalf("response has no routes array: %v", out) |
| + } |
| + byHost := map[string]map[string]any{} |
| + for _, r := range rows { |
| + m, _ := r.(map[string]any) |
| + byHost[str(t, m, "host")] = m |
| + } |
| + return byHost |
| +} |
| + |
| +// ---- the gate --------------------------------------------------------------- |
| + |
| +// TestAPIRoutesGateIsByteIdentical: a token belonging to no account, an |
| +// account segment that doesn't exist, and an app the account never claimed |
| +// are one and the same 404 — byte-identical to http.NotFound's own output, |
| +// so probing this door teaches a token holder nothing. |
| +func TestAPIRoutesGateIsByteIdentical(t *testing.T) { |
| + fx, token := routesFixture(t, "code-routes-gate") |
| + eve := cliToken(t, fx, "eve@keymail.dev", "code-routes-gate-eve") |
| + |
| + statusA, bodyA := getRaw(t, routesURL(fx, "titogo"), eve) |
| + statusB, bodyB := getRaw(t, fx.ts.URL+"/api/cli/apps/zzzz/titogo/routes", token) |
| + statusC, bodyC := getRaw(t, routesURL(fx, "ghost-app"), token) |
| + if statusA != http.StatusNotFound || statusB != http.StatusNotFound || statusC != http.StatusNotFound { |
| + t.Fatalf("routes gate = %d / %d / %d, want 404 for all three", statusA, statusB, statusC) |
| + } |
| + if bodyA != bodyB || bodyB != bodyC { |
| + t.Fatalf("refusals are not byte-identical:\n(a) %q\n(b) %q\n(c) %q", bodyA, bodyB, bodyC) |
| + } |
| + if bodyA != "404 page not found\n" { |
| + t.Fatalf("refusal body = %q, want the stdlib http.NotFound output", bodyA) |
| + } |
| +} |
| + |
| +// TestAPIRoutesNoToken: the same 401 cliUser always gives, before it can |
| +// matter what account or app was named. |
| +func TestAPIRoutesNoToken(t *testing.T) { |
| + fx, _ := routesFixture(t, "code-routes-notoken") |
| + status, out := getJSON(t, routesURL(fx, "titogo"), "") |
| + if status != http.StatusUnauthorized { |
| + t.Fatalf("GET with no token = %d %v, want 401", status, out) |
| + } |
| +} |
| + |
| +// ---- the answer ------------------------------------------------------------- |
| + |
| +// TestAPIRoutesReportsTheEnvironmentBinding is the whole point of issue |
| +// #64: after `carlos route -host <h> -environment relay`, a member can |
| +// read the binding back without a box shell. |
| +func TestAPIRoutesReportsTheEnvironmentBinding(t *testing.T) { |
| + fx, token := routesFixture(t, "code-routes-env") |
| + status, out := getJSON(t, routesURL(fx, "titogo"), token) |
| + if status != http.StatusOK { |
| + t.Fatalf("GET routes = %d %v", status, out) |
| + } |
| + byHost := hostsOf(t, out) |
| + relay, ok := byHost["connect.go.tito.io"] |
| + if !ok { |
| + t.Fatalf("routes = %v, want the bound route listed", out["routes"]) |
| + } |
| + if got := str(t, relay, "environment"); got != "relay" { |
| + t.Fatalf("environment = %q, want %q", got, "relay") |
| + } |
| + if got := str(t, relay, "channel"); got != "canary/relay" { |
| + t.Fatalf("channel = %q, want %q", got, "canary/relay") |
| + } |
| + if got := str(t, relay, "upstream"); got != "127.0.0.1:9001" { |
| + t.Fatalf("upstream = %q, want the route's addr", got) |
| + } |
| + // An UNBOUND route reports "" — a real answer (it takes the app's |
| + // default bundle), not missing data. |
| + unbound, ok := byHost["router-a.go.tito.io"] |
| + if !ok { |
| + t.Fatalf("routes = %v, want the unbound route listed too", out["routes"]) |
| + } |
| + if got := str(t, unbound, "environment"); got != "" { |
| + t.Fatalf("unbound route environment = %q, want empty", got) |
| + } |
| +} |
| + |
| +// 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. |
| +func TestAPIRoutesListsOnlyThisAppsRoutes(t *testing.T) { |
| + fx, token := routesFixture(t, "code-routes-scope") |
| + status, out := getJSON(t, routesURL(fx, "titogo"), token) |
| + if status != http.StatusOK { |
| + t.Fatalf("GET routes = %d %v", status, out) |
| + } |
| + byHost := hostsOf(t, out) |
| + if _, present := byHost["neighbour.example.com"]; present { |
| + t.Fatal("another app's route in the same account was listed") |
| + } |
| + if _, present := byHost["foreign.example.com"]; present { |
| + t.Fatal("a route stamped to another ACCOUNT was listed — the app name alone is not the boundary") |
| + } |
| + if len(byHost) != 2 { |
| + t.Fatalf("routes = %v, want exactly titogo's two rows", out["routes"]) |
| + } |
| +} |
| + |
| +// TestAPIRoutesSortsByHost: registry order is not stable across passes, so |
| +// the answer must not be either — a listing that reshuffles between calls |
| +// is one nobody can diff. |
| +func TestAPIRoutesSortsByHost(t *testing.T) { |
| + fx, token := routesFixture(t, "code-routes-sort") |
| + status, out := getJSON(t, routesURL(fx, "titogo"), token) |
| + if status != http.StatusOK { |
| + t.Fatalf("GET routes = %d %v", status, out) |
| + } |
| + rows, _ := out["routes"].([]any) |
| + if len(rows) != 2 { |
| + t.Fatalf("routes = %v, want two", out["routes"]) |
| + } |
| + first, _ := rows[0].(map[string]any) |
| + if str(t, first, "host") != "connect.go.tito.io" { |
| + t.Fatalf("first row = %v, want the alphabetically first host", first) |
| + } |
| +} |
| + |
| +// TestAPIRoutesSaysSoWhenTheConsoleServesNoRegistry: never an empty list. |
| +// "[]" would read as "this app has no routes", which is a different and |
| +// wrong answer. |
| +func TestAPIRoutesSaysSoWhenTheConsoleServesNoRegistry(t *testing.T) { |
| + fx, token := routesFixture(t, "code-routes-noregistry") |
| + fx.srv.cfg.Registry = "" |
| + status, out := getJSON(t, routesURL(fx, "titogo"), token) |
| + if status != http.StatusServiceUnavailable { |
| + t.Fatalf("GET routes with no registry = %d %v, want 503", status, out) |
| + } |
| + if _, present := out["routes"]; present { |
| + t.Fatalf("a console with no registry answered with a routes array: %v", out) |
| + } |
| +} |
| + |
| +// TestAPIRoutesAppTokenIsRefused: an app token is a machine credential |
| +// minted for one instance to register itself. This answer describes every |
| +// route of the app — upstreams, units, bindings — so it is a person's |
| +// view, and the narrower gate is the whole difference between this door |
| +// and the instance family's. |
| +func TestAPIRoutesAppTokenIsRefused(t *testing.T) { |
| + fx, _ := routesFixture(t, "code-routes-apptoken") |
| + appTok, err := fx.st.NewAppToken(OpsAccountID, "titogo", "paul@keymail.dev", "test", time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + status, _ := getRaw(t, routesURL(fx, "titogo"), appTok) |
| + if status == http.StatusOK { |
| + t.Fatal("an app token reached the routes door — it must not") |
| + } |
| +} |
diff --git a/internal/console/authcli.go b/internal/console/authcli.go| index ee508b3e7..78b7de62c 100644 |
| --- a/internal/console/authcli.go |
| +++ b/internal/console/authcli.go |
| @@ -298,6 +298,12 @@ func (s *Server) registerCLIRoutes() { |
| s.mux.HandleFunc("GET /api/cli/apps/{account}/{app}/config", s.handleAPIConfigGet) |
| s.mux.HandleFunc("PUT /api/cli/apps/{account}/{app}/config", s.handleAPIConfigPut) |
| s.mux.HandleFunc("POST /api/cli/apps/{account}/{app}/config/sync", s.handleAPIConfigSync) |
| + // The routes read (apiroutes.go, issue #64) — the member-only door onto |
| + // this box's registry rows for one app, so a route's environment |
| + // binding can be read back without a box shell. Deliberately NOT |
| + // instanceGate: see that file's package comment for why an app token |
| + // must not reach it. |
| + s.mux.HandleFunc("GET /api/cli/apps/{account}/{app}/routes", s.handleAPIRoutes) |
| // The customer fleets API (fleets.go) — account-owned remote-box groups |
| // and where an app's routes/instances place onto them. `new` is a |
| // literal beside the {fleet} wildcard for the same reason apps/new is: |
diff --git a/internal/host/agent.go b/internal/host/agent.go| index 09c483b4f..b7498b778 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -578,10 +578,31 @@ func bounce(svc *activator.Service) func(string) error { |
| // DirStore's is hex). |
| const genAbsent = "\x00absent" |
| +// genCache is everything configGenLoop remembers between ticks: one |
| +// generation etag per config-carrying app, and the previous tick's |
| +// route-shape digests. One object rather than two parameters because the |
| +// two are remembered for exactly one reason — deciding what "changed" |
| +// means — and a caller holding one without the other would silently lose |
| +// half the loop's sensitivity. |
| +type genCache struct { |
| + etags map[[2]string]string |
| + // exec/unit are appconfig.ConfigRouteFingerprints' two digests as of |
| + // the last pass that computed them; shapeSeen is false until the first |
| + // such pass. The bool is not redundant with an empty string: a box with |
| + // no config-carrying routes at all has a real, stable digest of the |
| + // empty set, and treating that as "never seen" would make its first |
| + // route look like a change on the tick after boot rather than on the |
| + // tick it appeared. |
| + exec, unit string |
| + shapeSeen bool |
| +} |
| + |
| +func newGenCache() *genCache { return &genCache{etags: map[[2]string]string{}} } |
| + |
| // genPollPass is one tick of configGenLoop, split out for tests: read the |
| // generation etag of every app the box can converge — exec-config apps and |
| -// unit-backed apps alike — and report which HALVES moved since the cache |
| -// was built. |
| +// unit-backed apps alike — plus the registry's own route shape, and report |
| +// which HALVES moved since the cache was built. |
| // |
| // Semantics, each of which is a deliberate choice: |
| // |
| @@ -613,6 +634,20 @@ const genAbsent = "\x00absent" |
| // re-added app is a first sight again rather than a comparison against |
| // an etag remembered from before it left (a route being re-added gets |
| // fresh config from the spawn path anyway). |
| +// - The ROUTE SHAPE is watched too, not just the bucket. A route's |
| +// Environment binding lives in the box's own registry and moves no etag |
| +// anywhere, so without this the loop cannot see a rebinding at all and |
| +// delivery waits out configSyncLoop's hourly floor: issue #196, where a |
| +// live route served the wrong merge for ten minutes and the only symptom |
| +// was the app reporting a credential missing — which reads as a BAD |
| +// credential rather than an undelivered one. First sight of the shape is |
| +// silent for the same reason first sight of an app is. This covers BOTH |
| +// doors onto the column: `carlos route -environment` on the box, and the |
| +// provisioning reconciler converging Environment from an instance record |
| +// — which is the sanctioned door for a Provisioned row, and which the |
| +// issue's own suggested fix (bump the generation from `carlos route`) |
| +// would have missed entirely. It costs no bucket read: see |
| +// appconfig.ConfigRouteFingerprints. |
| // - A read error that isn't "not found" leaves that app's cache entry |
| // exactly as it was and keeps evaluating the rest: the errors come back |
| // for the caller to aggregate into ONE log line per pass, never one per |
| @@ -622,7 +657,17 @@ const genAbsent = "\x00absent" |
| // The cache is mutated in place — it lives across ticks in the loop, which |
| // is the whole point (there is nowhere else to remember what "changed" |
| // means). |
| -func genPollPass(ctx context.Context, routes []registry.Route, store release.Store, cache map[[2]string]string) (execChanged, unitChanged bool, readErrs []error) { |
| +func genPollPass(ctx context.Context, routes []registry.Route, store release.Store, cache *genCache) (execChanged, unitChanged bool, readErrs []error) { |
| + // The registry-side half (issue #196). Free — no bucket read at all — |
| + // and evaluated first so a rebinding converges on the same tick a var |
| + // change would, rather than one behind it. |
| + execFP, unitFP := appconfig.ConfigRouteFingerprints(routes) |
| + if cache.shapeSeen { |
| + execChanged = execChanged || execFP != cache.exec |
| + unitChanged = unitChanged || unitFP != cache.unit |
| + } |
| + cache.exec, cache.unit, cache.shapeSeen = execFP, unitFP, true |
| + |
| execApps := appconfig.ExecConfigApps(routes) |
| unitApps := appconfig.UnitConfigApps(routes) |
| // The poll set is the UNION, read once per app: the two sets overlap |
| @@ -654,16 +699,16 @@ func genPollPass(ctx context.Context, routes []registry.Route, store release.Sto |
| readErrs = append(readErrs, fmt.Errorf("%s/%s: %w", k[0], k[1], err)) |
| continue // cache entry untouched: this app is unknown this tick, not changed |
| } |
| - prev, seen := cache[k] |
| - cache[k] = etag |
| + prev, seen := cache.etags[k] |
| + cache.etags[k] = etag |
| if seen && prev != etag { |
| execChanged = execChanged || inExec[k] |
| unitChanged = unitChanged || inUnit[k] |
| } |
| } |
| - for k := range cache { |
| + for k := range cache.etags { |
| if !live[k] { |
| - delete(cache, k) |
| + delete(cache.etags, k) |
| } |
| } |
| return execChanged, unitChanged, readErrs |
| @@ -808,7 +853,7 @@ func configGenLoop(reg *registry.Registry, svc *activator.Service, dd string) { |
| } |
| // One cache for the life of the process: what "changed" means is |
| // exactly "differs from the last tick that could read it". |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| log.Printf("agent: config generation loop armed (3s + jitter, exec-config and unit-backed apps)") |
| for { |
| time.Sleep(3*time.Second + rand.N(time.Second)) |
diff --git a/internal/host/agent_test.go b/internal/host/agent_test.go| index 7ce08214e..07758c854 100644 |
| --- a/internal/host/agent_test.go |
| +++ b/internal/host/agent_test.go |
| @@ -113,7 +113,7 @@ func TestGenPollPassFirstSightIsSilent(t *testing.T) { |
| ctx, s := genKit(t) |
| touch(t, ctx, s, "bab", "eleven") |
| routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")} |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| execChanged, unitChanged, errs := genPollPass(ctx, routes, s, cache) |
| if execChanged || unitChanged { |
| @@ -123,7 +123,7 @@ func TestGenPollPassFirstSightIsSilent(t *testing.T) { |
| t.Fatalf("unexpected read errors: %v", errs) |
| } |
| want := genEtag(t, ctx, s, "bab", "eleven") |
| - if got := cache[[2]string{"bab", "eleven"}]; got != want { |
| + if got := cache.etags[[2]string{"bab", "eleven"}]; got != want { |
| t.Fatalf("cache etag = %q, want the store's %q", got, want) |
| } |
| } |
| @@ -132,7 +132,7 @@ func TestGenPollPassUnchangedIsSilent(t *testing.T) { |
| ctx, s := genKit(t) |
| touch(t, ctx, s, "bab", "eleven") |
| routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")} |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| genPollPass(ctx, routes, s, cache) |
| execChanged, unitChanged, errs := genPollPass(ctx, routes, s, cache) |
| @@ -148,7 +148,7 @@ func TestGenPollPassTouchedGenerationTriggersSync(t *testing.T) { |
| ctx, s := genKit(t) |
| touch(t, ctx, s, "bab", "eleven") |
| routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")} |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| genPollPass(ctx, routes, s, cache) |
| touch(t, ctx, s, "bab", "eleven") |
| @@ -163,7 +163,7 @@ func TestGenPollPassTouchedGenerationTriggersSync(t *testing.T) { |
| t.Fatalf("unexpected read errors: %v", errs) |
| } |
| want := genEtag(t, ctx, s, "bab", "eleven") |
| - if got := cache[[2]string{"bab", "eleven"}]; got != want { |
| + if got := cache.etags[[2]string{"bab", "eleven"}]; got != want { |
| t.Fatalf("cache etag = %q, want the new %q (a stale cache would re-sync forever)", got, want) |
| } |
| if execChanged, unitChanged, _ := genPollPass(ctx, routes, s, cache); execChanged || unitChanged { |
| @@ -176,7 +176,7 @@ func TestGenPollPassAbsentGenerationStaysSilent(t *testing.T) { |
| // No touch at all: an app last written by a pre-generation CLI. Legal |
| // forever — the hourly configSyncLoop is its floor. |
| routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")} |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| for i := range 2 { |
| execChanged, unitChanged, errs := genPollPass(ctx, routes, s, cache) |
| @@ -187,7 +187,7 @@ func TestGenPollPassAbsentGenerationStaysSilent(t *testing.T) { |
| t.Fatalf("pass %d: absent is not a read error: %v", i, errs) |
| } |
| } |
| - if got := cache[[2]string{"bab", "eleven"}]; got != genAbsent { |
| + if got := cache.etags[[2]string{"bab", "eleven"}]; got != genAbsent { |
| t.Fatalf("cache = %q, want the known-absent sentinel %q", got, genAbsent) |
| } |
| } |
| @@ -195,7 +195,7 @@ func TestGenPollPassAbsentGenerationStaysSilent(t *testing.T) { |
| func TestGenPollPassAbsentToPresentTriggersSync(t *testing.T) { |
| ctx, s := genKit(t) |
| routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")} |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| genPollPass(ctx, routes, s, cache) // records known-absent |
| touch(t, ctx, s, "bab", "eleven") // the app's first ever config write |
| @@ -216,7 +216,7 @@ func TestGenPollPassOneChangedAppSyncsOnce(t *testing.T) { |
| execRoute("a.example.com", "bab", "eleven"), |
| execRoute("b.example.com", "bab", "titogo"), |
| } |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| genPollPass(ctx, routes, s, cache) |
| touch(t, ctx, s, "bab", "titogo") |
| @@ -230,7 +230,7 @@ func TestGenPollPassOneChangedAppSyncsOnce(t *testing.T) { |
| // One flag for the whole pass: the caller runs ONE SyncExec, which |
| // resolves every app itself. The unchanged app's cache entry must be |
| // carried forward untouched so it doesn't re-fire next tick. |
| - if got, want := cache[[2]string{"bab", "eleven"}], genEtag(t, ctx, s, "bab", "eleven"); got != want { |
| + if got, want := cache.etags[[2]string{"bab", "eleven"}], genEtag(t, ctx, s, "bab", "eleven"); got != want { |
| t.Fatalf("unchanged app's cache = %q, want %q", got, want) |
| } |
| if execChanged, _, _ := genPollPass(ctx, routes, s, cache); execChanged { |
| @@ -247,21 +247,37 @@ func TestGenPollPassDropsVanishedApps(t *testing.T) { |
| execRoute("b.example.com", "bab", "titogo"), |
| } |
| one := both[:1] |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| genPollPass(ctx, both, s, cache) |
| genPollPass(ctx, one, s, cache) // titogo's route is gone |
| - if _, ok := cache[[2]string{"bab", "titogo"}]; ok { |
| + if _, ok := cache.etags[[2]string{"bab", "titogo"}]; ok { |
| t.Fatal("a route that left the registry must leave no cache entry behind") |
| } |
| // The proof that dropping matters: while the app was unrouted its |
| - // config moved. Re-adding the route must record silently (first sight |
| - // again), not fire a sync off a remembered etag — a route being |
| - // re-added already gets fresh config from the spawn path. |
| + // config moved. The ETAG half must record that silently (first sight |
| + // again) rather than comparing against a remembered value — a route |
| + // being re-added already gets fresh config from the spawn path. |
| + // |
| + // The tick itself is NOT silent, and that is the route-shape half |
| + // speaking, not the etag one (issue #196): a route entering or leaving |
| + // scope changes what belongs on disk — a file to write, or one to reap |
| + // — so it earns a pass. The pass is cheap and cannot cost a restart: |
| + // SyncExec bounces only a route whose rendered bytes actually changed. |
| + // What this test still pins directly is the cache entry above; the |
| + // etag half's silence is no longer separately observable from here, |
| + // since the shape movement covers it either way. |
| touch(t, ctx, s, "bab", "titogo") |
| + _, _, errs := genPollPass(ctx, both, s, cache) |
| + if len(errs) != 0 { |
| + t.Fatalf("unexpected read errors: %v", errs) |
| + } |
| + // Settled again: with the route set stable and no bucket movement, the |
| + // next tick must be silent. A remembered etag or a mis-stored digest |
| + // would still be firing here. |
| execChanged, unitChanged, errs := genPollPass(ctx, both, s, cache) |
| if execChanged || unitChanged { |
| - t.Fatal("a re-added app is a first sight, not a change") |
| + t.Fatal("a settled tick after a re-add must be silent") |
| } |
| if len(errs) != 0 { |
| t.Fatalf("unexpected read errors: %v", errs) |
| @@ -276,9 +292,9 @@ func TestGenPollPassReadErrorLeavesCacheAndKeepsGoing(t *testing.T) { |
| execRoute("a.example.com", "bab", "eleven"), |
| execRoute("b.example.com", "bab", "titogo"), |
| } |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| genPollPass(ctx, routes, dir, cache) |
| - elevenEtag := cache[[2]string{"bab", "eleven"}] |
| + elevenEtag := cache.etags[[2]string{"bab", "eleven"}] |
| // eleven's generation object goes unreadable; titogo's moves. |
| boom := errors.New("bucket said no") |
| @@ -295,7 +311,7 @@ func TestGenPollPassReadErrorLeavesCacheAndKeepsGoing(t *testing.T) { |
| if !errors.Is(errs[0], boom) { |
| t.Fatalf("read error = %v, want it to wrap %v", errs[0], boom) |
| } |
| - if got := cache[[2]string{"bab", "eleven"}]; got != elevenEtag { |
| + if got := cache.etags[[2]string{"bab", "eleven"}]; got != elevenEtag { |
| t.Fatalf("unreadable app's cache = %q, want it untouched at %q (a cleared entry would re-fire on recovery)", got, elevenEtag) |
| } |
| } |
| @@ -304,7 +320,7 @@ func TestGenPollPassUnitFirstSightIsSilent(t *testing.T) { |
| ctx, s := genKit(t) |
| touch(t, ctx, s, "bab", "unitapp") |
| routes := []registry.Route{unitRoute("u.example.com", "bab", "unitapp")} |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| execChanged, unitChanged, errs := genPollPass(ctx, routes, s, cache) |
| if execChanged || unitChanged { |
| @@ -314,7 +330,7 @@ func TestGenPollPassUnitFirstSightIsSilent(t *testing.T) { |
| t.Fatalf("unexpected read errors: %v", errs) |
| } |
| want := genEtag(t, ctx, s, "bab", "unitapp") |
| - if got := cache[[2]string{"bab", "unitapp"}]; got != want { |
| + if got := cache.etags[[2]string{"bab", "unitapp"}]; got != want { |
| t.Fatalf("cache etag = %q, want the store's %q", got, want) |
| } |
| } |
| @@ -327,7 +343,7 @@ func TestGenPollPassUnitMovementRequestsRootPassOnly(t *testing.T) { |
| unitRoute("u.example.com", "bab", "unitapp"), |
| execRoute("a.example.com", "bab", "eleven"), |
| } |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| genPollPass(ctx, routes, s, cache) |
| touch(t, ctx, s, "bab", "unitapp") |
| @@ -357,7 +373,7 @@ func TestGenPollPassBothShapesSetBothHalvesOnOneRead(t *testing.T) { |
| unitRoute("mixed-unit.example.com", "zed", "mixed"), |
| } |
| s := &countingStore{Store: dir} |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| genPollPass(ctx, routes, s, cache) |
| touch(t, ctx, dir, "zed", "mixed") |
| @@ -381,7 +397,7 @@ func TestGenPollPassPrunesUnitAppsToo(t *testing.T) { |
| unitRoute("u.example.com", "bab", "unitapp"), |
| } |
| one := both[:1] |
| - cache := map[[2]string]string{} |
| + cache := newGenCache() |
| genPollPass(ctx, both, s, cache) |
| // The live set the cache is pruned against is the UNION: a unit app whose |
| @@ -389,13 +405,21 @@ func TestGenPollPassPrunesUnitAppsToo(t *testing.T) { |
| // remembered etag would fire a bogus root-pass request when the route |
| // comes back (adoption already delivers fresh config on the way in). |
| genPollPass(ctx, one, s, cache) |
| - if _, ok := cache[[2]string{"bab", "unitapp"}]; ok { |
| + if _, ok := cache.etags[[2]string{"bab", "unitapp"}]; ok { |
| t.Fatal("a unit-backed route that left the registry must leave no cache entry behind") |
| } |
| + // As in TestGenPollPassDropsVanishedApps: the re-add tick itself moves |
| + // the route-shape digest (issue #196), so the silence worth asserting |
| + // is the SETTLED tick after it. The dropped cache entry above is what |
| + // pins the etag half directly. |
| touch(t, ctx, s, "bab", "unitapp") |
| + _, _, errs := genPollPass(ctx, both, s, cache) |
| + if len(errs) != 0 { |
| + t.Fatalf("unexpected read errors: %v", errs) |
| + } |
| execChanged, unitChanged, errs := genPollPass(ctx, both, s, cache) |
| if execChanged || unitChanged { |
| - t.Fatal("a re-added unit-backed app is a first sight, not a change") |
| + t.Fatal("a settled tick after a unit-backed re-add must be silent") |
| } |
| if len(errs) != 0 { |
| t.Fatalf("unexpected read errors: %v", errs) |
| @@ -1666,3 +1690,103 @@ func TestStampedUnitNamesEmptyRegistry(t *testing.T) { |
| t.Fatalf("stampedUnitNames on an empty registry = %v, want nil", got) |
| } |
| } |
| + |
| +// --- issue #196: the registry side of "did anything change" --- |
| + |
| +// boundExecRoute is execRoute wired to a named config environment — the |
| +// binding whose whole point is that it changes which bundle merges into |
| +// the route's delivered config. |
| +func boundExecRoute(host, acct, app, env string) registry.Route { |
| + rt := execRoute(host, acct, app) |
| + rt.Environment = env |
| + return rt |
| +} |
| + |
| +// TestGenPollPassSyncsOnRouteRebinding is issue #196: binding a route to an |
| +// environment must converge from the FAST loop, not an hour later on |
| +// configSyncLoop's floor. The bucket does not move at all here — only the |
| +// registry column does, which is exactly why the etag poll could not see |
| +// it and titogo lost an hour to a symptom reading as a bad credential. |
| +func TestGenPollPassSyncsOnRouteRebinding(t *testing.T) { |
| + ctx, s := genKit(t) |
| + touch(t, ctx, s, "bab", "titogo") |
| + cache := newGenCache() |
| + genPollPass(ctx, []registry.Route{execRoute("connect.go.tito.io", "bab", "titogo")}, s, cache) |
| + |
| + rebound := []registry.Route{boundExecRoute("connect.go.tito.io", "bab", "titogo", "relay")} |
| + execChanged, unitChanged, errs := genPollPass(ctx, rebound, s, cache) |
| + if len(errs) != 0 { |
| + t.Fatalf("unexpected read errors: %v", errs) |
| + } |
| + if !execChanged { |
| + t.Fatal("binding a route to an environment must make the exec half sync") |
| + } |
| + if unitChanged { |
| + t.Fatal("an exec route's rebinding must not ask for a root config pass") |
| + } |
| + // And the pass after it is quiet again: a digest that re-fired every |
| + // tick would run SyncExec forever. |
| + if execChanged, _, _ := genPollPass(ctx, rebound, s, cache); execChanged { |
| + t.Fatal("the pass after a rebinding must be silent again (a stale digest would re-sync forever)") |
| + } |
| +} |
| + |
| +// TestGenPollPassSyncsOnUnitRouteRebinding: the unit half converges per |
| +// environment too (appconfig.Sync's group is (acct, app, environment)), so |
| +// a unit-backed route's rebinding must ask for the root pass — and must |
| +// not make the agent render anything itself. |
| +func TestGenPollPassSyncsOnUnitRouteRebinding(t *testing.T) { |
| + ctx, s := genKit(t) |
| + touch(t, ctx, s, "bae", "keymail") |
| + cache := newGenCache() |
| + genPollPass(ctx, []registry.Route{unitRoute("keymail.dev", "bae", "keymail")}, s, cache) |
| + |
| + rebound := []registry.Route{unitRoute("keymail.dev", "bae", "keymail")} |
| + rebound[0].Environment = "staging" |
| + execChanged, unitChanged, errs := genPollPass(ctx, rebound, s, cache) |
| + if len(errs) != 0 { |
| + t.Fatalf("unexpected read errors: %v", errs) |
| + } |
| + if !unitChanged { |
| + t.Fatal("binding a unit-backed route to an environment must request a root config pass") |
| + } |
| + if execChanged { |
| + t.Fatal("a unit route's rebinding must not run a SyncExec pass") |
| + } |
| +} |
| + |
| +// TestGenPollPassFirstSightOfRouteShapeIsSilent: a box coming up has never |
| +// seen the digest before, and must not treat that as a change — every |
| +// instance on the box would sync at once on boot. configSyncLoop's ~1min |
| +// first pass owns boot convergence, exactly as it does for etags. |
| +func TestGenPollPassFirstSightOfRouteShapeIsSilent(t *testing.T) { |
| + ctx, s := genKit(t) |
| + touch(t, ctx, s, "bab", "titogo") |
| + routes := []registry.Route{boundExecRoute("connect.go.tito.io", "bab", "titogo", "relay")} |
| + execChanged, unitChanged, errs := genPollPass(ctx, routes, s, newGenCache()) |
| + if len(errs) != 0 { |
| + t.Fatalf("unexpected read errors: %v", errs) |
| + } |
| + if execChanged || unitChanged { |
| + t.Fatal("the first pass must record the route shape silently") |
| + } |
| +} |
| + |
| +// TestGenPollPassRouteShapeCostsNoBucketReads: the registry half must not |
| +// add a single fetch to the tick — the whole economy of this loop is that |
| +// it reads generation objects and nothing else. |
| +func TestGenPollPassRouteShapeCostsNoBucketReads(t *testing.T) { |
| + ctx, _ := genKit(t) |
| + cs := &countingStore{Store: release.NewDirStore(t.TempDir())} |
| + touch(t, ctx, cs, "bab", "titogo") |
| + routes := []registry.Route{execRoute("connect.go.tito.io", "bab", "titogo")} |
| + cache := newGenCache() |
| + genPollPass(ctx, routes, cs, cache) |
| + key := appconfig.GenKey("bab", "titogo") |
| + before := cs.gets[key] |
| + rebound := []registry.Route{boundExecRoute("connect.go.tito.io", "bab", "titogo", "relay")} |
| + genPollPass(ctx, rebound, cs, cache) |
| + if got := cs.gets[key] - before; got != 1 { |
| + t.Fatalf("the rebinding tick made %d bucket reads, want exactly 1 (the app's generation object, as before)", got) |
| + } |
| +} |
diff --git a/main.go b/main.go| index 15f585435..fd96f0e7b 100644 |
| --- a/main.go |
| +++ b/main.go |
| @@ -53,6 +53,8 @@ func main() { |
| runRoute(args) |
| case "instances": |
| runInstances(args) |
| + case "routes": |
| + runRoutes(args) |
| case "domains": |
| runDomains(args) |
| case "ship": |
| @@ -158,6 +160,8 @@ Your apps — ship and run software on a CARLOS deployment |
| secrets sealed per-app secrets: set | unset | list | genkey |
| instances an app's instances: enable | create | list | delete | |
| set-upstreams |
| + routes an app's routes and what each is bound to: upstream, |
| + channel, config environment, status |
| domains a customer's own hostname attached to an app's route: |
| attach | detach | list |
| logs an app's merged log timeline: its own output plus platform |
diff --git a/routes_cli.go b/routes_cli.go| new file mode 100644 |
| index 000000000..a982da920 |
| --- /dev/null |
| +++ b/routes_cli.go |
| @@ -0,0 +1,85 @@ |
| +// `carlos routes` — what routes this app has, and what each one is bound |
| +// to (issue #64). |
| +// |
| +// API MODE ONLY, and that is the point of the verb existing at all. The |
| +// facts it prints live in a box's registry, and reading them used to mean |
| +// sqlite3 on the box: `carlos route` is a setter, `carlos apps` answers |
| +// from the console's app list, and `carlos instances list` answers for |
| +// instance RECORDS — so a hand-added route had no reader anywhere. Since a |
| +// route's ENVIRONMENT binding decides which sealed secrets it is |
| +// delivered, "which routes is this credential going to" was a question |
| +// only answerable with box access: the one step of titogo's whole secrets |
| +// procedure that needed it. |
| +// |
| +// The box-side listing is `carlos instances` (cli.go), which carries the |
| +// same columns for an operator already standing on the box. A second |
| +// box-local verb answering the same question would be two commands to keep |
| +// in step for no gain. |
| +package main |
| + |
| +import ( |
| + "context" |
| + "flag" |
| + "fmt" |
| + "log" |
| + "strings" |
| +) |
| + |
| +const routesUsage = `usage: carlos routes -app <a> [-account <sqid|name>] [-match <substr>] [-console <origin>] |
| + |
| +Lists this app's routes on the console's deployment: where each one sends |
| +traffic, which release channel it follows, and which named config |
| +environment it is bound to — the binding that decides which secrets the |
| +route's process is delivered (see carlos env, carlos secrets). |
| + |
| +Signed-in only (carlos auth login). On a box, the local listing is |
| +"carlos instances".` |
| + |
| +func runRoutes(args []string) { |
| + fs := flag.NewFlagSet("routes", flag.ExitOnError) |
| + app := fs.String("app", "", "app whose routes to list") |
| + match := fs.String("match", "", "only hosts containing this substring") |
| + consoleFlag := fs.String("console", "", "console to list through (default: the only logged-in one)") |
| + acctRaw := fs.String("account", "", "account (sqid or name) the app belongs to (default: your only account, or the configured/chosen one)") |
| + acctIDRaw := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account") |
| + fs.Parse(args) |
| + sess := requireAPISession("routes", *consoleFlag) |
| + acct := resolvedAccount(sess, acctRaw, acctIDRaw) |
| + app2 := resolvedApp(sess, acct, *app, routesUsage) |
| + rows, err := sess.routeList(context.Background(), acct, app2) |
| + if err != nil { |
| + log.Fatalf("routes: %v", err) |
| + } |
| + shown := 0 |
| + w := newTable(stdout) |
| + fmt.Fprintln(w, "HOST\tKIND\tUPSTREAM\tCHANNEL\tENVIRONMENT\tSTATUS\tHEALTH") |
| + for _, r := range rows { |
| + if *match != "" && !strings.Contains(r.Host, *match) { |
| + continue |
| + } |
| + shown++ |
| + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\n", |
| + r.Host, dashIfEmpty(r.Kind), dashIfEmpty(r.Upstream), dashIfEmpty(r.Channel), |
| + // An empty ENVIRONMENT is a real answer — the route takes the |
| + // app's default bundle — so it prints as a dash rather than a |
| + // blank cell a reader would take for missing data. |
| + dashIfEmpty(r.Environment), dashIfEmpty(r.Status), dashIfEmpty(r.Health)) |
| + } |
| + w.Flush() |
| + if shown == 0 { |
| + mutedf(stdout, "no routes for %s on this deployment's box", app2) |
| + } |
| + // Said every time, not only when the list is empty: this console reads |
| + // its OWN box's registry, so on a multi-box fleet a short answer is not |
| + // a complete one and must not be read as one (internal/fleetapi, #145). |
| + mutedf(stdout, "(routes on this deployment's own box; other boxes in a fleet are not listed)") |
| +} |
| + |
| +// dashIfEmpty renders an empty cell as an em dash, so a column that is |
| +// legitimately blank reads as an answer rather than as missing data. |
| +func dashIfEmpty(s string) string { |
| + if s == "" { |
| + return "—" |
| + } |
| + return s |
| +} |
diff --git a/routes_cli_test.go b/routes_cli_test.go| new file mode 100644 |
| index 000000000..34fc47e6f |
| --- /dev/null |
| +++ b/routes_cli_test.go |
| @@ -0,0 +1,144 @@ |
| +package main |
| + |
| +import ( |
| + "bytes" |
| + "context" |
| + "encoding/json" |
| + "net/http" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// routesConsole plants a credential for an httptest console that answers |
| +// GET /api/cli/apps (account resolution) and the routes door for titogo. |
| +func routesConsole(t *testing.T, rows []map[string]any) string { |
| + t.Helper() |
| + return signedInConsole(t, func(w http.ResponseWriter, r *http.Request) { |
| + switch { |
| + case r.URL.Path == "/api/cli/apps": |
| + json.NewEncoder(w).Encode(map[string]any{"accounts": []map[string]any{ |
| + {"sqid": "bae", "name": "tom@keymail.dev", "role": "owner", "apps": []string{"titogo"}}, |
| + }}) |
| + case strings.HasSuffix(r.URL.Path, "/titogo/routes"): |
| + json.NewEncoder(w).Encode(map[string]any{"routes": rows}) |
| + default: |
| + http.NotFound(w, r) |
| + } |
| + }) |
| +} |
| + |
| +// TestRouteListReportsTheEnvironmentBinding is issue #64 at the client |
| +// edge: the binding survives the wire and reaches the caller. |
| +func TestRouteListReportsTheEnvironmentBinding(t *testing.T) { |
| + useTempCredentials(t) |
| + origin := routesConsole(t, []map[string]any{ |
| + {"host": "connect.go.tito.io", "kind": "instance", "upstream": "127.0.0.1:9001", |
| + "channel": "canary/relay", "environment": "relay", "status": "ready", "health": "Running"}, |
| + }) |
| + sess, err := resolveMode(origin) |
| + if err != nil { |
| + t.Fatalf("session: %v", err) |
| + } |
| + rows, err := sess.routeList(context.Background(), apiAcct{Seg: "bae"}, "titogo") |
| + if err != nil { |
| + t.Fatalf("routeList: %v", err) |
| + } |
| + if len(rows) != 1 { |
| + t.Fatalf("rows = %v, want one", rows) |
| + } |
| + if rows[0].Environment != "relay" { |
| + t.Fatalf("environment = %q, want %q", rows[0].Environment, "relay") |
| + } |
| + if rows[0].Upstream != "127.0.0.1:9001" || rows[0].Channel != "canary/relay" { |
| + t.Fatalf("row = %+v, want the upstream and channel decoded too", rows[0]) |
| + } |
| +} |
| + |
| +// TestRouteListOldConsoleIsNamed: a console predating the endpoint 404s the |
| +// path itself. That must not be paraphrased as "no such app" — the |
| +// membership gate's 404 and a missing endpoint are different problems with |
| +// different fixes. |
| +func TestRouteListOldConsoleIsNamed(t *testing.T) { |
| + useTempCredentials(t) |
| + origin := signedInConsole(t, func(w http.ResponseWriter, r *http.Request) { |
| + http.NotFound(w, r) |
| + }) |
| + sess, err := resolveMode(origin) |
| + if err != nil { |
| + t.Fatalf("session: %v", err) |
| + } |
| + if _, err := sess.routeList(context.Background(), apiAcct{Seg: "bae"}, "titogo"); err == nil { |
| + t.Fatal("a 404 from the routes door must surface as an error, never as an empty list") |
| + } |
| +} |
| + |
| +// TestDashIfEmpty: an unbound route's empty ENVIRONMENT is a real answer — |
| +// it takes the app's default bundle — and must render as such rather than |
| +// as a blank cell a reader would take for missing data. |
| +func TestDashIfEmpty(t *testing.T) { |
| + if got := dashIfEmpty(""); got != "—" { |
| + t.Fatalf("dashIfEmpty(\"\") = %q, want an em dash", got) |
| + } |
| + if got := dashIfEmpty("relay"); got != "relay" { |
| + t.Fatalf("dashIfEmpty(%q) = %q, want it unchanged", "relay", got) |
| + } |
| +} |
| + |
| +// TestRoutesTableCarriesEveryColumn drives the VERB, not a copy of its |
| +// rendering: a column dropped from runRoutes' Fprintf must fail here |
| +// rather than in an operator's terminal. |
| +func TestRoutesTableCarriesEveryColumn(t *testing.T) { |
| + useTempCredentials(t) |
| + origin := routesConsole(t, []map[string]any{ |
| + {"host": "connect.go.tito.io", "kind": "instance", "upstream": "127.0.0.1:9001", |
| + "channel": "canary/relay", "environment": "relay", "status": "ready", "health": "Running"}, |
| + {"host": "router-a.go.tito.io", "kind": "instance", "upstream": "127.0.0.1:9002", |
| + "channel": "stable", "status": "ready", "health": "Running"}, |
| + }) |
| + var out bytes.Buffer |
| + old := stdout |
| + stdout = &out |
| + t.Cleanup(func() { stdout = old }) |
| + |
| + runRoutes([]string{"-app", "titogo", "-account-id", "bae", "-console", origin}) |
| + |
| + for _, want := range []string{ |
| + "HOST", "KIND", "UPSTREAM", "CHANNEL", "ENVIRONMENT", "STATUS", "HEALTH", |
| + "connect.go.tito.io", "canary/relay", "relay", "Running", "127.0.0.1:9001", |
| + } { |
| + if !strings.Contains(out.String(), want) { |
| + t.Fatalf("routes output missing %q:\n%s", want, out.String()) |
| + } |
| + } |
| + // The unbound row's ENVIRONMENT cell is the dash, not a blank: taking |
| + // the app's default bundle is an answer, not missing data. |
| + line := "" |
| + for _, l := range strings.Split(out.String(), "\n") { |
| + if strings.HasPrefix(l, "router-a.go.tito.io") { |
| + line = l |
| + } |
| + } |
| + if !strings.Contains(line, "\u2014") { |
| + t.Fatalf("unbound row = %q, want an em dash in the ENVIRONMENT cell", line) |
| + } |
| + // The fleet caveat is printed every time, not only on an empty list. |
| + if !strings.Contains(out.String(), "other boxes in a fleet are not listed") { |
| + t.Fatalf("routes output missing the multi-box caveat:\n%s", out.String()) |
| + } |
| +} |
| + |
| +// TestRoutesEmptyListSaysSo: an empty answer prints a line, not a bare |
| +// header an operator has to interpret. |
| +func TestRoutesEmptyListSaysSo(t *testing.T) { |
| + useTempCredentials(t) |
| + origin := routesConsole(t, []map[string]any{}) |
| + var out bytes.Buffer |
| + old := stdout |
| + stdout = &out |
| + t.Cleanup(func() { stdout = old }) |
| + |
| + runRoutes([]string{"-app", "titogo", "-account-id", "bae", "-console", origin}) |
| + if !strings.Contains(out.String(), "no routes for titogo") { |
| + t.Fatalf("empty routes output = %q, want a line saying so", out.String()) |
| + } |
| +} |