Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions apps/api/src/modules/services/service.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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 <command>` fallback. An explicit argv still wins, `[]` included.
commandArgv: data.commandArgv ?? commandToArgv(command), // #332
restart: data.restart ?? "unless-stopped",
advanced,
...routing,
Expand Down Expand Up @@ -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 <command>` 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 [
Expand Down
131 changes: 131 additions & 0 deletions apps/api/test/modules/services/service-command-argv.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof import("@repo/db")>();
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<typeof import("../../../src/lib/controller-helpers")>();
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<string, any>;
const createdRow = () => serviceRepo.create.mock.calls.at(-1)?.[0] as Record<string, any>;

/**
* `commandArgv` — not the text `command` — is what the runtime hands Docker
* (`resolveComposeCmd`), and it only falls back to `sh -c <command>` 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<string, unknown>) => ({
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"]);
});
});