From 15c367eeb86dc0d0de25772f993db167de1fdccc Mon Sep 17 00:00:00 2001 From: Nico <47644445+nicotsx@users.noreply.github.com> Date: Sat, 13 Jun 2026 10:29:36 +0200 Subject: [PATCH] feat(copy): pass custom restic params (#976) Restic copy (mirror) should inherit the compatible custom flags from the parent backup setup Related issue #960 --- .../__tests__/backups.mirror-sync.test.ts | 35 +++++++- .../backups.service.execution.test.ts | 29 +++++++ .../backups/helpers/backup-maintenance.ts | 2 + .../restic/commands/__tests__/copy.test.ts | 22 +++++ packages/core/src/restic/commands/copy.ts | 17 +++- .../__tests__/validate-custom-params.test.ts | 28 ++++++- .../restic/helpers/validate-custom-params.ts | 82 +++++++++++++++---- 7 files changed, 195 insertions(+), 20 deletions(-) diff --git a/app/server/modules/backups/__tests__/backups.mirror-sync.test.ts b/app/server/modules/backups/__tests__/backups.mirror-sync.test.ts index d5c01fdf..117f5bdd 100644 --- a/app/server/modules/backups/__tests__/backups.mirror-sync.test.ts +++ b/app/server/modules/backups/__tests__/backups.mirror-sync.test.ts @@ -152,7 +152,8 @@ describe("getMirrorSyncStatus", () => { describe("syncMirror", () => { test("should trigger sync and return success", async () => { - setup(); + const { mockCopy } = setup(); + mockCopy(); const volume = await createTestVolume(); const repository = await createTestRepository(); const mirrorRepository = await createTestRepository(); @@ -170,6 +171,38 @@ describe("syncMirror", () => { expect(result.success).toBe(true); }); + test("should pass custom restic params to manual mirror sync", async () => { + const { mockCopy } = setup(); + const copyMock = mockCopy(); + const volume = await createTestVolume(); + const repository = await createTestRepository(); + const mirrorRepository = await createTestRepository(); + const schedule = await createTestBackupSchedule({ + volumeId: volume.id, + repositoryId: repository.id, + customResticParams: ["--pack-size 64", "--ignore-inode"], + }); + await createTestBackupScheduleMirror(schedule.id, mirrorRepository.id); + + const result = await backupsService.syncMirror(schedule.shortId, mirrorRepository.shortId as ShortId, [ + "snap1", + ]); + + expect(result.success).toBe(true); + await waitForExpect(() => { + expect(copyMock).toHaveBeenCalledWith( + repository.config, + mirrorRepository.config, + expect.objectContaining({ + tag: schedule.shortId, + organizationId: TEST_ORG_ID, + snapshotIds: ["snap1"], + customResticParams: ["--pack-size 64", "--ignore-inode"], + }), + ); + }); + }); + test("should reject if mirror is already syncing", async () => { setup(); const volume = await createTestVolume(); diff --git a/app/server/modules/backups/__tests__/backups.service.execution.test.ts b/app/server/modules/backups/__tests__/backups.service.execution.test.ts index f72eb062..61036ed3 100644 --- a/app/server/modules/backups/__tests__/backups.service.execution.test.ts +++ b/app/server/modules/backups/__tests__/backups.service.execution.test.ts @@ -1179,6 +1179,35 @@ describe("mirror operations", () => { ); }); + test("should pass custom restic params to mirror copy", async () => { + // arrange + const { resticCopyMock } = setup(); + const volume = await createTestVolume(); + const sourceRepository = await createTestRepository(); + const mirrorRepository = await createTestRepository(); + const schedule = await createTestBackupSchedule({ + volumeId: volume.id, + repositoryId: sourceRepository.id, + customResticParams: ["--pack-size 64", "--ignore-inode"], + }); + + await createTestBackupScheduleMirror(schedule.id, mirrorRepository.id); + + // act + await backupsService.copyToMirrors(schedule.id, sourceRepository, null); + + // assert + expect(resticCopyMock).toHaveBeenCalledWith( + sourceRepository.config, + mirrorRepository.config, + expect.objectContaining({ + tag: schedule.shortId, + organizationId: TEST_ORG_ID, + customResticParams: ["--pack-size 64", "--ignore-inode"], + }), + ); + }); + test("should skip disabled mirrors", async () => { // arrange const { resticCopyMock } = setup(); diff --git a/app/server/modules/backups/helpers/backup-maintenance.ts b/app/server/modules/backups/helpers/backup-maintenance.ts index 1416cc9b..6236db0e 100644 --- a/app/server/modules/backups/helpers/backup-maintenance.ts +++ b/app/server/modules/backups/helpers/backup-maintenance.ts @@ -125,6 +125,7 @@ export async function syncSnapshotsToMirror( tag: schedule.shortId, organizationId, snapshotIds, + customResticParams: schedule.customResticParams ?? [], }), ); cache.delByPrefix(cacheKeys.repository.all(mirrorRepository.id)); @@ -214,6 +215,7 @@ async function copyToSingleMirror( restic.copy(sourceRepository.config, mirror.repository.config, { tag: schedule.shortId, organizationId, + customResticParams: schedule.customResticParams ?? [], }), ); cache.delByPrefix(cacheKeys.repository.all(mirror.repository.id)); diff --git a/packages/core/src/restic/commands/__tests__/copy.test.ts b/packages/core/src/restic/commands/__tests__/copy.test.ts index 0c21392f..3ff19dcb 100644 --- a/packages/core/src/restic/commands/__tests__/copy.test.ts +++ b/packages/core/src/restic/commands/__tests__/copy.test.ts @@ -74,6 +74,28 @@ describe("copy command", () => { expect(getArgs().slice(separatorIndex + 1)).toEqual(["latest"]); }); + test("passes only copy-compatible custom restic params", async () => { + const { getArgs } = setup(); + + await Effect.runPromise( + copy( + sourceConfig, + destConfig, + { + organizationId: "org-1", + tag: "daily", + customResticParams: ["--pack-size 64", "--ignore-inode", "--no-cache"], + }, + mockDeps, + ), + ); + + expect(getArgs()).toContain("--pack-size"); + expect(getArgs()).toContain("64"); + expect(getArgs()).toContain("--no-cache"); + expect(getArgs()).not.toContain("--ignore-inode"); + }); + test("passes multiple snapshot IDs after separator", async () => { const { getArgs } = setup(); diff --git a/packages/core/src/restic/commands/copy.ts b/packages/core/src/restic/commands/copy.ts index 88a21f3c..214aa5fa 100644 --- a/packages/core/src/restic/commands/copy.ts +++ b/packages/core/src/restic/commands/copy.ts @@ -3,6 +3,7 @@ import { addCommonArgs } from "../helpers/add-common-args"; import { buildEnv } from "../helpers/build-env"; import { buildRepoUrl } from "../helpers/build-repo-url"; import { cleanupTemporaryKeys } from "../helpers/cleanup-temporary-keys"; +import { getCopyCompatibleCustomResticParams } from "../helpers/validate-custom-params"; import type { RepositoryConfig } from "../schemas"; import { createResticError, isResticError } from "../error"; import { logger, safeExec } from "../../node"; @@ -18,7 +19,12 @@ class ResticCopyCommandError extends Data.TaggedError("ResticCopyCommandError")< export const copy = ( sourceConfig: RepositoryConfig, destConfig: RepositoryConfig, - options: { organizationId: string; tag?: string; snapshotIds?: string[] }, + options: { + organizationId: string; + tag?: string; + snapshotIds?: string[]; + customResticParams?: string[]; + }, deps: ResticDeps, ) => { return Effect.tryPromise({ @@ -41,6 +47,15 @@ export const copy = ( args.push("--tag", options.tag); } + if (options.customResticParams?.length) { + const customResticParams = getCopyCompatibleCustomResticParams(options.customResticParams); + + for (const param of customResticParams) { + const tokens = param.trim().split(/\s+/).filter(Boolean); + args.push(...tokens); + } + } + addCommonArgs(args, env, destConfig, { skipBandwidth: true }); const sourceDownloadLimit = formatBandwidthLimit(sourceConfig.downloadLimit); diff --git a/packages/core/src/restic/helpers/__tests__/validate-custom-params.test.ts b/packages/core/src/restic/helpers/__tests__/validate-custom-params.test.ts index 49a0c96c..44422620 100644 --- a/packages/core/src/restic/helpers/__tests__/validate-custom-params.test.ts +++ b/packages/core/src/restic/helpers/__tests__/validate-custom-params.test.ts @@ -1,6 +1,6 @@ import fc from "fast-check"; import { describe, expect, test } from "vitest"; -import { validateCustomResticParams } from "../validate-custom-params"; +import { getCopyCompatibleCustomResticParams, validateCustomResticParams } from "../validate-custom-params"; const supportedFlagsWithoutValues = [ "--verbose", @@ -41,7 +41,11 @@ const validCustomParamArb = fc.oneof( .tuple(fc.constantFrom(...positiveIntegerFlags), fc.integer({ min: 1, max: 100_000 }), fc.boolean()) .map(([flag, value, inline]) => (inline ? `${flag}=${value}` : `${flag} ${value}`)), fc - .tuple(fc.integer({ min: 1, max: 100_000 }), fc.constantFrom("", "K", "M", "G", "T", "KiB", "MiB"), fc.boolean()) + .tuple( + fc.integer({ min: 1, max: 100_000 }), + fc.constantFrom("", "K", "M", "G", "T", "KiB", "MiB"), + fc.boolean(), + ) .map(([value, suffix, inline]) => inline ? `--exclude-larger-than=${value}${suffix}` : `--exclude-larger-than ${value}${suffix}`, ), @@ -67,6 +71,26 @@ describe("validateCustomResticParams", () => { expect(result).toBeNull(); }); + test("filters custom params down to flags supported by copy", () => { + const result = getCopyCompatibleCustomResticParams([ + "--pack-size 64", + "--ignore-inode", + "--no-cache --exclude-caches", + "--limit-upload=2048", + "--read-concurrency 4", + "-v", + "--no-lock", + ]); + + expect(result).toEqual(["--pack-size 64", "--no-cache", "--limit-upload=2048", "-v", "--no-lock"]); + }); + + test("validates skipped backup-only flags while filtering copy params", () => { + expect(() => getCopyCompatibleCustomResticParams(["--read-concurrency"])).toThrow( + 'Flag "--read-concurrency" requires a value', + ); + }); + test("rejects positional arguments", () => { const result = validateCustomResticParams(["/etc"]); diff --git a/packages/core/src/restic/helpers/validate-custom-params.ts b/packages/core/src/restic/helpers/validate-custom-params.ts index 20ab1884..031de928 100644 --- a/packages/core/src/restic/helpers/validate-custom-params.ts +++ b/packages/core/src/restic/helpers/validate-custom-params.ts @@ -44,8 +44,25 @@ const FLAG_SPECS = new Map([ ["--no-lock", { requiresValue: false }], ]); +const COPY_COMPATIBLE_FLAGS = new Set([ + "--verbose", + "-v", + "--no-cache", + "--cleanup-cache", + "--limit-upload", + "--limit-download", + "--pack-size", + "--no-lock", +]); + +const SUPPORTED_FLAGS = new Set(FLAG_SPECS.keys()); const ALLOWED_FLAGS = [...FLAG_SPECS.keys()].join(", "); +type CollectCustomResticParamsOptions = { + allowedFlags?: Set; + skipDisallowed?: boolean; +}; + function parseFlagToken(token: string) { const eqIdx = token.indexOf("="); @@ -66,7 +83,12 @@ function validateFlagValue(flag: string, value: string, spec: FlagSpec): string return spec.validateValue?.(value) ?? null; } -export function validateCustomResticParams(params: string[]): string | null { +function collectCustomResticParams( + params: string[], + { allowedFlags = SUPPORTED_FLAGS, skipDisallowed = false }: CollectCustomResticParamsOptions = {}, +) { + const collectedParams: string[] = []; + for (const param of params) { const tokens = param.trim().split(/\s+/).filter(Boolean); @@ -88,34 +110,62 @@ export function validateCustomResticParams(params: string[]): string | null { return `Unknown or unsupported flag "${flag}" in customResticParams. Permitted flags: ${ALLOWED_FLAGS}`; } + let collectedParam = token; + if (!spec.requiresValue) { if (inlineValue !== null && inlineValue !== "") { return `Flag "${flag}" does not accept a value`; } - continue; - } - - if (inlineValue !== null) { + } else if (inlineValue !== null) { const error = validateFlagValue(flag, inlineValue, spec); if (error) { return error; } - continue; + } else { + const nextToken = tokens[index + 1]; + if (!nextToken) { + return `Flag "${flag}" requires a value`; + } + + const error = validateFlagValue(flag, nextToken, spec); + if (error) { + return error; + } + + collectedParam = `${token} ${nextToken}`; + index += 1; } - const nextToken = tokens[index + 1]; - if (!nextToken) { - return `Flag "${flag}" requires a value`; + if (!allowedFlags.has(flag)) { + if (skipDisallowed) { + continue; + } + + return `Unknown or unsupported flag "${flag}" in customResticParams. Permitted flags: ${ALLOWED_FLAGS}`; } - const error = validateFlagValue(flag, nextToken, spec); - if (error) { - return error; - } - - index += 1; + collectedParams.push(collectedParam); } } - return null; + return collectedParams; +} + +export function validateCustomResticParams(params: string[]): string | null { + const result = collectCustomResticParams(params); + + return typeof result === "string" ? result : null; +} + +export function getCopyCompatibleCustomResticParams(params: string[]): string[] { + const result = collectCustomResticParams(params, { + allowedFlags: COPY_COMPATIBLE_FLAGS, + skipDisallowed: true, + }); + + if (typeof result === "string") { + throw new Error(result); + } + + return result; }