pausing or resuming a backup schedule no longer clears the retention policy

This commit is contained in:
Jarek Krochmalski committed 2026-08-27 07:34:37 +02:00
1 parent e82b1aea89
commit c7eb8542a1
3 files changed
+63 -6

No files matched your search

+19
View File
@@ -503,6 +503,25 @@ export function retentionToStore(retention: any, schedule: unknown): string | nu
return scheduled ? JSON.stringify(DEFAULT_SCHEDULED_RETENTION) : null;
}
/**
* Resolve the retention value to persist on a PARTIAL config update. `undefined` means
* "leave the stored retention untouched" (the caller must skip the field). A minimal PUT
* that only flips `enabled` (pause/resume) omits `retention`, so re-applying the scheduled
* default here would silently wipe the user's policy (#1462). The default is applied ONLY
* when the retention is actually provided, or when the schedule is being added/changed -
* the transition the default exists to cover.
*/
export function resolveRetentionForUpdate(
retention: unknown,
newSchedule: unknown,
existingSchedule: unknown
): string | null | undefined {
const retentionProvided = retention !== undefined;
const scheduleChanged = newSchedule !== undefined && newSchedule !== existingSchedule;
if (!retentionProvided && !scheduleChanged) return undefined; // leave stored value as-is
return retentionToStore(retention, newSchedule ?? existingSchedule);
}
/**
* Decide the `enabled` flag when a backup config is updated. A "run once" backup persists a
* MANUAL, paused config (schedule=null, enabled=false); adding a cron schedule later should make
@@ -8,7 +8,7 @@ import {
} from '$lib/server/db';
import { registerSchedule, unregisterSchedule, isValidCron } from '$lib/server/scheduler';
import { isBackupRunning } from '$lib/server/backups';
import { validateRetention, retentionToStore, resolveEnabledOnScheduleChange } from '$lib/server/backups/helpers';
import { validateRetention, resolveRetentionForUpdate, resolveEnabledOnScheduleChange } from '$lib/server/backups/helpers';
import { requireBackups, loadConfigGateEnv } from '$lib/server/backups/route-guards';
/**
@@ -99,11 +99,11 @@ export const PUT: RequestHandler = async (event) => {
selectedVolumes: body.selectedVolumes ? JSON.stringify(body.selectedVolumes) : body.selectedVolumes,
stopBeforeBackup: body.stopBeforeBackup,
schedule: body.schedule,
// Apply the default scheduled retention the same way create does, so a
// config edited to add a schedule with no explicit retention doesn't end
// up with pruning disabled and an unbounded-growth repo. Uses the effective
// schedule (the incoming one, or the existing one if unchanged).
retention: retentionToStore(body.retention, body.schedule ?? existing.schedule),
// Leave the stored retention untouched when the client didn't send it and the
// schedule isn't changing - a minimal pause/resume PUT (only `enabled`) must not
// wipe the user's policy (#1462). The scheduled default is still applied when
// retention is provided or a schedule is being added/changed.
retention: resolveRetentionForUpdate(body.retention, body.schedule, existing.schedule),
options: body.options ? JSON.stringify(body.options) : body.options,
tags: body.tags ? JSON.stringify(body.tags) : body.tags
});
+38
View File
@@ -16,6 +16,7 @@ import {
isRepoNotInitializedError,
wouldDeleteAllSnapshots,
retentionToStore,
resolveRetentionForUpdate,
validateFlags,
validatePolicySchedules,
validateRetention,
@@ -691,6 +692,43 @@ describe('retentionToStore', () => {
});
});
// ============================================================================
// resolveRetentionForUpdate: partial-update guard. A minimal PUT that omits
// retention (e.g. pause/resume, which only sends `enabled`) must NOT re-apply
// the scheduled default and wipe the stored policy (#1462).
// ============================================================================
describe('resolveRetentionForUpdate', () => {
const SCHED = '0 4 * * *';
test('retention omitted and schedule unchanged -> undefined (leave stored value as-is)', () => {
// The pause/resume case: body has no retention, schedule not in the body.
expect(resolveRetentionForUpdate(undefined, undefined, SCHED)).toBeUndefined();
});
test('retention omitted but the SAME schedule resent -> still leaves it alone', () => {
expect(resolveRetentionForUpdate(undefined, SCHED, SCHED)).toBeUndefined();
});
test('an explicit retention is stored verbatim', () => {
expect(resolveRetentionForUpdate({ keepYearly: 3 }, undefined, SCHED)).toBe(
JSON.stringify({ keepYearly: 3 })
);
});
test('adding a schedule with no retention applies the scheduled default', () => {
// none -> cron transition: the default-injection the helper exists for.
expect(resolveRetentionForUpdate(undefined, SCHED, null)).toBe(
JSON.stringify({ keepDaily: 7, keepWeekly: 4, keepMonthly: 6 })
);
});
test('clearing to all-zero on a SCHEDULED config falls back to the default (no unbounded growth)', () => {
// retention is provided (so not skipped), but empty; with a schedule present the
// scheduled default applies rather than storing "no policy".
expect(resolveRetentionForUpdate({ keepLast: 0 }, undefined, SCHED)).toBe(
JSON.stringify({ keepDaily: 7, keepWeekly: 4, keepMonthly: 6 })
);
});
test('clearing to all-zero on an UNSCHEDULED config stores null', () => {
expect(resolveRetentionForUpdate({ keepLast: 0 }, undefined, null)).toBeNull();
});
});
// ============================================================================
// validateFlags: save-time wrapper over sanitizeResticFlags — surfaces the
// throw as an error string (or null when the flags are acceptable / absent).