Author SHA1 Message Date
sneak 60b71dd3e9 Count a file as deleted only if the previous snapshot of its name had it (closes #304)
check / check (push) Waiting to run
A backup counted as deleted every file in the local index under the
source path that the walk did not find. That took in files the
snapshot excludes but another snapshot backs up, and files removed long
ago, which every later backup, and the first backup of another name
over the same path, reported as deleted again.

The backup now looks up the most recent completed snapshot of the same
name and counts only the files it lists that the walk did not find and
that are not excluded.

The snapshots row stores no deleted figure; only the backup's output
changes.

Model: opus-5-5
2026-10-13 04:18:14 +00:00
clawbot 167c0a0746 Make snapshot remove fail for an unknown snapshot ID (closes #293)
check / check (push) Waiting to run
snapshot remove never checked that the ID existed: it deleted nothing,
reported the snapshot removed from the local database and from the
destination store, and exited 0, under --json, --dry-run and
--local-only too. It now looks the ID up in the local index and, only
when that has no row and --local-only is not set, on the destination
store; finding neither is an error. The command reports a removal from
the local database only when the local index had the row, and
remote_removed is true only when metadata was actually deleted.

Judgement call: under --local-only the destination store is not
consulted, so the snapshot must be in the local index.

Model: opus-5-5
2026-10-13 05:55:22 +02:00
13 changed files with 586 additions and 149 deletions
+4 -4
View File
@@ -279,8 +279,8 @@ Key methods:
- `Scan(ctx, path, snapshotID)` → Main scan operation - `Scan(ctx, path, snapshotID)` → Main scan operation
Scan phases: Scan phases:
1. **Phase 0**: Detect deleted files from previous snapshots 1. **Phase 1**: Walk directory, identify files needing processing
2. **Phase 1**: Walk directory, identify files needing processing 2. **Phase 1b**: Count as deleted the files that the previous snapshot of the same name lists and the walk did not find; an excluded file is not counted
3. **Phase 2**: Process files (chunk → pack → upload) 3. **Phase 2**: Process files (chunk → pack → upload)
#### SnapshotManager #### SnapshotManager
@@ -320,13 +320,13 @@ CreateSnapshot(opts)
│ │ │ │
│ ├─► scanner.Scan(ctx, path, snapshotID) │ ├─► scanner.Scan(ctx, path, snapshotID)
│ │ │ │ │ │
│ │ ├─► Phase 0: detectDeletedFiles()
│ │ │
│ │ ├─► Phase 1: scanPhase() │ │ ├─► Phase 1: scanPhase()
│ │ │ Walk directory │ │ │ Walk directory
│ │ │ Check file metadata changes │ │ │ Check file metadata changes
│ │ │ Build list of files to process │ │ │ Build list of files to process
│ │ │ │ │ │
│ │ ├─► Phase 1b: detectDeletedFiles()
│ │ │
│ │ └─► Phase 2: processPhase() │ │ └─► Phase 2: processPhase()
│ │ For each file: │ │ For each file:
│ │ chunker.ChunkReaderStreaming() │ │ chunker.ChunkReaderStreaming()
+5 -3
View File
@@ -349,9 +349,11 @@ the backup destination store. Blobs are NOT touched — deleting blobs
requires reading every remaining remote manifest (the destination store requires reading every remaining remote manifest (the destination store
may hold snapshots this host doesn't know about), which is what may hold snapshots this host doesn't know about), which is what
`vaultik prune` does. On success the command prints the exact `vaultik `vaultik prune` does. On success the command prints the exact `vaultik
prune` invocation to run as a follow-up. Local row cleanup (files, prune` invocation to run as a follow-up. An ID that names no snapshot in
chunks, blobs the snapshot was the last referrer for) runs the local index or on the destination store is an error; with
automatically. If the destination store is unreachable, the local-DB `--local-only`, the snapshot must be in the local index. Local row
cleanup (files, chunks, blobs the snapshot was the last referrer for)
runs automatically. If the destination store is unreachable, the local-DB
removal still completes and a warning is emitted; run `vaultik snapshot removal still completes and a warning is emitted; run `vaultik snapshot
remove <snapshot-id>` again once the store is reachable to remove the remove <snapshot-id>` again once the store is reachable to remove the
snapshot's metadata from it (`vaultik prune` does not). To wipe everything snapshot's metadata from it (`vaultik prune` does not). To wipe everything
+21 -8
View File
@@ -22,14 +22,27 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps # Completed Steps
- 2026-10-13: Made an exclude pattern starting with `**/` also match - 2026-10-13: Counted a file as deleted only when the previous snapshot
directly under the snapshot path of the same name had it
([issue #303](https://git.eeqj.de/sneak/vaultik/issues/303)). ([issue #304](https://git.eeqj.de/sneak/vaultik/issues/304)). The
`**/node_modules` and `**/*.pyc`, the patterns `config.example.yml` deleted figure of a backup counted every file in the local index under
uses, excluded `sub/node_modules` and `sub/y.pyc` but backed up the source path that the walk did not find. That took in files the
`node_modules/` and `x.pyc` at the top of the path, since the pattern snapshot excludes but another snapshot backs up, and files deleted
required a `/` before the name. As in gitignore, such a pattern now long ago, which every later backup, and the first backup of another
matches at every depth, the top included. name over the same path, reported as deleted again. A file is now
counted only when the previous completed snapshot of the same name
lists it and the walk did not find it, and an excluded file is never
counted.
- 2026-10-13: Made `snapshot remove` fail for a snapshot that does not
exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)).
The command never checked the ID: it reported the snapshot removed
from the local database and from the destination store and exited 0,
under `--json`, `--dry-run` and `--local-only` too. An ID that names
no snapshot in the local index or on the destination store is now an
error; with `--local-only`, the snapshot must be in the local index.
The command now reports a removal from either side only when one
happened, and `remote_removed` is true only when metadata was deleted.
- 2026-10-12: Bound the local index to the destination an `s3.*` config - 2026-10-12: Bound the local index to the destination an `s3.*` config
names ([issue #298](https://git.eeqj.de/sneak/vaultik/issues/298)). names ([issue #298](https://git.eeqj.de/sneak/vaultik/issues/298)).
+5 -1
View File
@@ -255,8 +255,12 @@ manifest (the destination store may hold snapshots this host doesn't
know about), which is what 'vaultik prune' does. On success the command know about), which is what 'vaultik prune' does. On success the command
prints the exact 'vaultik prune' invocation to run as a follow-up. prints the exact 'vaultik prune' invocation to run as a follow-up.
An ID that names no snapshot in the local index or on the destination
store is an error.
Use --local-only to skip the remote half (e.g. when you want to forget a Use --local-only to skip the remote half (e.g. when you want to forget a
snapshot locally without touching the destination store). snapshot locally without touching the destination store); the snapshot
must then be in the local index.
If the remote is unreachable, the local-database removal still completes If the remote is unreachable, the local-database removal still completes
and a warning is emitted; run 'vaultik snapshot remove <snapshot-id>' again and a warning is emitted; run 'vaultik snapshot remove <snapshot-id>' again
+44
View File
@@ -277,6 +277,50 @@ func (r *FileRepository) ListUnderPath(
return files, rows.Err() return files, rows.Err()
} }
// ListUnderPathInSnapshot returns the files that ListUnderPath returns
// for path and that the snapshot lists, in no particular order.
func (r *FileRepository) ListUnderPathInSnapshot(
ctx context.Context, path, snapshotID string,
) ([]*File, error) {
path = strings.TrimRight(path, "/")
dirPrefix := path + "/"
query := `
SELECT f.id, f.path, f.source_path, f.mtime, f.mtime_nsec, f.size,
f.mode, f.uid, f.gid, f.link_target
FROM files f
JOIN snapshot_files sf ON sf.file_id = f.id
WHERE sf.snapshot_id = ?
AND (f.path = ? OR substr(f.path, 1, length(?)) = ?)
`
rows, err := r.db.conn.QueryContext(ctx, query,
snapshotID, path, dirPrefix, dirPrefix)
if err != nil {
return nil, fmt.Errorf("querying files: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
var files []*File
for rows.Next() {
file, err := r.scanFileRows(rows)
if err != nil {
return nil, fmt.Errorf("scanning file: %w", err)
}
files = append(files, file)
}
return files, rows.Err()
}
// ListIDsWithChunksNotInUploadedBlobs returns the IDs of the files whose // ListIDsWithChunksNotInUploadedBlobs returns the IDs of the files whose
// path starts with prefix and that list at least one chunk held by no // path starts with prefix and that list at least one chunk held by no
// blob whose upload has completed (uploaded_ts set). A new snapshot // blob whose upload has completed (uploaded_ts set). A new snapshot
+59
View File
@@ -212,6 +212,65 @@ func TestFileRepositoryListUnderPath(t *testing.T) {
} }
} }
// ListUnderPathInSnapshot leaves out files outside path and files the
// snapshot does not list, though another snapshot lists them.
func TestFileRepositoryListUnderPathInSnapshot(t *testing.T) {
t.Parallel()
db, cleanup := setupTestDB(t)
defer cleanup()
ctx := context.Background()
files := database.NewFileRepository(db)
snapshots := database.NewSnapshotRepository(db)
listed := map[string][]string{
"one": {"/src/a.txt", "/src2/c.txt"},
"two": {"/src/b.txt"},
}
for snapshotID, paths := range listed {
err := snapshots.Create(ctx, nil, &database.Snapshot{
ID: types.SnapshotID(snapshotID),
Hostname: "host",
StartedAt: time.Now(),
})
if err != nil {
t.Fatalf("failed to create snapshot %s: %v", snapshotID, err)
}
for _, path := range paths {
err := files.Create(ctx, nil, &database.File{
Path: types.FilePath(path),
MTime: time.Now().Truncate(time.Second),
Mode: 0644,
})
if err != nil {
t.Fatalf("failed to create %s: %v", path, err)
}
err = snapshots.AddFile(ctx, nil, snapshotID, path)
if err != nil {
t.Fatalf("failed to add %s to %s: %v", path, snapshotID, err)
}
}
}
found, err := files.ListUnderPathInSnapshot(ctx, "/src", "one")
if err != nil {
t.Fatalf("failed to list files: %v", err)
}
got := make([]string, 0, len(found))
for _, f := range found {
got = append(got, f.Path.String())
}
if !slices.Equal(got, []string{"/src/a.txt"}) {
t.Errorf("got %q, want only /src/a.txt", got)
}
}
func TestFileRepositorySymlink(t *testing.T) { func TestFileRepositorySymlink(t *testing.T) {
t.Parallel() t.Parallel()
-47
View File
@@ -531,50 +531,3 @@ func TestExcludePatterns_UnanchoredPatternFile(t *testing.T) {
require.Equal(t, 1, result.FilesScanned, require.Equal(t, 1, result.FilesScanned,
"Unanchored pattern for file should match anywhere") "Unanchored pattern for file should match anywhere")
} }
func TestExcludePatterns_DoubleStarDirectoryAtEveryDepth(t *testing.T) {
t.Parallel()
// As in gitignore, a leading **/ also matches directly under the source dir
fs := setupAnchoredTestFS(t)
scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/projectname"})
defer cleanup()
ctx := context.Background()
createSnapshotRecord(ctx, t, repos, "test-snapshot")
result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
require.NoError(t, err)
// **/projectname should exclude BOTH:
// - /backup/projectname/file.txt
// - /backup/otherproject/projectname/file.txt
// Total files: 4, excluded: 2
require.Equal(t, 2, result.FilesScanned,
"**/ directory pattern should match at the top level and nested")
}
func TestExcludePatterns_DoubleStarFileAtEveryDepth(t *testing.T) {
t.Parallel()
// As in gitignore, a leading **/ also matches directly under the source dir
fs := setupAnchoredTestFS(t)
scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/*.txt"})
defer cleanup()
ctx := context.Background()
createSnapshotRecord(ctx, t, repos, "test-snapshot")
result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
require.NoError(t, err)
// **/*.txt should exclude ALL .txt files:
// - /backup/file.txt
// - /backup/projectname/file.txt
// - /backup/otherproject/projectname/file.txt
// Total files: 4, excluded: 3
require.Equal(t, 1, result.FilesScanned,
"**/ file pattern should match at the top level and nested")
}
+15 -11
View File
@@ -16,6 +16,9 @@ type ScannerParams struct {
Fs afero.Fs Fs afero.Fs
Exclude []string // Exclude patterns (combined global + snapshot-specific) Exclude []string // Exclude patterns (combined global + snapshot-specific)
SkipErrors bool // Skip file read errors (log loudly but continue) SkipErrors bool // Skip file read errors (log loudly but continue)
// PreviousSnapshotID is the most recent completed snapshot of the
// same name, "" for none.
PreviousSnapshotID string
} }
// Module exports backup functionality as an fx module. // Module exports backup functionality as an fx module.
@@ -44,17 +47,18 @@ func provideScannerFactory(
} }
return NewScanner(ScannerConfig{ return NewScanner(ScannerConfig{
FS: params.Fs, FS: params.Fs,
ChunkSize: cfg.ChunkSize.Int64(), ChunkSize: cfg.ChunkSize.Int64(),
Repositories: repos, Repositories: repos,
Storage: storer, Storage: storer,
MaxBlobSize: cfg.BlobSizeLimit.Int64(), MaxBlobSize: cfg.BlobSizeLimit.Int64(),
CompressionLevel: cfg.CompressionLevel, CompressionLevel: cfg.CompressionLevel,
AgeRecipients: cfg.AgeRecipients, AgeRecipients: cfg.AgeRecipients,
EnableProgress: params.EnableProgress, EnableProgress: params.EnableProgress,
UI: params.UI, UI: params.UI,
Exclude: excludes, Exclude: excludes,
SkipErrors: params.SkipErrors, SkipErrors: params.SkipErrors,
PreviousSnapshotID: params.PreviousSnapshotID,
}) })
} }
} }
+37 -27
View File
@@ -58,6 +58,10 @@ type Scanner struct {
compressionLevel int compressionLevel int
ageRecipient string ageRecipient string
snapshotID string // Current snapshot being processed snapshotID string // Current snapshot being processed
// previousSnapshotID is the most recent completed snapshot of the
// same name, "" for none. Only a file it lists can be counted as
// deleted.
previousSnapshotID string
// currentSourcePath is the source directory being scanned, stored with // currentSourcePath is the source directory being scanned, stored with
// each file record. // each file record.
currentSourcePath string currentSourcePath string
@@ -129,6 +133,9 @@ type ScannerConfig struct {
// SkipErrors skips files that cannot be opened or read (log loudly but // SkipErrors skips files that cannot be opened or read (log loudly but
// continue); packer, database, encryption, and upload errors still abort. // continue); packer, database, encryption, and upload errors still abort.
SkipErrors bool SkipErrors bool
// PreviousSnapshotID is the most recent completed snapshot of the
// same name, "" for none.
PreviousSnapshotID string
} }
// ScanResult contains the results of a scan operation. Files and bytes // ScanResult contains the results of a scan operation. Files and bytes
@@ -204,6 +211,7 @@ func NewScanner(cfg ScannerConfig) *Scanner {
compiledExclude: compiledExclude, compiledExclude: compiledExclude,
progress: progress, progress: progress,
skipErrors: cfg.SkipErrors, skipErrors: cfg.SkipErrors,
previousSnapshotID: cfg.PreviousSnapshotID,
ui: uiw, ui: uiw,
pendingChunkHashes: make(map[string]struct{}), pendingChunkHashes: make(map[string]struct{}),
} }
@@ -255,8 +263,9 @@ func (s *Scanner) Scan(
filesToProcess := scanResult.FilesToProcess filesToProcess := scanResult.FilesToProcess
// Phase 1b: Detect deleted files by comparing DB against scanned files // Phase 1b: Detect deleted files by comparing the previous snapshot
err = s.detectDeletedFilesFromMap(ctx, knownFiles, existingFiles, result) // of this name against scanned files
err = s.detectDeletedFiles(ctx, path, existingFiles, result)
if err != nil { if err != nil {
return nil, fmt.Errorf("detecting deleted files: %w", err) return nil, fmt.Errorf("detecting deleted files: %w", err)
} }
@@ -436,9 +445,8 @@ func (s *Scanner) summarizeScanPhase(
} }
// loadKnownFiles loads the known files at and beneath path from the // loadKnownFiles loads the known files at and beneath path from the
// database into a map for fast lookup. Every loaded file the scan does // database into a map for fast lookup. This avoids per-file database
// not find is counted as deleted. This avoids per-file database queries // queries during the scan phase.
// during the scan phase.
func (s *Scanner) loadKnownFiles( func (s *Scanner) loadKnownFiles(
ctx context.Context, path string, ctx context.Context, path string,
) (map[string]*database.File, error) { ) (map[string]*database.File, error) {
@@ -1933,21 +1941,28 @@ func (s *Scanner) queueFileForBatchInsert(
}) })
} }
// detectDeletedFilesFromMap finds files that existed in previous snapshots // detectDeletedFiles counts as deleted each file at or beneath path that
// but no longer exist. Uses pre-loaded maps to avoid any filesystem or // the previous snapshot of this name lists and the walk did not find.
// database access. // An excluded file is not counted: the walk skips it whether or not it
func (s *Scanner) detectDeletedFilesFromMap( // is still on disk.
func (s *Scanner) detectDeletedFiles(
ctx context.Context, ctx context.Context,
knownFiles map[string]*database.File, path string,
existingFiles map[string]struct{}, existingFiles map[string]struct{},
result *ScanResult, result *ScanResult,
) error { ) error {
if len(knownFiles) == 0 { if s.previousSnapshotID == "" {
return nil return nil
} }
// Check each known file against the enumerated set (no filesystem access needed) previousFiles, err := s.repos.Files.ListUnderPathInSnapshot(
for path, file := range knownFiles { ctx, path, s.previousSnapshotID)
if err != nil {
return fmt.Errorf("listing files of snapshot %s: %w",
s.previousSnapshotID, err)
}
for _, file := range previousFiles {
// Check context cancellation periodically // Check context cancellation periodically
select { select {
case <-ctx.Done(): case <-ctx.Done():
@@ -1955,13 +1970,16 @@ func (s *Scanner) detectDeletedFilesFromMap(
default: default:
} }
// Check if the file exists in our enumerated set filePath := file.Path.String()
if _, exists := existingFiles[path]; !exists {
// File has been deleted _, exists := existingFiles[filePath]
result.FilesDeleted++ if exists || s.shouldExclude(filePath, path) {
result.BytesDeleted += file.Size continue
log.Debug("Detected deleted file", "path", path, "size", file.Size)
} }
result.FilesDeleted++
result.BytesDeleted += file.Size
log.Debug("Detected deleted file", "path", filePath, "size", file.Size)
} }
if result.FilesDeleted > 0 { if result.FilesDeleted > 0 {
@@ -2013,14 +2031,6 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
// Remove trailing slash if present (directory indicator) // Remove trailing slash if present (directory indicator)
pattern = strings.TrimSuffix(pattern, "/") pattern = strings.TrimSuffix(pattern, "/")
// A leading "**/" demands a "/" before the name and so misses an
// entry directly under the source path. An unanchored pattern
// already matches at every depth, so drop the prefix: as in
// gitignore, "**/x" then also matches a top-level "x".
if !anchored {
pattern = strings.TrimPrefix(pattern, "**/")
}
// Compile the glob pattern // Compile the glob pattern
// For patterns without path separators, we need to match them as components // For patterns without path separators, we need to match them as components
// e.g., ".git" should match ".git" anywhere in the path // e.g., ".git" should match ".git" anywhere in the path
+12 -11
View File
@@ -661,17 +661,18 @@ func faultScannerFactory(
) snapshot.ScannerFactory { ) snapshot.ScannerFactory {
return func(params snapshot.ScannerParams) *snapshot.Scanner { return func(params snapshot.ScannerParams) *snapshot.Scanner {
return snapshot.NewScanner(snapshot.ScannerConfig{ return snapshot.NewScanner(snapshot.ScannerConfig{
FS: params.Fs, FS: params.Fs,
Storage: storer, Storage: storer,
ChunkSize: faultChunkSize, ChunkSize: faultChunkSize,
MaxBlobSize: faultMaxBlobSize, MaxBlobSize: faultMaxBlobSize,
CompressionLevel: cfg.CompressionLevel, CompressionLevel: cfg.CompressionLevel,
AgeRecipients: cfg.AgeRecipients, AgeRecipients: cfg.AgeRecipients,
Repositories: repos, Repositories: repos,
EnableProgress: params.EnableProgress, EnableProgress: params.EnableProgress,
UI: params.UI, UI: params.UI,
Exclude: params.Exclude, Exclude: params.Exclude,
SkipErrors: params.SkipErrors, SkipErrors: params.SkipErrors,
PreviousSnapshotID: params.PreviousSnapshotID,
}) })
} }
} }
+115
View File
@@ -3,17 +3,21 @@ package vaultik_test
import ( import (
"bytes" "bytes"
"context" "context"
"database/sql"
"io" "io"
"strings" "strings"
"sync" "sync"
"testing" "testing"
"time"
"github.com/klauspost/compress/zstd" "github.com/klauspost/compress/zstd"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/snapshot" "sneak.berlin/go/vaultik/internal/snapshot"
"sneak.berlin/go/vaultik/internal/storage" "sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/types"
"sneak.berlin/go/vaultik/internal/vaultik" "sneak.berlin/go/vaultik/internal/vaultik"
) )
@@ -214,6 +218,37 @@ func addBlob(t *testing.T, store *testStorer, hash string) {
require.NoError(t, err) require.NoError(t, err)
} }
// newLocalIndex returns an in-memory local index holding a completed
// snapshot row for each of ids.
func newLocalIndex(t *testing.T, ids ...string) *database.Repositories {
t.Helper()
ctx := context.Background()
db, err := database.New(ctx, ":memory:")
require.NoError(t, err)
t.Cleanup(func() { _ = db.Close() })
repos := database.NewRepositories(db)
for _, id := range ids {
completedAt := time.Now().UTC()
snap := &database.Snapshot{
ID: types.SnapshotID(id),
Hostname: snapHostname,
StartedAt: completedAt,
CompletedAt: &completedAt,
}
err = repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
return repos.Snapshots.Create(ctx, tx, snap)
})
require.NoError(t, err)
}
return repos
}
// ============================================================================ // ============================================================================
// Unit Tests for RemoveSnapshot // Unit Tests for RemoveSnapshot
// ============================================================================ // ============================================================================
@@ -233,6 +268,7 @@ func TestRemoveSnapshot_LocalOnly_PreservesRemote(t *testing.T) {
addBlob(t, store, blobA) addBlob(t, store, blobA)
tv := vaultik.NewForTesting(store) tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t, "snapshot-001")
opts := &vaultik.RemoveOptions{Force: true, LocalOnly: true} opts := &vaultik.RemoveOptions{Force: true, LocalOnly: true}
result, err := tv.RemoveSnapshot("snapshot-001", opts) result, err := tv.RemoveSnapshot("snapshot-001", opts)
@@ -268,6 +304,7 @@ func TestRemoveSnapshot_DefaultRemovesMetadataNotBlobs(t *testing.T) {
addBlob(t, store, blobShared) addBlob(t, store, blobShared)
tv := vaultik.NewForTesting(store) tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t, "snapshot-001", "snapshot-002")
opts := &vaultik.RemoveOptions{Force: true} opts := &vaultik.RemoveOptions{Force: true}
result, err := tv.RemoveSnapshot("snapshot-001", opts) result, err := tv.RemoveSnapshot("snapshot-001", opts)
@@ -316,6 +353,84 @@ func TestRemoveSnapshot_DryRun(t *testing.T) {
assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]") assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]")
} }
// TestRemoveSnapshot_UnknownIDIsError checks that an ID naming no
// snapshot in the local index or on the destination store is an error
// under every option, and that nothing is reported as removed.
func TestRemoveSnapshot_UnknownIDIsError(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
for name, opts := range map[string]vaultik.RemoveOptions{
"force": {Force: true},
"json": {JSON: true},
"dry-run": {Force: true, DryRun: true},
"local-only": {Force: true, LocalOnly: true},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t, "snapshot-001")
result, err := tv.RemoveSnapshot("snapshot-999", &opts)
require.ErrorContains(t, err, "snapshot not found")
assert.False(t, result.RemoteRemoved)
assert.Empty(t, tv.Stdout.String())
assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
})
}
}
// TestRemoveSnapshot_NoMetadataOnDestination checks that removing a
// snapshot whose metadata is not on the destination store does not
// report metadata removed from it.
func TestRemoveSnapshot_NoMetadataOnDestination(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
tv := vaultik.NewForTesting(newTestStorer())
tv.Repositories = newLocalIndex(t, "snapshot-001")
result, err := tv.RemoveSnapshot("snapshot-001",
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.False(t, result.RemoteRemoved)
out := tv.Stdout.String()
assert.Contains(t, out, "Removed snapshot 'snapshot-001' from local database")
assert.NotContains(t, out, "Removed snapshot metadata from remote storage")
}
// TestRemoveSnapshot_NotInLocalIndex checks that removing a snapshot
// only the destination store holds removes its metadata there without
// reporting a removal from the local database.
func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t)
result, err := tv.RemoveSnapshot("snapshot-001",
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.True(t, result.RemoteRemoved)
assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
out := tv.Stdout.String()
assert.NotContains(t, out, "from local database")
assert.Contains(t, out, "Removed snapshot metadata from remote storage")
}
func TestRemoveAllSnapshots_RequiresForce(t *testing.T) { func TestRemoveAllSnapshots_RequiresForce(t *testing.T) {
log.Initialize(log.Config{}) log.Initialize(log.Config{})
t.Parallel() t.Parallel()
+120 -37
View File
@@ -23,6 +23,7 @@ var (
errNoSnapshotsInConfig = errors.New("no snapshots configured") errNoSnapshotsInConfig = errors.New("no snapshots configured")
errSnapshotVerifyFailed = errors.New("verification failed") errSnapshotVerifyFailed = errors.New("verification failed")
errRemoveAllNeedsForce = errors.New("--all requires --force") errRemoveAllNeedsForce = errors.New("--all requires --force")
errSnapshotNotFound = errors.New("snapshot not found")
errInvalidTableName = errors.New("invalid table name") errInvalidTableName = errors.New("invalid table name")
errNoAgeRecipients = errors.New( errNoAgeRecipients = errors.New(
"creating a snapshot needs at least one public key in " + "creating a snapshot needs at least one public key in " +
@@ -213,12 +214,18 @@ func (v *Vaultik) createNamedSnapshot(
return err return err
} }
previousSnapshotID, err := v.previousSnapshotID(snapName)
if err != nil {
return err
}
scanner := v.ScannerFactory(snapshot.ScannerParams{ scanner := v.ScannerFactory(snapshot.ScannerParams{
EnableProgress: !opts.Cron, EnableProgress: !opts.Cron,
UI: v.UI, UI: v.UI,
Fs: v.Fs, Fs: v.Fs,
Exclude: v.Config.GetExcludes(snapName), Exclude: v.Config.GetExcludes(snapName),
SkipErrors: opts.SkipErrors, SkipErrors: opts.SkipErrors,
PreviousSnapshotID: previousSnapshotID,
}) })
snapshotID, err := v.SnapshotManager.CreateSnapshotWithName( snapshotID, err := v.SnapshotManager.CreateSnapshotWithName(
@@ -253,6 +260,24 @@ func (v *Vaultik) createNamedSnapshot(
return nil return nil
} }
// previousSnapshotID returns the ID of the most recent completed snapshot
// named snapName in the local index, or "" when there is none.
func (v *Vaultik) previousSnapshotID(snapName string) (string, error) {
snapshots, err := v.Repositories.Snapshots.ListRecent(v.ctx, listRecentLimit)
if err != nil {
return "", fmt.Errorf("listing snapshots: %w", err)
}
for _, s := range snapshots {
if s.CompletedAt != nil &&
parseSnapshotName(s.ID.String(), s.Hostname.String()) == snapName {
return s.ID.String(), nil
}
}
return "", nil
}
// resolveSnapshotPaths resolves source directories to absolute paths // resolveSnapshotPaths resolves source directories to absolute paths
// with symlink resolution. // with symlink resolution.
func (v *Vaultik) resolveSnapshotPaths(snapName string) ([]string, error) { func (v *Vaultik) resolveSnapshotPaths(snapName string) ([]string, error) {
@@ -643,7 +668,7 @@ func (v *Vaultik) confirmAndExecutePurge(
"snapshot_id", snapshotID, "error", err) "snapshot_id", snapshotID, "error", err)
} }
err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID)) _, err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))
if err != nil { if err != nil {
return fmt.Errorf("deleting snapshot %s from remote: %w", snapshotID, err) return fmt.Errorf("deleting snapshot %s from remote: %w", snapshotID, err)
} }
@@ -1153,6 +1178,11 @@ func (v *Vaultik) RemoveSnapshot(
return result, err return result, err
} }
inLocalIndex, err := v.findSnapshotToRemove(snapshotID, opts)
if err != nil {
return result, err
}
if opts.DryRun { if opts.DryRun {
return v.removeSnapshotDryRun(result, snapshotID, opts) return v.removeSnapshotDryRun(result, snapshotID, opts)
} }
@@ -1161,11 +1191,13 @@ func (v *Vaultik) RemoveSnapshot(
return result, nil return result, nil
} }
log.Info("Removing snapshot from local database", "snapshot_id", snapshotID) if inLocalIndex {
log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
err = v.deleteSnapshotFromLocalDB(snapshotID) err = v.deleteSnapshotFromLocalDB(snapshotID)
if err != nil { if err != nil {
return result, fmt.Errorf("removing from local database: %w", err) return result, fmt.Errorf("removing from local database: %w", err)
}
} }
if !opts.LocalOnly { if !opts.LocalOnly {
@@ -1184,9 +1216,11 @@ func (v *Vaultik) RemoveSnapshot(
return result, v.outputRemoveJSON(result) return result, v.outputRemoveJSON(result)
} }
v.stdoutf("Removed snapshot '%s' from local database\n", snapshotID) if inLocalIndex {
v.stdoutf("Removed snapshot '%s' from local database\n", snapshotID)
}
if !opts.LocalOnly && result.RemoteRemoved { if result.RemoteRemoved {
v.printlnStdout("Removed snapshot metadata from remote storage") v.printlnStdout("Removed snapshot metadata from remote storage")
v.stdoutf("\nNote: The removed snapshot's blobs remain on the remote. "+ v.stdoutf("\nNote: The removed snapshot's blobs remain on the remote. "+
"Run '%s' to delete any blobs no longer referenced by any "+ "Run '%s' to delete any blobs no longer referenced by any "+
@@ -1196,6 +1230,45 @@ func (v *Vaultik) RemoveSnapshot(
return result, nil return result, nil
} }
// findSnapshotToRemove reports whether the local index has a row for
// snapshotID, and returns errSnapshotNotFound when there is nothing to
// remove: no row and, unless LocalOnly is set, no metadata on the
// destination store either. The destination store is consulted only
// when the local index has no row, so a snapshot in the local index can
// still be removed from it while the destination store is unreachable.
func (v *Vaultik) findSnapshotToRemove(
snapshotID string, opts *RemoveOptions,
) (bool, error) {
if v.Repositories != nil {
snap, err := v.Repositories.Snapshots.GetByID(v.ctx, snapshotID)
if err != nil {
return false, fmt.Errorf("looking up snapshot in local index: %w", err)
}
if snap != nil {
return true, nil
}
}
if opts.LocalOnly {
return false, fmt.Errorf("%w in the local index: %s",
errSnapshotNotFound, snapshotID)
}
objects, err := v.remoteSnapshotObjects(snapshot.RemoteSnapshotKey(snapshotID))
if err != nil {
return false, fmt.Errorf("looking up snapshot on destination store: %w", err)
}
if len(objects) == 0 {
return false, fmt.Errorf(
"%w in the local index or on the destination store: %s",
errSnapshotNotFound, snapshotID)
}
return false, nil
}
// removeSnapshotDryRun reports what RemoveSnapshot would do without // removeSnapshotDryRun reports what RemoveSnapshot would do without
// making changes. // making changes.
func (v *Vaultik) removeSnapshotDryRun( func (v *Vaultik) removeSnapshotDryRun(
@@ -1255,14 +1328,14 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
// didn't finish and to run `vaultik snapshot remove` for the snapshot // didn't finish and to run `vaultik snapshot remove` for the snapshot
// again once the destination store is reachable (`vaultik prune` never // again once the destination store is reachable (`vaultik prune` never
// removes snapshot metadata). Returns true when the remote removal // removes snapshot metadata). Returns true when the remote removal
// succeeded. // succeeded and deleted something.
func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) bool { func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) bool {
log.Info("Removing snapshot metadata from remote storage", log.Info("Removing snapshot metadata from remote storage",
"snapshot_id", snapshotID) "snapshot_id", snapshotID)
remoteKey := snapshot.RemoteSnapshotKey(snapshotID) remoteKey := snapshot.RemoteSnapshotKey(snapshotID)
err := v.deleteRemoteSnapshotByKey(remoteKey) deleted, err := v.deleteRemoteSnapshotByKey(remoteKey)
if err != nil { if err != nil {
log.Warn("Could not remove snapshot metadata from remote storage; "+ log.Warn("Could not remove snapshot metadata from remote storage; "+
"run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+ "run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+
@@ -1280,7 +1353,7 @@ func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) b
return false return false
} }
return true return deleted > 0
} }
// RemoveAllSnapshots removes every snapshot known to the local // RemoveAllSnapshots removes every snapshot known to the local
@@ -1572,7 +1645,7 @@ func (v *Vaultik) removeAllLocalSnapshots(
} }
if !opts.LocalOnly { if !opts.LocalOnly {
err = v.deleteRemoteSnapshotByKey( _, err = v.deleteRemoteSnapshotByKey(
snapshot.RemoteSnapshotKey(snapshotID)) snapshot.RemoteSnapshotKey(snapshotID))
if err != nil { if err != nil {
log.Warn("Failed to remove snapshot metadata from remote", log.Warn("Failed to remove snapshot metadata from remote",
@@ -1598,7 +1671,7 @@ func (v *Vaultik) removeAllOrphanRemotes(
for _, key := range orphanRemoteKeys { for _, key := range orphanRemoteKeys {
log.Info("Removing orphan remote snapshot", "remote_key", key) log.Info("Removing orphan remote snapshot", "remote_key", key)
err := v.deleteRemoteSnapshotByKey(key) _, err := v.deleteRemoteSnapshotByKey(key)
if err != nil { if err != nil {
log.Warn("Failed to remove orphan from remote", log.Warn("Failed to remove orphan from remote",
"remote_key", key, "error", err) "remote_key", key, "error", err)
@@ -1645,34 +1718,44 @@ func (v *Vaultik) deleteSnapshotFromLocalDB(snapshotID string) error {
} }
// deleteRemoteSnapshotByKey removes everything under // deleteRemoteSnapshotByKey removes everything under
// metadata/<remoteKey>/ on the destination store. The argument is a // metadata/<remoteKey>/ on the destination store and returns how many
// remote key (double-SHA256 derivation), not a human snapshot ID; // objects it deleted. The argument is a remote key (double-SHA256
// callers that have a human ID must hash via snapshot.RemoteSnapshotKey // derivation), not a human snapshot ID; callers that have a human ID
// first. // must hash via snapshot.RemoteSnapshotKey first.
func (v *Vaultik) deleteRemoteSnapshotByKey(remoteKey string) error { func (v *Vaultik) deleteRemoteSnapshotByKey(remoteKey string) (int, error) {
prefix := fmt.Sprintf("metadata/%s/", remoteKey) objectsToDelete, err := v.remoteSnapshotObjects(remoteKey)
objectCh := v.Storage.ListStream(v.ctx, prefix) if err != nil {
return 0, err
var objectsToDelete []string
for object := range objectCh {
if object.Err != nil {
return fmt.Errorf("listing objects: %w", object.Err)
}
objectsToDelete = append(objectsToDelete, object.Key)
} }
for _, key := range objectsToDelete { for i, key := range objectsToDelete {
err := v.Storage.Delete(v.ctx, key) err = v.Storage.Delete(v.ctx, key)
if err != nil { if err != nil {
return fmt.Errorf("removing %s: %w", key, err) return i, fmt.Errorf("removing %s: %w", key, err)
} }
log.Debug("Deleted remote object", "key", key) log.Debug("Deleted remote object", "key", key)
} }
return nil return len(objectsToDelete), nil
}
// remoteSnapshotObjects lists the keys of every object under
// metadata/<remoteKey>/ on the destination store.
func (v *Vaultik) remoteSnapshotObjects(remoteKey string) ([]string, error) {
prefix := fmt.Sprintf("metadata/%s/", remoteKey)
var keys []string
for object := range v.Storage.ListStream(v.ctx, prefix) {
if object.Err != nil {
return nil, fmt.Errorf("listing objects: %w", object.Err)
}
keys = append(keys, object.Key)
}
return keys, nil
} }
// outputRemoveJSON outputs the removal result as JSON // outputRemoveJSON outputs the removal result as JSON
@@ -0,0 +1,149 @@
package vaultik_test
import (
"bytes"
"context"
"fmt"
"os"
"path/filepath"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/config"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/ui"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// These tests cover https://git.eeqj.de/sneak/vaultik/issues/304: a
// backup counts a file as deleted only when the previous snapshot of
// the same name had it and it is gone, and never counts an excluded
// file.
// deletedCountEnv is a backup setup over the source directory src whose
// user-facing output is kept in out. Each test configures its snapshots
// with setSnapshot.
type deletedCountEnv struct {
v *vaultik.Vaultik
out *bytes.Buffer
src string
}
// newDeletedCountEnv writes files, keyed by path relative to src.
func newDeletedCountEnv(t *testing.T, files map[string][]byte) *deletedCountEnv {
t.Helper()
ctx := context.Background()
fs := afero.NewOsFs()
tempDir := t.TempDir()
srcDir := filepath.Join(tempDir, "src")
for rel, content := range files {
path := filepath.Join(srcDir, rel)
require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0o755))
require.NoError(t, afero.WriteFile(fs, path, content, 0o644))
}
cfg := faultTestConfig()
cfg.IndexPath = filepath.Join(tempDir, "index.sqlite")
cfg.Snapshots = map[string]config.SnapshotConfig{}
store, err := storage.NewFileStorer(filepath.Join(tempDir, "remote"))
require.NoError(t, err)
db, err := database.New(ctx, cfg.IndexPath)
require.NoError(t, err)
t.Cleanup(func() { _ = db.Close() })
repos := database.NewRepositories(db)
out := &bytes.Buffer{}
v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
v.UI = ui.NewWithColor(out, false)
return &deletedCountEnv{v: v, out: out, src: srcDir}
}
// setSnapshot configures the snapshot name to back up the path rel
// beneath src, with the given excludes.
func (e *deletedCountEnv) setSnapshot(name, rel string, exclude ...string) {
e.v.Config.Snapshots[name] = config.SnapshotConfig{
Paths: []string{filepath.Join(e.src, rel)},
Exclude: exclude,
}
}
// backUp runs a backup of the named snapshot and returns its output.
func (e *deletedCountEnv) backUp(t *testing.T, name string) string {
t.Helper()
e.out.Reset()
require.NoError(t, e.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
Snapshots: []string{name},
}))
return e.out.String()
}
// A file this snapshot excludes is not counted as deleted: neither one
// that another snapshot backs up, nor one that the previous snapshot of
// this name had before the exclude was added.
func TestBackupDoesNotCountExcludedFileAsDeleted(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newDeletedCountEnv(t, map[string][]byte{
"top.txt": []byte("top"),
"cache/tmp.bin": []byte("cache"),
"photos/a.jpg": []byte("photo a"),
"photos/b.jpg": []byte("photo b"),
"photos/c.jpg": []byte("photo c"),
})
env.setSnapshot("photos", "photos")
env.setSnapshot("home", "", "/photos")
env.backUp(t, "photos")
assert.Contains(t, env.backUp(t, "home"), filesLine(2, 2, 0))
env.setSnapshot("home", "", "/photos", "/cache")
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
}
// A file removed from disk is counted as deleted by the next backup of
// the snapshot that had it, and not by any later backup of that
// snapshot, nor by the first backup of another name over the same path.
func TestBackupCountsDeletedFileOnce(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
aContent := bytes.Repeat([]byte("a"), 1000)
bContent := bytes.Repeat([]byte("b"), 2000)
keptContent := bytes.Repeat([]byte("k"), 500)
env := newDeletedCountEnv(t, map[string][]byte{
"a.txt": aContent,
"b.txt": bContent,
"kept.txt": keptContent,
})
env.setSnapshot("home", "")
env.setSnapshot("other", "")
assert.Contains(t, env.backUp(t, "home"), filesLine(3, 3, 0))
require.NoError(t, os.Remove(filepath.Join(env.src, "a.txt")))
require.NoError(t, os.Remove(filepath.Join(env.src, "b.txt")))
summary := env.backUp(t, "home")
assert.Contains(t, summary,
"Files: 1 examined, 0 backed up, 1 unchanged, 2 deleted.")
assert.Contains(t, summary,
fmt.Sprintf("Data: %s total (%s backed up), %s deleted.",
env.v.UI.Size(int64(len(keptContent))), env.v.UI.Size(0),
env.v.UI.Size(int64(len(aContent)+len(bContent)))))
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
assert.Contains(t, env.backUp(t, "other"), filesLine(1, 0, 1))
}