Merge pull request #39 from claude-hdb/fix/ensure-environment
fix: apply creates the environment its resources name (#38)
This commit is contained in:
Daniel Marin
GitHub
commit
19f8d6f14e
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