fix: apply creates the environment its resources name (#38)
POST /projects hands a new project Coolify's OWN default environment,
`production` — never ours. cast then created every resource with
`environment_name: <our --env>`, so the first apply against a project that
did not exist yet 404'd on its first resource ("Environment not found") and
left the project behind, created and empty.
Two comments in the source already asserted the behaviour as though it were
implemented (cli.ts:716, :808), and the README says it outright — the route
existed in the vendored 4.1.2 spec, cast just never called it. It went unseen
because every environment cast had touched until now was hand-built in a UI
and adopted, so it already existed under whatever name someone typed. The
genuinely-from-nothing apply is the one path nobody had run.
apply now reconciles project + environment once per run, before the first
create. Read-before-write: an environment that already exists is never written
to, so adoption is untouched and this cannot regress an apply that works today.
A 409 is read as "present" (the race between our read and our write).
Coolify's default environment is LEFT ALONE, per apply-never-deletes — deleting
it would be the first delete cast ever performs. 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.
The regression test drives the real failure, not a call count: the fake Coolify
404s a create whose environment_name it does not carry, exactly as a live box
does — against the old executor it reproduces the reported error verbatim.
This commit is contained in:
claude-hdb
parent
2b202580ea
commit
6b363cb5be
3 changed files with 302 additions and 4 deletions
|
|
@ -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.
|
||||
|
|
|
|||
90
src/cli.ts
90
src/cli.ts
|
|
@ -1664,9 +1664,9 @@ async function main(): Promise<number> {
|
|||
async function resolveOrCreateProject(
|
||||
client: CoolifyClient,
|
||||
name: string,
|
||||
): Promise<string> {
|
||||
): 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<void> {
|
||||
// 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<string> {
|
||||
let once: Promise<string> | 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,
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
Loading…
Reference in a new issue