diff --git a/README.md b/README.md index 4154af4..4fb46ae 100644 --- a/README.md +++ b/README.md @@ -77,9 +77,13 @@ cast team [--env ] - **`apply`** — idempotent create-or-update of every manifest resource, then redeploy what changed. One-way: it never deletes a resource that Coolify has - and the manifest doesn't. Clones the repo's default branch unless `--path` - points at a local checkout (refused with `--env prod` — prod always reads the - default branch). + and the manifest doesn't. It creates the **project** and its **environment** + when they are absent — the two things a resource create has to name — and then + removes the empty `production` that Coolify hands every new project, which is + the single delete cast performs and never touches a project built by hand + ([docs/semantics.md](docs/semantics.md)). Clones the repo's default branch + unless `--path` points at a local checkout (refused with `--env prod` — prod + always reads the default branch). - **`diff`** — reports drift, manifest → Coolify. Structural by default; `--full` also compares env vars. Exits non-zero when dirty, so CI can gate on it. - **`--all`** — on `apply`/`diff`, act on **every project the registry lists for @@ -677,12 +681,49 @@ split placement: these resources sit on 2 different destinations verify it. That is deliberate. A setting that reads back as *absent* rather than *wrong* is the failure this whole file keeps trying not to be. +When you declare **nothing**, every `diff` says that too: + +``` +placement: server's default destination (none declared) — cast sends no destination_uuid, + so Coolify picks; a server with more than one destination refuses the create outright. +``` + +Declaring nothing is not the absence of a placement decision. It is one, and it +used to be the only one cast made silently — the inference sat in a source comment +("the server's only destination, which is what Coolify picks anyway"), which is +exactly where an assumption is invisible until it is wrong. + One sharp edge worth knowing: on a server with exactly **one** destination, Coolify ignores the `destination_uuid` you send and never validates it — a typo there is invisible until a second destination exists. On a server with more than one, a create that omits it is a hard `400`, which is why cast could not deploy -onto a shared box at all until it could send this. Details, with citations: -[reference/README.md](reference/README.md). +onto a shared box at all until it could send this. The asymmetry hides itself: +the day a server gains its second destination, every project on it that declared +no destination stops being able to create. + +cast cannot warn you before that create — Coolify 4.1.2 serves no destinations +API, so a server's destination *count* is not knowable until a create has already +been attempted, and the 400 therefore lands **after** apply has made the project +and the environment. What cast does instead is answer it: + +``` +cannot create application core: prod-box has multiple destinations, so a create must say which one to use. + + Coolify said: POST /applications/private-github-app → 400: {"message":"Server has multiple destinations and you do not set destination_uuid."} + +Read the destination UUID from the Coolify UI (4.1.2 exposes no API for it) and +declare it as: + + environments.prod.projects.heavy-duty/incubator.destination_uuid + +Placement is create-time — a resource cannot be moved between networks later, so a +wrong or missing destination is repaired by delete + recreate, never by a later apply. + +Re-run this apply once the UUID is declared: anything it already created (the project, +its environment) is adopted, not made twice — apply reads before it writes. +``` + +Details, with citations: [reference/README.md](reference/README.md). ## Guarding an environment diff --git a/docs/semantics.md b/docs/semantics.md index 474528c..58cb803 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -152,9 +152,21 @@ softened by an implementation detail): and leaves the project behind, created and empty (#38). Read-before-write, so an environment that already exists is never written to: adoption keeps working exactly as it did, and this cannot regress an apply that works today. -- That default environment is **left alone**, per *apply never deletes*. An - empty `production` beside the environment everything lives in is reported - (the same courtesy an orphan gets) and removed by hand, or not at all. +- That default environment is then **removed** — the one delete cast performs, + and the only exception to *apply never deletes* (#40). What that rule protects + is things cast did not make; this is a byproduct of cast's own `POST /projects` + seconds earlier, holding nothing and having never held anything. Leaving it + meant every project cast created from nothing carried a permanently-empty + `production` beside the environment everything actually lives in — precisely + the shape that makes a box unreadable later (on the box being migrated away + from, `production` is empty and everything runs in `staging`, and *"the obvious + guess is the wrong one"* is a note we had to write for ourselves). All three + conditions hold jointly or nothing is touched: **cast created the project in + this run** (never a project someone built by hand, whatever it carries), the + environment is **empty** (asked of Coolify — the details route is the only one + that eager-loads resources — not assumed from the first condition), and its + name is **not ours** (an `--environment production` keeps its `production`). + Best-effort: a delete that fails is reported and never fails the apply. - On drift in a field the API cannot update in place (`build_pack`, a database's `type`/`version`, a service's `type`), apply **fails loudly naming the field** rather than recreating. @@ -325,6 +337,30 @@ anything else. Two consequences, both deliberate: - Whenever a destination is declared, `diff` says explicitly that it was *not* compared. Silence would make an unverified setting read as a verified one — the failure shape this document exists to avoid. +- Whenever one is **not** declared, `diff` says *that*, too: + `placement: server's default destination (none declared)`. Declaring nothing is + not the absence of a placement decision — it is one (cast sends no + `destination_uuid`, Coolify picks), and it was the only placement decision made + in silence until #41. It is reported even on a clean run that creates nothing, + because the trap is set precisely for projects that are already built: the day + their server gains a second destination, every project on it that declared no + destination stops being able to create at all. + +**The multi-destination 400 is translated, not passed through** (#41). A create +against a server with more than one destination that names none is rejected with +*"Server has multiple destinations and you do not set destination_uuid."* — a +message that names neither the remedy nor the file the remedy goes in, and that +arrives at the **first create**, after `apply` has already made the project and +the environment. cast **cannot** pre-flight the condition: 4.1.2 serves no +destinations API at all, and `GET /servers/{uuid}` does not carry them either, so +a server's destination count is unknowable until a create has been attempted. +What cast can do is answer the question the 400 raises, and it does: the failing +resource, the server by the name the operator wrote (not its UUID), the exact +state-file path the UUID goes in +(`environments..projects./.destination_uuid`), the create-time +warning, and Coolify's own words kept verbatim. A run interrupted this way is +safe to re-run once the UUID is declared — the project and environment it already +made are adopted, not remade. **Coolify's create-time behavior** (`ApplicationsController` ~L1003, `DatabasesController` ~L1700, `ServicesController` ~L378 @ v4.1.2): a server with diff --git a/src/cli.ts b/src/cli.ts index 2935d3a..04505d7 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -29,6 +29,7 @@ import { } from "./config.js"; import { CoolifyClient, HttpError } from "./coolify.js"; import { + type Change, type Live, type ResourceKind, computeDiff, @@ -811,6 +812,11 @@ async function runProject( envName: coolifyEnv, serverUuid, githubAppUuid, + // Not the wire names above (see buildExecutor): the names the operator wrote, + // and the state-file path a missing destination has to be declared at. + serverName: ctx.binding.server, + orgRepo, + bindingEnv: ctx.envName, destinationUuid: projectBinding?.destination_uuid, s3DestinationUuid: ctx.binding.s3_destination, backupSchedules, @@ -1704,11 +1710,35 @@ async function resolveOrCreateProject( // duplicate name), reached when something else wins the race between our read // and our write. // -// Coolify's own default environment is left exactly where it is: cast does not -// remove things (see renderDiff — an orphan is reported and NOT repaired, -// "removal is a manual runbook act"), and an empty `production` beside the -// environment everything lives in is the mildest possible case of that. It is -// reported for the same reason an orphan is: so the operator knows, and decides. +// Coolify's default environment is then REMOVED — the one delete cast performs, +// and the exception that has to argue for itself against *apply never deletes* +// (#40). +// +// What that rule protects is things cast did not make: a resource, an env var, a +// project someone built by hand. This is none of those. It is a byproduct of +// cast's own `POST /projects` seconds earlier, in this run, holding nothing and +// having never held anything — cast declining to leave litter behind itself. The +// alternative is what #39 shipped and #40 was filed against: every project cast +// creates from nothing carries a permanently-empty `production` beside the +// environment everything actually lives in, which is *precisely* the shape that +// makes a box unreadable later. We have the live example — on the box being +// migrated away from, `production` is empty and everything runs in `staging`, +// and "the obvious guess is the wrong one" is a note we had to write down for +// ourselves. Shipping more of those is not neutrality; it is a bug with a +// changelog entry. +// +// All three conditions are load-bearing, and removeDefaultEnvironment enforces +// them jointly: +// +// cast created the project, in THIS run — never touch a project someone built +// by hand, whatever it happens to carry. +// the environment is EMPTY — asked of Coolify, not assumed from the above. +// its name is NOT ours — a project whose --environment legitimately IS +// `production` keeps it (it is the one everything is about to live in). +// +// And it is best-effort: a delete that fails leaves the environment reported, +// exactly as #39 left it, and never fails an apply that has otherwise worked. +// Tidying is not worth a half-applied run. async function ensureEnvironment( client: CoolifyClient, projectUuid: string, @@ -1717,7 +1747,7 @@ async function ensureEnvironment( projectWasCreated: boolean, ): Promise { // The read that decides. On a project cast just created, it is also the list - // of environments Coolify gave it by itself — which is what `strays` reports. + // of environments Coolify gave it by itself — which is what `strays` holds. const existing = await client.environments(projectUuid); if (!existing.includes(envName)) { try { @@ -1728,10 +1758,39 @@ async function ensureEnvironment( if (!(err instanceof HttpError) || err.status !== 409) throw err; } } - const strays = projectWasCreated ? existing.filter((e) => e !== envName) : []; - if (strays.length > 0) { + if (!projectWasCreated) return; + for (const stray of existing.filter((e) => e !== envName)) { + await removeDefaultEnvironment(client, projectUuid, projectName, stray); + } +} + +// The delete itself, and the two ways it declines to happen. Nothing here throws: +// every path ends in a line of output, because the operator's project is either +// tidy or carrying an environment they now know about. +async function removeDefaultEnvironment( + client: CoolifyClient, + projectUuid: string, + projectName: string, + envName: string, +): Promise { + try { + // Asked, not inferred. It is empty by construction — Coolify made it a + // moment ago and only cast has written to this project since — but "it must + // be empty" is a belief, and this is a delete. The check costs one GET and + // is what makes the guarantee a fact rather than an argument. + if (!(await client.environmentIsEmpty(projectUuid, envName))) { + console.log( + `note: left Coolify's default environment ${envName} on new project ${projectName} — it is NOT empty (cast deletes nothing that holds anything)`, + ); + return; + } + await client.deleteEnvironment(projectUuid, envName); console.log( - `note: new project ${projectName} carries Coolify's default environment(s): ${strays.join(", ")} — empty, unused, and cast never removes (delete by hand if unwanted)`, + `removed Coolify's default environment ${envName} from new project ${projectName} (empty — created by Coolify's POST /projects, never by the manifest)`, + ); + } catch (err) { + console.log( + `note: new project ${projectName} carries Coolify's default environment ${envName} — empty and unused, and cast could not remove it (${err instanceof Error ? err.message : String(err)}). Delete it by hand, or leave it.`, ); } } @@ -1841,6 +1900,55 @@ export function serviceApiFields( return rest; } +// Coolify's answer when a server has more than one destination and the create did +// not say which one to use (all three controllers, identically, v4.1.2): +// +// POST /applications/private-github-app → 400: +// {"message":"Server has multiple destinations and you do not set destination_uuid."} +// +// It names neither the remedy nor the file the remedy goes in, and it arrives at +// the FIRST create — after apply has already made the project and the environment. +// So the operator is holding a half-applied run and a message about a field they +// may never have heard of. (#41) +// +// cast cannot pre-flight this and that part is not fixable here: 4.1.2 serves no +// destinations API at all — not list, not read, not create — and GET /servers/{uuid} +// does not carry them either, so a server's destination COUNT is not knowable until +// a create has already been attempted. What IS fixable is the diagnosis, and this is +// the whole of it: catch the one message, and answer the question it raises. +function isMultiDestination400(err: unknown): err is HttpError { + return ( + err instanceof HttpError && + err.status === 400 && + err.message.includes("Server has multiple destinations") + ); +} + +export function multiDestinationRemedy(where: { + server: string; + env: string; + project: string; + resource: string; + coolify: string; +}): string { + return [ + `cannot create ${where.resource}: ${where.server} has multiple destinations, so a create must say which one to use.`, + "", + ` Coolify said: ${where.coolify}`, + "", + "Read the destination UUID from the Coolify UI (4.1.2 exposes no API for it) and", + "declare it as:", + "", + ` environments.${where.env}.projects.${where.project}.destination_uuid`, + "", + "Placement is create-time — a resource cannot be moved between networks later, so a", + "wrong or missing destination is repaired by delete + recreate, never by a later apply.", + "", + "Re-run this apply once the UUID is declared: anything it already created (the project,", + "its environment) is adopted, not made twice — apply reads before it writes.", + ].join("\n"); +} + export function buildExecutor( client: CoolifyClient, ctx: { @@ -1848,6 +1956,16 @@ export function buildExecutor( envName: string; serverUuid: string; githubAppUuid: string; + // The three names the multi-destination 400 has to be able to say back, and + // the only reason they are here: none of them is on the wire. A create sends + // `serverUuid`, but the operator wrote a server NAME — and the UUID they now + // have to go and read lands at `environments..projects./`, a + // path keyed by cast's OWN env name and the repo, never by the Coolify + // project/environment names above (which `--project`/`--environment` are free + // to make something else entirely). + serverName: string; + orgRepo: string; + bindingEnv: string; // The Docker network to create resources on. A raw UUID from // environments.yaml for the same reason s3DestinationUuid is one: Coolify // 4.1.2 has no destinations API, so there is no name for cast to resolve. @@ -1888,70 +2006,96 @@ export function buildExecutor( ctx.projectName, ctx.envName, ); + // Wrapped around all three creates rather than around each one: Coolify runs + // the same destination logic in ApplicationsController, DatabasesController and + // ServicesController, so whichever kind happens to be created first is the one + // that 400s, and which one that is depends only on the order of the manifest. + const withDestinationDiagnosis = async ( + change: Change, + create: () => Promise, + ): Promise => { + try { + return await create(); + } catch (err) { + if (!isMultiDestination400(err)) throw err; + throw new Error( + multiDestinationRemedy({ + server: ctx.serverName, + env: ctx.bindingEnv, + project: ctx.orgRepo, + resource: `${change.kind} ${change.name}`, + coolify: err.message, + }), + { cause: err }, + ); + } + }; return { async createResource(change) { - // Field payloads assembled from change.fieldDiffs (desired values): - const fields = Object.fromEntries( - change.fieldDiffs.map((f) => [f.field, f.desired]), - ); - const projectUuid = await projectEnv(); - if (change.kind === "application") { - const res = (await client.post("/applications/private-github-app", { - project_uuid: projectUuid, - environment_name: ctx.envName, - server_uuid: ctx.serverUuid, - ...destination, - github_app_uuid: ctx.githubAppUuid, - name: change.name, - instant_deploy: false, - ...applicationApiFields(fields), - // A compose stack must reach the managed Postgres/Redis resources - // (the box-B lesson, DEPLOY.md §0/§3) — Coolify only wires that up - // when this flag is set on create. - ...(fields.build_pack === "dockercompose" - ? { connect_to_docker_network: true } - : {}), - })) as { uuid: string }; - return res.uuid; - } - if (change.kind === "database") { - const type = String(fields.type); - const res = (await client.post( - `/databases/${type === "postgresql" ? "postgresql" : "redis"}`, - { + return withDestinationDiagnosis(change, async () => { + // Field payloads assembled from change.fieldDiffs (desired values): + const fields = Object.fromEntries( + change.fieldDiffs.map((f) => [f.field, f.desired]), + ); + const projectUuid = await projectEnv(); + if (change.kind === "application") { + const res = (await client.post("/applications/private-github-app", { project_uuid: projectUuid, environment_name: ctx.envName, server_uuid: ctx.serverUuid, ...destination, + github_app_uuid: ctx.githubAppUuid, name: change.name, - ...databaseApiFields(fields), - }, - )) as { uuid: string }; - const schedule = ctx.backupSchedules[change.name]; - if (schedule) { - if (!ctx.s3DestinationUuid) { - throw new Error( - `database ${change.name} declares a backup schedule but environments.yaml has no s3_destination UUID for this environment`, - ); - } - await client.post(`/databases/${res.uuid}/backups`, { - frequency: schedule.frequency, - database_backup_retention_amount_locally: schedule.retention, - save_s3: true, - s3_storage_uuid: ctx.s3DestinationUuid, - }); + instant_deploy: false, + ...applicationApiFields(fields), + // A compose stack must reach the managed Postgres/Redis resources + // (the box-B lesson, DEPLOY.md §0/§3) — Coolify only wires that up + // when this flag is set on create. + ...(fields.build_pack === "dockercompose" + ? { connect_to_docker_network: true } + : {}), + })) as { uuid: string }; + return res.uuid; } + if (change.kind === "database") { + const type = String(fields.type); + const res = (await client.post( + `/databases/${type === "postgresql" ? "postgresql" : "redis"}`, + { + project_uuid: projectUuid, + environment_name: ctx.envName, + server_uuid: ctx.serverUuid, + ...destination, + name: change.name, + ...databaseApiFields(fields), + }, + )) as { uuid: string }; + const schedule = ctx.backupSchedules[change.name]; + if (schedule) { + if (!ctx.s3DestinationUuid) { + throw new Error( + `database ${change.name} declares a backup schedule but environments.yaml has no s3_destination UUID for this environment`, + ); + } + await client.post(`/databases/${res.uuid}/backups`, { + frequency: schedule.frequency, + database_backup_retention_amount_locally: schedule.retention, + save_s3: true, + s3_storage_uuid: ctx.s3DestinationUuid, + }); + } + return res.uuid; + } + const res = (await client.post("/services", { + project_uuid: projectUuid, + environment_name: ctx.envName, + server_uuid: ctx.serverUuid, + ...destination, + name: change.name, + ...serviceApiFields(fields), + })) as { uuid: string }; return res.uuid; - } - const res = (await client.post("/services", { - project_uuid: projectUuid, - environment_name: ctx.envName, - server_uuid: ctx.serverUuid, - ...destination, - name: change.name, - ...serviceApiFields(fields), - })) as { uuid: string }; - return res.uuid; + }); }, async updateFields(uuid, kind, fields) { const base = diff --git a/src/coolify.ts b/src/coolify.ts index 364d94b..cae1bf0 100644 --- a/src/coolify.ts +++ b/src/coolify.ts @@ -147,6 +147,37 @@ export class CoolifyClient { return names(project?.environments); } + // Does this environment hold anything at all? + // + // The LIST route above cannot answer it: an `Environment` carries id, name, + // project_id, description, timestamps — no relations, so an environment with + // five applications in it looks exactly like an empty one. This route is the + // one that eager-loads them (ProjectController@environment_details, v4.1.2: + // applications, postgresqls, redis, mongodbs, mysqls, mariadbs, services). + // + // The question is asked of the SHAPE rather than of those seven names: any + // non-empty array in the response is a resource list, because everything else + // there is a scalar. Naming the seven instead would mean a Coolify that grows + // an eighth database type could answer "empty" about an environment holding + // one — and this answer is the guard on a delete. + async environmentIsEmpty( + projectUuid: string, + envName: string, + ): Promise { + const env = (await this.get( + `/projects/${projectUuid}/${encodeURIComponent(envName)}`, + )) as Record | null; + // Not "empty" — unreadable. The caller must not delete on this answer. + if (!env) return false; + return !Object.values(env).some((v) => Array.isArray(v) && v.length > 0); + } + + async deleteEnvironment(projectUuid: string, envName: string): Promise { + await this.delete_( + `/projects/${projectUuid}/environments/${encodeURIComponent(envName)}`, + ); + } + async deploy(uuid: string): Promise { await this.post(`/deploy?uuid=${encodeURIComponent(uuid)}`); } diff --git a/src/diff.ts b/src/diff.ts index 04fee78..2555bb4 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -241,6 +241,29 @@ export function renderDiff(report: DiffReport): string { " destination_uuid on write and returns destination_id on read, and has no endpoint", " mapping one to the other. cast sends it on create; nothing can verify it after.", ); + } else { + // The other half of the same principle, and #41: declaring NOTHING is also a + // decision about placement — cast sends no destination_uuid and lets Coolify + // pick — and it was the one placement decision made in silence. The inference + // lived in a source comment ("the server's only destination, which is what + // Coolify picks anyway"), which is exactly where an assumption is invisible + // until it is wrong. + // + // This reverses a judgment cast used to hold explicitly ("a line on every diff + // that says nothing is how a report stops being read" — the test this replaces). + // The line does not say nothing: it says which network the next create lands on, + // which is a fact about this run and a wrong one to have to infer from a blank + // space. It stays on a run that creates nothing, too, because the trap is set + // precisely for projects that are already built and clean — the day their server + // gains a second destination, every one of them that declared no destination + // stops being able to create at all, and nothing will have warned them. + // + // Two lines, not three: the old judgment was not wrong about noise, only about + // which side of it silence was on. + lines.push( + "placement: server's default destination (none declared) — cast sends no destination_uuid,", + " so Coolify picks; a server with more than one destination refuses the create outright.", + ); } lines.push( report.clean diff --git a/test/diff.test.ts b/test/diff.test.ts index 5534626..d0bf930 100644 --- a/test/diff.test.ts +++ b/test/diff.test.ts @@ -186,15 +186,44 @@ describe("renderDiff placement", () => { // The whole reason placement is in the report at all: a destination that read // back as absent rather than wrong is the failure shape #12/#14/#17/#18 are - // about. Silence is the bug — but so is noise on the happy path. - it("stays silent about placement when nothing is declared and nothing is split", () => { + // about. + // + // This test used to assert the opposite — that an undeclared, unsplit box says + // NOTHING about placement, on the grounds that a line on every diff is how a + // report stops being read. #41 reversed it. Declaring nothing is not the absence + // of a placement decision, it is a placement decision: cast sends no + // destination_uuid and Coolify picks. Leaving that inference in a source comment + // is what made it invisible until the day it was wrong — a first apply against a + // multi-destination server, 400ing after the run had already created the project + // and the environment. It is a fact about what the next create will do, so it is + // on screen while it is still true. + it("says out loud that an undeclared placement is the server's default", () => { const out = renderDiff( computeDiff([want("a")], [got("a", 3)], "structural"), ); - expect(out).not.toMatch(/placement/); + expect(out).toMatch( + /placement: server's default destination \(none declared\)/, + ); + // ...and the consequence, which is the only part that can hurt: on a server + // with more than one destination this is not a default, it is a 400. + expect(out).toMatch(/refuses the create/); + // Still clean: an undeclared destination is an assumption, not drift. expect(out).toContain("clean"); }); + // The claim above is about the UNDECLARED case only. A declared destination + // makes the opposite statement (it was sent, and cannot be verified) and must + // never make both. + it("does not call a declared destination the server's default", () => { + const out = renderDiff( + computeDiff([want("a")], [got("a", 3)], "structural", { + declaredDestination: "dest-abc", + }), + ); + expect(out).toMatch(/NOT compared/); + expect(out).not.toMatch(/none declared/); + }); + it("names every resource on each side of a split", () => { const out = renderDiff( computeDiff( diff --git a/test/placement-cli.test.ts b/test/placement-cli.test.ts index f20d6b6..15f44af 100644 --- a/test/placement-cli.test.ts +++ b/test/placement-cli.test.ts @@ -219,14 +219,21 @@ describe("cast diff — placement (#21)", () => { expect(r.output).not.toMatch(/NOT compared/); }); - // The state of every box today: one project, one server, one network, nothing - // declared. Placement must be silent — a line on every diff that says nothing - // is how a report stops being read. - it("says nothing at all about placement on an undeclared, unsplit box", async () => { + // The state of most boxes today: one project, one server, one network, nothing + // declared. This used to assert silence. #41 made it speak: an undeclared + // destination is cast choosing to let Coolify pick, and a run says which network + // its creates land on rather than leaving it to be inferred from a blank space. + // The clean exit is the part that must not move — an assumption is not drift. + it("names the assumption on an undeclared, unsplit box, and stays clean", async () => { const f = fixture((await stubCoolify({ core: 5, landing: 5 })).url); const r = await run(base(f)); expect(r.code).toBe(0); - expect(r.output).not.toMatch(/placement/); + expect(r.output).toMatch( + /placement: server's default destination \(none declared\)/, + ); expect(r.output).toContain("clean"); + // The declared-destination warning is the other branch, and belongs to a run + // that declared one. Saying both would be saying nothing. + expect(r.output).not.toMatch(/NOT compared/); }); }); diff --git a/test/wire.test.ts b/test/wire.test.ts index e7283cf..92ad045 100644 --- a/test/wire.test.ts +++ b/test/wire.test.ts @@ -195,6 +195,9 @@ describe("buildExecutor createResource (application, dockercompose)", () => { envName: "prod", serverUuid: "srv-1", githubAppUuid: "gh-1", + serverName: "prod-box", + orgRepo: "acme/widget", + bindingEnv: "prod", backupSchedules: {}, }); const uuid = await exec.createResource({ @@ -249,6 +252,9 @@ describe("buildExecutor createResource (application, dockercompose)", () => { envName: "prod", serverUuid: "srv-1", githubAppUuid: "gh-1", + serverName: "prod-box", + orgRepo: "acme/widget", + bindingEnv: "prod", backupSchedules: {}, }); await exec.createResource({ @@ -351,6 +357,9 @@ describe("buildExecutor createResource (destination placement)", () => { envName: "prod", serverUuid: "srv-1", githubAppUuid: "gh-1", + serverName: "prod-box", + orgRepo: "acme/widget", + bindingEnv: "prod", destinationUuid: "dest-abc", backupSchedules: {}, }); @@ -378,6 +387,9 @@ describe("buildExecutor createResource (destination placement)", () => { envName: "prod", serverUuid: "srv-1", githubAppUuid: "gh-1", + serverName: "prod-box", + orgRepo: "acme/widget", + bindingEnv: "prod", backupSchedules: {}, }); await exec.createResource(change); @@ -401,9 +413,16 @@ describe("buildExecutor createResource (environment reconcile)", () => { projects?: Array<{ uuid: string; name: string }>; environments?: string[]; envCreateStatus?: number; + // What an environment HOLDS, by name — the guard on #40's delete. A project + // cast just created cannot really hold anything, which is exactly why the + // mock has to be able to say otherwise: the guard is worth only as much as + // the case it refuses. + holds?: Record; + envDeleteStatus?: number; }) { const projects = opts.projects ?? []; let environments = opts.environments ?? []; + const holds = opts.holds ?? {}; const calls: string[] = []; const fetchImpl = vi.fn(async (url: string | URL, init?: RequestInit) => { const path = new URL(String(url)).pathname; @@ -458,6 +477,46 @@ describe("buildExecutor createResource (environment reconcile)", () => { environments.push(name); return new Response(JSON.stringify({ uuid: "env-1" }), { status: 201 }); } + // DELETE /projects/{uuid}/environments/{name} — #40. Three segments, so the + // two-segment details route below cannot swallow it. + const envDelete = + /^\/api\/v1\/projects\/([^/]+)\/environments\/([^/]+)$/.exec(path); + if (envDelete && method === "DELETE") { + const status = opts.envDeleteStatus ?? 200; + if (status !== 200) + return new Response(JSON.stringify({ message: "boom" }), { status }); + environments = environments.filter((e) => e !== envDelete[2]); + return new Response( + JSON.stringify({ message: "Environment deleted." }), + { status: 200 }, + ); + } + // GET /projects/{uuid}/{environment} — the details route, the ONLY one that + // eager-loads an environment's resources, and so the only one that can + // answer "is it empty?". The list route's Environment objects carry no + // relations at all. Checked after the /environments routes above, which + // this pattern would otherwise match. + const envDetail = /^\/api\/v1\/projects\/([^/]+)\/([^/]+)$/.exec(path); + if (envDetail && method === "GET") { + const name = envDetail[2]; + if (!environments.includes(name)) + return new Response(JSON.stringify({ message: "Not found." }), { + status: 404, + }); + return new Response( + JSON.stringify({ + id: 1, + name, + project_id: 1, + description: "", + applications: holds[name] ?? [], + postgresqls: [], + redis: [], + services: [], + }), + { status: 200 }, + ); + } if (path === "/api/v1/applications/private-github-app") { // Coolify's actual rule, and the whole of #38: a create names an // environment, and an environment that is not there is a 404. Without @@ -488,14 +547,21 @@ describe("buildExecutor createResource (environment reconcile)", () => { envDiffs: [], }; - function exec(fetchImpl: typeof fetch) { + // envName is a parameter because #40's third guard is about it: an environment + // named `production` is Coolify's leftover default in every run EXCEPT the one + // that asked for `--environment production`, where it is the environment + // everything is about to live in. + function exec(fetchImpl: typeof fetch, envName = "prod") { return buildExecutor( new CoolifyClient("https://coolify.test", "tok", fetchImpl), { projectName: "widget", - envName: "prod", + envName, serverUuid: "srv-1", githubAppUuid: "gh-1", + serverName: "prod-box", + orgRepo: "acme/widget", + bindingEnv: "prod", backupSchedules: {}, }, ); @@ -575,6 +641,250 @@ describe("buildExecutor createResource (environment reconcile)", () => { coolify.calls.filter((c) => c === "POST /api/v1/projects").length, ).toBe(1); }); + + // #40: what #39 left behind. POST /projects hands the new project Coolify's own + // default environment, and #39 then created OURS beside it — so every project cast + // creates from nothing carried a permanently-empty `production` next to the + // environment everything actually lives in. That is the shape that makes a box + // unreadable later; the box being migrated away from has an empty `production` and + // runs everything in `staging`, and "the obvious guess is the wrong one" is a note + // we had to write for ourselves. + // + // This is also the ONE delete cast performs, so the guards are the test: it happens + // only to a project cast made in this run, only to an environment that is empty, and + // only when the name is not the one we asked for. + describe("buildExecutor createResource (default environment removal, #40)", () => { + const DELETE_DEFAULT = + "DELETE /api/v1/projects/proj-new/environments/production"; + + it("removes the empty default environment from a project it just created", async () => { + const coolify = fakeCoolify({ projects: [] }); + const notes = vi.spyOn(console, "log").mockImplementation(() => {}); + + const uuid = await exec(coolify.fetchImpl).createResource(app); + + expect(uuid).toBe("app-1"); + expect(coolify.calls).toContain(DELETE_DEFAULT); + // The point of the whole issue: what is left is ours, and only ours. + expect(coolify.environments()).toEqual(["prod"]); + expect(notes.mock.calls.flat().join("\n")).toMatch( + /removed Coolify's default environment production/, + ); + notes.mockRestore(); + }); + + // Guard 1. The rule cast does not get to break: a project someone built by hand + // is not cast's to tidy, whatever it happens to carry. `production` sitting empty + // next to `staging` on an ADOPTED project is exactly the live box we migrate from + // — and it stays untouched. + it("never removes an environment from a project it adopted", async () => { + const coolify = fakeCoolify({ + projects: [{ uuid: "proj-1", name: "widget" }], + environments: ["production", "prod"], + }); + + await exec(coolify.fetchImpl).createResource(app); + + expect(coolify.calls.some((c) => c.startsWith("DELETE"))).toBe(false); + expect(coolify.environments()).toContain("production"); + }); + + // Guard 2. Asked of Coolify, not inferred from "we just made this project". The + // mock is lying here — a fresh project cannot hold an application — and it must + // be able to, because a guard that is only ever handed the safe case is not a + // guard. cast declines, and says why. + it("leaves the default environment alone when it holds anything", async () => { + const coolify = fakeCoolify({ + projects: [], + holds: { production: [{ uuid: "app-x", name: "legacy" }] }, + }); + const notes = vi.spyOn(console, "log").mockImplementation(() => {}); + + await exec(coolify.fetchImpl).createResource(app); + + expect(coolify.calls.some((c) => c.startsWith("DELETE"))).toBe(false); + expect(coolify.environments()).toContain("production"); + expect(notes.mock.calls.flat().join("\n")).toMatch( + /left Coolify's default environment production .* NOT empty/, + ); + notes.mockRestore(); + }); + + // Guard 3. `production` is a leftover in every run except the one that asked for + // it, where it is the environment everything is about to live in. Deleting it + // there would delete the target of the very apply doing the deleting. + it("keeps the default environment when it is the one we asked for", async () => { + const coolify = fakeCoolify({ projects: [] }); + + await exec(coolify.fetchImpl, "production").createResource(app); + + expect(coolify.calls.some((c) => c.startsWith("DELETE"))).toBe(false); + expect(coolify.environments()).toEqual(["production"]); + // ...and it was never re-created either: it was already there. + expect(coolify.calls).not.toContain( + "POST /api/v1/projects/proj-new/environments", + ); + }); + + // Tidying is a courtesy, and a courtesy that can fail an apply is not one. The + // resource still gets created; the operator is told what was left behind. + it("does not fail the apply when the delete fails", async () => { + const coolify = fakeCoolify({ projects: [], envDeleteStatus: 500 }); + const notes = vi.spyOn(console, "log").mockImplementation(() => {}); + + const uuid = await exec(coolify.fetchImpl).createResource(app); + + expect(uuid).toBe("app-1"); + expect(coolify.environments()).toContain("production"); + expect(notes.mock.calls.flat().join("\n")).toMatch( + /could not remove it .*500/s, + ); + notes.mockRestore(); + }); + }); +}); + +// #41: a first apply against a server with more than one destination 400s on the +// first create — with the project and the environment already made. Coolify's own +// message names neither the remedy nor the file it goes in, and cast cannot +// pre-flight the condition (4.1.2 serves no destinations API at all, so a server's +// destination count is unknowable until a create has been attempted). The diagnosis +// is the whole of what is fixable, so the diagnosis is what is tested. +describe("buildExecutor createResource (multi-destination 400, #41)", () => { + const app = { + kind: "application" as const, + name: "core", + op: "create" as const, + fieldDiffs: [ + { field: "build_pack", desired: "nixpacks", updatable: false }, + ], + envDiffs: [], + }; + + // Coolify's real answer, verbatim, from all three create controllers. + function multiDestinationCoolify() { + return vi.fn(async (url: string | URL, init?: RequestInit) => { + const path = new URL(String(url)).pathname; + const method = init?.method ?? "GET"; + if (path === "/api/v1/projects" && method === "GET") + return new Response( + JSON.stringify([{ uuid: "proj-1", name: "widget" }]), + { status: 200 }, + ); + if (path === "/api/v1/projects/proj-1/environments" && method === "GET") + return new Response(JSON.stringify([{ name: "prod" }]), { + status: 200, + }); + return new Response( + JSON.stringify({ + message: + "Server has multiple destinations and you do not set destination_uuid.", + }), + { status: 400 }, + ); + }) as unknown as typeof fetch; + } + + const exec = (fetchImpl: typeof fetch) => + buildExecutor(new CoolifyClient("https://coolify.test", "tok", fetchImpl), { + projectName: "widget", + envName: "prod", + serverUuid: "srv-1", + githubAppUuid: "gh-1", + // The names the message has to be able to say back. None is on the wire: + // Coolify knows srv-1, the operator wrote prod-box. + serverName: "prod-box", + orgRepo: "heavy-duty/incubator", + bindingEnv: "prod", + backupSchedules: {}, + }); + + const kinds = [ + { label: "application", change: app }, + { + label: "database", + change: { + kind: "database" as const, + name: "postgres", + op: "create" as const, + fieldDiffs: [ + { field: "type", desired: "postgresql", updatable: false }, + ], + envDiffs: [], + }, + }, + { + label: "service", + change: { + kind: "service" as const, + name: "umami", + op: "create" as const, + fieldDiffs: [{ field: "type", desired: "umami", updatable: false }], + envDiffs: [], + }, + }, + ]; + + // Every kind, because which one 400s first depends only on the order of the + // manifest — Coolify runs the same destination logic in all three controllers. + it.each(kinds)( + "answers the $label 400 with the state-file path the UUID goes in", + async ({ change }) => { + const err = await exec(multiDestinationCoolify()) + .createResource(change) + .catch((e: Error) => e); + + expect(err).toBeInstanceOf(Error); + const message = (err as Error).message; + // The remedy: the exact key, in the exact place, with the repo and the env + // the operator actually named. + expect(message).toContain( + "environments.prod.projects.heavy-duty/incubator.destination_uuid", + ); + // The server, by the name the operator wrote — not srv-1. + expect(message).toContain("prod-box has multiple destinations"); + // Which resource it died on, so a half-applied run can be read. + expect(message).toContain(`cannot create ${change.kind} ${change.name}`); + // Create-time: the part that decides whether they can fix this with an + // apply (they cannot) or a delete + recreate (they must). + expect(message).toMatch(/cannot be moved between networks later/); + // Coolify's own words survive — a translation that hides the original + // makes the next person's search fail. + expect(message).toContain( + "Server has multiple destinations and you do not set destination_uuid.", + ); + }, + ); + + // The translation must be about THIS 400, not about 400s. A create rejected for + // any other reason has to arrive unmolested, or the next bug gets a confident + // answer about a destination it has nothing to do with. + it("leaves every other failure exactly as Coolify sent it", async () => { + const fetchImpl = vi.fn(async (url: string | URL, init?: RequestInit) => { + const path = new URL(String(url)).pathname; + const method = init?.method ?? "GET"; + if (path === "/api/v1/projects" && method === "GET") + return new Response( + JSON.stringify([{ uuid: "proj-1", name: "widget" }]), + { status: 200 }, + ); + if (path === "/api/v1/projects/proj-1/environments" && method === "GET") + return new Response(JSON.stringify([{ name: "prod" }]), { + status: 200, + }); + return new Response( + JSON.stringify({ message: "The name field is required." }), + { status: 422 }, + ); + }) as unknown as typeof fetch; + + const err = await exec(fetchImpl) + .createResource(app) + .catch((e: Error) => e); + + expect((err as Error).message).toContain("The name field is required."); + expect((err as Error).message).not.toMatch(/destination/); + }); }); describe("databaseVersionFromImage / defaultDatabaseImage", () => {