From bbca3cfff066b0e6368dabef1cea0079bb852099 Mon Sep 17 00:00:00 2001 From: claude-hdb Date: Thu, 16 Jul 2026 15:14:29 +0000 Subject: [PATCH] feat(service): set and diff per-container service hostnames via `urls` (#72) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Services could not carry hostnames through cast: `desiredFromManifest` dropped a service's `domains` and warned they were a manual Coolify UI act, citing a re-checked "no flat `domains` on a 4.1.2 service, on any route." The audit (#72) disproved that — the same failure mode #51 corrected for backup schedules. The FLAT shape genuinely has no route; the per-container CAPABILITY was there at 4.1.2 all along. `POST /services` and `PATCH /services/{uuid}` both take a `urls` list ([{name, url}], url comma-joined) that `applyServiceUrls` matches to a `ServiceApplication` by name and stores as its `fqdn`; `GET /services/{uuid}` loads `applications` and returns each `fqdn` (verified against ServicesController v4.1.2). So services now speak the SAME per-container vocabulary a dockercompose app does: - **Manifest:** `service_domains: { : [url] }` replaces the flat, unhonorable `domains` on a service (a flat list cannot name which container a hostname belongs to — exactly what `urls` requires). Canonicalized (keys and each URL array sorted) so container order never false-drifts. - **Write:** `serviceApiFields` builds `urls` on create and update. - **Read/diff:** a supplementary `GET /services/{uuid}` per service (`attachServiceDomains`, gated to `diff`/`apply` like backups) projects `applications[].fqdn` back into `service_domains`, so a declared hostname is compared every run — no perpetual drift, no manual UI step. - **Pre-flight:** a service create's `service_domains` joins `desiredDomainsOfCreate`, the more important because a service create whose domain conflicts is DELETED server-side before the 409 (rollback). Two limits stated out loud: the read is fail-closed (an unreachable/ unrecognized `GET /services/{uuid}` aborts rather than projecting empty and re-PATCHing forever), and `inventory --emit-draft` does not yet make the per-service GET, so a drafted service's hostnames are still declared by hand (same as backups) — draft/semantics say so. `npm run check` clean · 514 tests pass. Co-Authored-By: Claude Opus 4.8 --- docs/semantics.md | 47 ++++++++------ src/cli.ts | 118 +++++++++++++++++++++++++++------- src/coolify.ts | 11 ++++ src/draft.ts | 20 +++--- src/manifest.ts | 11 +++- src/resolve.ts | 61 +++++++++++------- test/domain-preflight.test.ts | 30 ++++++++- test/fixtures/manifest.yaml | 3 +- test/live-lookup.test.ts | 78 +++++++++++++++++++++- test/resolve.test.ts | 100 +++++++++++++++++----------- test/wire.test.ts | 26 +++++++- 11 files changed, 382 insertions(+), 123 deletions(-) diff --git a/docs/semantics.md b/docs/semantics.md index b52a35f..99e0b4f 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -1056,8 +1056,11 @@ express everything a Coolify holds, and a blueprint that omits those things without saying so is worse than no blueprint — in a disaster you would trust it and rebuild a *different box*. Per resource, it names what was seen and could not be written: `destination_id` (which Docker network — no destinations API in 4.1.2 -to resolve it to the UUID `destination_uuid:` wants, #21), service hostnames (no -flat `domains` on a Coolify 4.1.2 service), Basic Auth / custom Traefik labels, +to resolve it to the UUID `destination_uuid:` wants, #21), service hostnames +(settable/readable via the API and carried by `diff`/`apply` as `service_domains` +since #72, but the same as backups below — `inventory --emit-draft` does not yet +make the per-service `GET /services/{uuid}`, so a drafted service has none until +you declare them), Basic Auth / custom Traefik labels, build and deploy command overrides, backup schedules (**a rebuild has no backups until you declare them** — *not* because they cannot be read, which is what this line used to say and #51 disproved, but because `inventory --emit-draft` has not @@ -1228,24 +1231,28 @@ one used — verified against a live private clone. target) and how each says so out loud. Kept here, struck through, because this entry is *why nobody looked*: a limitation filed as a defect gets fixed, and a defect filed as a limitation does not. -- **A service's `domains` cannot be applied via the API in Coolify 4.1.2, - and is deliberately kept out of the diffed `fields` for idempotency.** (This - used to cite backup schedules as its precedent; it can't any more — that - reasoning was disproved above. This one was re-checked and holds: Coolify - 4.1.2 exposes no flat `domains` on a service, on any route. If that is ever - disproved the same way, `domains` belongs in `fields` too.) The `/services` - create/update payload takes a structured per-container `urls` list, not - the manifest's flat `domains: string[]`, and the manifest has no - per-container name to build that list correctly from — so cast - drops it rather than send a malformed payload. Live Coolify service state - doesn't expose a flat `domains` back either, so if it stayed in `fields` - every domain-bearing service would diff as a perpetual update and every - `apply` would needlessly restart it — `desiredFromManifest` drops - `domains` from the service's `fields` and **warns** - (`service declares domains (...), but apply cannot set them on - Coolify 4.1.2 services — configure hostnames manually in the Coolify UI`) - once per run for every service that declared any. Set service hostnames - in the Coolify UI by hand. +- **~~A service's hostnames cannot be applied via the API in Coolify 4.1.2.~~** + **Corrected (#72).** This entry used to claim, re-checked, that "Coolify 4.1.2 + exposes no flat `domains` on a service, on any route" — and drop a service's + domains from `fields`, warning that hostnames were a manual Coolify UI act. The + claim was true of the FLAT shape and false of the CAPABILITY, the same failure + mode #51 corrected for backups: the per-container route was there at 4.1.2 all + along. `POST /services` and `PATCH /services/{uuid}` both take a structured + `urls` list (`[{name, url}]`, `url` comma-joined) that `applyServiceUrls` + matches to a `ServiceApplication` by name and stores as its `fqdn`; `GET + /services/{uuid}` loads `applications` and returns each `fqdn` + (`ServicesController` v4.1.2). So services now speak the SAME per-container + vocabulary a dockercompose app does — a `service_domains: { : [url] }` + map — carried in `fields`, written on create *and* update, and compared on + every run off the read-back (one supplementary `GET /services/{uuid}` per + service, gated to `diff`/`apply` like backups). A flat `domains: string[]` is + gone: it could never name which container a hostname belongs to, which is + exactly what `urls` requires. Two limits remain, stated out loud: the read is + **fail-closed** (an unreachable/unrecognized `GET /services/{uuid}` aborts + rather than projecting empty and re-PATCHing forever), and a service create + whose domain conflicts is **deleted server-side before the 409** + (`applyServiceUrls` rollback) — which is why `service_domains` on a create is + pre-flighted (`desiredDomainsOfCreate`) alongside application domains. - **"Include Source Commit in Build" cannot be enabled via the API in Coolify 4.1.2 — `apply` warns instead.** A dockercompose application whose build consumes `SOURCE_COMMIT` as a **build arg** only receives it if the diff --git a/src/cli.ts b/src/cli.ts index a752788..584047d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -93,6 +93,7 @@ import { import { assertNoReservedEnvNames, reservedHits } from "./reserved.js"; import { PATH_IN_PROD_REFUSAL, + canonicalizeServiceDomains, desiredFromManifest, fillDesiredDerived, manifestResources, @@ -493,15 +494,55 @@ export async function attachBackup( }; } +// Read a service's per-container hostnames off GET /services/{uuid} and project +// them into `service_domains` on the Live's fields, so a declared hostname diffs +// like any other field (cast#72). The environment-list GET fetchLive reads does +// not eager-load `service.applications`, so this is a supplementary per-service +// read (see serviceByUuid). +// +// Unlike backup's not-compared escape, this FAILS CLOSED: a service whose domains +// cannot be read is NOT projected empty — that would diff a declared hostname as +// "will set" and let apply re-PATCH it every run — the read throws and aborts. +// GET /services/{uuid} for a service the environment list just named is not +// expected to fail; when it does, refusing beats a confident-but-blind plan +// (#12/#14/#17). A service with genuinely NO hostnames leaves service_domains +// absent, so a manifest declaring none stays clean and one declaring some drifts. +export async function attachServiceDomains( + client: CoolifyClient, + svc: Live, +): Promise { + const raw = (await client.serviceByUuid(svc.uuid)) as { + applications?: Array<{ name?: unknown; fqdn?: unknown }>; + } | null; + if (!raw || !Array.isArray(raw.applications)) { + throw new Error( + `GET /services/${svc.uuid} returned no applications array — cannot read service ${svc.name}'s hostnames to diff them`, + ); + } + const map: Record = {}; + for (const app of raw.applications) { + const name = typeof app.name === "string" ? app.name : undefined; + const fqdn = typeof app.fqdn === "string" ? app.fqdn : ""; + const urls = fqdn + .split(",") + .map((u) => u.trim()) + .filter(Boolean); + if (name && urls.length > 0) map[name] = urls; + } + if (Object.keys(map).length > 0) { + svc.fields.service_domains = canonicalizeServiceDomains(map); + } +} + export async function fetchLive( client: CoolifyClient, projectName: string, envName: string, - // Backups cost one extra GET per database, so only the callers that actually - // compare them ask for them: `diff` and `apply`. The read-side sweeps - // (inventory, capture, smoke) walk every project on a box and would pay it on - // every database for an answer they never look at. - opts: { backups?: boolean } = {}, + // Backups and service hostnames each cost one extra GET per resource, so only + // the callers that actually compare them ask: `diff` and `apply`. The read-side + // sweeps (inventory, capture, smoke) walk every project on a box and would pay + // it on every resource for an answer they never look at. + opts: { backups?: boolean; serviceDomains?: boolean } = {}, ): Promise { const projects = (await client.get("/projects")) as Array<{ uuid: string; @@ -599,6 +640,11 @@ export async function fetchLive( await attachBackup(client, db); } } + if (opts.serviceDomains) { + for (const svc of live.filter((l) => l.kind === "service")) { + await attachServiceDomains(client, svc); + } + } return { found: true, live }; } @@ -1067,9 +1113,11 @@ async function runProject( parseYaml(readFileSync(ctx.hostnameOverlay, "utf8")), ); } - // `backups: true` — this is the one path that compares them (see fetchLive). + // `backups`/`serviceDomains` — diff and apply are the one path that compares + // each, and each costs a supplementary GET per resource (see fetchLive). const lookup = await fetchLive(ctx.client, projectName, coolifyEnv, { backups: true, + serviceDomains: true, }); // apply and diff take opposite (and both correct) positions on absence: // apply is *allowed* to be the thing that brings a project into existence, @@ -2621,14 +2669,29 @@ export function databaseApiFields( export function serviceApiFields( fields: Record, ): Record { - // /services accepts `urls`, a structured per-container list - // ({name, url}[]), not the flat `domains` string list the manifest - // speaks. manifest.ts's ServiceSpecSchema has no notion of per-container - // name, so we can't build a correct `urls` payload from `domains` alone — - // dropped rather than sent malformed. Known limitation: service hostnames - // need manual Coolify UI configuration (see README, Task 10). - const { domains: _domains, ...rest } = fields; - return rest; + // /services accepts `urls`, a per-container list ({name, url}[]) — the create + // (POST /services) and update (PATCH /services/{uuid}) allowlists both carry + // it, and applyServiceUrls matches `urls[].name` to a ServiceApplication and + // sets its `fqdn`, `url` being that container's URLs comma-joined (verified + // against ServicesController v4.1.2, cast#72). `service_domains` speaks the + // internal map vocabulary (container -> string[]); this is the exact shape + // dockercompose apps' `docker_compose_domains` is written with, one route over. + // + // A `url` whose `name` matches no container is a 422 on update and, on CREATE, + // deletes the just-made service before answering 422 (applyServiceUrls's + // rollback) — so the name must be a real container. buildExecutor surfaces + // either as-is; the operator reads the right name off a `cast diff` read-back. + const { service_domains, ...rest } = fields; + return { + ...rest, + ...(service_domains !== undefined + ? { + urls: Object.entries(service_domains as Record).map( + ([name, urls]) => ({ name, url: urls.join(",") }), + ), + } + : {}), + }; } // --- Domain uniqueness: instance-wide, while cast plans project-scoped (#44) --- @@ -2684,14 +2747,18 @@ function nakedDomain(raw: string): string { return d.endsWith("/") ? d.slice(0, -1) : d; } -// The domains a planned CREATE would claim. Applications only, and that is not an -// oversight: databases have no domains, and cast's service creates drop `domains` -// on the wire entirely (serviceApiFields above), so an application create is the -// only way an apply can claim one. +// The domains a planned CREATE would claim: an application's flat `domains` or +// compose `docker_compose_domains`, and now a service's `service_domains` +// (cast#72 — service creates send `urls`; databases have no domains). Worth +// pre-flighting for services in particular, because a service create whose +// domain conflicts does not just 409 — applyServiceUrls DELETES the half-made +// service first (v4.1.2 rollback), so catching it here costs nothing where the +// server-side failure costs a resurrection. export function desiredDomainsOfCreate( change: Change, ): Array<{ domain: string; service?: string }> { - if (change.kind !== "application" || change.op !== "create") return []; + if (change.op !== "create") return []; + if (change.kind !== "application" && change.kind !== "service") return []; const fields = Object.fromEntries( change.fieldDiffs.map((f) => [f.field, f.desired]), ); @@ -2702,11 +2769,14 @@ export function desiredDomainsOfCreate( if (typeof d === "string" && d.length > 0) out.push({ domain: nakedDomain(d) }); } - const compose = fields.docker_compose_domains as - | Record - | undefined; - if (compose) { - for (const [service, urls] of Object.entries(compose)) + // dockercompose apps and services share the per-container map shape + // (docker_compose_domains / service_domains); the two never coexist on one + // resource. A container's name rides out as `service` so a conflict can say + // which container wanted the domain. + const perContainer = (fields.docker_compose_domains ?? + fields.service_domains) as Record | undefined; + if (perContainer) { + for (const [service, urls] of Object.entries(perContainer)) for (const d of urls ?? []) if (typeof d === "string" && d.length > 0) out.push({ domain: nakedDomain(d), service }); diff --git a/src/coolify.ts b/src/coolify.ts index c667b39..7375ee0 100644 --- a/src/coolify.ts +++ b/src/coolify.ts @@ -330,6 +330,17 @@ export class CoolifyClient { return this.get(`/databases/${encodeURIComponent(uuid)}/backups`); } + // A service's per-container hostnames live on `service.applications[].fqdn`, + // and only GET /services/{uuid} loads that relation ($service->load(['appli- + // cations','databases']), v4.1.2) — the environment-list GET that fetchLive + // reads does NOT. So reading a service's domains back to diff them (cast#72) + // costs this one extra GET per service. Returns the raw body; attachService- + // Domains projects `applications[].fqdn` into the manifest's service_domains + // shape and fails closed on any unrecognized answer. + async serviceByUuid(uuid: string): Promise { + return this.get(`/services/${encodeURIComponent(uuid)}`); + } + // What a Coolify DELETE removes, made explicit rather than inherited. // // All four are query parameters on DELETE /applications|databases|services/{uuid}, diff --git a/src/draft.ts b/src/draft.ts index c01b3b0..94a7a73 100644 --- a/src/draft.ts +++ b/src/draft.ts @@ -603,17 +603,21 @@ function serviceSpec( hasEnv: boolean, uncaptured: UncapturedItem[], ): Spec { - // Coolify 4.1.2's Service model carries no flat `domains` — hostnames live - // per-container on `service.applications[].fqdn`, which no endpoint cast uses - // returns (see projectLiveFields / serviceApiFields in cli.ts, and - // desiredFromManifest's warning). Not readable, not writable, not in the - // draft: a service that serves a hostname today would come back serving none. + // A service's per-container hostnames (`service.applications[].fqdn`) ARE + // settable and readable via the API — `urls` on create/PATCH, and + // GET /services/{uuid} on read — so `diff`/`apply` now carry them as + // `service_domains` (cast#72). What the DRAFT path cannot yet do is CAPTURE + // them: the inventory sweep reads the environment list, which does not + // eager-load `service.applications`, and does not make the supplementary + // per-service GET. So a service that serves a hostname today comes back with + // none in this draft — until it is declared by hand. Same shape as the backup + // schedule (#51): the API answers, the draft path has not been taught to ask. uncaptured.push({ project, resource: r.name, - setting: "domains (hostnames)", + setting: "service_domains (hostnames)", detail: - "Coolify 4.1.2 exposes no flat `domains` on a service — hostnames live per-container on service.applications[].fqdn, which cast can neither read nor write. Whatever hostnames this service answers on are NOT in this draft. Read them off the Coolify UI and set them there after a rebuild.", + "a service's per-container hostnames ARE settable/readable via the API (`urls` on create/PATCH, `service.applications[].fqdn` on GET /services/{uuid}) — `diff`/`apply` carry them as `service_domains` (cast#72) — but `inventory --emit-draft` does not yet make that per-service GET, so they are NOT captured here. Read them off a `cast diff` or the Coolify UI and declare `service_domains: { : [url] }` yourself.", }); return { type: String(r.raw.service_type ?? r.raw.type ?? ""), @@ -786,7 +790,7 @@ const NO_API_COVERAGE: Array<[string, string]> = [ ], [ "service hostnames", - "no flat `domains` on a service — they live per-container on `service.applications[].fqdn`, which cast can neither read nor write (Coolify 4.1.2).", + "settable/readable via the API (`urls` on create/PATCH, `service.applications[].fqdn` on GET /services/{uuid}) — `diff`/`apply` carry them as `service_domains` (cast#72) — but `inventory --emit-draft` does not yet make the per-service GET, so a drafted service has none until you declare them.", ], [ "Basic Auth / custom Traefik labels", diff --git a/src/manifest.ts b/src/manifest.ts index d6ebdc2..4ba4c49 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -147,7 +147,16 @@ const DatabaseSpecSchema = z const ServiceSpecSchema = z .object({ type: z.string(), - domains: z.array(z.string()).optional(), + // Per-container hostnames, exactly the vocabulary a dockercompose app uses + // (a map of container name -> URLs). A Coolify service is a bundle of + // containers (`ServiceApplication`s), and a hostname is set on ONE of them — + // so a flat `domains: string[]` cannot say which, and cannot build the + // `urls: [{name, url}]` payload the API matches to a container by name + // (cast#72, verified against ServicesController@applyServiceUrls v4.1.2). + // The name is the container's, discoverable from a `cast diff` read-back or + // the Coolify UI. Written on create/PATCH, read back off + // `service.applications[].fqdn`, and diffed like any other field. + service_domains: z.record(z.array(z.string())).optional(), env_template: z.string().optional(), }) .strict(); diff --git a/src/resolve.ts b/src/resolve.ts index e891c85..da2c059 100644 --- a/src/resolve.ts +++ b/src/resolve.ts @@ -509,6 +509,24 @@ export function manifestResources( return resources; } +// Sort the keys and each URL array of a service's `service_domains` map, so the +// same set of per-container hostnames compares equal whatever order the manifest +// authored them in or Coolify returns them in. Both the desired side +// (desiredFromManifest) and the live read-back (attachServiceDomains in cli.ts) +// run this before computeDiff compares by JSON.stringify — without it, a service +// whose containers Coolify lists in a different order than the manifest would +// diff forever. Order is meaningless for hostnames (Coolify matches `urls[].name` +// to a container and treats the URLs as a set), so canonicalizing loses nothing. +export function canonicalizeServiceDomains( + map: Record, +): Record { + return Object.fromEntries( + Object.keys(map) + .sort() + .map((k) => [k, [...map[k]].sort()]), + ); +} + export function desiredFromManifest( checkoutDir: string, envName: string, @@ -663,33 +681,30 @@ export function desiredFromManifest( }); } for (const [name, svc] of Object.entries(envSpec.services ?? {})) { - if (svc.domains && svc.domains.length > 0) { - // Coolify 4.1.2's service executor has no flat `domains` concept — - // hostnames live per-container on `urls` (see serviceApiFields in - // cli.ts) — so a manifest-declared service `domains` list is silently - // unhonorable by apply. Warn at build time, once per run, while the - // service name is still in scope. - console.warn( - `service ${name} declares domains (${svc.domains.join(", ")}), but apply cannot set them on Coolify 4.1.2 services — configure hostnames manually in the Coolify UI`, - ); - } desired.push({ kind: "service", name, - // domains dropped from fields: the live side (projectLiveFields in - // cli.ts) can't read service domains and the write side - // (serviceApiFields) drops them, so keeping domains in fields makes - // every domain-bearing service diff as a perpetual update. Hostnames - // stay a manual Coolify UI act (warned above). + // service_domains is a DIFFED FIELD (cast#72). Coolify 4.1.2 sets a + // service's per-container hostnames via `urls` on create/PATCH and returns + // them on `service.applications[].fqdn` — so a declared hostname is now + // WRITTEN by apply and VERIFIED by diff, not a manual Coolify UI act. // - // This USED to cite database `backup` as its precedent. It no longer - // can: `backup` was dropped on the same reasoning and the reasoning - // turned out to be false there (a read route existed, unlooked-for — - // see the databases loop above). The difference is that this one was - // re-checked: Coolify 4.1.2 genuinely exposes no flat `domains` on a - // service, on any route. If that is ever disproved the same way, this - // belongs in `fields` too. - fields: { type: svc.type }, + // Canonicalized (see canonicalizeServiceDomains) so container order in the + // manifest never diffs against Coolify's own ordering on read-back. The + // live side (attachServiceDomains in cli.ts) canonicalizes identically. + // + // This block USED to warn that a service's domains were unhonorable and + // drop them, citing a re-checked "no flat `domains` on a 4.1.2 service, on + // any route". That was true of the FLAT shape and false of the capability: + // the per-container `urls` route was there at 4.1.2 all along — the same + // arc as `backup` (#51), which was dropped on the same "can't read it back" + // reasoning that also turned out false. + fields: { + type: svc.type, + ...(svc.service_domains + ? { service_domains: canonicalizeServiceDomains(svc.service_domains) } + : {}), + }, env: resolveEnvFile(name, svc.env_template), }); } diff --git a/test/domain-preflight.test.ts b/test/domain-preflight.test.ts index b05a633..09ef2d6 100644 --- a/test/domain-preflight.test.ts +++ b/test/domain-preflight.test.ts @@ -82,9 +82,10 @@ describe("reading the domains of a plan and of an instance", () => { ]); }); - // Databases have none, and cast's service creates drop `domains` on the wire - // (serviceApiFields) — an application create is the only way an apply claims one. - it("claims nothing for a database or service create, or for an update", () => { + // Databases have no domains, and an update never claims (apply diffs the field + // and PATCHes it, it does not create). A SERVICE create does now claim — see + // the next test. + it("claims nothing for a database create or an update", () => { const db: Change = { kind: "database", name: "postgres", @@ -101,6 +102,29 @@ describe("reading the domains of a plan and of an instance", () => { expect(desiredDomainsOfCreate(update)).toEqual([]); }); + // cast#72: a service create sends `urls`, so its per-container service_domains + // must be pre-flighted — the more so because a service create whose domain + // conflicts is DELETED server-side before the 409 (applyServiceUrls rollback). + it("claims a service create's per-container service_domains", () => { + const svc: Change = { + kind: "service", + name: "umami", + op: "create", + fieldDiffs: [ + { field: "type", desired: "umami", updatable: false }, + { + field: "service_domains", + desired: { umami: ["https://analytics.example.com"] }, + updatable: true, + }, + ], + envDiffs: [], + }; + expect(desiredDomainsOfCreate(svc)).toEqual([ + { domain: "https://analytics.example.com", service: "umami" }, + ]); + }); + it("reads both live shapes: fqdn, and per-service compose domains", () => { expect( liveApplicationDomains( diff --git a/test/fixtures/manifest.yaml b/test/fixtures/manifest.yaml index 5c2a3f7..d72d9a1 100644 --- a/test/fixtures/manifest.yaml +++ b/test/fixtures/manifest.yaml @@ -19,7 +19,8 @@ environments: services: metabase: type: metabase - domains: ["https://metabase.example.com"] + service_domains: + metabase: ["https://metabase.example.com"] staging: applications: core-api: diff --git a/test/live-lookup.test.ts b/test/live-lookup.test.ts index 0e78a30..ce6872e 100644 --- a/test/live-lookup.test.ts +++ b/test/live-lookup.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it, vi } from "vitest"; -import { attachBackup, fetchLive, renderAbsentTarget } from "../src/cli.js"; +import { + attachBackup, + attachServiceDomains, + fetchLive, + renderAbsentTarget, +} from "../src/cli.js"; import { CoolifyClient } from "../src/coolify.js"; // A Coolify that answers GET /projects with `projects`, and @@ -270,3 +275,74 @@ describe("attachBackup", () => { expect("backup" in l.fields).toBe(false); }); }); + +describe("attachServiceDomains (cast#72)", () => { + const svc = () => ({ + kind: "service" as const, + name: "umami", + uuid: "svc-1", + fields: { type: "umami" }, + }); + // A Coolify whose GET /services/svc-1 answers with `body`. + const client = (body: unknown, status = 200) => + new CoolifyClient( + "https://coolify.test", + "tok", + vi.fn( + async () => + new Response(status === 200 ? JSON.stringify(body) : "boom", { + status, + }), + ) as unknown as typeof fetch, + ); + + it("projects applications[].fqdn into canonicalized service_domains", async () => { + const l = svc(); + await attachServiceDomains( + client({ + applications: [ + // Out of order, comma-joined, one container with two URLs — all + // canonicalized so the read matches the desired side regardless. + { name: "web", fqdn: "https://b.example.com,https://a.example.com" }, + { name: "collector", fqdn: "https://collect.example.com" }, + ], + }), + l, + ); + expect(l.fields.service_domains).toEqual({ + collector: ["https://collect.example.com"], + web: ["https://a.example.com", "https://b.example.com"], + }); + }); + + it("leaves service_domains absent when no container has a hostname", async () => { + const l = svc(); + await attachServiceDomains( + client({ + applications: [ + { name: "web", fqdn: "" }, + { name: "collector", fqdn: null }, + ], + }), + l, + ); + // A service with no hostnames stays clean against a manifest that declares + // none, and drifts against one that declares some. + expect("service_domains" in l.fields).toBe(false); + }); + + it("FAILS CLOSED — throws rather than projecting empty — when the read is unreachable", async () => { + const l = svc(); + // A blind empty projection would diff a declared hostname as "will set" and + // re-PATCH it forever; refusing is the safe answer (#12/#14/#17). + await expect(attachServiceDomains(client(null, 500), l)).rejects.toThrow(); + expect("service_domains" in l.fields).toBe(false); + }); + + it("FAILS CLOSED when the answer has no applications array", async () => { + const l = svc(); + await expect( + attachServiceDomains(client({ uuid: "svc-1" }), l), + ).rejects.toThrow(/applications array/); + }); +}); diff --git a/test/resolve.test.ts b/test/resolve.test.ts index 0e1c48f..ebe90dc 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -270,30 +270,7 @@ environments: // alone, like every other thing apply never removes. expect("backup" in desired[0].fields).toBe(false); }); - it("warns when a service declares domains (unhonorable by apply on Coolify 4.1.2)", () => { - const dir = mkdtempSync(join(tmpdir(), "infra-co-")); - mkdirSync(join(dir, ".infra"), { recursive: true }); - writeFileSync( - join(dir, ".infra", "manifest.yaml"), - `project: widget -environments: - staging: - applications: {} - services: - plausible: - type: plausible - domains: ["https://stats.staging.example.com"] -`, - ); - const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); - const { desired } = desiredFromManifest(dir, "staging", {}); - expect(desired[0]).toMatchObject({ kind: "service", name: "plausible" }); - expect(warn).toHaveBeenCalledTimes(1); - expect(warn.mock.calls[0][0]).toMatch(/plausible/); - expect(warn.mock.calls[0][0]).toMatch(/domains/); - warn.mockRestore(); - }); - it("drops domains from a domain-bearing service's fields (mirrors database backup handling)", () => { + it("emits a service's service_domains into fields, canonicalized (cast#72)", () => { const dir = mkdtempSync(join(tmpdir(), "infra-co-")); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( @@ -305,15 +282,21 @@ environments: services: umami: type: umami - domains: ["https://analytics.example.com"] + service_domains: + umami: ["https://b.example.com", "https://a.example.com"] `, ); - const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); const { desired } = desiredFromManifest(dir, "prod", {}); - expect(desired[0].fields).toEqual({ type: "umami" }); - warn.mockRestore(); + // Keys and each URL array are sorted so container order never false-drifts + // against Coolify's read-back ordering. + expect(desired[0].fields).toEqual({ + type: "umami", + service_domains: { + umami: ["https://a.example.com", "https://b.example.com"], + }, + }); }); - it("computeDiff is clean for a domain-bearing service against a matching live service (no perpetual update)", () => { + it("is clean for a service whose live per-container hostnames match (cast#72, no perpetual update)", () => { const dir = mkdtempSync(join(tmpdir(), "infra-co-")); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( @@ -325,12 +308,45 @@ environments: services: umami: type: umami - domains: ["https://analytics.example.com"] + service_domains: + umami: ["https://analytics.example.com"] +`, + ); + const { desired } = desiredFromManifest(dir, "prod", {}); + const report = computeDiff( + desired, + [ + { + kind: "service", + name: "umami", + uuid: "svc-uuid", + fields: { + type: "umami", + service_domains: { umami: ["https://analytics.example.com"] }, + }, + }, + ], + "full", + ); + expect(report.clean).toBe(true); + }); + it("diffs a service whose declared hostname is missing live (apply will set it)", () => { + const dir = mkdtempSync(join(tmpdir(), "infra-co-")); + mkdirSync(join(dir, ".infra"), { recursive: true }); + writeFileSync( + join(dir, ".infra", "manifest.yaml"), + `project: widget +environments: + prod: + applications: {} + services: + umami: + type: umami + service_domains: + umami: ["https://analytics.example.com"] `, ); - const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); const { desired } = desiredFromManifest(dir, "prod", {}); - warn.mockRestore(); const report = computeDiff( desired, [ @@ -341,11 +357,19 @@ environments: fields: { type: "umami" }, }, ], - "structural", + "full", ); - expect(report.clean).toBe(true); + expect(report.clean).toBe(false); + expect(report.changes[0].fieldDiffs).toEqual([ + { + field: "service_domains", + desired: { umami: ["https://analytics.example.com"] }, + live: undefined, + updatable: true, + }, + ]); }); - it("does not warn for a service with no domains", () => { + it("a service with no service_domains carries only its type", () => { const dir = mkdtempSync(join(tmpdir(), "infra-co-")); mkdirSync(join(dir, ".infra"), { recursive: true }); writeFileSync( @@ -359,10 +383,8 @@ environments: type: plausible `, ); - const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); - desiredFromManifest(dir, "staging", {}); - expect(warn).not.toHaveBeenCalled(); - warn.mockRestore(); + const { desired } = desiredFromManifest(dir, "staging", {}); + expect(desired[0].fields).toEqual({ type: "plausible" }); }); it("resolves a dockercompose app to docker_compose_location/docker_compose_domains and no port/healthcheck/domains keys", () => { const dir = mkdtempSync(join(tmpdir(), "infra-co-")); diff --git a/test/wire.test.ts b/test/wire.test.ts index 8bceabc..aec3015 100644 --- a/test/wire.test.ts +++ b/test/wire.test.ts @@ -56,13 +56,33 @@ describe("databaseApiFields", () => { }); describe("serviceApiFields", () => { - it("drops domains (services have no flat-domains create/update field)", () => { + // cast#72: a service's per-container hostnames go on the wire as `urls` + // ({name, url}[], url comma-joined) — matched to a ServiceApplication by name + // on create/PATCH. `service_domains` is the internal map that becomes it. + it("builds `urls` from service_domains", () => { const out = serviceApiFields({ type: "plausible", - domains: ["https://stats.example.com"], + service_domains: { + web: ["https://stats.example.com", "https://alt.example.com"], + collector: ["https://collect.example.com"], + }, }); + expect(out).toEqual({ + type: "plausible", + urls: [ + { + name: "web", + url: "https://stats.example.com,https://alt.example.com", + }, + { name: "collector", url: "https://collect.example.com" }, + ], + }); + expect(out).not.toHaveProperty("service_domains"); + }); + it("sends no `urls` for a service with no service_domains", () => { + const out = serviceApiFields({ type: "plausible" }); expect(out).toEqual({ type: "plausible" }); - expect(out).not.toHaveProperty("domains"); + expect(out).not.toHaveProperty("urls"); }); });