diff --git a/apps/api/src/modules/services/service.service.ts b/apps/api/src/modules/services/service.service.ts index 03101c430..4acf4e393 100644 --- a/apps/api/src/modules/services/service.service.ts +++ b/apps/api/src/modules/services/service.service.ts @@ -3,7 +3,7 @@ */ import { normalizeRoutingFields, repos, composeSpecDiff, type Project, type Service, type ServicePublicEndpoint } from "@repo/db"; -import { aliasConflictsWithSiblings, getProjectType, mergeAdvanced, normalizeServiceLabel, normalizeAliasStrict, safeErrorMessage, withTimeout, type ComposeAdvanced, type ServiceContainerState, type StackId } from "@repo/core"; +import { aliasConflictsWithSiblings, commandToArgv, getProjectType, mergeAdvanced, normalizeServiceLabel, normalizeAliasStrict, safeErrorMessage, withTimeout, type ComposeAdvanced, type ServiceContainerState, type StackId } from "@repo/core"; import { BuildLogger, DockerRuntime, @@ -484,6 +484,8 @@ export async function createService( // service counts as a sibling, which is exactly right for a new row. await validateServiceAlias(projectId, "", advanced, project.internalAlias); + const command = trimOrNull(data.command); + const created = await repos.service.create({ projectId, name, @@ -495,8 +497,15 @@ export async function createService( dependsOn: data.dependsOn ?? [], environment: data.environment ?? {}, volumes: data.volumes ?? [], - command: trimOrNull(data.command), - commandArgv: data.commandArgv ?? null, // #332 + command, + // #332: derive the argv when the caller sent only the text `command` — the + // dashboard's service form and the app installer (app-install.service.ts, + // which forwards a template's string command) both do. A row stored with a + // text command and a null argv is only rescued by the deploy-time backfill in + // `toComposeSpec`; deriving it at the insert keeps the two columns consistent + // without depending on that, and out of resolveComposeCmd's legacy + // `sh -c ` fallback. An explicit argv still wins, `[]` included. + commandArgv: data.commandArgv ?? commandToArgv(command), // #332 restart: data.restart ?? "unless-stopped", advanced, ...routing, @@ -590,6 +599,17 @@ export async function updateService( patch[key] = trimOrNull(patch[key]); } } + // #332: `commandArgv` is what the runtime actually runs — resolveComposeCmd + // prefers it and only falls back to `sh -c ` for legacy rows that have + // no argv at all. The dashboard's service form sends the one-line `command` and + // no argv, so a text-only edit left the PREVIOUS argv in the row: the container + // went on running the old command while the UI showed the new one, and clearing + // the field never handed the image's own CMD back. Re-derive alongside the trim + // so the two columns can't disagree. An explicit argv from an argv-aware caller + // (the compose parser, the CLI, a snapshot replay) still wins, `[]` included. + if ("command" in patch && patch.commandArgv === undefined) { + patch.commandArgv = commandToArgv(patch.command); + } // Monorepo sub-app build settings: same trim-or-null treatment so empty // strings become null in DB (matches the rest of the service columns). for (const key of [ diff --git a/apps/api/test/modules/services/service-command-argv.test.ts b/apps/api/test/modules/services/service-command-argv.test.ts new file mode 100644 index 000000000..9a1ecd846 --- /dev/null +++ b/apps/api/test/modules/services/service-command-argv.test.ts @@ -0,0 +1,131 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const projectRepo = vi.hoisted(() => ({ findById: vi.fn() })); +const serviceRepo = vi.hoisted(() => ({ + findById: vi.fn(), + findByName: vi.fn(), + listByProject: vi.fn(), + listByDeployment: vi.fn(), + create: vi.fn(), + update: vi.fn(), +})); +const deploymentRepo = vi.hoisted(() => ({ findById: vi.fn() })); +const domainRepo = vi.hoisted(() => ({ listByProject: vi.fn() })); +const freeGate = vi.hoisted(() => ({ assertFreeEndpointsAllowed: vi.fn() })); + +vi.mock("@repo/db", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + repos: { + ...actual.repos, + project: projectRepo, + service: serviceRepo, + deployment: deploymentRepo, + domain: domainRepo, + }, + }; +}); + +vi.mock("../../../src/lib/free-domain-guard", () => freeGate); +vi.mock("../../../src/lib/controller-helpers", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, platform: () => ({ runtime: { name: "docker" } }) }; +}); + +import { createService, updateService } from "../../../src/modules/services/service.service"; + +const ctx = { organizationId: "org_1" } as never; +const project = { id: "proj_1", organizationId: "org_1", slug: "acme" }; + +/** A compose row imported from a file, so it carries BOTH the text and the argv. */ +const composeRow = () => ({ + id: "svc_1", + projectId: project.id, + name: "server", + kind: "compose", + enabled: true, + environment: {}, + ports: [], + command: "ak server", + commandArgv: ["ak", "server"], + exposed: false, + publicEndpoints: [], +}); + +const writtenPatch = () => serviceRepo.update.mock.calls.at(-1)?.[1] as Record; +const createdRow = () => serviceRepo.create.mock.calls.at(-1)?.[0] as Record; + +/** + * `commandArgv` — not the text `command` — is what the runtime hands Docker + * (`resolveComposeCmd`), and it only falls back to `sh -c ` for legacy + * rows that have no argv. Both editors here take the dashboard's one-line + * `command` field and NO argv, which left the two columns free to disagree. + * + * The edit half is the live bug: `toComposeSpec` backfills an argv only when the + * stored one is null, so a STALE argv survives the deploy-time `syncFromCompose` + * and the container keeps running the old command. The create half stores no argv + * at all, which that same backfill does cover — asserted here so a row is written + * consistently instead of depending on a later repair step. + */ +describe("service command → argv (#332)", () => { + beforeEach(() => { + projectRepo.findById.mockReset().mockResolvedValue(project); + serviceRepo.findById.mockReset().mockResolvedValue(composeRow()); + serviceRepo.findByName.mockReset().mockResolvedValue(null); + serviceRepo.listByProject.mockReset().mockResolvedValue([]); + serviceRepo.listByDeployment.mockReset().mockResolvedValue([]); + serviceRepo.update.mockReset().mockResolvedValue(undefined); + serviceRepo.create.mockReset().mockImplementation(async (row: Record) => ({ + id: "svc_new", + ...row, + })); + deploymentRepo.findById.mockReset().mockResolvedValue(null); + domainRepo.listByProject.mockReset().mockResolvedValue([]); + freeGate.assertFreeEndpointsAllowed.mockReset().mockResolvedValue(undefined); + }); + + it("re-derives the argv when an edit sends only the text command", async () => { + await updateService(ctx, project.id, "svc_1", { command: "ak worker" } as never); + + const patch = writtenPatch(); + expect(patch.command).toBe("ak worker"); + // Without this the row kept ["ak","server"] and the container never switched. + expect(patch.commandArgv).toEqual(["ak", "worker"]); + }); + + it("clears the argv when an edit clears the command", async () => { + await updateService(ctx, project.id, "svc_1", { command: "" } as never); + + const patch = writtenPatch(); + expect(patch.command).toBeNull(); + // Null argv AND null command is the only shape that hands the image's own + // CMD back; a stale argv would keep overriding it forever. + expect(patch.commandArgv).toBeNull(); + }); + + it("keeps an explicit argv over the text command", async () => { + await updateService(ctx, project.id, "svc_1", { + command: "sh -c 'echo hi && ak server'", + commandArgv: ["sh", "-c", "echo hi && ak server"], + } as never); + + // An argv-aware caller (compose parser, CLI, snapshot replay) is never + // second-guessed — re-splitting its argv would mangle the quoted script. + expect(writtenPatch().commandArgv).toEqual(["sh", "-c", "echo hi && ak server"]); + }); + + it("derives the argv on create instead of storing none", async () => { + await createService(ctx, project.id, { + name: "worker", + kind: "compose", + image: "ghcr.io/goauthentik/server:latest", + command: "ak worker", + } as never); + + // The app installer forwards a template's string command through here. A null + // argv leaves the row one missed backfill away from `sh -c "ak worker"`, which + // hands `sh` to the image's `dumb-init -- ak` entrypoint. + expect(createdRow().commandArgv).toEqual(["ak", "worker"]); + }); +});