From db4b72b04d4cb59530a237da2394ee9e292756e8 Mon Sep 17 00:00:00 2001 From: "detail-app[bot]" <180357370+detail-app[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 02:52:24 +0000 Subject: [PATCH] fix(registry): allow cloud-mode registry edit with untouched Server dropdown --- .../__test__/registry/update-registry.test.ts | 209 ++++++++++++++++++ packages/server/src/services/registry.ts | 60 +++-- 2 files changed, 236 insertions(+), 33 deletions(-) create mode 100644 apps/dokploy/__test__/registry/update-registry.test.ts diff --git a/apps/dokploy/__test__/registry/update-registry.test.ts b/apps/dokploy/__test__/registry/update-registry.test.ts new file mode 100644 index 000000000..acedbc8fe --- /dev/null +++ b/apps/dokploy/__test__/registry/update-registry.test.ts @@ -0,0 +1,209 @@ +import { TRPCError } from "@trpc/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + updatedRow: { + registryId: "reg-1", + registryName: "AWS ECR", + username: "AWS", + password: "dXNlcm5hbWU6cGFzc3dvcmQ=", + registryUrl: "123456789.dkr.ecr.us-east-1.amazonaws.com", + registryType: "cloud", + imagePrefix: null, + organizationId: "org-1", + } as Record, + execAsync: vi.fn(), + execAsyncRemote: vi.fn(), + transaction: vi.fn(), + txUpdate: vi.fn(), + txUpdateSet: vi.fn(), + txUpdateWhere: vi.fn(), + txUpdateReturning: vi.fn(), + cloud: { IS_CLOUD: false }, +})); + +const DEFAULT_ROW = { ...mocks.updatedRow }; + +vi.mock("@dokploy/server/db", () => { + const chain = { + set: mocks.txUpdateSet, + where: mocks.txUpdateWhere, + returning: mocks.txUpdateReturning, + }; + mocks.txUpdateSet.mockReturnValue(chain); + mocks.txUpdateWhere.mockReturnValue(chain); + mocks.txUpdateReturning.mockImplementation(() => + Promise.resolve([mocks.updatedRow]), + ); + mocks.txUpdate.mockReturnValue(chain); + mocks.transaction.mockImplementation( + async (cb: (tx: unknown) => Promise) => + cb({ update: mocks.txUpdate }), + ); + return { + db: { transaction: mocks.transaction, update: mocks.txUpdate }, + }; +}); + +vi.mock("@dokploy/server/utils/process/execAsync", () => ({ + execAsync: mocks.execAsync, + execAsyncRemote: mocks.execAsyncRemote, +})); + +vi.mock("@dokploy/server/constants", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + get IS_CLOUD() { + return mocks.cloud.IS_CLOUD; + }, + }; +}); + +import { + safeDockerLoginCommand, + updateRegistry, +} from "@dokploy/server/services/registry"; + +const wireChain = () => { + const chain = { + set: mocks.txUpdateSet, + where: mocks.txUpdateWhere, + returning: mocks.txUpdateReturning, + }; + mocks.txUpdateSet.mockReturnValue(chain); + mocks.txUpdateWhere.mockReturnValue(chain); + mocks.txUpdateReturning.mockImplementation(() => + Promise.resolve([mocks.updatedRow]), + ); + mocks.txUpdate.mockReturnValue(chain); + mocks.transaction.mockImplementation( + async (cb: (tx: unknown) => Promise) => + cb({ update: mocks.txUpdate }), + ); +}; + +const expectedLoginCommand = () => + safeDockerLoginCommand( + mocks.updatedRow.registryUrl as string, + mocks.updatedRow.username as string, + mocks.updatedRow.password as string, + ); + +beforeEach(() => { + vi.clearAllMocks(); + wireChain(); + mocks.execAsync.mockResolvedValue({ stdout: "", stderr: "" }); + mocks.execAsyncRemote.mockResolvedValue({ stdout: "", stderr: "" }); + mocks.cloud.IS_CLOUD = false; + Object.assign(mocks.updatedRow, DEFAULT_ROW); +}); + +describe("updateRegistry", () => { + it("succeeds without running docker login when serverId is falsy in cloud mode (untouched Server dropdown)", async () => { + mocks.cloud.IS_CLOUD = true; + + const result = await updateRegistry("reg-1", { + username: "AWS", + registryUrl: "123456789.dkr.ecr.us-east-1.amazonaws.com", + registryType: "cloud", + serverId: "", + }); + + expect(mocks.transaction).toHaveBeenCalledTimes(1); + expect(mocks.txUpdate).toHaveBeenCalledTimes(1); + expect(mocks.execAsync).not.toHaveBeenCalled(); + expect(mocks.execAsyncRemote).not.toHaveBeenCalled(); + expect(result).toEqual(mocks.updatedRow); + }); + + it("runs docker login locally when the user explicitly selects 'None' in cloud mode", async () => { + mocks.cloud.IS_CLOUD = true; + + await updateRegistry("reg-1", { + registryType: "cloud", + serverId: "none", + }); + + expect(mocks.execAsync).toHaveBeenCalledTimes(1); + expect(mocks.execAsync).toHaveBeenCalledWith(expectedLoginCommand()); + expect(mocks.execAsyncRemote).not.toHaveBeenCalled(); + }); + + it("runs docker login on the selected remote server in cloud mode", async () => { + mocks.cloud.IS_CLOUD = true; + + await updateRegistry("reg-1", { + registryType: "cloud", + serverId: "server-42", + }); + + expect(mocks.execAsyncRemote).toHaveBeenCalledTimes(1); + expect(mocks.execAsyncRemote).toHaveBeenCalledWith( + "server-42", + expectedLoginCommand(), + ); + expect(mocks.execAsync).not.toHaveBeenCalled(); + }); + + it("runs docker login locally for a falsy serverId when self-hosted (no shared control plane)", async () => { + const result = await updateRegistry("reg-1", { + registryType: "cloud", + serverId: "", + }); + + expect(mocks.execAsync).toHaveBeenCalledTimes(1); + expect(mocks.execAsync).toHaveBeenCalledWith(expectedLoginCommand()); + expect(mocks.execAsyncRemote).not.toHaveBeenCalled(); + expect(result).toEqual(mocks.updatedRow); + }); + + it("builds the login command from the post-update DB row (keep-existing-password flow)", async () => { + mocks.cloud.IS_CLOUD = false; + Object.assign(mocks.updatedRow, { password: "kept-existing-secret" }); + + await updateRegistry("reg-1", { + registryType: "cloud", + serverId: "none", + }); + + const cmd = mocks.execAsync.mock.calls[0]?.[0] as string; + expect(cmd).toContain("kept-existing-secret"); + expect( + safeDockerLoginCommand( + "123456789.dkr.ecr.us-east-1.amazonaws.com", + "AWS", + "kept-existing-secret", + ), + ).toBe(cmd); + }); + + it("throws a BAD_REQUEST TRPCError with the password redacted when docker login fails", async () => { + mocks.cloud.IS_CLOUD = false; + const password = mocks.updatedRow.password as string; + mocks.execAsync.mockRejectedValueOnce( + new Error(`login failed for ${password}`), + ); + + await expect( + updateRegistry("reg-1", { registryType: "cloud", serverId: "none" }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: "login failed for ***", + }); + + expect(mocks.execAsync).toHaveBeenCalledTimes(1); + }); + + it("runs the update inside db.transaction so a failed docker login rolls back the credential row", async () => { + mocks.cloud.IS_CLOUD = false; + mocks.execAsync.mockRejectedValueOnce(new Error("login failed")); + + await expect( + updateRegistry("reg-1", { registryType: "cloud", serverId: "none" }), + ).rejects.toBeInstanceOf(TRPCError); + + expect(mocks.transaction).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/server/src/services/registry.ts b/packages/server/src/services/registry.ts index e395dd4c9..607794662 100644 --- a/packages/server/src/services/registry.ts +++ b/packages/server/src/services/registry.ts @@ -118,43 +118,37 @@ export const updateRegistry = async ( registryData: Partial & { serverId?: string | null }, ) => { try { - const response = await db - .update(registry) - .set({ - ...registryData, - }) - .where(eq(registry.registryId, registryId)) - .returning() - .then((res) => res[0]); + return await db.transaction(async (tx) => { + const response = await tx + .update(registry) + .set({ + ...registryData, + }) + .where(eq(registry.registryId, registryId)) + .returning() + .then((res) => res[0]); - const loginCommand = safeDockerLoginCommand( - response?.registryUrl, - response?.username, - response?.password, - ); + const loginCommand = safeDockerLoginCommand( + response?.registryUrl, + response?.username, + response?.password, + ); - if ( - IS_CLOUD && - !registryData?.serverId && - registryData?.serverId !== "none" - ) { - throw new TRPCError({ - code: "NOT_FOUND", - message: "Select a server to add the registry", - }); - } - - try { - if (registryData?.serverId && registryData?.serverId !== "none") { - await execAsyncRemote(registryData.serverId, loginCommand); - } else if (response?.registryType === "cloud") { - await execAsync(loginCommand); + try { + if (registryData?.serverId && registryData?.serverId !== "none") { + await execAsyncRemote(registryData.serverId, loginCommand); + } else if ( + response?.registryType === "cloud" && + (!IS_CLOUD || registryData?.serverId === "none") + ) { + await execAsync(loginCommand); + } + } catch (execError) { + throw new Error(sanitizeRegistryError(execError, response?.password)); } - } catch (execError) { - throw new Error(sanitizeRegistryError(execError, response?.password)); - } - return response; + return response; + }); } catch (error) { const message = error instanceof TRPCError