diff --git a/docs/semantics.md b/docs/semantics.md index d43d93b..474528c 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -144,6 +144,17 @@ softened by an implementation detail): - Apply never deletes. Resource removal or rename is a manual runbook act; `diff` reports the orphan as such until that act happens. - Apply never recreates a database resource under any circumstances. +- Apply creates the **project** and its **environment** when they are absent — + the two things a resource create has to name before it can name anything + else. Coolify hands a project it has just created its OWN default environment + (`production`), never ours, so without this the first apply against a + from-nothing project 404s on its first resource — *"Environment not found"* — + 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. - 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. diff --git a/src/cli.ts b/src/cli.ts index 7f6d012..2935d3a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1664,9 +1664,9 @@ async function main(): Promise { async function resolveOrCreateProject( client: CoolifyClient, name: string, -): Promise { +): Promise<{ uuid: string; created: boolean }> { try { - return await client.projectUuid(name); + return { uuid: await client.projectUuid(name), created: false }; } catch (err) { // projectUuid's resolver-miss (CoolifyClient.resolve) throws this exact // message with no `status` — that's the only case we treat as "create @@ -1677,12 +1677,86 @@ async function resolveOrCreateProject( err.message === `not found in Coolify: project ${name}` ) { const p = (await client.post("/projects", { name })) as { uuid: string }; - return p.uuid; + return { uuid: p.uuid, created: true }; } throw err; } } +// The environment every resource create names in `environment_name` has to +// EXIST before the create, and cast is the only thing that can be relied on to +// make it so: POST /projects gives a new project Coolify's OWN default +// environment ("production"), not ours, so the first apply against a project +// cast itself created 404s on the first resource — "Environment not found" — +// with the project left behind, created and empty (#38). +// +// It went unseen for as long as it did because every environment cast had met +// until then was built by hand in a UI and adopted, so it already existed under +// whatever name someone typed — which is the same history that put `--environment` +// in the tool. The genuinely-from-nothing apply is the one path nobody had run. +// +// Idempotent by construction, so it is safe on EVERY apply and not just the +// first: absent -> create, present -> nothing. Reading before writing is also +// what keeps this change from being able to break an apply that works TODAY — +// an environment that already exists (every environment cast has ever touched) +// takes the read and stops, and the create route is never called at all. The +// 409 is the same answer as "present" (Coolify's create-environment 409s on a +// 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. +async function ensureEnvironment( + client: CoolifyClient, + projectUuid: string, + projectName: string, + envName: string, + 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. + const existing = await client.environments(projectUuid); + if (!existing.includes(envName)) { + try { + await client.post(`/projects/${projectUuid}/environments`, { + name: envName, + }); + } catch (err) { + if (!(err instanceof HttpError) || err.status !== 409) throw err; + } + } + const strays = projectWasCreated ? existing.filter((e) => e !== envName) : []; + if (strays.length > 0) { + 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)`, + ); + } +} + +// Project + environment, reconciled once per run and then remembered — the pair +// a resource create has to name before it can name anything else. +function projectEnvironmentResolver( + client: CoolifyClient, + projectName: string, + envName: string, +): () => Promise { + let once: Promise | undefined; + return () => { + once ??= (async () => { + const { uuid, created } = await resolveOrCreateProject( + client, + projectName, + ); + await ensureEnvironment(client, uuid, projectName, envName, created); + return uuid; + })(); + return once; + }; +} + // --- Desired-vocabulary -> Coolify wire-vocabulary field mapping --- // // `fields` (from Desired/Change) speaks the internal vocabulary used for @@ -1806,13 +1880,21 @@ export function buildExecutor( const destination = ctx.destinationUuid ? { destination_uuid: ctx.destinationUuid } : {}; + // Lazy, so a run with nothing to create touches neither route, and memoized, + // so a run with five creates reconciles the project and its environment once + // rather than five times. + const projectEnv = projectEnvironmentResolver( + client, + ctx.projectName, + ctx.envName, + ); 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 resolveOrCreateProject(client, ctx.projectName); + const projectUuid = await projectEnv(); if (change.kind === "application") { const res = (await client.post("/applications/private-github-app", { project_uuid: projectUuid, diff --git a/test/wire.test.ts b/test/wire.test.ts index 2c08fac..e7283cf 100644 --- a/test/wire.test.ts +++ b/test/wire.test.ts @@ -177,6 +177,10 @@ describe("buildExecutor createResource (application, dockercompose)", () => { JSON.stringify([{ uuid: "proj-1", name: "widget" }]), { status: 200 }, ); + if (path === "/api/v1/projects/proj-1/environments") + return new Response(JSON.stringify([{ name: "prod" }]), { + status: 200, + }); if (path === "/api/v1/applications/private-github-app") { createBody = JSON.parse(String(init?.body)); return new Response(JSON.stringify({ uuid: "app-1" }), { @@ -227,6 +231,10 @@ describe("buildExecutor createResource (application, dockercompose)", () => { JSON.stringify([{ uuid: "proj-1", name: "widget" }]), { status: 200 }, ); + if (path === "/api/v1/projects/proj-1/environments") + return new Response(JSON.stringify([{ name: "prod" }]), { + status: 200, + }); if (path === "/api/v1/applications/private-github-app") { createBody = JSON.parse(String(init?.body)); return new Response(JSON.stringify({ uuid: "app-2" }), { @@ -272,6 +280,12 @@ describe("buildExecutor createResource (destination placement)", () => { JSON.stringify([{ uuid: "proj-1", name: "widget" }]), { status: 200 }, ); + // The environment a create names has to exist, so apply reads it first + // (#38). This project is an existing one and already carries `prod`. + if (path === "/api/v1/projects/proj-1/environments") + return new Response(JSON.stringify([{ name: "prod" }]), { + status: 200, + }); if ( path === "/api/v1/applications/private-github-app" || path === "/api/v1/databases/postgresql" || @@ -372,6 +386,197 @@ describe("buildExecutor createResource (destination placement)", () => { ); }); +// #38: the first apply against a project that does not exist yet. POST /projects +// hands the new project Coolify's OWN default environment ("production"), never +// ours — so a create that names `environment_name: prod` 404s with "Environment +// not found" and leaves the project behind, created and empty. Every environment +// cast had touched until then was hand-built in a UI and adopted, which is why +// the from-nothing path is the one that had never run. +describe("buildExecutor createResource (environment reconcile)", () => { + // A Coolify with ONE project's worth of state, driven by what the test hands + // it: `projects` is what GET /projects answers, `environments` what the + // project carries. Both mutate as cast writes, so the mock stays honest about + // what a second read would see. + function fakeCoolify(opts: { + projects?: Array<{ uuid: string; name: string }>; + environments?: string[]; + envCreateStatus?: number; + }) { + const projects = opts.projects ?? []; + let environments = opts.environments ?? []; + const calls: string[] = []; + const fetchImpl = vi.fn(async (url: string | URL, init?: RequestInit) => { + const path = new URL(String(url)).pathname; + const method = init?.method ?? "GET"; + calls.push(`${method} ${path}`); + if (path === "/api/v1/projects" && method === "GET") + return new Response(JSON.stringify(projects), { status: 200 }); + if (path === "/api/v1/projects" && method === "POST") { + projects.push({ uuid: "proj-new", name: "widget" }); + // Coolify's doing, not ours: a brand-new project comes with this. + environments = ["production"]; + return new Response(JSON.stringify({ uuid: "proj-new" }), { + status: 201, + }); + } + const envRoute = /^\/api\/v1\/projects\/([^/]+)\/environments$/.exec( + path, + ); + if (envRoute && method === "GET") + return new Response( + JSON.stringify(environments.map((name) => ({ name }))), + { status: 200 }, + ); + // CoolifyClient.environments falls back to the project show route when + // the list route answers empty — it carries the same names as a relation. + if (/^\/api\/v1\/projects\/[^/]+$/.test(path) && method === "GET") + return new Response( + JSON.stringify({ + environments: environments.map((name) => ({ name })), + }), + { status: 200 }, + ); + if (envRoute && method === "POST") { + const name = JSON.parse(String(init?.body)).name as string; + const status = opts.envCreateStatus ?? 201; + if (status === 409) { + // A 409 is Coolify saying the name is TAKEN — so in the world the + // mock is modelling it exists, created by whoever won the race + // between our read and our write. The environment is there; only our + // create lost. A 409 whose environment did not exist is not a state + // Coolify can be in, and pretending otherwise would test nothing. + if (!environments.includes(name)) environments.push(name); + return new Response( + JSON.stringify({ + message: "Environment with this name already exists.", + }), + { status: 409 }, + ); + } + if (status !== 201) + return new Response(JSON.stringify({ message: "boom" }), { status }); + environments.push(name); + return new Response(JSON.stringify({ uuid: "env-1" }), { status: 201 }); + } + 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 + // it this mock would happily accept the create that a real box refuses, + // and the test below would pass against the very bug it exists to catch. + const body = JSON.parse(String(init?.body)) as { + environment_name: string; + }; + if (!environments.includes(body.environment_name)) + return new Response( + JSON.stringify({ message: "Environment not found." }), + { status: 404 }, + ); + return new Response(JSON.stringify({ uuid: "app-1" }), { status: 200 }); + } + return new Response("not found", { status: 404 }); + }) as unknown as typeof fetch; + return { calls, fetchImpl, environments: () => environments }; + } + + const app = { + kind: "application" as const, + name: "core", + op: "create" as const, + fieldDiffs: [ + { field: "build_pack", desired: "nixpacks", updatable: false }, + ], + envDiffs: [], + }; + + function exec(fetchImpl: typeof fetch) { + return buildExecutor( + new CoolifyClient("https://coolify.test", "tok", fetchImpl), + { + projectName: "widget", + envName: "prod", + serverUuid: "srv-1", + githubAppUuid: "gh-1", + backupSchedules: {}, + }, + ); + } + + it("creates the environment on a project it just created, and the create names it", async () => { + const coolify = fakeCoolify({ projects: [] }); + const uuid = await exec(coolify.fetchImpl).createResource(app); + + expect(uuid).toBe("app-1"); + // The fix: our environment is created BEFORE the resource that names it. + expect(coolify.calls).toContain( + "POST /api/v1/projects/proj-new/environments", + ); + expect(coolify.environments()).toContain("prod"); + const order = coolify.calls.indexOf( + "POST /api/v1/projects/proj-new/environments", + ); + const create = coolify.calls.indexOf( + "POST /api/v1/applications/private-github-app", + ); + expect(order).toBeGreaterThan(-1); + expect(order).toBeLessThan(create); + }); + + // The half of idempotence that protects every apply that works today: an + // environment that already exists must not be written to at all. + it("never touches the create route when the environment already exists", async () => { + const coolify = fakeCoolify({ + projects: [{ uuid: "proj-1", name: "widget" }], + environments: ["prod", "staging"], + }); + await exec(coolify.fetchImpl).createResource(app); + + expect(coolify.calls).toContain("GET /api/v1/projects/proj-1/environments"); + expect(coolify.calls).not.toContain( + "POST /api/v1/projects/proj-1/environments", + ); + }); + + // Coolify 409s a duplicate environment name — the same answer as "present", + // reached when something else wins the race between our read and our write. + it("treats a 409 from the environment create as already-there", async () => { + const coolify = fakeCoolify({ + projects: [{ uuid: "proj-1", name: "widget" }], + environments: [], + envCreateStatus: 409, + }); + const uuid = await exec(coolify.fetchImpl).createResource(app); + expect(uuid).toBe("app-1"); + }); + + // A 5xx is NOT "already there" — apply must not go on to create resources + // into an environment it has no reason to believe exists. + it("surfaces a non-409 failure from the environment create", async () => { + const coolify = fakeCoolify({ + projects: [{ uuid: "proj-1", name: "widget" }], + environments: [], + envCreateStatus: 500, + }); + await expect(exec(coolify.fetchImpl).createResource(app)).rejects.toThrow( + /environments → 500/, + ); + }); + + // Five creates in a run must reconcile the project and its environment once, + // not five times. + it("reconciles once across several creates", async () => { + const coolify = fakeCoolify({ projects: [] }); + const e = exec(coolify.fetchImpl); + await e.createResource(app); + await e.createResource({ ...app, name: "core-api" }); + + const envReads = coolify.calls.filter((c) => c.endsWith("/environments")); + expect(envReads).toHaveLength(2); // one GET + one POST, not two of each + expect( + coolify.calls.filter((c) => c === "POST /api/v1/projects").length, + ).toBe(1); + }); +}); + describe("databaseVersionFromImage / defaultDatabaseImage", () => { it("round-trips through defaultDatabaseImage for postgres", () => { const image = defaultDatabaseImage("postgresql", "17");