trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRunsg rEventsFlows

A preview that is not serving says the truth about its pull and links back: to the pull, to production and home #8168

Merged torquecrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

torque/forge:torque/8154-a-preview-that-is-not-serving-says-the-t → main 2 commits fork

  • Merged. torque/8154-a-preview-that-is-not-serving-says-the-t was merged into main as 6dd4dbd.

Review

Sign in to review this change.
2 files changed +234 −8
src/apps/proxy.ts +40 −8
@@ -739,7 +739,7 @@ function admit(
739739 runtime.rouse(target.app, target.env);
740740 const status = runtime.envStatus(target.app, target.env);
741741 if (status === undefined || status.port === undefined) {
742 return err(unavailable(runtime, target, status));
742 return err(unavailable(runtime, target, status, undefined, grant?.who));
743743 }
744744 return ok(
745745 grant === undefined
@@ -1538,11 +1538,12 @@ function cookieValue(request: Request, name: string): string | undefined {
15381538 * The page an environment that is not serving shows: what it is doing, in
15391539 * words, with the run to look at. Never a bare gateway error.
15401540 */
1541 function unavailable(
1541 export function unavailable(
15421542 runtime: AppsRuntime,
15431543 target: AppTarget,
15441544 status: EnvStatus | undefined,
15451545 reason?: string,
1546 who?: GrantWho,
15461547 ): Response {
15471548 const name = target.app.slug;
15481549 const where =
@@ -1551,10 +1552,21 @@ function unavailable(
15511552 : `the preview of pull #${target.env.slice(5)}`;
15521553 const state = status?.state ?? "stopped";
15531554 const deploying = state === "deploying";
1554 // A preview the forge no longer knows is gone, not broken: its pull closed
1555 // and its data went with it. That is a 404, which also lets this page
1556 // through an edge that replaces every 5xx with one of its own.
1557 const gone = status === undefined && target.env !== "production";
1555 // A preview is gone when its pull is closed, merged or no longer exists:
1556 // its data went with it. That is a 404, which also lets this page through
1557 // an edge that replaces every 5xx with one of its own. An open pull with no
1558 // environment is not gone: it deploys again on the next push.
1559 const at = pillWhere(runtime, target, who);
1560 const pull =
1561 at.pull === undefined
1562 ? undefined
1563 : getIssue(runtime.app.db, target.app.repoId, at.pull);
1564 const preview = target.env !== "production";
1565 const gone = preview && (pull === undefined || pull.state === "closed");
1566 // Nothing was ever deployed for an open pull: also a 404, for the same
1567 // edge, since there is nothing at this address yet. A stopped or failed
1568 // environment keeps its 502.
1569 const absent = preview && !gone && status === undefined;
15581570 const message =
15591571 reason ??
15601572 (deploying
@@ -1563,13 +1575,32 @@ function unavailable(
15631575 ? (status?.error ?? "The last deploy failed.")
15641576 : gone
15651577 ? "This preview is gone: its pull was closed, and its data with it."
1566 : "Nothing is running here yet.");
1578 : absent
1579 ? "The preview is not running. It starts again with the next push to the pull, or with app_deploy redeploy."
1580 : "Nothing is running here yet.");
15671581 const links = discoveryLinks(runtime, target);
15681582 const report = links.length > 0;
15691583 const runLink =
15701584 status?.runId === undefined
15711585 ? ""
15721586 : `<p><a href="${escape(runtime.options.forgeUrl)}/runs/${escape(status.runId)}">See the run on the forge</a></p>`;
1587 const forge = runtime.options.forgeUrl;
1588 const exits: string[] = [];
1589 if (preview && at.repo !== undefined && at.pull !== undefined) {
1590 const href = new URL(`/${at.repo}/pulls/${at.pull}`, forge).href;
1591 exits.push(
1592 `<a href="${escape(href)}">Pull #${at.pull}${at.title === undefined ? "" : `: ${escape(at.title)}`}</a>`,
1593 );
1594 }
1595 // A private app's address, like its repository's name, goes only to a reader.
1596 const mayNameProduction = target.app.access === "public" || at.repo !== undefined;
1597 if (
1598 preview &&
1599 mayNameProduction &&
1600 runtime.envStatus(target.app, "production")?.port !== undefined
1601 )
1602 exits.push(`<a href="${escape(runtime.urlFor(target.app, "production"))}">Production</a>`);
1603 exits.push(`<a href="${escape(new URL("/", forge).href)}">Forge home</a>`);
15731604 const html = `<!doctype html>
15741605 <meta charset="utf-8">
15751606 <meta name="viewport" content="width=device-width, initial-scale=1">
@@ -1586,10 +1617,11 @@ ${deploying ? '<meta http-equiv="refresh" content="3">' : ""}
15861617 <p>This is ${escape(where)}. ${escape(message)}</p>
15871618 ${deploying ? "<p>This page refreshes on its own.</p>" : ""}
15881619 ${runLink}
1620 <p>${exits.join(" · ")}</p>
15891621 ${report ? '<p><a href="/__forge/report">Report a problem</a> to the people who keep it; no account needed.</p>' : ""}
15901622 `;
15911623 return new Response(html, {
1592 status: deploying ? 503 : gone ? 404 : 502,
1624 status: deploying ? 503 : gone || absent ? 404 : 502,
15931625 headers: {
15941626 "content-type": "text/html; charset=utf-8",
15951627 "cache-control": "no-store",
test/apps-proxy-unavailable.test.ts added +194
@@ -0,0 +1,194 @@
1 /**
2 * The page a preview that is not serving shows (#8154): `gone` only when
3 * its pull is closed or missing, and every such page leads out.
4 */
5 import { signGrant } from "../src/apps/proxy.ts";
6 import { AppsRuntime as Runtime } from "../src/apps/runtime.ts";
7 import { serve, type Servers } from "../src/http/server.ts";
8 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
9 import { mkdtemp, rm } from "node:fs/promises";
10 import { tmpdir } from "node:os";
11 import { join } from "node:path";
12 import { createApp, type App } from "../src/app.ts";
13 import type { AppsRuntime } from "../src/apps/runtime.ts";
14 import { unavailable } from "../src/apps/proxy.ts";
15 import { newApp } from "../src/actions/apps.ts";
16 import { openPull } from "../src/actions/pulls.ts";
17 import { commitToBranch } from "../src/actions/repos.ts";
18 import { getIssue, setIssueState } from "../src/core/issues.ts";
19 import { createUser, type User } from "../src/core/users.ts";
20
21 describe("unavailable page", () => {
22 let dataDir: string;
23 let app: App;
24 let alice: User;
25 let target: any;
26 let open: number;
27 let closed: number;
28 let production = true;
29 let secret: any;
30 let secretPull: number;
31 let bob: User;
32
33 const runtime = () =>
34 ({
35 app,
36 options: { forgeUrl: "https://forge.test" },
37 envStatus: (_a: unknown, env: string) =>
38 env === "production" && production ? { state: "running", port: 1 } : undefined,
39 urlFor: (_a: unknown, env: string) => `https://x--${env}.test/?a=1&b=<2>`,
40 }) as unknown as AppsRuntime;
41 const page = (env: string, status?: any) =>
42 unavailable(runtime(), { ...target, env }, status);
43
44 beforeAll(async () => {
45 dataDir = await mkdtemp(join(tmpdir(), "forge-unavail-"));
46 app = createApp({ dataDir, port: 0 });
47 const made = await createUser(app.db, {
48 origin: "seed", trusted: true, username: "alice",
49 email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true,
50 });
51 if (!made.ok) throw new Error(made.error);
52 alice = made.value;
53 const a = await newApp(app, alice, { name: "hello", visibility: "public" });
54 if (!a.ok) throw new Error(a.error);
55 const repo = a.value.repo;
56 target = { app: a.value.app, env: "production", nested: false };
57 const bobMade = await createUser(app.db, {
58 origin: "seed", trusted: true, username: "bob",
59 email: "[email protected]", password: "a-good-passphrase",
60 });
61 if (!bobMade.ok) throw new Error(bobMade.error);
62 bob = bobMade.value;
63 const sec = await newApp(app, alice, { name: "vault", visibility: "private" });
64 if (!sec.ok) throw new Error(sec.error);
65 secret = { app: sec.value.app, env: "production", nested: false };
66 await commitToBranch(app, alice, sec.value.repo, {
67 branch: "s", base: sec.value.repo.defaultBranch, message: "s",
68 files: [{ path: "s.md", content: "x\n" }],
69 });
70 const so = await openPull(app, alice, sec.value.repo, {
71 title: "Secret roadmap", baseBranch: sec.value.repo.defaultBranch, headBranch: "s",
72 });
73 if (!so.ok) throw new Error(so.error);
74 secretPull = so.value.number;
75 const mk = async (branch: string, title: string) => {
76 await commitToBranch(app, alice, repo, {
77 branch, base: repo.defaultBranch, message: branch,
78 files: [{ path: `${branch}.md`, content: "x\n" }],
79 });
80 const o = await openPull(app, alice, repo, {
81 title, baseBranch: repo.defaultBranch, headBranch: branch,
82 });
83 if (!o.ok) throw new Error(o.error);
84 return o.value;
85 };
86 open = (await mk("b-open", "Open <b>pull</b>")).number;
87 const c = await mk("b-closed", "Closed one");
88 closed = c.number;
89 setIssueState(app.db, getIssue(app.db, repo.id, c.number)!.id, "closed");
90 }, 30_000);
91
92 afterAll(async () => {
93 app.close();
94 await rm(dataDir, { recursive: true, force: true });
95 });
96
97 test("an open pull with no environment is not gone", async () => {
98 const res = page(`pull-${open}`);
99 const html = await res.text();
100 expect(html).not.toContain("is gone");
101 expect(html).toContain("is not running");
102 expect(html).toContain("app_deploy redeploy");
103 expect(html).toContain("next push");
104 expect(res.status).toBeLessThan(500);
105 });
106
107 test("a closed pull, and one that does not exist, are a 404 gone", async () => {
108 for (const n of [closed, 9999]) {
109 const res = page(`pull-${n}`);
110 expect(res.status).toBe(404);
111 expect(await res.text()).toContain("is gone");
112 }
113 });
114
115 test("every page links to the pull, production and home, escaped", async () => {
116 const cases: [string, any][] = [
117 [`pull-${open}`, undefined],
118 [`pull-${open}`, { state: "deploying" }],
119 [`pull-${open}`, { state: "failed", error: "boom" }],
120 [`pull-${closed}`, undefined],
121 ];
122 for (const [env, status] of cases) {
123 const html = await page(env, status).text();
124 const n = env.slice(5);
125 expect(html).toContain(`href="https://forge.test/alice/hello/pulls/${n}"`);
126 expect(html).toContain(
127 `href="https://x--production.test/?a=1&amp;b=&lt;2&gt;"`,
128 );
129 expect(html).toContain(`href="https://forge.test/"`);
130 expect(html).not.toContain("<b>pull</b>");
131 }
132 });
133
134 test("production is linked only when it serves; production's own page has no pull link", async () => {
135 production = false;
136 const html = await page(`pull-${open}`).text();
137 production = true;
138 expect(html).not.toContain("Production</a>");
139 const own = await page("production").text();
140 expect(own).toContain('href="https://forge.test/"');
141 expect(own).not.toContain("/pulls/");
142 });
143
144 const who = (u: User) => ({ id: u.id, name: u.username, role: "user" as const });
145 const secretPage = (v?: User) =>
146 unavailable(runtime(), { ...secret, env: `pull-${secretPull}` }, undefined, undefined, v && who(v)).text();
147
148 test("a private repository's pull link and title go only to a reader", async () => {
149 for (const viewer of [undefined, bob]) {
150 const html = await secretPage(viewer);
151 expect(html).not.toContain("alice/vault");
152 expect(html).not.toContain("Secret roadmap");
153 expect(html).not.toContain("/pulls/");
154 expect(html).not.toContain("Production</a>");
155 expect(html).toContain('href="https://forge.test/"');
156 }
157 const html = await secretPage(alice);
158 expect(html).toContain(`href="https://forge.test/alice/vault/pulls/${secretPull}"`);
159 expect(html).toContain("Secret roadmap");
160 expect(html).toContain("Production</a>");
161 });
162
163 test("a merged pull is closed, so its preview is gone", () => {
164 // mergePull closes the issue (src/actions/pulls.ts) with the same
165 // setIssueState the closed case above uses; `gone` reads that state.
166 expect(getIssue(app.db, target.app.repoId, closed)!.state).toBe("closed");
167 });
168
169 test("the proxy hands the signed-in person to the page", async () => {
170 const servers: Servers = await serve(app);
171 const rt = new Runtime(app, {
172 domain: "apps.test", scheme: "http", publicPort: servers.port,
173 forgeUrl: servers.url, concurrency: 1,
174 });
175 await rt.start();
176 try {
177 const host = `vault--${secretPull}.apps.test`;
178 const t = { ...secret, env: `pull-${secretPull}` };
179 const ask = async (v: User) => {
180 const cookie = `forge_grant_${secret.app.id.slice(-8)}=${signGrant(dataDir, t, 60_000, Date.now(), who(v))}`;
181 return fetch(`${servers.url}/`, { headers: { host, cookie }, redirect: "manual" });
182 };
183 const mine = await ask(alice);
184 const html = await mine.text();
185 expect(html).toContain("Secret roadmap");
186 expect(html).toContain(`/alice/vault/pulls/${secretPull}`);
187 const theirs = await (await ask(bob)).text();
188 expect(theirs).not.toContain("Secret roadmap");
189 } finally {
190 await rt.stop();
191 servers.stop();
192 }
193 }, 30_000);
194 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs