Merge branch 'deploy-build-probe' into main
10 files changed,
+395
−12
apimode.go+32 −0apps.go+13 −1apps_buildprobe.go+97 −0deploy.go+31 −2deploy_e2e_test.go+16 −2docs/site/cli.md+25 −4internal/console/apibuildprobe.go+77 −0internal/console/apibuildprobe_test.go+88 −0internal/console/authcli.go+6 −0internal/console/svcroutes_test.go+10 −3
CI passed — run details
diff --git a/apimode.go b/apimode.go| index 0267d6b43..9650bccf4 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -3130,6 +3130,38 @@ func (s *apiSession) place(ctx context.Context, a apiAcct, app, target string) e |
| return nil |
| } |
| +// setBuildProbe stamps (or, with "", clears) an existing app's build-probe |
| +// path — the path `carlos deploy` polls INSTEAD of the X-Carlos-Version |
| +// header, which reports the adopted release rather than the running process. |
| +// |
| +// It returns what the console echoes back, which is what the store actually |
| +// holds: this is the field the deploy watch trusts, so a write that silently |
| +// failed has to be a loud error rather than a quiet lie. Same discipline the |
| +// claim door's own echo has. |
| +func (s *apiSession) setBuildProbe(ctx context.Context, a apiAcct, app, probe string) (string, error) { |
| + in := struct { |
| + BuildProbe string `json:"build_probe"` |
| + }{probe} |
| + status, body, err := s.postJSON(ctx, apiPath(a, app, "/build-probe"), in) |
| + if err != nil { |
| + return "", err |
| + } |
| + if status != http.StatusOK { |
| + if status == http.StatusNotFound { |
| + return "", fmt.Errorf("not found — either %s is not an app in this account, "+ |
| + "or this console predates `carlos apps build-probe` (deploy a console built from current main)", app) |
| + } |
| + return "", apiErrorFromBody(status, body) |
| + } |
| + var out struct { |
| + BuildProbe string `json:"build_probe"` |
| + } |
| + if err := json.Unmarshal(body, &out); err != nil { |
| + return "", fmt.Errorf("apps build-probe: unreadable response: %w", err) |
| + } |
| + return out.BuildProbe, nil |
| +} |
| + |
| // ---- service credentials (services.go's `carlos services` verbs) --------- |
| // servicePath builds one /api/cli/services/{account}... path — the service |
diff --git a/apps.go b/apps.go| index 6efc462d9..558e0f41e 100644 |
| --- a/apps.go |
| +++ b/apps.go |
| @@ -34,7 +34,7 @@ import ( |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| -const appsUsage = `usage: carlos apps <create|place|delete|restore> [flags] <app> |
| +const appsUsage = `usage: carlos apps <create|place|build-probe|delete|restore> [flags] <app> |
| create --app <name> [flags] claim a new app name in an account, through |
| the console API — the same claim the console's New app |
| @@ -73,6 +73,16 @@ const appsUsage = `usage: carlos apps <create|place|delete|restore> [flags] <app |
| silent clear (a fleet box retires an app's routes on its |
| next reconcile the moment placement reads empty). |
| + build-probe --app <name> </path|-> set (or clear, with "-") the path whose |
| + response body names the RUNNING build, on an app that |
| + already exists. "carlos deploy" polls it instead of the |
| + X-Carlos-Version header, which the edge stamps from the |
| + route row and which therefore reports the new build before |
| + the process has actually restarted — the whole reason a |
| + deploy can print "live" over an old binary. Settable only |
| + at claim time until now, which meant the fix was out of |
| + reach of every app that already had the problem. |
| + |
| delete [flags] <app> move an app to the Trash: every route drops |
| (custom domains included), its name stays reserved, and it |
| is restorable for 30 days. Refused while the app is |
| @@ -121,6 +131,8 @@ func RunApps(args []string) { |
| runAppsCreate(args[1:]) |
| case "place": |
| runAppsPlace(args[1:]) |
| + case "build-probe": |
| + runAppsBuildProbe(args[1:]) |
| case "delete": |
| runAppsDelete(args[1:]) |
| case "restore": |
diff --git a/apps_buildprobe.go b/apps_buildprobe.go| new file mode 100644 |
| index 000000000..3bd7f9215 |
| --- /dev/null |
| +++ b/apps_buildprobe.go |
| @@ -0,0 +1,97 @@ |
| +package main |
| + |
| +// `carlos apps build-probe` — set, on an app that already exists, the path |
| +// `carlos deploy` polls to learn which build is ACTUALLY running. |
| +// |
| +// The probe already existed and could only be stamped at claim time: |
| +// SetAppBuildProbe had exactly one caller, the create door. So every app made |
| +// before the flag existed, and every app whose author did not think to pass |
| +// it, was permanently stuck on header-watching — the X-Carlos-Version header |
| +// the edge stamps from the route row the moment adoption moves a symlink, |
| +// whether or not any process restarted. Which means the fix for "deploy said |
| +// live over the old binary" was out of reach of exactly the apps that had the |
| +// problem (discussion #13). |
| + |
| +import ( |
| + "context" |
| + "flag" |
| + "fmt" |
| + "log" |
| + "strings" |
| +) |
| + |
| +const buildProbeUsage = `usage: carlos apps build-probe --app <name> </path|-> [--account <sqid|name>] [--account-id <sqid>] [--console <origin>] |
| + |
| + </path> an absolute path, no query, whose response body names the running |
| + build — "/healthz" serving "ok abc1234" is the shape. carlos deploy |
| + polls it instead of the X-Carlos-Version header, which reports the |
| + release adoption INTENDED rather than the build actually serving. |
| + - clear the probe, back to watching the header.` |
| + |
| +// runAppsBuildProbe stamps the probe path, or clears it. |
| +// |
| +// "-" is the clear sentinel, the same one `carlos route --unit` and |
| +// `--environment` use and for the same reason: "" is a LEGAL value of this |
| +// field — it means header-watching — so an empty argument cannot be told from |
| +// an argument nobody meant to pass. The doors that clear say so out loud. |
| +// |
| +// The stored value is echoed back and compared rather than assumed, the same |
| +// discipline `apps create` applies to this exact field. It is what the deploy |
| +// watch trusts; a write that half-landed would leave a deploy quietly |
| +// believing a header again, which is the whole bug. |
| +func runAppsBuildProbe(args []string) { |
| + fs := flag.NewFlagSet("apps build-probe", flag.ExitOnError) |
| + app := appFlag(fs, "app whose build probe to set") |
| + 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") |
| + consoleFlag := fs.String("console", "", "console to act through (default: the only logged-in one)") |
| + fs.Usage = flagUsage(fs, buildProbeUsage) |
| + fs.Parse(args) |
| + if strings.TrimSpace(*app) == "" || fs.NArg() != 1 { |
| + log.Fatal(buildProbeUsage) |
| + } |
| + if err := validAppName(strings.TrimSpace(*app)); err != nil { |
| + log.Fatalf("apps build-probe: %v", err) |
| + } |
| + probe := strings.TrimSpace(fs.Arg(0)) |
| + clearProbe := probe == "-" |
| + if clearProbe { |
| + probe = "" |
| + } else { |
| + if probe == "" { |
| + log.Fatal(`apps build-probe: an empty probe is refused — pass "-" to clear it back to header-watching, or name the path`) |
| + } |
| + // Checked here, before the call, the same client-side grammar gate |
| + // `apps create` applies: a shape the server is certain to refuse |
| + // should cost no round trip and reach nothing. |
| + if err := validBuildProbe(probe); err != nil { |
| + log.Fatalf("apps build-probe: %v", err) |
| + } |
| + } |
| + |
| + sess, err := resolveMode(*consoleFlag) |
| + if err != nil { |
| + log.Fatal(err) |
| + } |
| + if sess == nil { |
| + log.Fatal("apps build-probe works through a console, and this shell is in direct bucket mode\n" + |
| + "(CARLOS_DEPLOYMENT_BUCKET/CARLOS_DEPLOYMENT_DIR is set). The probe is console-tracked\n" + |
| + "state on the app row: log in instead — carlos auth login --console <url>") |
| + } |
| + acct := resolvedAccount(sess, acctRaw, acctIDRaw) |
| + got, err := sess.setBuildProbe(context.Background(), acct, strings.TrimSpace(*app), probe) |
| + if err != nil { |
| + log.Fatalf("apps build-probe: %v", err) |
| + } |
| + if got != probe { |
| + log.Fatalf("apps build-probe: did not stick (console echoed %q, want %q) — the probe is what "+ |
| + "carlos deploy trusts, so a write that half-landed is worse than none", got, probe) |
| + } |
| + if clearProbe { |
| + fmt.Fprintf(stdout, "cleared the build probe for %s — carlos deploy watches the X-Carlos-Version header again\n", strings.TrimSpace(*app)) |
| + fmt.Fprintf(stdout, " that header is stamped by the edge from the route row, so it reports the release adoption intended, not the build serving\n") |
| + return |
| + } |
| + fmt.Fprintf(stdout, "build probe for %s: %s\n", strings.TrimSpace(*app), got) |
| + fmt.Fprintf(stdout, " carlos deploy now asks the app's own process which build it is running, instead of trusting the router's header\n") |
| +} |
diff --git a/deploy.go b/deploy.go| index 0f664742e..ea045fb40 100644 |
| --- a/deploy.go |
| +++ b/deploy.go |
| @@ -45,11 +45,21 @@ func runDeploy(args []string) { |
| channelFlag := channelFlag(fs, "channel to promote to (default: resolved from the app's instances, or its one channel if it has no instances; error if ambiguous)") |
| hostFlag := fs.String("host", "", "serve on this hostname instead of the app's own platform host (also scopes channel resolution and the watch to it)") |
| timeout := fs.Duration("timeout", defaultDeployTimeout, "how long to watch the URL before failing") |
| + // A deploy IS a ship plus a promote, and the ship half has carried |
| + // --label and --notes since it existed — they are the line a person |
| + // doing a rollback at 2am reads. Deploy passed "" for both, so |
| + // `carlos deploy --label "..."` failed with "flag provided but not |
| + // defined" and dumped usage, and the only way to label a release was to |
| + // stop using the one-command form (discussion #13, filed alongside the |
| + // watch problem). Nothing had to change below: runShipAPI already takes |
| + // both. |
| + label := fs.String("label", "", "one line describing this release, for carlos releases and the console") |
| + notes := fs.String("notes", "", "longer release notes") |
| fs.Usage = flagUsage(fs, "") |
| fs.Parse(args) |
| usage := "usage: carlos deploy --app <a> [--version <sha>] [--kind binary|static] [--target linux-arm64] " + |
| "[--account <sqid-or-name>] [--account-id <sqid>] [--console <origin>] [--capabilities <file.json>] " + |
| - "[--channel <ch>] [--host <host>] [--timeout 120s] <files-or-site-dir>\n" + |
| + "[--channel <ch>] [--host <host>] [--label <text>] [--notes <text>] [--timeout 120s] <files-or-site-dir>\n" + |
| "or, in a project with saved defaults (or at a terminal, to create them): carlos deploy" |
| if *ver == "" { |
| *ver = gitShortHead() |
| @@ -235,7 +245,7 @@ func runDeploy(args []string) { |
| // printBanner=false: the "using ..." line already printed above, before |
| // the instance list — so a listing failure still names the console — |
| // and runShipAPI would otherwise print it a second time. |
| - shipRes, err := runShipAPI(sess, acct, fs, *app, *ver, *kind, *target, "", "", capsSet, false) |
| + shipRes, err := runShipAPI(sess, acct, fs, *app, *ver, *kind, *target, *label, *notes, capsSet, false) |
| if err != nil { |
| log.Fatalf("deploy: ship: %v", err) |
| } |
| @@ -314,6 +324,25 @@ func runDeploy(args []string) { |
| } |
| if probe != "" { |
| mutedf(stdout, "watching %s for the build %s reports itself\n", probe, *app) |
| + } else { |
| + // Said out loud, every time, because the silence is what the bug was |
| + // made of (discussion #13). With no probe this watch can only ask |
| + // the ROUTER what release was adopted; it cannot ask the process |
| + // what it is running. A deploy that goes green here has proved that |
| + // adoption happened, not that anything restarted — and an app whose |
| + // instance failed to cycle looks identical. It is a one-line fix |
| + // per app, so the line names it rather than describing the problem |
| + // and leaving the reader to find the verb. |
| + mutedf(stdout, "watching the X-Carlos-Version header: it reports the release ADOPTED, not the build running\n") |
| + // The advisory below assumes a process to probe. A static route has |
| + // none — no /api/version, no exec child to cycle — so the suggested |
| + // command would only teach the operator to configure a probe that |
| + // 404s, which then fails and times out every future `carlos deploy` |
| + // for that site. Static deploys get the line above (still true: the |
| + // header reports adoption, not a running build) and stop there. |
| + if !staticDeploy { |
| + mutedf(stdout, " a probe proves the process actually cycled — carlos apps build-probe --app %s /api/version\n", *app) |
| + } |
| } |
| targets := make([]watchTarget, 0, len(hosts)) |
| for _, h := range hosts { |
diff --git a/deploy_e2e_test.go b/deploy_e2e_test.go| index 76f9e5357..dfc9ae2f9 100644 |
| --- a/deploy_e2e_test.go |
| +++ b/deploy_e2e_test.go |
| @@ -212,6 +212,13 @@ func TestE2EStaticDeployThroughConsole(t *testing.T) { |
| if strings.Contains(out, "watching "+host) { |
| t.Fatalf("static note still repeats the host the deploying line just printed:\n%s", out) |
| } |
| + // A static route has no process and no /api/version, so the no-probe |
| + // advisory (deploy.go, gated on !staticDeploy) must not tell the |
| + // operator to set one — following that advice on a static site sets a |
| + // probe that 404s and fails every later `carlos deploy` for it. |
| + if strings.Contains(out, "build-probe") { |
| + t.Fatalf("static deploy suggested a build probe, which a static route can never serve:\n%s", out) |
| + } |
| ptr, err := release.Channel(context.Background(), relStore, pub, release.OpsAccount, app, channel) |
| if err != nil { |
| t.Fatalf("channel after deploy: %v", err) |
| @@ -266,6 +273,7 @@ func TestE2EDeployThroughConsole(t *testing.T) { |
| app = "titogo" |
| channel = "canary/e2e" |
| version = "abc1234" |
| + label = "e2e rollout" |
| ) |
| tmp := t.TempDir() |
| @@ -377,10 +385,13 @@ func TestE2EDeployThroughConsole(t *testing.T) { |
| // --- the verb ------------------------------------------------------ |
| buf.Reset() |
| - runDeploy([]string{"-app", app, "-account", "ops", "-version", version, "-timeout", "30s", artifact}) |
| + runDeploy([]string{"-app", app, "-account", "ops", "-version", version, "-label", label, "-timeout", "30s", artifact}) |
| out := buf.String() |
| - // Ship happened, against the bucket, signed. |
| + // Ship happened, against the bucket, signed — and --label, wired |
| + // straight through to runShipAPI (deploy.go), landed in the manifest |
| + // rather than the silently-passed "" this branch exists to fix |
| + // (discussion #13). |
| m, err := release.GetManifest(context.Background(), relStore, release.OpsAccount, app, version) |
| if err != nil { |
| t.Fatalf("manifest after deploy: %v", err) |
| @@ -388,6 +399,9 @@ func TestE2EDeployThroughConsole(t *testing.T) { |
| if m.Version != version || m.Kind != "binary" { |
| t.Fatalf("manifest = %+v, want %s binary", m, version) |
| } |
| + if m.Label != label { |
| + t.Fatalf("manifest label = %q, want %q — carlos deploy --label is not reaching the release", m.Label, label) |
| + } |
| // Promote happened, onto the channel the RECORD named — never typed. |
| ptr, err := release.Channel(context.Background(), relStore, pub, release.OpsAccount, app, channel) |
| if err != nil { |
diff --git a/docs/site/cli.md b/docs/site/cli.md| index 22561c166..7c2997622 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -84,6 +84,19 @@ the route row and which therefore reports the new build before the process has |
| actually restarted. Both are checked against what the console echoes back, so a |
| write that silently failed is an error rather than a quiet lie. |
| +`carlos apps build-probe` sets that same probe on an app that already exists, |
| +which is the common case: most apps were claimed before anyone thought about |
| +it. |
| + |
| +```sh |
| +carlos apps build-probe --app docs /api/version |
| +``` |
| + |
| +Pass `-` instead of a path to clear it and go back to watching the header. It |
| +is worth setting on anything you deploy often. Without it, a green deploy tells |
| +you the release was adopted, not that the process running the old one ever |
| +restarted — and those look identical from outside. |
| + |
| ### carlos ship |
| Publishes a release. The artifact is hashed, stored under its content |
| @@ -133,10 +146,18 @@ and the console refuses it by name, so the flag never quietly does nothing. |
| ### carlos deploy |
| Ship, promote, and then wait until the app's URL actually serves the new |
| -build. It watches the `X-Carlos-Version` header on the live host and does not |
| -report success until that header names what you shipped, over a 2xx. Reach |
| -for this one by default; ship and promote separately when you want the gap |
| -between them. |
| +build. Reach for this one by default; ship and promote separately when you |
| +want the gap between them. |
| + |
| +What it watches is worth knowing, because the two answers differ. If the app |
| +declares a build probe, deploy asks the app's own process which build it is |
| +running, and `live` means the process cycled. If it does not, deploy watches |
| +the `X-Carlos-Version` header — and the edge stamps that from the route row |
| +the moment adoption moves the symlink, whether or not anything restarted. So |
| +a probe-less deploy can print `live` over a process still running the old |
| +binary, and it says so while it waits. Set one with |
| +[carlos apps build-probe](/docs/cli#carlos-apps); it is a single command per |
| +app. |
| ```sh |
| carlos deploy --app hello ./hello |
diff --git a/internal/console/apibuildprobe.go b/internal/console/apibuildprobe.go| new file mode 100644 |
| index 000000000..b2bb42f54 |
| --- /dev/null |
| +++ b/internal/console/apibuildprobe.go |
| @@ -0,0 +1,77 @@ |
| +// The build probe, settable on an app that already exists (discussion #13). |
| +// |
| +// `carlos deploy` printed "live" and exited 0 while the previous binary was |
| +// still serving, and every platform-side signal agreed with it, because they |
| +// all describe the ADOPTED release rather than the running process: the edge |
| +// stamps X-Carlos-Version from the route row the moment adoption moves the |
| +// symlink, whether or not anything restarted. |
| +// |
| +// The platform already has the answer to that. An app may declare a BUILD |
| +// PROBE — a path whose response body names the build actually running — and |
| +// the deploy watch polls the app's own process instead of the header |
| +// (deploy.go's watchTarget.Probe). It closes exactly the gap #13 describes. |
| +// |
| +// It could only be set at CLAIM time. `SetAppBuildProbe` had precisely one |
| +// caller, handleAPICreateApp, so every app created before the feature existed |
| +// — or created without the flag, which is every app anyone forgot it for — |
| +// could never acquire one. The fix for the bug was unreachable by the apps |
| +// that had the bug. That is what this door is for. |
| +// |
| +// GATE: apiAccountApp proves membership, and the route is registered with an |
| +// EMPTY role set — no service credential of any role reaches it. This is a |
| +// policy door in the sense svcroutes_test.go means: the probe is what the |
| +// deploy watch TRUSTS, so a credential able to rewrite it is a credential |
| +// able to make its own deploys report success. A person sets it once. |
| +package console |
| + |
| +import ( |
| + "net/http" |
| + "strings" |
| + "time" |
| +) |
| + |
| +func (s *Server) handleAPIBuildProbe(w http.ResponseWriter, r *http.Request) { |
| + addr, acct, app, ok := s.apiAccountApp(w, r) |
| + if !ok { |
| + return |
| + } |
| + p, _ := principalFrom(r.Context()) |
| + if !s.allowWrite(p, time.Now()) { |
| + jsonError(w, http.StatusTooManyRequests, "too many requests — wait a minute and try again") |
| + return |
| + } |
| + var req struct { |
| + BuildProbe string `json:"build_probe"` |
| + } |
| + if !readJSON(w, r, &req) { |
| + return |
| + } |
| + // "" is a legal value: it clears the probe back to header-watching, the |
| + // same meaning SetAppBuildProbe's own doc comment gives it. The CLI's |
| + // clear sentinel is "-", resolved client-side, so nothing here has to |
| + // guess whether an empty body meant "clear" or "not mentioned". |
| + probe := strings.TrimSpace(req.BuildProbe) |
| + if probe != "" && !validBuildProbe(probe) { |
| + jsonError(w, http.StatusBadRequest, |
| + `build_probe must be an absolute URL path with no query, e.g. "/healthz"`) |
| + return |
| + } |
| + if err := s.store.SetAppBuildProbe(acct.ID, app, probe); err != nil { |
| + jsonError(w, http.StatusInternalServerError, "could not save the build probe — try again") |
| + return |
| + } |
| + // Read back and echo what was actually STORED, exactly as the claim door |
| + // does: a write that silently failed must be a loud error rather than a |
| + // quiet lie, and this is the field whose whole job is to be trusted. |
| + _, got, err := s.store.AppFlags(acct.ID, app) |
| + if err != nil { |
| + jsonError(w, http.StatusInternalServerError, "saved, but could not read the build probe back — check it with carlos apps") |
| + return |
| + } |
| + detail := "cleared the build probe for " + app + " (cli)" |
| + if got != "" { |
| + detail = "set the build probe for " + app + " to " + got + " (cli)" |
| + } |
| + s.audit(acct, app, addr, "build-probe", detail) |
| + writeJSON(w, http.StatusOK, map[string]any{"build_probe": got}) |
| +} |
diff --git a/internal/console/apibuildprobe_test.go b/internal/console/apibuildprobe_test.go| new file mode 100644 |
| index 000000000..2823bab85 |
| --- /dev/null |
| +++ b/internal/console/apibuildprobe_test.go |
| @@ -0,0 +1,88 @@ |
| +package console |
| + |
| +import ( |
| + "encoding/json" |
| + "net/http" |
| + "testing" |
| + "time" |
| +) |
| + |
| +// TestAPIBuildProbeSetsAndClearsOnAnExistingApp is the whole point of the |
| +// door. Until it existed, SetAppBuildProbe had exactly one caller — the claim |
| +// door — so an app created without the flag could never acquire a probe, and |
| +// the fix for "deploy reported live over the old binary" was unreachable by |
| +// every app that already had the problem (discussion #13). |
| +func TestAPIBuildProbeSetsAndClearsOnAnExistingApp(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-buildprobe") |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, "docs", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatalf("create app: %v", err) |
| + } |
| + |
| + probeOf := func() string { |
| + t.Helper() |
| + _, probe, err := fx.st.AppFlags(OpsAccountID, "docs") |
| + if err != nil { |
| + t.Fatalf("AppFlags: %v", err) |
| + } |
| + return probe |
| + } |
| + if got := probeOf(); got != "" { |
| + t.Fatalf("a fresh app already carries a probe: %q", got) |
| + } |
| + |
| + u := fx.ts.URL + "/api/cli/apps/" + fx.sqid + "/docs/build-probe" |
| + status, _, body := rawReq(t, http.MethodPost, u, token, `{"build_probe":"/api/version"}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("set = %d %q", status, body) |
| + } |
| + var out struct { |
| + BuildProbe string `json:"build_probe"` |
| + } |
| + if err := json.Unmarshal([]byte(body), &out); err != nil { |
| + t.Fatalf("unreadable: %v: %s", err, body) |
| + } |
| + // The echo is the contract: this is the field the deploy watch trusts |
| + // instead of the header, so a write that silently failed has to be a loud |
| + // error rather than a quiet lie. Response and store both. |
| + if out.BuildProbe != "/api/version" { |
| + t.Errorf("echo = %q, want /api/version", out.BuildProbe) |
| + } |
| + if got := probeOf(); got != "/api/version" { |
| + t.Errorf("stored = %q, want /api/version", got) |
| + } |
| + |
| + // "" clears back to header-watching — a legal value of this field, not a |
| + // missing one. |
| + status, _, body = rawReq(t, http.MethodPost, u, token, `{"build_probe":""}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("clear = %d %q", status, body) |
| + } |
| + if got := probeOf(); got != "" { |
| + t.Errorf("after clear, stored = %q, want empty", got) |
| + } |
| +} |
| + |
| +// TestAPIBuildProbeRefusesABadShape: the watch builds "https://" + host + |
| +// probe, so a probe not starting with "/" turns the poll into a request to a |
| +// machine of somebody else's choosing, which would then report ITS build as |
| +// this deploy's. Refused at the write door, not only where it is read. |
| +func TestAPIBuildProbeRefusesABadShape(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-buildprobebad") |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, "docs", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatalf("create app: %v", err) |
| + } |
| + |
| + u := fx.ts.URL + "/api/cli/apps/" + fx.sqid + "/docs/build-probe" |
| + for _, bad := range []string{"@evil.example/x", "api/version", "/x?y=1", "https://evil.example/x"} { |
| + payload, _ := json.Marshal(map[string]string{"build_probe": bad}) |
| + status, _, body := rawReq(t, http.MethodPost, u, token, string(payload)) |
| + if status != http.StatusBadRequest { |
| + t.Errorf("probe %q = %d %q, want 400", bad, status, body) |
| + } |
| + } |
| + if _, probe, _ := fx.st.AppFlags(OpsAccountID, "docs"); probe != "" { |
| + t.Errorf("a refused probe still landed: %q", probe) |
| + } |
| +} |
diff --git a/internal/console/authcli.go b/internal/console/authcli.go| index 232143bbb..2d1243302 100644 |
| --- a/internal/console/authcli.go |
| +++ b/internal/console/authcli.go |
| @@ -711,6 +711,12 @@ func (s *Server) registerCLIRoutes() { |
| // Placement lives under the apps family, not fleets: its subject is an |
| // APP, and a target fleet name is only ever the request body's data. |
| s.cliRoute("POST /api/cli/apps/{account}/{app}/place", svcrole.Allow(svcrole.Admin), s.handleAPIPlace) |
| + // The build probe on an EXISTING app (discussion #13). An empty role set: |
| + // the probe is what the deploy watch trusts instead of the header, so a |
| + // credential that could rewrite it could make its own deploys report |
| + // success over a process that never restarted — which is the exact bug |
| + // the probe exists to prevent, handed to the wrong caller. |
| + s.cliRoute("POST /api/cli/apps/{account}/{app}/build-probe", svcrole.Set{}, s.handleAPIBuildProbe) |
| // The object-storage API (apistore.go, issue #55) — declare a store, read |
| // what became of it, and drive the walks that make it real. Beside the |
| // config routes on purpose: a store's credentials are DELIVERED as config, |
diff --git a/internal/console/svcroutes_test.go b/internal/console/svcroutes_test.go| index 2c3fb99a4..0f3d30e82 100644 |
| --- a/internal/console/svcroutes_test.go |
| +++ b/internal/console/svcroutes_test.go |
| @@ -67,7 +67,7 @@ import ( |
| var ( |
| mintDoors = []string{"/instance-token", "/api/cli/services/", "/email/credentials"} |
| fleetDoors = []string{"/api/cli/fleets/"} |
| - policyDoors = []string{"/instances-manifest", "/{app}/routes", "/email/pause"} |
| + policyDoors = []string{"/instances-manifest", "/{app}/routes", "/email/pause", "/build-probe"} |
| ) |
| // closedDoors is the exact set of /api/cli/ patterns registered with the zero |
| @@ -130,6 +130,13 @@ var closedDoors = []string{ |
| // whose rules it never met. Shut for the same reason the manifest door |
| // is — an app may not rewrite the bounds it runs inside. |
| "POST /api/cli/apps/{account}/{app}/instances/{host}/channel", |
| + // The build probe is what `carlos deploy`'s watch TRUSTS in place of the |
| + // X-Carlos-Version header — the header being the thing that reports a |
| + // release before any process has restarted. A credential that could |
| + // repoint the probe could therefore make its own deploys report success |
| + // over a process that never cycled, which is the precise failure the |
| + // probe exists to prevent. Setting it is a person's one-time act. |
| + "POST /api/cli/apps/{account}/{app}/build-probe", |
| } |
| // TestEveryCLIDoorNamesItsRoles is the default-deny guarantee for the |
| @@ -686,8 +693,8 @@ func TestEveryClosedDoorRefusesAServiceCredential(t *testing.T) { |
| } |
| seen++ |
| } |
| - if seen != len(closedDoors) || seen != 17 { |
| - t.Errorf("walked %d closed doors, want all %d (and 17 today: instance-token, the routes door, instances-manifest, the route-channel move, six fleets, four services, two email credentials, email pause)", |
| + if seen != len(closedDoors) || seen != 18 { |
| + t.Errorf("walked %d closed doors, want all %d (and 18 today: instance-token, the routes door, instances-manifest, the route-channel move, six fleets, four services, two email credentials, email pause, the build probe)", |
| seen, len(closedDoors)) |
| } |
| } |