diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md index d331367dbd..433cc51025 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md @@ -14,12 +14,12 @@ ## Files Written -| Path | Format | When | -| ----------------------------------------------- | ------ | -------------------------------------------------------------------------- | -| `/supabase/config.toml` | TOML | on success — appends `[workers.]`, preserving surrounding formatting | -| `/supabase/workers//*` | varies | on success, unless `--source` names another directory | -| `//*` | varies | on success, when `--source` is given | -| `/telemetry.json` | JSON | whenever the handler runs — flushed on success and on failure | +| Path | Format | When | +| ----------------------------------------------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/supabase/config.toml` | TOML | on success — appends `[workers.]` with `runtime`, `size` and `exposure` always, `instances` only when it differs from the default of 1, and `source` only when `--source` was passed, preserving surrounding formatting | +| `/supabase/workers//*` | varies | on success, unless `--source` names another directory | +| `//*` | varies | on success, when `--source` is given | +| `/telemetry.json` | JSON | whenever the handler runs — flushed on success and on failure | Workers are recorded in `config.toml` only. The project config loader prefers `supabase/config.json` when one exists, but the entry writer is a TOML text @@ -40,13 +40,22 @@ prompt refuses a name that is not a DNS label or that `config.toml` already records — so nothing is asked, and nothing written, for a name the command was going to refuse. With `-o json|yaml|toml|env`, a redirected stdout, or a stdin that is not a terminal, there is nowhere to ask, and the command fails instead -of defaulting: unlike the runtime and size, the name has no default to fall back -on. Every prompt is gated on both streams, so +of defaulting: unlike the runtime, size and exposure, the name has no default to +fall back on. Every prompt is gated on both streams, so `printf 'api\n' | supabase experimental workers new` takes that failure path rather than reading the worker name off the pipe. +`runtime`, `size` and `exposure` are always written, defaults included: they are +closed sets the command prompts for, and pinning the answer is the point of +recording it. `instances` is written only when it differs from the default of 1 — +it has no prompt, because how many instances a worker needs is not something a +scaffold can guess, and an absent `instances` means exactly what `instances = 1` +means to `push`. A `0` is an explicit count that scales the worker to nothing, so +it is written like any other. It is rendered as a bare TOML number rather than a +quoted string, because the config schema types it as a number. + Writes to `config.toml` are append-only. A worker already recorded under -`[workers.]` is refused outright — before the runtime and size prompts, +`[workers.]` is refused outright — before the dial prompts, and before anything reaches disk — because editing an entry the user owns is not this command's job. @@ -95,7 +104,8 @@ No custom events — only the `cli_command_executed` that the instrumentation wrapper emits for every command. Nothing is emitted for a failure the parser catches, such as a -`--runtime`/`--size` value outside the choice list. The wrapper is installed by +`--runtime`/`--size`/`--exposure` value outside the choice list, or a negative +`--instances`. The wrapper is installed by `Command.withHandler`, so a command that never reaches its handler never reaches the instrumentation either — and `telemetry.json` is not written. A missing name is _not_ one of those: the argument is optional, so a bare `workers new` reaches diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts index ce93798dd9..a16c4e5a43 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts @@ -3,7 +3,11 @@ import { Argument, Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; import { withJsonErrorHandling } from "../../../../../shared/output/json-error-handling.ts"; import { commandRuntimeLayer } from "../../../../../shared/runtime/command-runtime.layer.ts"; -import { WORKER_RUNTIMES, WORKER_SIZES } from "../../../../../shared/workers/worker-runtimes.ts"; +import { + WORKER_EXPOSURES, + WORKER_RUNTIMES, + WORKER_SIZES, +} from "../../../../../shared/workers/worker-runtimes.ts"; import { legacyCliSettingsLayer } from "../../../../config/legacy-cli-settings.layer.ts"; import { legacyDebugLoggerLayer } from "../../../../shared/legacy-debug-logger.layer.ts"; import { legacyTelemetryStateLayer } from "../../../../telemetry/legacy-telemetry-state.layer.ts"; @@ -29,6 +33,24 @@ const config = { ), Flag.optional, ), + exposure: Flag.choice("exposure", WORKER_EXPOSURES).pipe( + Flag.withDescription( + "Whether the worker is reachable from the internet, recorded as `exposure` in supabase/config.toml. Prompted when omitted.", + ), + Flag.optional, + ), + instances: Flag.integer("instances").pipe( + // Bounded at the parser, the same way `push --instances` and the config + // schema's own `instances` are. + Flag.filter( + (instances) => instances >= 0, + (instances) => `--instances ${instances} is negative; pass zero or more.`, + ), + Flag.withDescription( + "Number of instances to record in supabase/config.toml. Not prompted for, and recorded only when it differs from the default of 1.", + ), + Flag.optional, + ), source: Flag.string("source").pipe( Flag.withDescription( "Scaffold the worker here instead of the default workers directory, recorded as `source` in supabase/config.toml.", @@ -50,22 +72,30 @@ const legacyWorkersNewRuntimeLayer = Layer.mergeAll( export const legacyWorkersNewCommand = Command.make("new", config).pipe( Command.withDescription( - "Scaffold a worker directory from a runtime's starter files and record the choice in supabase/config.toml. Nothing is deployed.", + "Scaffold a worker directory from a runtime's starter files and record the choices in supabase/config.toml. Nothing is deployed.", ), Command.withShortDescription("Scaffold a worker locally"), Command.withExamples([ { command: "supabase experimental workers new", - description: "Prompt for the name, then for runtime and size", + description: "Prompt for the name, then for runtime, size and exposure", }, { command: "supabase experimental workers new api", - description: "Scaffold supabase/workers/api, prompting for runtime and size", + description: "Scaffold supabase/workers/api, prompting for runtime, size and exposure", }, { command: "supabase experimental workers new api --runtime node", description: "Scaffold supabase/workers/api on the node runtime", }, + { + command: "supabase experimental workers new api --exposure private", + description: "Scaffold a worker with no internet-facing URL", + }, + { + command: "supabase experimental workers new api --instances 3", + description: "Scaffold a worker that deploys at three instances", + }, { command: "supabase experimental workers new api --source packages/api", description: "Scaffold the worker outside the workers directory", diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts index 79b195dc54..af30205547 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts @@ -22,15 +22,21 @@ import { resolveWorkerSource, } from "../../../../../shared/workers/worker-paths.ts"; import { + DEFAULT_WORKER_EXPOSURE, + DEFAULT_WORKER_INSTANCES, DEFAULT_WORKER_RUNTIME, DEFAULT_WORKER_SIZE, + parseWorkerExposure, parseWorkerRuntime, parseWorkerSize, validateWorkerNameMessage, vcpuForSize, + WORKER_EXPOSURE_DESCRIPTIONS, + WORKER_EXPOSURES, WORKER_RUNTIME_DESCRIPTIONS, WORKER_RUNTIMES, WORKER_SIZES, + type WorkerExposure, type WorkerRuntime, type WorkerSize, } from "../../../../../shared/workers/worker-runtimes.ts"; @@ -51,8 +57,10 @@ import type { LegacyWorkersNewFlags } from "./new.command.ts"; * chosen runtime's starter files and record the choice in `config.toml`. * Nothing is deployed; this is entirely local-disk work. * - * The name, runtime and size are all resolved *before* anything is written, so a - * cancelled prompt leaves nothing behind for this worker at all. + * The name, runtime, size and exposure are all resolved *before* anything is + * written, so a cancelled prompt leaves nothing behind for this worker at all. + * `--instances` is recorded rather than resolved: it has no prompt, and it only + * reaches `config.toml` when it differs from the default. */ /** `values`, with `defaultValue` first, so a prompt pre-selects what it shows first. */ @@ -174,6 +182,55 @@ const resolveSize = Effect.fnUntraced(function* (options: { return DEFAULT_WORKER_SIZE; }); +/** + * Recorded on every scaffold, not just when it is asked for: `push` sends a + * complete spec each time, so a worker whose `exposure` is absent from + * `config.toml` is deployed public by the next bare `push`. Writing the value + * down — default included, the way `runtime` and `size` are — is what makes + * `--exposure private` stick past the deploy that chose it. + */ +const resolveExposure = Effect.fnUntraced(function* (options: { + readonly explicit: Option.Option; + /** Whether there is a terminal to ask on — see `canPromptFor`. */ + readonly canPrompt: boolean; +}) { + if (Option.isSome(options.explicit)) { + return options.explicit.value; + } + + if (options.canPrompt) { + const output = yield* Output; + const selected = yield* output.promptSelect( + "Should this worker be reachable from the internet?", + defaultFirst([...WORKER_EXPOSURES], DEFAULT_WORKER_EXPOSURE).map((exposure) => ({ + value: exposure, + label: exposure, + hint: WORKER_EXPOSURE_DESCRIPTIONS[exposure], + })), + ); + return parseWorkerExposure(selected) ?? DEFAULT_WORKER_EXPOSURE; + } + + return DEFAULT_WORKER_EXPOSURE; +}); + +/** + * The instance count to record, and whether to record it at all. + * + * Not prompted for, unlike the other dials: how many instances a worker needs is + * an operational answer nobody has while scaffolding it, so the flag records + * one when it is given and the file stays quiet when it is not. + * + * `undefined` — meaning "write no key" — for the default count, because an + * absent `instances` and `instances = 1` mean the same thing to `push`, and a + * scaffold should not commit a line that says nothing. A `0` is not that: it + * scales the worker to nothing, so it is written like any other explicit count. + */ +function recordedInstances(explicit: Option.Option): number | undefined { + const instances = Option.getOrUndefined(explicit); + return instances === undefined || instances === DEFAULT_WORKER_INSTANCES ? undefined : instances; +} + /** * Whether the destination is free for a scaffold: nothing there, or an empty * directory. A plain file counts as occupied, so it is refused by name rather @@ -226,11 +283,13 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun ); } - // Resolved before anything is written, so cancelling either prompt leaves + // Resolved before anything is written, so cancelling any prompt leaves // nothing behind — the name included. With nowhere to ask, the defaults // stand; only the name has nothing to fall back to. const runtime = yield* resolveRuntime({ explicit: flags.runtime, canPrompt }); const size = yield* resolveSize({ explicit: flags.size, canPrompt }); + const exposure = yield* resolveExposure({ explicit: flags.exposure, canPrompt }); + const instances = recordedInstances(flags.instances); // Validated before anything is written: this is the directory the starter // files land in, so a value naming the project root, `supabase/`, or @@ -289,6 +348,8 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun patch: { runtime, size, + exposure, + ...(instances === undefined ? {} : { instances }), ...(source === undefined ? {} : { source }), }, }); @@ -310,6 +371,11 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun runtime, size, vcpu: vcpuForSize(size), + exposure, + // The count a deploy will use, whether or not it was written down — a + // payload that omitted it for the default would read as "unknown" rather + // than "one". + instances: instances ?? DEFAULT_WORKER_INSTANCES, source: sourceDisplay, config_path: project.configPath, }; @@ -335,7 +401,10 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun legacyRenderWorkerDetails([ ["Runtime", runtime], ["Size", `${size} (${vcpuForSize(size)} vCPU)`], - ["Access", "public"], + ["Access", exposure], + // `declared`, the way `workers status` labels the same number: nothing + // is running yet, so a bare count would read as a live tally. + ["Instances", `${instances ?? DEFAULT_WORKER_INSTANCES} declared`], ]), ); // On the success trailer rather than inline, the way `bootstrap` emits its diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts index 07e4285ee2..a0dd81bd9c 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts @@ -31,6 +31,8 @@ function flags(overrides: Partial = {}): LegacyWorkersNew name: Option.some("api"), runtime: Option.none(), size: Option.none(), + exposure: Option.none(), + instances: Option.none(), source: Option.none(), ...overrides, }; @@ -60,7 +62,7 @@ describe("legacy workers new", () => { const workerDir = join(repo.dir, "supabase", "workers", "api"); expect(existsSync(join(workerDir, "index.mjs"))).toBe(true); expect(repo.config()).toBe( - `${CONFIG_WITH_COMMENTS}\n[workers.api]\nruntime = "node"\nsize = "2gb"\n`, + `${CONFIG_WITH_COMMENTS}\n[workers.api]\nruntime = "node"\nsize = "2gb"\nexposure = "public"\n`, ); // Declarative line first, then the detail rows, then the next step — @@ -94,7 +96,7 @@ describe("legacy workers new", () => { // without ending the run, so it refuses both there rather than after asking. it.live("refuses a bad or already-recorded name at the name prompt", () => { const repo = project({ - "supabase/config.toml": `${CONFIG_WITH_COMMENTS}\n[workers.api]\nruntime = "node"\nsize = "2gb"\n`, + "supabase/config.toml": `${CONFIG_WITH_COMMENTS}\n[workers.api]\nruntime = "node"\nsize = "2gb"\nexposure = "public"\n`, }); const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, @@ -150,11 +152,11 @@ describe("legacy workers new", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); - it.live("prompts for runtime and size when neither is given", () => { + it.live("prompts for runtime, size and exposure when none is given", () => { const repo = project(); const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, - promptSelectResponses: ["node", "4gb"], + promptSelectResponses: ["node", "4gb", "private"], }); return Effect.gen(function* () { @@ -163,13 +165,113 @@ describe("legacy workers new", () => { expect(out.promptSelectCalls.map((call) => call.message)).toEqual([ "Which runtime should this worker use?", "Which instance size should this worker use?", + "Should this worker be reachable from the internet?", ]); expect(repo.config()).toContain('runtime = "node"'); expect(repo.config()).toContain('size = "4gb"'); + expect(repo.config()).toContain('exposure = "private"'); expect(existsSync(join(repo.dir, "supabase", "workers", "api", "index.mjs"))).toBe(true); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // The whole reason `new` records it: `push` sends a complete spec every time, + // so an entry with no `exposure` is deployed public by the next bare `push`. + // Recording the answer is what makes a private worker stay private. + it.live("records the chosen exposure so a later push keeps it", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ workdir: repo.dir }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ exposure: Option.some("private") })); + + expect(repo.config()).toContain('exposure = "private"'); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The count a scaffold cannot guess: `--instances` has no prompt, so it is + // recorded when given and left out when not. + it.live("records an instance count that differs from the default", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ workdir: repo.dir }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ instances: Option.some(3) })); + + // Bare, not quoted: the config schema types `instances` as a number, so a + // quoted count would render a config.toml that no longer loads. + expect(repo.config()).toContain("instances = 3"); + expect(repo.config()).not.toContain('instances = "3"'); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The end-to-end proof that the count is written as a number: the config + // schema types `instances` as one, so a quoted `"3"` renders a config.toml + // that no longer decodes — which only shows up on the *next* load, not on the + // write that caused it. Scaffolding a second worker is that next load. + it.live("writes a count the config loader can read back", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ workdir: repo.dir }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ name: Option.some("api"), instances: Option.some(3) })); + yield* legacyWorkersNew(flags({ name: Option.some("web") })); + + expect(repo.config()).toContain("instances = 3"); + expect(repo.config()).toContain("[workers.web]"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // Zero is an explicit count — it scales the worker to nothing — not an absent + // one, so it has to survive the "only record a non-default" rule. + it.live("records a zero instance count", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ workdir: repo.dir }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ instances: Option.some(0) })); + + expect(repo.config()).toContain("instances = 0"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // An absent `instances` and `instances = 1` mean the same thing to `push`, so + // the scaffold does not commit a line that says nothing. + it.live("writes no instance count when nothing names one", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ workdir: repo.dir }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags()); + + expect(repo.config()).not.toContain("instances"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + it.live("writes no instance count when the default is named explicitly", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ workdir: repo.dir }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ instances: Option.some(1) })); + + expect(repo.config()).not.toContain("instances"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // Written even when it is the default, the same way `runtime` and `size` are: + // an absent key and `public` mean the same thing to `push` today, but only the + // written one survives a change of default. + it.live("records the default exposure when nothing names one", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ workdir: repo.dir, format: "json" }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ name: Option.some("api") })); + + expect(repo.config()).toContain('exposure = "public"'); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + // The runtime and size prompts do have defaults to fall back on, so a piped // stdin must leave them unasked rather than consuming the pipe. it.live("takes the defaults without prompting when stdin is piped", () => { @@ -177,7 +279,7 @@ describe("legacy workers new", () => { const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, stdinIsTty: false, - promptSelectResponses: ["node", "4gb"], + promptSelectResponses: ["node", "4gb", "private"], }); return Effect.gen(function* () { @@ -186,6 +288,7 @@ describe("legacy workers new", () => { expect(out.promptSelectCalls).toEqual([]); expect(repo.config()).toContain('runtime = "deno"'); expect(repo.config()).toContain('size = "2gb"'); + expect(repo.config()).toContain('exposure = "public"'); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); @@ -210,7 +313,12 @@ describe("legacy workers new", () => { return Effect.gen(function* () { yield* legacyWorkersNew( - flags({ name: Option.some("api"), runtime: Option.some("deno"), size: Option.some("4gb") }), + flags({ + name: Option.some("api"), + runtime: Option.some("deno"), + size: Option.some("4gb"), + exposure: Option.some("public"), + }), ); const recorded = repo.config(); @@ -298,7 +406,7 @@ describe("legacy workers new", () => { expect(existsSync(join(created.dir, "supabase", "workers", "api", "index.mjs"))).toBe(true); expect(readFileSync(join(created.dir, "supabase", "config.toml"), "utf8")).toBe( - `[workers.api]\nruntime = "node"\nsize = "2gb"\n`, + `[workers.api]\nruntime = "node"\nsize = "2gb"\nexposure = "public"\n`, ); }).pipe( Effect.provide(layer), @@ -373,7 +481,63 @@ describe("legacy workers new", () => { yield* legacyWorkersNew(flags({ runtime: Option.some("node") })); const payload: unknown = JSON.parse(out.stdoutText); - expect(payload).toMatchObject({ runtime: "node", size: "2gb" }); + // Every dial the scaffold settled, not just the two it is named for: a + // caller reading this payload is deciding what to deploy, and an omitted + // `exposure` or `instances` reads as "unknown" rather than as the default + // the run actually chose. + expect(payload).toMatchObject({ + worker_name: "api", + runtime: "node", + size: "2gb", + vcpu: 1, + exposure: "public", + instances: 1, + }); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The count and the exposure are recorded sparsely — `instances` is left out + // of config.toml at the default — so the payload is the only place a caller + // can read what this scaffold will actually deploy as. + it.live("reports the chosen exposure and count under -o json", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, goOutput: "json" }); + + return Effect.gen(function* () { + yield* legacyWorkersNew( + flags({ + runtime: Option.some("node"), + exposure: Option.some("private"), + instances: Option.some(3), + }), + ); + + expect(JSON.parse(out.stdoutText)).toMatchObject({ + exposure: "private", + instances: 3, + }); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + it.live("shows the exposure and declared count in the details block", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ workdir: repo.dir }); + + return Effect.gen(function* () { + yield* legacyWorkersNew( + flags({ + runtime: Option.some("node"), + exposure: Option.some("private"), + instances: Option.some(3), + }), + ); + + // `Access`, the way `workers status` and `push` label the same field. + expect(out.stdoutText).toContain("Access"); + expect(out.stdoutText).toContain("private"); + // `declared`, because nothing is running yet — a bare count would read as + // a live tally. + expect(out.stdoutText).toContain("3 declared"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); @@ -417,7 +581,7 @@ describe("legacy workers new", () => { // The worker is recorded in config.toml, which is the TOML editor's file. expect(repo.config()).toBe( - `${CONFIG_WITH_COMMENTS}\n[workers.api]\nruntime = "node"\nsize = "2gb"\n`, + `${CONFIG_WITH_COMMENTS}\n[workers.api]\nruntime = "node"\nsize = "2gb"\nexposure = "public"\n`, ); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); @@ -442,7 +606,7 @@ describe("legacy workers new", () => { // The workdir got both the entry and the scaffold it points at. expect(readFileSync(join(workdir, "supabase", "config.toml"), "utf8")).toBe( - '[workers.api]\nruntime = "node"\nsize = "2gb"\n', + '[workers.api]\nruntime = "node"\nsize = "2gb"\nexposure = "public"\n', ); expect(existsSync(join(workdir, "supabase", "workers", "api", "index.mjs"))).toBe(true); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); @@ -542,7 +706,7 @@ describe("legacy workers new", () => { const repo = project(); const { layer } = setupLegacyWorkers({ workdir: repo.dir, - promptSelectResponses: ["cobol", "colossal"], + promptSelectResponses: ["cobol", "colossal", "sideways"], }); return Effect.gen(function* () { @@ -550,11 +714,14 @@ describe("legacy workers new", () => { name: Option.some("api"), runtime: Option.none(), size: Option.none(), + exposure: Option.none(), + instances: Option.none(), source: Option.none(), }); expect(repo.config()).toContain(`runtime = "deno"`); expect(repo.config()).toContain(`size = "2gb"`); + expect(repo.config()).toContain(`exposure = "public"`); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/push/SIDE_EFFECTS.md index a7437d1e42..c4af0f5100 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/push/SIDE_EFFECTS.md @@ -7,13 +7,13 @@ ## Files Read -| Path | Format | When | -| ---------------------------------------- | ---------- | ----------------------------------------------------------------------------------------------------------- | -| `/supabase/config.json` | JSON | always when present — preferred over `config.toml`; each worker's runtime, size, instances, source | -| `/supabase/config.toml` | TOML | always when no `config.json` exists — the same worker fields | -| `/**` | any | always — packaged into the build context | -| `/profile` | plain text | when neither `--profile` nor `SUPABASE_PROFILE` is set — names the profile, defaulting to `supabase` | -| `` (YAML) | YAML | when `SUPABASE_PROFILE` is a filesystem path rather than a built-in name; a read failure aborts the command | +| Path | Format | When | +| ---------------------------------------- | ---------- | ------------------------------------------------------------------------------------------------------------ | +| `/supabase/config.json` | JSON | always when present — preferred over `config.toml`; each worker's runtime, size, exposure, instances, source | +| `/supabase/config.toml` | TOML | always when no `config.json` exists — the same worker fields | +| `/**` | any | always — packaged into the build context | +| `/profile` | plain text | when neither `--profile` nor `SUPABASE_PROFILE` is set — names the profile, defaulting to `supabase` | +| `` (YAML) | YAML | when `SUPABASE_PROFILE` is a filesystem path rather than a built-in name; a read failure aborts the command | ## Files Written @@ -43,6 +43,7 @@ run reports the accepted spec the deploy response returned. | ---- | -------------------------------------------------------------------- | | `0` | success | | `1` | no workers named and none found in the project | +| `1` | config records a runtime, size or exposure the CLI does not know | | `1` | a worker's source is missing, not a directory, or empty | | `1` | a worker's source directory cannot be read | | `1` | a worker's source links to a path outside itself | @@ -94,6 +95,14 @@ part of the question the failure raises. The second report also covers a real gap, since `runCli` drains success trailers only on exit code 0, so a failing run discards every follow-up hint it had queued. +`--exposure` decides one deploy and nothing writes it down, so an override the +config does not already agree with is reported on stderr, naming the +`[workers.] exposure` line to add. Unguarded by format, like the +runtime-guess nudge: every deploy sends a complete spec, so a worker taken off +the internet by the flag goes back on it at the next bare push, and a CI run is +where that matters most. Silent when the config already resolves to the same +exposure, case included. + Under `--no-wait` the `Image` row and the payload's `image_version` are omitted while `build_state` is `building`. The deploy response may carry an `image_version` — a re-push of a worker that is already serving echoes the image diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.command.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.command.ts index 73be5300f1..c890f119b4 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.command.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.command.ts @@ -1,6 +1,7 @@ import { Argument, Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; import { withJsonErrorHandling } from "../../../../../shared/output/json-error-handling.ts"; +import { WORKER_EXPOSURES } from "../../../../../shared/workers/worker-runtimes.ts"; import { legacyManagementApiRuntimeLayer } from "../../../../shared/legacy-management-api-runtime.layer.ts"; import { withLegacyCommandInstrumentation } from "../../../../telemetry/legacy-command-instrumentation.ts"; import { legacyWorkersPush } from "./push.handler.ts"; @@ -24,6 +25,17 @@ const config = { ), Flag.optional, ), + exposure: Flag.choice("exposure", WORKER_EXPOSURES).pipe( + // A closed set at the parser, the way `new --runtime` and `new --size` are: + // the accepted values get listed in the refusal, and nothing unrecognized + // reaches the deploy endpoint after a build context has been uploaded. + // `[workers.] exposure` stays a plain string, so a value the API + // grows before this CLI does can still be recorded there. + Flag.withDescription( + "Whether the worker is reachable from the internet, overriding `exposure` in supabase/config.toml for this deploy. Falls back to the recorded value, then public.", + ), + Flag.optional, + ), noWait: Flag.boolean("no-wait").pipe( // The deploy POST is answered once the platform has accepted the spec and // the uploaded context, and the server-side container build that follows @@ -47,7 +59,7 @@ export type LegacyWorkersPushFlags = CliCommand.Command.Config.Infer legacyWorkersPush(flags).pipe( - withLegacyCommandInstrumentation({ flags }), + withLegacyCommandInstrumentation({ flags, config }), withJsonErrorHandling, ), ), diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts index 9b9936cd83..8d11562cf1 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts @@ -21,13 +21,17 @@ import { displayPath } from "../../../../../shared/workers/worker-paths.ts"; import type { WorkerEntry } from "../../../../../shared/workers/worker-config.ts"; import { apiSizeFor, + DEFAULT_WORKER_EXPOSURE, DEFAULT_WORKER_INSTANCES, DEFAULT_WORKER_SIZE, formatApiSize, + parseWorkerExposure, parseWorkerRuntime, parseWorkerSize, + WORKER_EXPOSURES, WORKER_RUNTIMES, WORKER_SIZES, + type WorkerExposure, } from "../../../../../shared/workers/worker-runtimes.ts"; import { workerUrl } from "../../../../../shared/workers/worker-url.ts"; import { @@ -39,6 +43,7 @@ import { } from "../../../../../shared/workers/workers-api.ts"; import { NoWorkersToDeployError, + UnknownWorkerExposureError, UnknownWorkerRuntimeError, UnknownWorkerSizeError, WorkerBuildFailedError, @@ -61,8 +66,8 @@ import type { LegacyWorkersPushFlags } from "./push.command.ts"; * deploy the worker into the linked project. Registered under `deploy` as an * alias, for anyone reaching for the `supabase functions` verb out of habit. * - * The runtime, size and source directory come from `[workers.]` in - * `supabase/config.toml`. A directory pushed without ever running `new` gets + * The runtime, size, exposure and source directory come from `[workers.]` + * in `supabase/config.toml`. A directory pushed without ever running `new` gets * its runtime guessed from marker files instead — reported, with a nudge to pin * it down rather than re-guess on every push. * @@ -104,7 +109,7 @@ const resolveRuntime = Effect.fnUntraced(function* (options: { // payload stdout is carrying. yield* output.raw( `No runtime configured for ${options.name}: guessed ${classified.runtime} (${classified.reason}). ` + - `Pin it down by adding [workers.${options.name}] runtime = "${classified.runtime}" to supabase/config.toml.\n`, + `Set [workers.${options.name}] runtime = "${classified.runtime}" in supabase/config.toml.\n`, "stderr", ); return classified.runtime; @@ -144,6 +149,75 @@ function resolveInstances(options: { return Option.getOrElse(options.override, () => options.recorded ?? DEFAULT_WORKER_INSTANCES); } +/** + * `--exposure` for one deploy, then the recorded exposure, then + * {@link DEFAULT_WORKER_EXPOSURE}. Never left unset, because every deploy sends a + * complete spec and an omitted exposure would re-expose a worker somebody had + * deliberately made private. + * + * `--exposure` is a `Flag.choice`, so only a recorded value can be unrecognized + * — and that is refused rather than coerced, the same way `resolveSize` treats a + * size it does not know: silently deploying a `private`-typo'd worker as public + * is the one outcome nobody asked for. + * + * The flag decides one deploy and nothing writes it down, so an override the + * config does not already agree with is reported the way `resolveRuntime` + * reports a guess: on stderr, naming the line to set. Without it, taking a + * worker off the internet with `--exposure private` lasts exactly until the next + * bare `push` puts it back. + */ +const resolveExposure = Effect.fnUntraced(function* (options: { + readonly name: string; + readonly recorded: string | undefined; + readonly override: Option.Option; +}) { + if (Option.isSome(options.override)) { + const chosen = options.override.value; + // What a later bare `push` would resolve to: the recorded value if the CLI + // knows it, the default if there is none, and `undefined` for one it cannot + // read — which is not `chosen` either, so that case is nudged too. + const withoutTheFlag = + options.recorded === undefined + ? DEFAULT_WORKER_EXPOSURE + : parseWorkerExposure(options.recorded); + if (withoutTheFlag !== chosen) { + const output = yield* Output; + // stderr, so it never lands inside a payload stdout is carrying — and + // unguarded by format, like the runtime nudge: a CI run is exactly where + // a one-deploy exposure quietly reverting matters most. + yield* output.raw( + `--exposure ${chosen} applies to this deploy only: supabase/config.toml ${ + options.recorded === undefined + ? `records no exposure for ${options.name}` + : `records exposure = "${options.recorded}"` + }, so the next bare push will not use ${chosen}. ` + + `Set [workers.${options.name}] exposure = "${chosen}" in supabase/config.toml.\n`, + "stderr", + ); + } + return chosen; + } + if (options.recorded === undefined) { + return DEFAULT_WORKER_EXPOSURE; + } + const recorded = parseWorkerExposure(options.recorded); + if (recorded === undefined) { + return yield* Effect.fail( + new UnknownWorkerExposureError({ + // A blank value gets its own sentence: `an unknown exposure ""` reads + // like a parser quirk, when what actually happened is that the key is + // there and says nothing. + detail: + options.recorded.trim() === "" + ? `supabase/config.toml records a blank exposure for "${options.name}".` + : `supabase/config.toml records an unknown exposure "${options.recorded}" for "${options.name}".`, + suggestion: `Set [workers.${options.name}] exposure to one of: ${WORKER_EXPOSURES.join(", ")}.`, + }), + ); + } + return recorded; +}); + /** * What to do about a worker whose source directory is not there at all. * @@ -194,6 +268,7 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { */ readonly refSuffix: string; readonly instances: Option.Option; + readonly exposure: Option.Option; /** `--no-wait`: return once the deploy is accepted instead of blocking on the build. */ readonly noWait: boolean; readonly pollSchedule?: Schedule.Schedule; @@ -286,6 +361,15 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { override: input.instances, }); + // Resolved before anything is packaged or uploaded, alongside the runtime and + // size, so a config that records an exposure this CLI does not know is refused + // while the refusal is still free. + const exposure = yield* resolveExposure({ + name, + recorded: worker.entry?.exposure, + override: input.exposure, + }); + let contextUploadId: string; { const packaging = yield* output.task("Packaging worker..."); @@ -328,9 +412,7 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { // context carries its own Dockerfile and is built as-is. ...(runtime === "dockerfile" ? {} : { runtime }), size: apiSizeFor(size), - // Every runtime offered today serves HTTP. A sandbox runtime would need a - // branch here. - exposure: "public", + exposure, instances, }; @@ -599,6 +681,7 @@ export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(f projectRef, refSuffix, instances: flags.instances, + exposure: flags.exposure, noWait: flags.noWait, machineOutput, ...(options.pollSchedule === undefined ? {} : { pollSchedule: options.pollSchedule }), diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts index 1f219a80d5..16e5feb083 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts @@ -14,6 +14,7 @@ import { LegacyProjectNotLinkedError } from "../../../../config/legacy-project-r import { LegacyWorkersEnvNotSupportedError } from "../workers.errors.ts"; import { NoWorkersToDeployError, + UnknownWorkerExposureError, UnknownWorkerRuntimeError, UnknownWorkerSizeError, WorkerBuildFailedError, @@ -45,6 +46,7 @@ function flags(overrides: Partial = {}): LegacyWorkersPu return { names: ["api"], instances: Option.none(), + exposure: Option.none(), // Mirrors the command default: a push waits for the build, and only the // scenarios that are about the early return opt out of it. noWait: false, @@ -365,6 +367,196 @@ describe("legacy workers push", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // The whole point of recording it: every deploy sends a complete spec, so a + // worker deliberately made private has to stay private across pushes rather + // than being re-exposed by the next one. + it.live("keeps a worker private when config records it that way", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nsize = "2gb"\nexposure = "private"\n`, + }); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + yield* push(); + + const deploy = http.requests.find((request) => request.url.endsWith("/deploy")); + expect(JSON.parse(deploy?.body ?? "{}").data.attributes.spec.exposure).toBe("private"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // Hand-written config, so the casing is the user's own — `PRIVATE` plainly + // means `private`, and the canonical form is what gets sent. + it.live("reads a recorded exposure case-insensitively", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nexposure = "PRIVATE"\n`, + }); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + yield* push(); + + const deploy = http.requests.find((request) => request.url.endsWith("/deploy")); + expect(JSON.parse(deploy?.body ?? "{}").data.attributes.spec.exposure).toBe("private"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + it.live("lets --exposure override the recorded exposure for one deploy", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nsize = "2gb"\nexposure = "private"\n`, + }); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + yield* push({ exposure: Option.some("public") }); + + const deploy = http.requests.find((request) => request.url.endsWith("/deploy")); + expect(JSON.parse(deploy?.body ?? "{}").data.attributes.spec.exposure).toBe("public"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // `[workers.*] exposure` is a plain string in the config schema, so a typo + // reaches the handler. Coercing it to the default would deploy a `privat` + // worker to the whole internet — refused before anything is packaged instead. + it.live("names the exposures on offer when config records one it does not know", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nexposure = "privat"\n`, + }); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + const error = yield* push().pipe(Effect.flip); + + expect(error).toBeInstanceOf(UnknownWorkerExposureError); + expect((error as UnknownWorkerExposureError).detail).toContain("privat"); + expect((error as UnknownWorkerExposureError).suggestion).toContain("public, private"); + expect(http.requests).toHaveLength(0); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The blank case, which reads as "not recorded" if the config reader collapses + // it: absent means the `public` default, so a worker whose config plainly + // tried to say something would go to the whole internet. Refused like any + // other value the CLI does not recognize. + it.live("refuses a blank recorded exposure instead of defaulting it to public", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nexposure = ""\n`, + }); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + const error = yield* push().pipe(Effect.flip); + + expect(error).toBeInstanceOf(UnknownWorkerExposureError); + // Named as blank rather than as an unknown `""`, which reads like a + // parser quirk instead of an empty key. + expect((error as UnknownWorkerExposureError).detail).toContain("blank exposure"); + expect((error as UnknownWorkerExposureError).suggestion).toContain("public, private"); + // Nothing was packaged, uploaded or deployed — least of all publicly. + expect(http.requests).toHaveLength(0); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // `--exposure` decides one deploy and nothing writes it down. Every deploy + // sends a complete spec, so a worker taken off the internet by the flag goes + // back on it at the next bare push — quietly, unless the run says so. + describe("says when --exposure will not outlive the deploy", () => { + const pushWith = (config: string, exposure: "public" | "private") => { + const repo = project({ "supabase/config.toml": config }); + const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + return { repo, layer, out, run: () => push({ exposure: Option.some(exposure) }) }; + }; + + it.live("nudges when the config records nothing", () => { + const { repo, layer, out, run } = pushWith( + `project_id = "demo"\n\n[workers.api]\nruntime = "node"\n`, + "private", + ); + + return Effect.gen(function* () { + yield* run(); + + expect(out.stderrText).toContain("records no exposure for api"); + // The exact line to set, the way the runtime guess names its own. + expect(out.stderrText).toContain('[workers.api] exposure = "private"'); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + it.live("nudges when the config records the opposite", () => { + const { repo, layer, out, run } = pushWith( + `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nexposure = "public"\n`, + "private", + ); + + return Effect.gen(function* () { + yield* run(); + + expect(out.stderrText).toContain('records exposure = "public"'); + expect(out.stderrText).toContain('exposure = "private"'); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // A recorded value the CLI cannot read is not `chosen` either: the next bare + // push refuses rather than deploying, which is still not what this run did. + it.live("nudges when the config records something it cannot read", () => { + const { repo, layer, out, run } = pushWith( + `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nexposure = "privat"\n`, + "private", + ); + + return Effect.gen(function* () { + yield* run(); + + expect(out.stderrText).toContain('records exposure = "privat"'); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // Nothing drifts, so nothing to say — the flag restated what the config + // already holds, case-insensitively. + it.live("stays quiet when the config already agrees", () => { + const { repo, layer, out, run } = pushWith( + `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nexposure = "PRIVATE"\n`, + "private", + ); + + return Effect.gen(function* () { + yield* run(); + + expect(out.stderrText).not.toContain("applies to this deploy only"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The same non-drift, reached the other way: no recorded exposure and a flag + // naming the default a bare push would have picked anyway. + it.live("stays quiet when the flag restates the default", () => { + const { repo, layer, out, run } = pushWith( + `project_id = "demo"\n\n[workers.api]\nruntime = "node"\n`, + "public", + ); + + return Effect.gen(function* () { + yield* run(); + + expect(out.stderrText).not.toContain("applies to this deploy only"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + }); + + // The flag is the authority for the deploy it runs, so an unrecognized + // recorded value it replaces is moot rather than fatal. + it.live("lets --exposure stand in for an exposure config records badly", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nexposure = "privat"\n`, + }); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + yield* push({ exposure: Option.some("private") }); + + const deploy = http.requests.find((request) => request.url.endsWith("/deploy")); + expect(JSON.parse(deploy?.body ?? "{}").data.attributes.spec.exposure).toBe("private"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("polls until the build leaves `building`", () => { const repo = project(); const { layer, http } = setupLegacyWorkers({ @@ -448,10 +640,10 @@ describe("legacy workers push", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); - // Every deploy this CLI sends asks for public exposure, but the accepted spec - // is the platform's answer, not the request echoed back. A worker it did not - // expose has no URL to print, and inventing one from the ref would name an - // address that does not resolve. + // The accepted spec is the platform's answer, not the request echoed back — so + // a worker the platform did not expose has no URL to print even when the deploy + // asked for `public`, and inventing one from the ref would name an address + // that does not resolve. it.live("omits the URL for a worker the platform did not expose publicly", () => { const repo = project(); const { layer, out } = setupLegacyWorkers({ diff --git a/apps/cli/src/legacy/shared/legacy-db-target-flags.ts b/apps/cli/src/legacy/shared/legacy-db-target-flags.ts index 4f28bc40b5..8d2c8d76c6 100644 --- a/apps/cli/src/legacy/shared/legacy-db-target-flags.ts +++ b/apps/cli/src/legacy/shared/legacy-db-target-flags.ts @@ -114,6 +114,7 @@ export const VALUE_CONSUMING_LONG_FLAGS = new Set([ "env-file", "exclude", "exp", + "exposure", "file", "from", "from-backup", diff --git a/apps/cli/src/shared/workers/toml-section.ts b/apps/cli/src/shared/workers/toml-section.ts index 8baab4025d..b37290e2ea 100644 --- a/apps/cli/src/shared/workers/toml-section.ts +++ b/apps/cli/src/shared/workers/toml-section.ts @@ -53,14 +53,40 @@ function quote(value: string): string { return `"${escaped}"`; } +/** + * Whether `value` is a number this module can render without lying about it. + * + * Bare-rendering is faithful only for a whole, finite, non-negative count — + * which is what `[workers.] instances` is bounded to. Everything else is + * either valid TOML the schema refuses (`1.5`, `-1`, `1e21`) or a token TOML has + * no reading for at all (`String(NaN)` is `NaN`, not `nan`; `String(Infinity)` + * is `Infinity`, not `inf`). Only the second kind fails a re-parse, so the first + * kind has to be stopped here. + */ +export function isRenderableTomlNumber(value: number): boolean { + return Number.isSafeInteger(value) && value >= 0; +} + /** Render `key` for use in a table header or key position. */ export function tomlKey(key: string): string { return isBareKey(key) ? key : quote(key); } -/** `key = "value"` — every value the worker commands write is a string. */ -function renderPair(key: string, value: string): string { - return `${tomlKey(key)} = ${quote(value)}`; +/** + * `key = "value"`, or `key = value` for a number. + * + * A number has to be rendered bare: quoting it would write a TOML string, and + * the config schema types `[workers.] instances` as a number — so a quoted + * count produces a `config.toml` that no longer loads at all. + * + * Rendering is all this does, and a bare number is only as good as the caller's. + * `1.5` and `-1` render as perfectly valid TOML that the worker schema rejects, + * so `planWorkerEntry`'s re-parse — a syntax check, not a schema one — would + * pass them straight through. {@link isRenderableTomlNumber} is the guard that + * makes that impossible, and it runs before this does. + */ +function renderPair(key: string, value: string | number): string { + return `${tomlKey(key)} = ${typeof value === "number" ? String(value) : quote(value)}`; } /** @@ -74,7 +100,7 @@ function renderPair(key: string, value: string): string { export function appendTomlSection( text: string, header: string, - values: Readonly>, + values: Readonly>, ): string { const block = [ `[${header}]`, diff --git a/apps/cli/src/shared/workers/toml-section.unit.test.ts b/apps/cli/src/shared/workers/toml-section.unit.test.ts index d00fca6933..8cd27d4214 100644 --- a/apps/cli/src/shared/workers/toml-section.unit.test.ts +++ b/apps/cli/src/shared/workers/toml-section.unit.test.ts @@ -65,6 +65,20 @@ size = "2gb" ); }); + // Quoting a count would write a TOML string, and the config schema types + // `instances` as a number — so the rendered file would stop loading entirely. + test("writes a number bare rather than quoting it", () => { + expect(appendTomlSection("", "workers.api", { size: "2gb", instances: 3 })).toBe( + '[workers.api]\nsize = "2gb"\ninstances = 3\n', + ); + }); + + test("writes a zero count, which is a real value rather than an absent one", () => { + expect(appendTomlSection("", "workers.api", { instances: 0 })).toBe( + "[workers.api]\ninstances = 0\n", + ); + }); + test("writes a header with no keys when there is nothing to set", () => { expect(appendTomlSection("", "workers.api", {})).toBe("[workers.api]\n"); }); diff --git a/apps/cli/src/shared/workers/worker-config.ts b/apps/cli/src/shared/workers/worker-config.ts index 6d09c9ccb3..8351f9b9c0 100644 --- a/apps/cli/src/shared/workers/worker-config.ts +++ b/apps/cli/src/shared/workers/worker-config.ts @@ -6,7 +6,7 @@ import { type CliErrorActionabilityDeclaration, ErrorActionabilityId, } from "../telemetry/error-actionability.ts"; -import { appendTomlSection, tomlKey } from "./toml-section.ts"; +import { appendTomlSection, isRenderableTomlNumber, tomlKey } from "./toml-section.ts"; /** * The `[workers]` section of `supabase/config.toml`, read through the decoded @@ -21,6 +21,7 @@ import { appendTomlSection, tomlKey } from "./toml-section.ts"; export interface WorkerEntry { readonly runtime?: string; readonly size?: string; + readonly exposure?: string; readonly instances?: number; readonly source?: string; } @@ -72,6 +73,23 @@ export class WorkerConfigWriteUnsafeError extends Data.TaggedError("WorkerConfig const stringOrUndefined = (value: unknown): string | undefined => typeof value === "string" && value !== "" ? value : undefined; +/** + * As {@link stringOrUndefined}, but an explicitly empty string survives. + * + * For `exposure`, "recorded but unusable" must not read as "not recorded". + * Absent means the `public` default, so folding `exposure = ""` into `undefined` + * hands a config that plainly tried to say something to the most open setting + * there is — the exact silent-widening `push`'s `resolveExposure` exists to + * refuse. Kept verbatim so it reaches that check like any other value the CLI + * does not recognize. + * + * `runtime`, `size` and `source` keep the collapsing reader: their fallbacks are + * a marker-file guess, a default size and the conventional directory, none of + * which widens anything. + */ +const recordedStringOrUndefined = (value: unknown): string | undefined => + typeof value === "string" ? value : undefined; + /** A plain object — a `[workers.]` table rather than a scalar or a list. */ const isRecord = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); @@ -107,6 +125,10 @@ export function readWorkersSection(workers: unknown): WorkersSection { entries[key] = { runtime: stringOrUndefined(value["runtime"]), size: stringOrUndefined(value["size"]), + // Left as whatever string was written, empty included: `push` is what + // names the accepted values, and dropping an unrecognized one here would + // silently deploy a worker at the default exposure instead. + exposure: recordedStringOrUndefined(value["exposure"]), instances: instanceCountOrUndefined(value["instances"]), source: stringOrUndefined(value["source"]), }; @@ -131,7 +153,8 @@ export interface WorkerEntryWrite { export const planWorkerEntry = Effect.fnUntraced(function* (options: { readonly configPath: string; readonly name: string; - readonly patch: Readonly>; + /** Rendered as written: strings are quoted, numbers are not. */ + readonly patch: Readonly>; /** The already-parsed config — the authority on whether an entry exists. */ readonly existingWorkers: Readonly>; }) { @@ -150,6 +173,22 @@ export const planWorkerEntry = Effect.fnUntraced(function* (options: { ); } + // Before rendering, because the re-parse below cannot catch this. A number + // like `1.5` or `-1` renders as valid TOML that only the *schema* rejects, so + // it would sail through a syntax check and land in the user's config as a + // `[workers]` section the loader then refuses. + const unrenderable = Object.entries(options.patch).find( + ([, value]) => typeof value === "number" && !isRenderableTomlNumber(value), + ); + if (unrenderable !== undefined) { + return yield* Effect.fail( + new WorkerConfigWriteUnsafeError({ + detail: `Recording "${options.name}" would write ${unrenderable[0]} = ${String(unrenderable[1])} to ${options.configPath}, which is not a whole, non-negative count.`, + suggestion: `Pass a whole number of zero or more, or add [workers.${options.name}] to ${options.configPath} yourself.`, + }), + ); + } + const exists = yield* fs.exists(options.configPath); const text = exists ? yield* fs.readFileString(options.configPath) : ""; const header = `workers.${tomlKey(options.name)}`; diff --git a/apps/cli/src/shared/workers/worker-config.unit.test.ts b/apps/cli/src/shared/workers/worker-config.unit.test.ts index d1439e57ac..5c4c3c9b4c 100644 --- a/apps/cli/src/shared/workers/worker-config.unit.test.ts +++ b/apps/cli/src/shared/workers/worker-config.unit.test.ts @@ -1,8 +1,8 @@ -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { BunServices } from "@effect/platform-bun"; -import { Effect } from "effect"; +import { Effect, Exit } from "effect"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { readWorkersSection, @@ -16,13 +16,31 @@ describe("readWorkersSection", () => { test("reads each worker's recorded dials", () => { expect( readWorkersSection({ - api: { runtime: "node", size: "2gb", instances: 4, source: "packages/api" }, + api: { + runtime: "node", + size: "2gb", + exposure: "private", + instances: 4, + source: "packages/api", + }, box: { runtime: "sandbox" }, }), ).toEqual({ workers: { - api: { runtime: "node", size: "2gb", instances: 4, source: "packages/api" }, - box: { runtime: "sandbox", size: undefined, instances: undefined, source: undefined }, + api: { + runtime: "node", + size: "2gb", + exposure: "private", + instances: 4, + source: "packages/api", + }, + box: { + runtime: "sandbox", + size: undefined, + exposure: undefined, + instances: undefined, + source: undefined, + }, }, }); }); @@ -30,7 +48,13 @@ describe("readWorkersSection", () => { test("drops non-object values so a stray scalar is not read as a worker", () => { expect(readWorkersSection({ stray: "oops", api: {} })).toEqual({ workers: { - api: { runtime: undefined, size: undefined, instances: undefined, source: undefined }, + api: { + runtime: undefined, + size: undefined, + exposure: undefined, + instances: undefined, + source: undefined, + }, }, }); }); @@ -51,6 +75,16 @@ describe("readWorkersSection", () => { expect(readWorkersSection({ api: { instances: 0 } }).workers["api"]?.instances).toBe(0); }); + // Unlike the instance count, an unrecognized exposure is kept and carried to + // `push`, which names the values it accepts. Dropping it here would deploy the + // worker at the default exposure — public — which is the opposite of what a + // misspelled `private` was asking for. + test("keeps an exposure it does not recognize, for push to refuse by name", () => { + expect(readWorkersSection({ api: { exposure: "privat" } }).workers["api"]?.exposure).toBe( + "privat", + ); + }); + test("treats a missing or malformed section as empty", () => { expect(readWorkersSection(undefined)).toEqual({ workers: {} }); expect(readWorkersSection([])).toEqual({ workers: {} }); @@ -76,6 +110,42 @@ describe("planWorkerEntry + commitWorkerEntry", () => { const writeWorkerEntry = (options: Parameters[0]) => planWorkerEntry(options).pipe(Effect.flatMap(commitWorkerEntry)); + // The re-parse below is a syntax check, not a schema one: `instances = 1.5` + // is perfectly valid TOML that the worker schema rejects, so it would reach + // the user's config and only fail later, when the loader refuses the file. + test.each([ + ["a fraction", 1.5], + ["a negative count", -1], + ["a value past the safe integer range", 1e21], + ["not a number at all", Number.NaN], + ])("refuses %s rather than rendering it", async (_label, instances) => { + const exit = await Effect.runPromise( + writeWorkerEntry({ + configPath, + name: "api", + existingWorkers: {}, + patch: { runtime: "node", instances }, + }).pipe(Effect.provide(BunServices.layer), Effect.exit), + ); + + expect(Exit.isFailure(exit)).toBe(true); + // Refused before anything reaches disk, the way every other unsafe write is. + expect(existsSync(configPath)).toBe(false); + }); + + test("writes a whole, non-negative count unquoted", async () => { + await run( + writeWorkerEntry({ + configPath, + name: "api", + existingWorkers: {}, + patch: { runtime: "node", instances: 0 }, + }).pipe(Effect.provide(BunServices.layer)), + ); + + expect(readFileSync(configPath, "utf8")).toContain("instances = 0"); + }); + test("creates the file when there is none yet", async () => { await run( writeWorkerEntry({ @@ -220,6 +290,30 @@ describe("planWorkerEntry + commitWorkerEntry", () => { }); }); +describe("readWorkersSection blank values", () => { + // Absent means the `public` default, so a blank `exposure` must not read as + // absent — that would silently widen a worker whose config tried to say + // something. `push` refuses the value instead. + test("keeps an explicitly blank exposure so push can refuse it", () => { + const section = readWorkersSection({ api: { runtime: "node", exposure: "" } }); + + expect(section.workers["api"]?.exposure).toBe(""); + }); + + // The mirror: nothing else here widens anything on absence — a missing + // runtime is guessed, a missing size defaults, a missing source is the + // conventional directory — so blank keeps collapsing to absent for those. + test("still folds the other blank dials into absent", () => { + const section = readWorkersSection({ api: { runtime: "", size: "", source: "" } }); + + expect(section.workers["api"]).toMatchObject({ + runtime: undefined, + size: undefined, + source: undefined, + }); + }); +}); + describe("readWorkersSection prototype safety", () => { // `constructor` is a valid DNS label, so it is a valid worker name. Read into // a plain `{}`, looking it up would return `Object.prototype.constructor` and diff --git a/apps/cli/src/shared/workers/worker-runtimes.ts b/apps/cli/src/shared/workers/worker-runtimes.ts index f72fdf0d99..f5f5b96a84 100644 --- a/apps/cli/src/shared/workers/worker-runtimes.ts +++ b/apps/cli/src/shared/workers/worker-runtimes.ts @@ -1,12 +1,13 @@ /** - * The alpha envelope a worker is described by: which runtime it is built on, - * and how big an instance it runs as. + * The alpha envelope a worker is described by: which runtime it is built on, how + * big an instance it runs as, and whether it is reachable from the internet. * - * Both are deliberately small closed sets. The Workers API takes `spec.size` as - * one opaque string (`2gb-1vcpu`) rather than independent cpu/memory dials, so - * the CLI offers exactly the sizes that string has values for and derives the - * vCPU count from the memory the user picked — one choice, not two that could - * be combined into a shape the platform does not run. + * All three are deliberately small closed sets, and the CLI's own rather than + * the API's: the Workers API takes `spec.size` as one opaque string + * (`2gb-1vcpu`) rather than independent cpu/memory dials, and `spec.exposure` as + * an unconstrained string. So the CLI offers exactly the sizes that string has + * values for and derives the vCPU count from the memory the user picked — one + * choice, not two that could be combined into a shape the platform does not run. */ /** A worker's runtime: its own Dockerfile, or one of the catalog base images. */ @@ -82,6 +83,42 @@ export function parseWorkerSize(value: string): WorkerSize | undefined { return isWorkerSize(canonical) ? canonical : undefined; } +/** + * How a worker is reached: `public` gives it an internet-facing URL, `private` + * keeps it reachable only from inside the project. + * + * `spec.exposure` is an unconstrained string in the Management API's schema, so + * this closed set is the CLI's own — the same arrangement as {@link WORKER_SIZES}, + * and the reason output renders the *accepted* exposure verbatim rather than + * forcing it back into this enum. + */ +export const WORKER_EXPOSURES = ["public", "private"] as const; + +export type WorkerExposure = (typeof WORKER_EXPOSURES)[number]; + +/** + * The exposure a worker gets when neither `--exposure` nor `[workers.] + * exposure` says otherwise. Public, because every runtime offered today serves + * HTTP and a worker nobody has locked down is one you can call. + */ +export const DEFAULT_WORKER_EXPOSURE: WorkerExposure = "public"; + +/** One-line description of each exposure, for `--exposure`'s prompt and help. */ +export const WORKER_EXPOSURE_DESCRIPTIONS: Record = { + public: "Reachable from the internet at the worker's own URL.", + private: "Reachable only from inside the project; no URL is issued.", +}; + +function isWorkerExposure(value: string): value is WorkerExposure { + return WORKER_EXPOSURES.some((exposure) => exposure === value); +} + +/** As {@link parseWorkerRuntime}, for exposures. */ +export function parseWorkerExposure(value: string): WorkerExposure | undefined { + const canonical = value.trim().toLowerCase(); + return isWorkerExposure(canonical) ? canonical : undefined; +} + const VCPU_FOR_SIZE: Record = { "2gb": 1, "4gb": 2 }; /** The vCPU count that comes with `size` — not independently choosable. */ diff --git a/apps/cli/src/shared/workers/worker-runtimes.unit.test.ts b/apps/cli/src/shared/workers/worker-runtimes.unit.test.ts index 1eb1f9bccd..7569ec4992 100644 --- a/apps/cli/src/shared/workers/worker-runtimes.unit.test.ts +++ b/apps/cli/src/shared/workers/worker-runtimes.unit.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "vitest"; import { apiSizeFor, formatApiSize, + parseWorkerExposure, parseWorkerRuntime, parseWorkerSize, validateWorkerNameMessage, @@ -46,6 +47,21 @@ describe("sizes", () => { }); }); +describe("parseWorkerExposure", () => { + test("accepts both exposures case-insensitively, and canonicalizes them", () => { + expect(parseWorkerExposure("Public")).toBe("public"); + expect(parseWorkerExposure(" PRIVATE ")).toBe("private"); + }); + + // A typo here would otherwise read as the default and put a worker somebody + // meant to keep private on the internet, so nothing near-miss is accepted. + test("rejects anything outside the pair, including near misses", () => { + expect(parseWorkerExposure("privat")).toBeUndefined(); + expect(parseWorkerExposure("internal")).toBeUndefined(); + expect(parseWorkerExposure("")).toBeUndefined(); + }); +}); + describe("validateWorkerNameMessage", () => { test("accepts DNS labels", () => { expect(validateWorkerNameMessage("api")).toBeUndefined(); diff --git a/apps/cli/src/shared/workers/workers.errors.ts b/apps/cli/src/shared/workers/workers.errors.ts index 8d9ba34515..e5edc0897f 100644 --- a/apps/cli/src/shared/workers/workers.errors.ts +++ b/apps/cli/src/shared/workers/workers.errors.ts @@ -96,6 +96,15 @@ export class UnknownWorkerSizeError extends Data.TaggedError("UnknownWorkerSizeE } } +export class UnknownWorkerExposureError extends Data.TaggedError("UnknownWorkerExposureError")<{ + readonly detail: string; + readonly suggestion: string; +}> { + get [ErrorActionabilityId](): CliErrorActionabilityDeclaration { + return actionability.provideFlags; + } +} + export class WorkerDirectoryExistsError extends Data.TaggedError("WorkerDirectoryExistsError")<{ readonly detail: string; readonly suggestion: string; diff --git a/apps/docs/public/cli/config.schema.json b/apps/docs/public/cli/config.schema.json index c8977ea00a..41bfb54b32 100644 --- a/apps/docs/public/cli/config.schema.json +++ b/apps/docs/public/cli/config.schema.json @@ -2314,6 +2314,11 @@ "description": "Instance size, denominated by memory. Each size implies its own vCPU count,\nso it is the one dial rather than two.", "examples": ["2gb"] }, + "exposure": { + "type": "string", + "description": "How the worker is reached: `public` gives it an internet-facing URL,\n`private` keeps it reachable only from inside the project. Every deploy\nsends a complete spec, so the value recorded here is what keeps a private\nworker private; `--exposure` overrides it for one deploy. Defaults to\n`public`.", + "examples": ["private"] + }, "instances": { "type": "integer", "minimum": 0, @@ -4753,6 +4758,11 @@ "description": "Instance size, denominated by memory. Each size implies its own vCPU count,\nso it is the one dial rather than two.", "examples": ["2gb"] }, + "exposure": { + "type": "string", + "description": "How the worker is reached: `public` gives it an internet-facing URL,\n`private` keeps it reachable only from inside the project. Every deploy\nsends a complete spec, so the value recorded here is what keeps a private\nworker private; `--exposure` overrides it for one deploy. Defaults to\n`public`.", + "examples": ["private"] + }, "instances": { "type": "integer", "minimum": 0, diff --git a/apps/docs/public/cli/project-config.schema.json b/apps/docs/public/cli/project-config.schema.json index 7d639503d8..73b6747a72 100644 --- a/apps/docs/public/cli/project-config.schema.json +++ b/apps/docs/public/cli/project-config.schema.json @@ -1862,6 +1862,11 @@ "description": "Instance size, denominated by memory. Each size implies its own vCPU count,\nso it is the one dial rather than two.", "examples": ["2gb"] }, + "exposure": { + "type": "string", + "description": "How the worker is reached: `public` gives it an internet-facing URL,\n`private` keeps it reachable only from inside the project. Every deploy\nsends a complete spec, so the value recorded here is what keeps a private\nworker private; `--exposure` overrides it for one deploy. Defaults to\n`public`.", + "examples": ["private"] + }, "instances": { "type": "integer", "minimum": 0, diff --git a/packages/config/src/workers.ts b/packages/config/src/workers.ts index 83cbcb360d..c3bc2a4178 100644 --- a/packages/config/src/workers.ts +++ b/packages/config/src/workers.ts @@ -41,6 +41,24 @@ const worker = Schema.Struct({ links, }), ), + exposure: Schema.optionalKey( + // A plain string, like `runtime` and `size`: the Management API takes + // `spec.exposure` as an unconstrained string, and the CLI names the values it + // accepts when it reads one it does not know. Constraining it here would + // report a config that a newer CLI understands as unloadable. + Schema.String.annotate({ + description: dedent` + How the worker is reached: \`public\` gives it an internet-facing URL, + \`private\` keeps it reachable only from inside the project. Every deploy + sends a complete spec, so the value recorded here is what keeps a private + worker private; \`--exposure\` overrides it for one deploy. Defaults to + \`public\`. + `, + examples: ["private"], + tags, + links, + }), + ), instances: Schema.optionalKey( // Bounded to match `spec.instances` in the Management API's input schema. A // value that gets past here is dropped rather than sent, so leaving it diff --git a/packages/config/src/workers.unit.test.ts b/packages/config/src/workers.unit.test.ts index dd9d9dd403..251f106de5 100644 --- a/packages/config/src/workers.unit.test.ts +++ b/packages/config/src/workers.unit.test.ts @@ -8,9 +8,28 @@ const workerNamePattern = "^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"; describe("workers schema", () => { test("decodes a worker table with every dial set", () => { - expect( - decode({ api: { runtime: "node", size: "4gb", instances: 3, source: "packages/api" } }), - ).toEqual({ api: { runtime: "node", size: "4gb", instances: 3, source: "packages/api" } }); + const every = { + api: { + runtime: "node", + size: "4gb", + exposure: "private", + instances: 3, + source: "packages/api", + }, + }; + expect(decode(every)).toEqual(every); + }); + + // Unconstrained, like `runtime` and `size`: the Management API takes + // `spec.exposure` as a plain string, and `push` is what names the values it + // accepts. Pinning an enum here would make a config a newer CLI understands + // fail to load at all. + test("accepts an exposure it does not itself recognize", () => { + expect(decode({ api: { exposure: "internal" } })).toEqual({ api: { exposure: "internal" } }); + }); + + test("rejects a non-string exposure", () => { + expect(() => decode({ api: { exposure: true } })).toThrow(); }); test("defaults to an empty section when the key is absent", () => { @@ -66,6 +85,7 @@ describe("workers schema", () => { expect(workerSchema?.properties?.runtime).toBeDefined(); expect(workerSchema?.properties?.size).toBeDefined(); + expect(workerSchema?.properties?.exposure).toBeDefined(); expect(workerSchema?.properties?.instances).toBeDefined(); expect(workerSchema?.properties?.source).toBeDefined(); });