Merge pull request #107 from dan-claude-bot/fix/lazy-github-app
fix: resolve the GitHub App only when the manifest declares applications (#103)
This commit is contained in:
commit
5f2e856a7d
4 changed files with 360 additions and 4 deletions
16
CHANGELOG.md
16
CHANGELOG.md
|
|
@ -7,6 +7,22 @@ actually cutting it, and this file starts there.
|
|||
|
||||
## Unreleased
|
||||
|
||||
### Fixed
|
||||
|
||||
- **`apply` no longer demands a GitHub App for a manifest that declares no
|
||||
applications** (#103) — found live in the 2026-07-19 release drill, where a
|
||||
databases-only manifest (`applications: {}`) rendered its plan of two
|
||||
creates and then died in preflight on `no GitHub App bound`, over a binding
|
||||
nothing in the run would ever have used: a GitHub App exists to clone
|
||||
application source, cast reads it in exactly one call (the application
|
||||
create), and databases and services never touch it. That unconditional
|
||||
resolution gated infra-only projects — the databases a fleet's other
|
||||
projects share — behind the GitHub-App browser-registration ceremony for no
|
||||
reason. `apply` now resolves the App only when the desired state actually
|
||||
contains an application; a manifest that does declare one still refuses on
|
||||
a missing binding exactly as before, clean plan or not, because that
|
||||
binding is state the next create will need.
|
||||
|
||||
### Added
|
||||
|
||||
- **Tagged releases with a prebuilt dist asset, and an installer that
|
||||
|
|
|
|||
34
src/cli.ts
34
src/cli.ts
|
|
@ -1275,9 +1275,22 @@ async function runProject(
|
|||
);
|
||||
}
|
||||
const serverUuid = await ctx.client.serverUuid(ctx.binding.server);
|
||||
const githubAppUuid = await ctx.client.githubAppUuid(
|
||||
githubAppNameFor(ctx.bindings, orgRepo),
|
||||
);
|
||||
// A GitHub App is how Coolify clones application SOURCE, and cast reads it in
|
||||
// exactly one place — the application create (buildExecutor's POST
|
||||
// /applications/private-github-app). Databases and services never touch it,
|
||||
// so it is resolved only when the manifest actually declares an application.
|
||||
// Resolving it unconditionally is what killed a databases-only apply in
|
||||
// preflight (#103, found live in the 2026-07-19 release drill): the plan
|
||||
// rendered its two creates and then githubAppNameFor threw over a binding
|
||||
// nothing in the run would ever have used — gating infra-only projects
|
||||
// behind the GitHub-App browser-registration ceremony. Keyed off DESIRED
|
||||
// rather than the plan's changes, deliberately: a manifest that declares an
|
||||
// application keeps the refusal even on a clean plan, exactly as before —
|
||||
// a missing binding there is state the next create will need, and the
|
||||
// operator should hear about it now, not mid-bootstrap.
|
||||
const githubAppUuid = desired.some((d) => d.kind === "application")
|
||||
? await ctx.client.githubAppUuid(githubAppNameFor(ctx.bindings, orgRepo))
|
||||
: null;
|
||||
const exec = buildExecutor(ctx.client, {
|
||||
projectName,
|
||||
// The name the environment gets ON COOLIFY when apply creates it — so an
|
||||
|
|
@ -3155,7 +3168,10 @@ export function buildExecutor(
|
|||
projectName: string;
|
||||
envName: string;
|
||||
serverUuid: string;
|
||||
githubAppUuid: string;
|
||||
// null when the desired state declares no applications (#103): the App is
|
||||
// read by the application create alone, and runProject only resolves one
|
||||
// when there is an application for it to clone.
|
||||
githubAppUuid: string | null;
|
||||
// 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
|
||||
|
|
@ -3379,6 +3395,16 @@ export function buildExecutor(
|
|||
);
|
||||
const projectUuid = await projectEnv();
|
||||
if (change.kind === "application") {
|
||||
// Unreachable by construction (#103): githubAppUuid is null only
|
||||
// when the desired state holds no applications, and a plan can only
|
||||
// create resources the desired state holds. Guarded anyway — this is
|
||||
// the uuid's single consumer, and a null slipping onto the wire
|
||||
// would surface as a Coolify 422 about somebody else's field.
|
||||
if (ctx.githubAppUuid === null) {
|
||||
throw new Error(
|
||||
"internal: application create reached an executor built without a GitHub App uuid — resolution was skipped as applications-free, yet the plan creates an application",
|
||||
);
|
||||
}
|
||||
const res = (await client.post("/applications/private-github-app", {
|
||||
project_uuid: projectUuid,
|
||||
environment_name: ctx.envName,
|
||||
|
|
|
|||
213
test/github-app-cli.test.ts
Normal file
213
test/github-app-cli.test.ts
Normal file
|
|
@ -0,0 +1,213 @@
|
|||
import { execFileSync, spawn } from "node:child_process";
|
||||
import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs";
|
||||
import { createServer } from "node:http";
|
||||
import type { AddressInfo } from "node:net";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
|
||||
// When does `apply` need a GitHub App at all? (#103, found live in the
|
||||
// 2026-07-19 release drill.)
|
||||
//
|
||||
// A GitHub App exists to clone application source, and cast reads it in exactly
|
||||
// one call — the application create. But apply used to resolve it
|
||||
// unconditionally, after the plan had already rendered: a manifest declaring
|
||||
// only databases printed its creates and then died in preflight on "no GitHub
|
||||
// App bound", over a binding nothing in the run would ever have used. That
|
||||
// gates infra-only projects (databases shared by other projects) behind the
|
||||
// GitHub-App browser-registration ceremony for no reason.
|
||||
//
|
||||
// Both directions are pinned here, end to end against a stub Coolify:
|
||||
// a databases-only manifest applies with NO github_apps binding and the stub
|
||||
// never sees a /github-apps request — and a manifest that DOES declare an
|
||||
// application still refuses on the missing binding, with the same message.
|
||||
|
||||
let recipient: string;
|
||||
let keyFile: string;
|
||||
|
||||
beforeAll(() => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "cast-age-"));
|
||||
keyFile = join(dir, "age.key");
|
||||
execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" });
|
||||
recipient = execFileSync("age-keygen", ["-y", keyFile], {
|
||||
encoding: "utf8",
|
||||
}).trim();
|
||||
});
|
||||
|
||||
type Stub = { url: string; hits: string[]; close: () => Promise<void> };
|
||||
const stubs: Stub[] = [];
|
||||
|
||||
// A box holding the project and its (empty) environment, so the plan is pure
|
||||
// creates — the drill's shape. `hits` records "METHOD path" so a test can
|
||||
// assert which routes a run touched, and which it never did.
|
||||
async function stubCoolify(): Promise<Stub> {
|
||||
const hits: string[] = [];
|
||||
const server = createServer((req, res) => {
|
||||
const path = new URL(req.url ?? "", "http://x").pathname.replace(
|
||||
"/api/v1",
|
||||
"",
|
||||
);
|
||||
hits.push(`${req.method} ${path}`);
|
||||
const json = (body: unknown) => {
|
||||
res.writeHead(200, { "content-type": "application/json" });
|
||||
res.end(JSON.stringify(body));
|
||||
};
|
||||
if (path === "/teams/current") return json({ id: 0, name: "Root Team" });
|
||||
if (path === "/servers") return json([{ uuid: "s1", name: "drill-box" }]);
|
||||
if (path === "/projects" && req.method === "GET")
|
||||
return json([{ uuid: "p1", name: "drill-widget" }]);
|
||||
if (path === "/projects/p1/staging") return json({});
|
||||
if (path === "/projects/p1/environments")
|
||||
return json([{ name: "staging" }]);
|
||||
// The domain preflight's instance-wide read (only an application plan asks).
|
||||
if (path === "/applications" && req.method === "GET") return json([]);
|
||||
if (path === "/databases/redis" && req.method === "POST")
|
||||
return json({ uuid: "db-9" });
|
||||
if (path === "/deploy") return json({});
|
||||
res.writeHead(404);
|
||||
res.end("{}");
|
||||
});
|
||||
await new Promise<void>((r) => {
|
||||
server.listen(0, "127.0.0.1", r);
|
||||
});
|
||||
const stub: Stub = {
|
||||
url: `http://127.0.0.1:${(server.address() as AddressInfo).port}`,
|
||||
hits,
|
||||
close: () =>
|
||||
new Promise<void>((r) => {
|
||||
server.close(() => r());
|
||||
}),
|
||||
};
|
||||
stubs.push(stub);
|
||||
return stub;
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(stubs.splice(0).map((s) => s.close()));
|
||||
});
|
||||
|
||||
// The drill's manifest: applications declared, and empty — this project IS its
|
||||
// databases.
|
||||
const DATABASES_ONLY = `project: drill-widget
|
||||
environments:
|
||||
staging:
|
||||
applications: {}
|
||||
databases:
|
||||
cache:
|
||||
type: redis
|
||||
`;
|
||||
|
||||
const WITH_APPLICATION = `project: drill-widget
|
||||
environments:
|
||||
staging:
|
||||
applications:
|
||||
web:
|
||||
source: { repo: heavy-duty/drill-widget, branch: main }
|
||||
build: { pack: nixpacks, base_directory: / }
|
||||
domains: ["http://web.example.com"]
|
||||
databases:
|
||||
cache:
|
||||
type: redis
|
||||
`;
|
||||
|
||||
// The state file the issue is about: no github_apps entry for this repo at all.
|
||||
function fixture(url: string, manifest: string) {
|
||||
const checkout = mkdtempSync(join(tmpdir(), "cast-co-"));
|
||||
mkdirSync(join(checkout, ".infra", "env"), { recursive: true });
|
||||
writeFileSync(join(checkout, ".infra", "manifest.yaml"), manifest);
|
||||
|
||||
const state = mkdtempSync(join(tmpdir(), "cast-state-"));
|
||||
mkdirSync(join(state, "secrets"));
|
||||
writeFileSync(
|
||||
join(state, ".coolify.env"),
|
||||
`COOLIFY_BASE_URL="${url}"\nCOOLIFY_ACCESS_TOKEN="t"\n`,
|
||||
);
|
||||
// No template refs any secret, but the store still has to exist and open.
|
||||
execFileSync("age", ["-r", recipient, "-o", "drill-widget.staging.env.age"], {
|
||||
input: "\n",
|
||||
cwd: join(state, "secrets"),
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
writeFileSync(
|
||||
join(state, "environments.yaml"),
|
||||
[
|
||||
"environments:",
|
||||
" staging:",
|
||||
" server: drill-box",
|
||||
" team: { id: 0, name: Root Team }",
|
||||
"github_apps: {}",
|
||||
"",
|
||||
].join("\n"),
|
||||
);
|
||||
return { checkout, state };
|
||||
}
|
||||
|
||||
function run(f: {
|
||||
checkout: string;
|
||||
state: string;
|
||||
}): Promise<{ code: number; output: string }> {
|
||||
return new Promise((resolve) => {
|
||||
const child = spawn(
|
||||
"node",
|
||||
[
|
||||
"dist/cli.js",
|
||||
"apply",
|
||||
"heavy-duty/drill-widget",
|
||||
"--env",
|
||||
"staging",
|
||||
"--path",
|
||||
f.checkout,
|
||||
"--state",
|
||||
f.state,
|
||||
],
|
||||
{
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
env: { ...process.env, CAST_AGE_KEY_FILE_STAGING: keyFile },
|
||||
},
|
||||
);
|
||||
let output = "";
|
||||
child.stdout.on("data", (d) => {
|
||||
output += String(d);
|
||||
});
|
||||
child.stderr.on("data", (d) => {
|
||||
output += String(d);
|
||||
});
|
||||
child.on("close", (code) => resolve({ code: code ?? 0, output }));
|
||||
});
|
||||
}
|
||||
|
||||
describe("cast apply — GitHub App resolved only for applications (#103)", () => {
|
||||
it("applies a databases-only manifest with no github_apps binding, never asking for one", async () => {
|
||||
const stub = await stubCoolify();
|
||||
const f = fixture(stub.url, DATABASES_ONLY);
|
||||
const r = await run(f);
|
||||
expect(r.code).toBe(0);
|
||||
// The apply went all the way through: the database was created and
|
||||
// redeployed, not merely planned.
|
||||
expect(r.output).toContain("applied + redeployed: cache");
|
||||
expect(stub.hits).toContain("POST /databases/redis");
|
||||
// The load-bearing absence: nothing in this run may ask Coolify for a
|
||||
// GitHub App — the resolution, not just the create, must be skipped.
|
||||
expect(stub.hits.some((h) => h.includes("/github-apps"))).toBe(false);
|
||||
expect(r.output).not.toContain("no GitHub App bound");
|
||||
});
|
||||
|
||||
it("still refuses a manifest WITH an application when the binding is missing", async () => {
|
||||
const stub = await stubCoolify();
|
||||
const f = fixture(stub.url, WITH_APPLICATION);
|
||||
const r = await run(f);
|
||||
expect(r.code).toBe(1);
|
||||
// The message githubAppNameFor has always thrown, unchanged.
|
||||
expect(r.output).toContain(
|
||||
"no GitHub App bound for heavy-duty/drill-widget",
|
||||
);
|
||||
expect(r.output).toContain('github_apps["heavy-duty/drill-widget"]');
|
||||
expect(r.output).toContain("bound repos: (none)");
|
||||
// Refused in preflight: nothing was created — not even the database the
|
||||
// manifest also declares.
|
||||
expect(stub.hits.some((h) => h.startsWith("POST /databases"))).toBe(false);
|
||||
expect(
|
||||
stub.hits.some((h) => h.includes("/applications/private-github-app")),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -498,6 +498,107 @@ describe("buildExecutor createResource (application, dockercompose)", () => {
|
|||
});
|
||||
});
|
||||
|
||||
// githubAppUuid is null when the desired state declares no applications (#103)
|
||||
// — legal for an executor that will only ever create databases and services,
|
||||
// and unreachable for an application create by construction. The guard exists
|
||||
// so that IF the construction is ever broken, the failure is cast's own
|
||||
// sentence and not a Coolify 422 about somebody else's field.
|
||||
describe("buildExecutor createResource (no GitHub App resolved, #103)", () => {
|
||||
type Call = { method: string; path: string; body?: unknown };
|
||||
|
||||
// Records every request, so a test can assert what was NOT sent — the
|
||||
// silently-included github_app_uuid is the shape of bug this pins against.
|
||||
function recorder(handler: (path: string) => Response): {
|
||||
calls: Call[];
|
||||
fetchImpl: typeof fetch;
|
||||
} {
|
||||
const calls: Call[] = [];
|
||||
const fetchImpl = vi.fn(async (url: string | URL, init?: RequestInit) => {
|
||||
const path = new URL(String(url)).pathname;
|
||||
calls.push({
|
||||
method: init?.method ?? "GET",
|
||||
path,
|
||||
body: init?.body ? JSON.parse(String(init.body)) : undefined,
|
||||
});
|
||||
return handler(path);
|
||||
}) as unknown as typeof fetch;
|
||||
return { calls, fetchImpl };
|
||||
}
|
||||
|
||||
const ctx = {
|
||||
projectName: "drill-widget",
|
||||
envName: "staging",
|
||||
serverUuid: "srv-1",
|
||||
githubAppUuid: null,
|
||||
serverName: "drill-box",
|
||||
orgRepo: "heavy-duty/drill-widget",
|
||||
bindingEnv: "staging",
|
||||
};
|
||||
|
||||
it("creates a database without one, sending no github_app_uuid", async () => {
|
||||
const { calls, fetchImpl } = recorder((path) => {
|
||||
if (path === "/api/v1/projects")
|
||||
return new Response(
|
||||
JSON.stringify([{ uuid: "proj-1", name: "drill-widget" }]),
|
||||
{ status: 200 },
|
||||
);
|
||||
if (path === "/api/v1/projects/proj-1/environments")
|
||||
return new Response(JSON.stringify([{ name: "staging" }]), {
|
||||
status: 200,
|
||||
});
|
||||
return new Response(JSON.stringify({ uuid: "db-9" }), { status: 201 });
|
||||
});
|
||||
const exec = buildExecutor(
|
||||
new CoolifyClient("https://coolify.test", "tok", fetchImpl),
|
||||
ctx,
|
||||
);
|
||||
const uuid = await exec.createResource({
|
||||
kind: "database",
|
||||
name: "cache",
|
||||
op: "create",
|
||||
fieldDiffs: [{ field: "type", desired: "redis", updatable: false }],
|
||||
envDiffs: [],
|
||||
});
|
||||
expect(uuid).toBe("db-9");
|
||||
const create = calls.find((c) => c.path === "/api/v1/databases/redis");
|
||||
expect(create?.body).not.toHaveProperty("github_app_uuid");
|
||||
});
|
||||
|
||||
it("refuses an application create with cast's own internal error, not a wire 422", async () => {
|
||||
const { calls, fetchImpl } = recorder((path) => {
|
||||
if (path === "/api/v1/projects")
|
||||
return new Response(
|
||||
JSON.stringify([{ uuid: "proj-1", name: "drill-widget" }]),
|
||||
{ status: 200 },
|
||||
);
|
||||
if (path === "/api/v1/projects/proj-1/environments")
|
||||
return new Response(JSON.stringify([{ name: "staging" }]), {
|
||||
status: 200,
|
||||
});
|
||||
return new Response("{}", { status: 200 });
|
||||
});
|
||||
const exec = buildExecutor(
|
||||
new CoolifyClient("https://coolify.test", "tok", fetchImpl),
|
||||
ctx,
|
||||
);
|
||||
await expect(
|
||||
exec.createResource({
|
||||
kind: "application",
|
||||
name: "web",
|
||||
op: "create",
|
||||
fieldDiffs: [
|
||||
{ field: "build_pack", desired: "nixpacks", updatable: false },
|
||||
],
|
||||
envDiffs: [],
|
||||
}),
|
||||
).rejects.toThrow(/internal: application create/);
|
||||
// The refusal happened before anything went near the create route.
|
||||
expect(
|
||||
calls.some((c) => c.path === "/api/v1/applications/private-github-app"),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// Placement is create-time only, and every kind needs it: Coolify runs the same
|
||||
// destination logic in ApplicationsController, DatabasesController and
|
||||
// ServicesController, and 400s on a multi-destination server for whichever one
|
||||
|
|
|
|||
Loading…
Reference in a new issue