scope stack backup snapshots to their own environment
This commit is contained in:
3 files changed
+137
-5
No files matched your search
@@ -388,3 +388,49 @@ export async function fetchBackupExecutions(
|
||||
}));
|
||||
return computeExecutionTally(all);
|
||||
}
|
||||
|
||||
/**
|
||||
* Does a snapshot (by its restic tags) belong to the stack/container Backups view for a
|
||||
* given set of ENV-SCOPED config ids? A same-name stack on another environment writes to
|
||||
* the same shared repo and carries the same `dockhand:name=<target>` tag, so filtering by
|
||||
* name alone mixed snapshots (and "latest") across environments (#1546). `configIds` are
|
||||
* already env-scoped by the caller and `dockhand:configid` uniquely identifies a
|
||||
* (target, env) config, so we match on that. When no config ids are known (should not
|
||||
* happen once a target has any backup config) fall back to the name tag rather than hide
|
||||
* everything. Pure + unit-testable.
|
||||
*/
|
||||
export function snapshotMatchesConfigScope(
|
||||
tags: string[] | undefined,
|
||||
configIds: number[],
|
||||
targetName: string
|
||||
): boolean {
|
||||
const t = tags ?? [];
|
||||
if (configIds.length > 0) {
|
||||
for (const tag of t) {
|
||||
if (tag.startsWith('dockhand:configid=')) {
|
||||
const n = Number(tag.slice('dockhand:configid='.length));
|
||||
if (Number.isInteger(n) && configIds.includes(n)) return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
return t.includes(`dockhand:name=${targetName}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Pick THIS target's snapshots from one destination's raw restic list and stamp each with
|
||||
* the destination it came from - the exact filter+map the Backups panel applies per
|
||||
* destination. Extracted so the wired call site (not just snapshotMatchesConfigScope in
|
||||
* isolation) is covered by a test: a regression back to a name-only filter changes this
|
||||
* helper's output and fails the suite (#1546).
|
||||
*/
|
||||
export function selectOwnSnapshotsFromDestination<S extends { tags?: string[] }>(
|
||||
snaps: S[],
|
||||
dest: { id: number; name: string; repository: string },
|
||||
configIds: number[],
|
||||
targetName: string
|
||||
): (S & { _destinationId: number; _destinationName: string; _destinationRepository: string })[] {
|
||||
return snaps
|
||||
.filter((s) => snapshotMatchesConfigScope(s.tags, configIds, targetName))
|
||||
.map((s) => ({ ...s, _destinationId: dest.id, _destinationName: dest.name, _destinationRepository: dest.repository }));
|
||||
}
|
||||
@@ -7,7 +7,7 @@
|
||||
import { formatDateTime, formatRelativeTime } from '$lib/stores/settings';
|
||||
import { formatBytes } from '$lib/utils/format';
|
||||
import { readJobResponse } from '$lib/utils/sse-fetch';
|
||||
import { getRepoTypeIcon } from '$lib/utils/backup';
|
||||
import { getRepoTypeIcon, selectOwnSnapshotsFromDestination } from '$lib/utils/backup';
|
||||
import SnapshotBrowser from './SnapshotBrowser.svelte';
|
||||
import RestoreModal from './RestoreModal.svelte';
|
||||
import SnapshotDiffModal from '../backups/SnapshotDiffModal.svelte';
|
||||
@@ -102,7 +102,11 @@
|
||||
try {
|
||||
const destRes = await fetch('/api/backup/destinations');
|
||||
const destinations: { id: number; name: string; repository: string }[] = destRes.ok ? await destRes.json() : [];
|
||||
const nameTag = `dockhand:name=${targetName}`;
|
||||
// Scope to THIS env's configs, not just the name: same-name stacks on different
|
||||
// environments write to a shared repo and all carry dockhand:name=<stack>, so a
|
||||
// name-only filter mixed their snapshots (and "latest") across envs (#1546).
|
||||
// configIds is env-scoped (BackupPanel fetches configs with ?env=), so a snapshot's
|
||||
// own dockhand:configid tag uniquely identifies (stack, env). See snapshotMatchesConfigScope.
|
||||
const all: Snapshot[] = [];
|
||||
loadDone = 0;
|
||||
loadTotal = destinations.length;
|
||||
@@ -119,9 +123,7 @@
|
||||
const data = await readJobResponse(res);
|
||||
if (data?.error) return;
|
||||
const snaps: any[] = data.snapshots ?? data;
|
||||
const mine = snaps
|
||||
.filter((s) => (s.tags || []).some((t: string) => t === nameTag))
|
||||
.map((s) => ({ ...s, _destinationId: dest.id, _destinationName: dest.name, _destinationRepository: dest.repository }));
|
||||
const mine = selectOwnSnapshotsFromDestination(snaps, dest, configIds, targetName);
|
||||
if (mine.length) {
|
||||
all.push(...mine);
|
||||
all.sort((a, b) => new Date(b.time).getTime() - new Date(a.time).getTime());
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
/**
|
||||
* snapshotMatchesConfigScope: the stack/container Backups view must show only THIS
|
||||
* env's snapshots. Same-name stacks on different envs share one repo and all carry
|
||||
* `dockhand:name=<stack>`, so a name-only filter leaked snapshots (and "latest")
|
||||
* across environments (#1546). Scoping by the env-scoped configid tag fixes it.
|
||||
*/
|
||||
import { describe, it, expect } from 'bun:test';
|
||||
import { snapshotMatchesConfigScope, selectOwnSnapshotsFromDestination } from '../src/lib/utils/backup';
|
||||
|
||||
const tags = (name: string, configid: number, envid: number | 'local') => [
|
||||
'dockhand:instance=inst-1',
|
||||
`dockhand:configid=${configid}`,
|
||||
`dockhand:envid=${envid}`,
|
||||
'dockhand:type=stack',
|
||||
`dockhand:name=${name}`,
|
||||
];
|
||||
|
||||
describe('snapshotMatchesConfigScope', () => {
|
||||
it('keeps a snapshot whose configid is in this env’s config set', () => {
|
||||
expect(snapshotMatchesConfigScope(tags('caddy', 1, 1), [1], 'caddy')).toBe(true);
|
||||
});
|
||||
|
||||
it('DROPS a same-name snapshot from another env (different configid) - the #1546 leak', () => {
|
||||
// env-1 view has config 1; a caddy snapshot from env-2 (config 5) must not show.
|
||||
expect(snapshotMatchesConfigScope(tags('caddy', 5, 2), [1], 'caddy')).toBe(false);
|
||||
});
|
||||
|
||||
it('matches any of several env-scoped configs (a stack with configs on >1 destination)', () => {
|
||||
expect(snapshotMatchesConfigScope(tags('caddy', 8, 3), [7, 8, 9], 'caddy')).toBe(true);
|
||||
expect(snapshotMatchesConfigScope(tags('caddy', 5, 2), [7, 8, 9], 'caddy')).toBe(false);
|
||||
});
|
||||
|
||||
it('name tag alone is NOT enough once configIds are known', () => {
|
||||
// Same name, but its configid (99) is not one of ours -> excluded despite the name match.
|
||||
expect(snapshotMatchesConfigScope(tags('caddy', 99, 4), [1], 'caddy')).toBe(false);
|
||||
});
|
||||
|
||||
it('falls back to the name tag when no configIds are known (defensive, pre-config)', () => {
|
||||
expect(snapshotMatchesConfigScope(tags('caddy', 1, 1), [], 'caddy')).toBe(true);
|
||||
expect(snapshotMatchesConfigScope(tags('other', 1, 1), [], 'caddy')).toBe(false);
|
||||
});
|
||||
|
||||
it('handles missing/empty tags without throwing', () => {
|
||||
expect(snapshotMatchesConfigScope(undefined, [1], 'caddy')).toBe(false);
|
||||
expect(snapshotMatchesConfigScope([], [1], 'caddy')).toBe(false);
|
||||
expect(snapshotMatchesConfigScope([], [], 'caddy')).toBe(false);
|
||||
});
|
||||
|
||||
it('ignores a configid substring/prefix false match', () => {
|
||||
// configIds=[1] must NOT match dockhand:configid=15 or =10
|
||||
expect(snapshotMatchesConfigScope(tags('caddy', 15, 1), [1], 'caddy')).toBe(false);
|
||||
expect(snapshotMatchesConfigScope(tags('caddy', 10, 1), [1], 'caddy')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// The wired filter the Backups panel actually applies per destination. Testing THIS
|
||||
// (not just snapshotMatchesConfigScope in isolation) makes the panel's call site
|
||||
// load-bearing: a regression back to a name-only filter fails here (#1546 review).
|
||||
describe('selectOwnSnapshotsFromDestination', () => {
|
||||
const dest = { id: 7, name: 'Restic Server', repository: 'rest:http://repo' };
|
||||
|
||||
it('keeps only this env-scoped config’s snapshots and stamps the destination', () => {
|
||||
const snaps = [
|
||||
{ id: 'a', tags: tags('caddy', 1, 1) }, // env A (config 1)
|
||||
{ id: 'b', tags: tags('caddy', 5, 2) }, // env B (config 5) - same name, must be dropped
|
||||
];
|
||||
const mine = selectOwnSnapshotsFromDestination(snaps, dest, [1], 'caddy');
|
||||
expect(mine.map((s) => s.id)).toEqual(['a']); // NOT 'b' - the #1546 leak is closed
|
||||
expect(mine[0]._destinationId).toBe(7);
|
||||
expect(mine[0]._destinationName).toBe('Restic Server');
|
||||
expect(mine[0]._destinationRepository).toBe('rest:http://repo');
|
||||
});
|
||||
|
||||
it('a name-only filter would have kept both - this asserts we do NOT', () => {
|
||||
const snaps = [
|
||||
{ id: 'a', tags: tags('caddy', 1, 1) },
|
||||
{ id: 'b', tags: tags('caddy', 5, 2) },
|
||||
];
|
||||
// name-only (the old, buggy behaviour) keeps both; the wired helper keeps one.
|
||||
const nameOnly = snaps.filter((s) => s.tags.includes('dockhand:name=caddy'));
|
||||
expect(nameOnly.length).toBe(2);
|
||||
expect(selectOwnSnapshotsFromDestination(snaps, dest, [1], 'caddy').length).toBe(1);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user