From 60c897429fc795ab9d91b662230468acc7ae4adc 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:48:50 +0000 Subject: [PATCH] fix(backups): remove repeatable job using pre-update cron when disabling in cloud --- .../cloud-disable-uses-old-cron.test.ts | 287 ++++++++++++++++++ .../backups/schedules-runJobs-gate.test.ts | 269 ++++++++++++++++ apps/dokploy/server/api/routers/backup.ts | 2 +- apps/dokploy/server/api/routers/schedule.ts | 6 +- .../server/api/routers/volume-backups.ts | 4 +- apps/schedules/src/utils.ts | 4 + 6 files changed, 566 insertions(+), 6 deletions(-) create mode 100644 apps/dokploy/__test__/backups/cloud-disable-uses-old-cron.test.ts create mode 100644 apps/dokploy/__test__/backups/schedules-runJobs-gate.test.ts diff --git a/apps/dokploy/__test__/backups/cloud-disable-uses-old-cron.test.ts b/apps/dokploy/__test__/backups/cloud-disable-uses-old-cron.test.ts new file mode 100644 index 000000000..1c6495d99 --- /dev/null +++ b/apps/dokploy/__test__/backups/cloud-disable-uses-old-cron.test.ts @@ -0,0 +1,287 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * Regression coverage for the cloud (`IS_CLOUD`) `update` mutation in + * `apps/dokploy/server/api/routers/backup.ts`. + * + * BullMQ repeatable jobs are keyed by `name + pattern (+ tz)`. When a backup + * that was originally scheduled with cron `A` is updated to cron `B` AND + * disabled in the same request, the disable branch must ask `apps/schedules` + * to remove the repeatable job using the *stored* (pre-update) cron `A` — not + * the post-update cron `B` — otherwise `removeRepeatable` misses and the old + * schedule keeps firing forever. + * + * The router fetches the row twice around `updateBackupById`: `existing` + * (pre-update) and `backup` (post-update). The disable branch must source the + * removal cron from `existing.schedule`. + */ + +const mocks = vi.hoisted(() => ({ + // @dokploy/server barrel — backup CRUD + helpers used by the router. + findBackupById: vi.fn(), + updateBackupById: vi.fn(), + // Cloud fetch wrappers in @/server/utils/backup (the things we assert on). + removeJob: vi.fn().mockResolvedValue(true), + updateJob: vi.fn().mockResolvedValue(true), + schedule: vi.fn().mockResolvedValue(true), + // Permission + audit side effects. + checkServicePermissionAndAccess: vi.fn().mockResolvedValue(undefined), + audit: vi.fn().mockResolvedValue(undefined), + assertDatabaseBackupLimit: vi.fn().mockResolvedValue(undefined), +})); + +// Cloud fetch wrappers (capture the cron handed to apps/schedules). +vi.mock("@/server/utils/backup", () => ({ + removeJob: mocks.removeJob, + updateJob: mocks.updateJob, + schedule: mocks.schedule, +})); + +vi.mock("@/server/api/utils/audit", () => ({ audit: mocks.audit })); + +vi.mock("@/server/api/utils/plan-limits", () => ({ + assertDatabaseBackupLimit: mocks.assertDatabaseBackupLimit, + assertScheduledJobLimit: vi.fn().mockResolvedValue(undefined), + assertVolumeBackupLimit: vi.fn().mockResolvedValue(undefined), + assertOrganizationLimit: vi.fn().mockResolvedValue(undefined), + assertMemberLimit: vi.fn().mockResolvedValue(undefined), + assertEnvironmentLimit: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@dokploy/server/services/permission", () => ({ + checkPermission: vi.fn().mockResolvedValue(undefined), + checkServicePermissionAndAccess: mocks.checkServicePermissionAndAccess, + findMemberByUserId: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@dokploy/server/services/destination", () => ({ + findDestinationById: vi.fn(), +})); + +vi.mock("@dokploy/server/utils/backups/compose", () => ({ + runComposeBackup: vi.fn(), +})); + +vi.mock("@dokploy/server/utils/backups/utils", () => ({ + getS3Credentials: vi.fn(), + normalizeS3Path: vi.fn(), +})); + +vi.mock("@dokploy/server/utils/process/execAsync", () => ({ + execAsync: vi.fn(), + execAsyncRemote: vi.fn(), + execAsyncStream: vi.fn(), +})); + +vi.mock("@dokploy/server/utils/restore", () => ({ + restoreComposeBackup: vi.fn(), + restoreLibsqlBackup: vi.fn(), + restoreMariadbBackup: vi.fn(), + restoreMongoBackup: vi.fn(), + restoreMySqlBackup: vi.fn(), + restorePostgresBackup: vi.fn(), + restoreWebServerBackup: vi.fn(), +})); + +// trpc.ts imports validateRequest at module load. +vi.mock("@dokploy/server/lib/auth", () => ({ validateRequest: vi.fn() })); + +// The @dokploy/server barrel is imported by the router AND by trpc.ts (as +// @dokploy/server/index for `hasValidLicense`). Provide every named export the +// router references so module load is side-effect free. `IS_CLOUD` pins the +// cloud branch under test. +vi.mock("@dokploy/server", () => ({ + IS_CLOUD: true, + hasValidLicense: vi.fn().mockResolvedValue(true), + // CRUD / lookups used by the router. + findBackupById: mocks.findBackupById, + updateBackupById: mocks.updateBackupById, + createBackup: vi.fn(), + findBackupsByDbId: vi.fn().mockResolvedValue([]), + findComposeByBackupId: vi.fn(), + findComposeById: vi.fn(), + findLibsqlByBackupId: vi.fn(), + findLibsqlById: vi.fn(), + findMariadbByBackupId: vi.fn(), + findMariadbById: vi.fn(), + findMongoByBackupId: vi.fn(), + findMongoById: vi.fn(), + findMySqlByBackupId: vi.fn(), + findMySqlById: vi.fn(), + findPostgresByBackupId: vi.fn(), + findPostgresById: vi.fn(), + findServerById: vi.fn(), + keepLatestNBackups: vi.fn(), + removeBackupById: vi.fn(), + removeScheduleBackup: vi.fn(), + runLibsqlBackup: vi.fn(), + runMariadbBackup: vi.fn(), + runMongoBackup: vi.fn(), + runMySqlBackup: vi.fn(), + runPostgresBackup: vi.fn(), + runWebServerBackup: vi.fn(), + scheduleBackup: vi.fn(), +})); + +// @/server/db/schema re-exports @dokploy/server/db/schema. Provide permissive +// Zod input schemas so createCaller parses any input shape we pass. +vi.mock("@dokploy/server/db/schema", async () => { + const { z } = await import("zod"); + const any = z.any(); + return { + apiCreateBackup: any, + apiFindOneBackup: any, + apiRemoveBackup: any, + apiRestoreBackup: any, + apiUpdateBackup: any, + createScheduleSchema: any, + updateScheduleSchema: any, + createVolumeBackupSchema: any, + updateVolumeBackupSchema: any, + VOLUME_NAME_MESSAGE: "invalid", + VOLUME_NAME_REGEX: /.*/, + backups: {}, + schedules: {}, + volumeBackups: {}, + deployments: {}, + environments: {}, + member: {}, + organization: {}, + }; +}); + +// Distinct subpath modules imported by the sibling routers / plan-limits; mock +// them so the barrel re-export above is the single source of schema exports. +vi.mock("@dokploy/server/db/schema/schedule", async () => { + const { z } = await import("zod"); + return { + createScheduleSchema: z.any(), + updateScheduleSchema: z.any(), + schedules: {}, + }; +}); + +vi.mock("@dokploy/server/db/schema/deployment", () => ({ deployments: {} })); + +import { backupRouter } from "@/server/api/routers/backup"; + +const buildBackup = (over: Partial> = {}) => ({ + backupId: "backup-1", + schedule: "*/2 * * * *", + enabled: true, + postgresId: "pg-1", + databaseType: "postgres", + backupType: "database", + destinationId: "dest-1", + prefix: "pg", + database: "db", + serviceName: "pg", + keepLatestCount: 5, + metadata: {}, + ...over, +}); + +const ctx = () => + ({ + session: { activeOrganizationId: "org-1" }, + user: { + id: "u1", + email: "u@example.com", + role: "owner", + ownerId: "u1", + enableEnterpriseFeatures: false, + isValidEnterpriseLicense: false, + }, + db: {}, + }) as any; + +describe("backupRouter.update — cloud disable branch", () => { + const OLD_CRON = "*/2 * * * *"; + const NEW_CRON = "0 0 * * *"; + + beforeEach(() => { + mocks.findBackupById.mockReset(); + mocks.updateBackupById.mockReset(); + mocks.removeJob.mockClear(); + mocks.updateJob.mockClear(); + mocks.schedule.mockClear(); + mocks.checkServicePermissionAndAccess.mockClear(); + mocks.audit.mockClear(); + }); + + it("removes the repeatable job using the PRE-update cron when cron changes and enabled is flipped off", async () => { + // 1st read: pre-update row (scheduled with OLD_CRON, enabled). + // 2nd read: post-update row (NEW_CRON, disabled). + mocks.findBackupById + .mockResolvedValueOnce(buildBackup({ schedule: OLD_CRON, enabled: true })) + .mockResolvedValueOnce( + buildBackup({ schedule: NEW_CRON, enabled: false }), + ); + + const caller = backupRouter.createCaller(ctx()); + await caller.update({ + backupId: "backup-1", + schedule: NEW_CRON, + enabled: false, + } as any); + + // The disable branch must remove using the stored (old) cron, not the + // post-update cron — otherwise the BullMQ repeatable member keyed by + // OLD_CRON is left in Redis and keeps firing. + expect(mocks.removeJob).toHaveBeenCalledTimes(1); + expect(mocks.removeJob).toHaveBeenCalledWith({ + cronSchedule: OLD_CRON, + backupId: "backup-1", + type: "backup", + }); + expect(mocks.updateJob).not.toHaveBeenCalled(); + }); + + it("still removes correctly when only disabling (cron unchanged)", async () => { + mocks.findBackupById + .mockResolvedValueOnce(buildBackup({ schedule: OLD_CRON, enabled: true })) + .mockResolvedValueOnce( + buildBackup({ schedule: OLD_CRON, enabled: false }), + ); + + const caller = backupRouter.createCaller(ctx()); + await caller.update({ + backupId: "backup-1", + schedule: OLD_CRON, + enabled: false, + } as any); + + expect(mocks.removeJob).toHaveBeenCalledTimes(1); + expect(mocks.removeJob).toHaveBeenCalledWith({ + cronSchedule: OLD_CRON, + backupId: "backup-1", + type: "backup", + }); + expect(mocks.updateJob).not.toHaveBeenCalled(); + }); + + it("does not regress the enabled path: updateJob receives the post-update cron", async () => { + mocks.findBackupById + .mockResolvedValueOnce(buildBackup({ schedule: OLD_CRON, enabled: true })) + .mockResolvedValueOnce( + buildBackup({ schedule: NEW_CRON, enabled: true }), + ); + + const caller = backupRouter.createCaller(ctx()); + await caller.update({ + backupId: "backup-1", + schedule: NEW_CRON, + enabled: true, + } as any); + + // The enabled branch delegates to /update-backup (which does its own + // getJobRepeatable lookup), so it receives the NEW cron to reschedule. + expect(mocks.updateJob).toHaveBeenCalledTimes(1); + expect(mocks.updateJob).toHaveBeenCalledWith({ + cronSchedule: NEW_CRON, + backupId: "backup-1", + type: "backup", + }); + expect(mocks.removeJob).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/dokploy/__test__/backups/schedules-runJobs-gate.test.ts b/apps/dokploy/__test__/backups/schedules-runJobs-gate.test.ts new file mode 100644 index 000000000..5768067c6 --- /dev/null +++ b/apps/dokploy/__test__/backups/schedules-runJobs-gate.test.ts @@ -0,0 +1,269 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * Defense-in-depth gate test for `apps/schedules/src/utils.ts` `runJobs` + * `backup` branch (G6, G7). + * + * `runJobs` is imported directly from the schedules service source (via + * relative path) so this exercises the real module. `bullmq` is mocked so the + * module-load-time `new Queue("backupQueue", { connection: ... })` in `queue.ts` + * does not open a Redis connection. The `@dokploy/server` barrel and the + * schedules `logger` are mocked so we can assert dispatch behaviour. + */ + +const mocks = vi.hoisted(() => ({ + findBackupById: vi.fn(), + findServerById: vi.fn(), + findScheduleById: vi.fn(), + findVolumeBackupById: vi.fn(), + runPostgresBackup: vi.fn().mockResolvedValue(undefined), + runMySqlBackup: vi.fn().mockResolvedValue(undefined), + runMongoBackup: vi.fn().mockResolvedValue(undefined), + runMariadbBackup: vi.fn().mockResolvedValue(undefined), + runLibsqlBackup: vi.fn().mockResolvedValue(undefined), + runComposeBackup: vi.fn().mockResolvedValue(undefined), + runVolumeBackup: vi.fn().mockResolvedValue(undefined), + keepLatestNBackups: vi.fn().mockResolvedValue(undefined), + runCommand: vi.fn().mockResolvedValue(undefined), + cleanupAll: vi.fn().mockResolvedValue(undefined), + loggerInfo: vi.fn(), + loggerError: vi.fn(), +})); + +// Mock the schedules queue module (imported by utils.ts as ./queue.js) so the +// real `queue.ts` — which instantiates `new Queue("backupQueue", { connection })` +// and would connect to Redis — is never loaded. Mocking by the path utils.ts +// resolves (`./queue.js` -> apps/schedules/src/queue.ts) ensures the stub is +// used; a bare `vi.mock("bullmq")` would NOT match because pnpm's nested +// node_modules resolves the schedules package's `bullmq` to a different +// physical path than this test file's. +vi.mock("../../../schedules/src/queue", () => ({ + jobQueue: { + add: vi.fn(), + removeRepeatable: vi.fn(), + getRepeatableJobs: vi.fn().mockResolvedValue([]), + obliterate: vi.fn(), + }, + scheduleJob: vi.fn().mockResolvedValue(undefined), + removeJob: vi.fn().mockResolvedValue(true), + getJobRepeatable: vi.fn().mockResolvedValue(null), + cleanQueue: vi.fn().mockResolvedValue(undefined), +})); + +// Mock the schedules logger (imported by utils.ts as ./logger.js). +vi.mock("../../../schedules/src/logger", () => ({ + logger: { info: mocks.loggerInfo, error: mocks.loggerError, warn: vi.fn() }, +})); + +vi.mock("@dokploy/server", () => ({ + findBackupById: mocks.findBackupById, + findScheduleById: mocks.findScheduleById, + findServerById: mocks.findServerById, + findVolumeBackupById: mocks.findVolumeBackupById, + keepLatestNBackups: mocks.keepLatestNBackups, + runCommand: mocks.runCommand, + runComposeBackup: mocks.runComposeBackup, + runLibsqlBackup: mocks.runLibsqlBackup, + runMariadbBackup: mocks.runMariadbBackup, + runMongoBackup: mocks.runMongoBackup, + runMySqlBackup: mocks.runMySqlBackup, + runPostgresBackup: mocks.runPostgresBackup, + runVolumeBackup: mocks.runVolumeBackup, + cleanupAll: mocks.cleanupAll, + CLEANUP_CRON_JOB: "0 0 * * *", +})); + +// Relative import to the real schedules source (resolved by vite). +import { runJobs } from "../../../schedules/src/utils"; + +const activeServer = { serverStatus: "active" } as const; + +describe("runJobs — backup enabled gate (G6)", () => { + beforeEach(() => { + mocks.findBackupById.mockReset(); + mocks.findServerById.mockReset(); + mocks.findScheduleById.mockReset(); + mocks.findVolumeBackupById.mockReset(); + for (const fn of [ + mocks.runPostgresBackup, + mocks.runMySqlBackup, + mocks.runMongoBackup, + mocks.runMariadbBackup, + mocks.runLibsqlBackup, + mocks.runComposeBackup, + mocks.runVolumeBackup, + mocks.keepLatestNBackups, + mocks.runCommand, + mocks.cleanupAll, + mocks.loggerInfo, + mocks.loggerError, + ]) { + fn.mockClear(); + } + mocks.findServerById.mockResolvedValue(activeServer); + }); + + it("skips a disabled (enabled=false) postgres backup without dispatching", async () => { + mocks.findBackupById.mockResolvedValue({ + enabled: false, + databaseType: "postgres", + backupType: "database", + postgres: { serverId: "s1" }, + }); + + await runJobs({ + type: "backup", + backupId: "b1", + cronSchedule: "*/2 * * * *", + }); + + expect(mocks.runPostgresBackup).not.toHaveBeenCalled(); + expect(mocks.keepLatestNBackups).not.toHaveBeenCalled(); + expect(mocks.loggerInfo).toHaveBeenCalledWith( + "Backup b1 is disabled; skipping", + ); + }); + + it("skips a backup whose enabled is null (treats as falsy)", async () => { + mocks.findBackupById.mockResolvedValue({ + enabled: null, + databaseType: "mysql", + backupType: "database", + mysql: { serverId: "s1" }, + }); + + await runJobs({ + type: "backup", + backupId: "b2", + cronSchedule: "*/2 * * * *", + }); + + expect(mocks.runMySqlBackup).not.toHaveBeenCalled(); + expect(mocks.keepLatestNBackups).not.toHaveBeenCalled(); + }); + + it("skips a disabled compose backup without dispatching", async () => { + mocks.findBackupById.mockResolvedValue({ + enabled: false, + backupType: "compose", + compose: { serverId: "s1" }, + }); + + await runJobs({ + type: "backup", + backupId: "b3", + cronSchedule: "0 0 * * *", + }); + + expect(mocks.runComposeBackup).not.toHaveBeenCalled(); + expect(mocks.keepLatestNBackups).not.toHaveBeenCalled(); + }); + + it("does not gate schedule jobs (already gated on schedule.enabled, unchanged behaviour)", async () => { + // schedule disabled → runCommand NOT called; schedule enabled → runCommand called. + mocks.findScheduleById.mockResolvedValue({ enabled: false }); + await runJobs({ + type: "schedule", + scheduleId: "s1", + cronSchedule: "*/2 * * * *", + }); + expect(mocks.runCommand).not.toHaveBeenCalled(); + + mocks.findScheduleById.mockResolvedValue({ enabled: true }); + await runJobs({ + type: "schedule", + scheduleId: "s2", + cronSchedule: "*/2 * * * *", + }); + expect(mocks.runCommand).toHaveBeenCalledTimes(1); + }); + + it("does not gate volume-backup jobs (already gated on volumeBackup.enabled)", async () => { + mocks.findVolumeBackupById.mockResolvedValue({ enabled: false }); + await runJobs({ + type: "volume-backup", + volumeBackupId: "v1", + cronSchedule: "*/2 * * * *", + }); + expect(mocks.runVolumeBackup).not.toHaveBeenCalled(); + }); +}); + +describe("runJobs — backup enabled dispatch still runs (G7)", () => { + beforeEach(() => { + mocks.findBackupById.mockReset(); + mocks.findServerById.mockReset(); + for (const fn of [ + mocks.runPostgresBackup, + mocks.runMySqlBackup, + mocks.runMongoBackup, + mocks.runMariadbBackup, + mocks.runLibsqlBackup, + mocks.runComposeBackup, + mocks.keepLatestNBackups, + mocks.loggerInfo, + ]) { + fn.mockClear(); + } + mocks.findServerById.mockResolvedValue(activeServer); + }); + + const cases: Array< + [string, Record, ReturnType] + > = [ + [ + "postgres", + { databaseType: "postgres", backupType: "database" }, + mocks.runPostgresBackup, + ], + [ + "mysql", + { databaseType: "mysql", backupType: "database" }, + mocks.runMySqlBackup, + ], + [ + "mongo", + { databaseType: "mongo", backupType: "database" }, + mocks.runMongoBackup, + ], + [ + "mariadb", + { databaseType: "mariadb", backupType: "database" }, + mocks.runMariadbBackup, + ], + [ + "libsql", + { databaseType: "libsql", backupType: "database" }, + mocks.runLibsqlBackup, + ], + [ + "compose", + { backupType: "compose" } as Record, + mocks.runComposeBackup, + ], + ]; + + for (const [label, over, runFn] of cases) { + it(`enabled=true ${label} backup dispatches to run*Backup + keepLatestNBackups`, async () => { + mocks.findBackupById.mockResolvedValue({ + enabled: true, + postgres: { serverId: "s1" }, + mysql: { serverId: "s1" }, + mongo: { serverId: "s1" }, + mariadb: { serverId: "s1" }, + libsql: { serverId: "s1" }, + compose: { serverId: "s1" }, + ...over, + } as any); + + await runJobs({ + type: "backup", + backupId: "b1", + cronSchedule: "*/2 * * * *", + }); + + expect(runFn).toHaveBeenCalledTimes(1); + expect(mocks.keepLatestNBackups).toHaveBeenCalledTimes(1); + }); + } +}); diff --git a/apps/dokploy/server/api/routers/backup.ts b/apps/dokploy/server/api/routers/backup.ts index 9bc73ce77..b7c4651e2 100644 --- a/apps/dokploy/server/api/routers/backup.ts +++ b/apps/dokploy/server/api/routers/backup.ts @@ -220,7 +220,7 @@ export const backupRouter = createTRPCRouter({ }); } else { await removeJob({ - cronSchedule: backup.schedule, + cronSchedule: existing.schedule, backupId: backup.backupId, type: "backup", }); diff --git a/apps/dokploy/server/api/routers/schedule.ts b/apps/dokploy/server/api/routers/schedule.ts index a7417a92c..f40f380e0 100644 --- a/apps/dokploy/server/api/routers/schedule.ts +++ b/apps/dokploy/server/api/routers/schedule.ts @@ -138,10 +138,10 @@ export const scheduleRouter = createTRPCRouter({ }); } else { await removeJob({ - cronSchedule: updatedSchedule.cronExpression, - scheduleId: updatedSchedule.scheduleId, + cronSchedule: existingSchedule.cronExpression, + scheduleId: existingSchedule.scheduleId, type: "schedule", - timezone: updatedSchedule.timezone, + timezone: existingSchedule.timezone, }); } } else { diff --git a/apps/dokploy/server/api/routers/volume-backups.ts b/apps/dokploy/server/api/routers/volume-backups.ts index 8bce3ef31..b6321b598 100644 --- a/apps/dokploy/server/api/routers/volume-backups.ts +++ b/apps/dokploy/server/api/routers/volume-backups.ts @@ -222,8 +222,8 @@ export const volumeBackupsRouter = createTRPCRouter({ }); } else { await removeJob({ - cronSchedule: updatedVolumeBackup.cronExpression, - volumeBackupId: updatedVolumeBackup.volumeBackupId, + cronSchedule: existingVb.cronExpression, + volumeBackupId: existingVb.volumeBackupId, type: "volume-backup", }); } diff --git a/apps/schedules/src/utils.ts b/apps/schedules/src/utils.ts index 85de85f9a..31a8feea7 100644 --- a/apps/schedules/src/utils.ts +++ b/apps/schedules/src/utils.ts @@ -33,6 +33,10 @@ export const runJobs = async (job: QueueJob) => { if (job.type === "backup") { const { backupId } = job; const backup = await findBackupById(backupId); + if (!backup.enabled) { + logger.info(`Backup ${backupId} is disabled; skipping`); + return; + } const { databaseType, postgres,