Compare commits

..
Author SHA1 Message Date
clawbot ad7941c215 List a file:// destination through a symlinked directory (closes #296)
check / check (push) Waiting to run
List and ListStream walked the destination with afero.Walk, which uses
Lstat and so reported a symlinked metadata/ or blobs/ directory as one
file, while Put, Get and Stat follow the link. The store then listed as
empty, and prune removed the local snapshot records and deleted every
blob. Both listings now walk a view of the filesystem without its Lstat
method, so afero falls back to Stat and enters the link. A listed prefix
that is a symlink whose target is missing, such as a link to a volume
that is not mounted, fails the listing, as a missing destination
directory does.

A symlink loop is not detected: it is followed until the system refuses
the path for too many levels of links, and the listing fails.

Model: opus-5-5
2026-10-13 07:36:02 +00:00
clawbot 94412d9ef3 Stop remote info counting a non-blob file under blobs/ (closes #295)
check / check (push) Waiting to run
remote info counted every key under blobs/ with four or more path parts
as a blob, while prune skips a name that is not a blob hash. A stray
file such as .DS_Store was reported as an orphaned blob, with advice to
run prune, which then deleted nothing. remote info now takes its blob
list from prune's listAllRemoteBlobs, so its blob and orphan figures,
in the table and in --json, cover exactly what prune would consider.

blobKeyParts moves to prune.go, its only remaining user.

Model: opus-5-5
2026-10-13 08:33:27 +02: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 609 additions and 129 deletions
+8 -4
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
@@ -399,7 +401,9 @@ orphaned blob figures are reported as unknown; `--json` gives them as
`null`, lists the remote key of each unreadable manifest in `null`, lists the remote key of each unreadable manifest in
`unreadable_manifests` and counts the manifests under skipped names in `unreadable_manifests` and counts the manifests under skipped names in
`skipped_manifest_count`. An unreadable manifest also leaves its `skipped_manifest_count`. An unreadable manifest also leaves its
snapshot's blob count and blob size unknown, `null` in `--json`. snapshot's blob count and blob size unknown, `null` in `--json`. A file
under `blobs/` whose name is not a blob hash is not counted as a blob,
since `prune` never deletes it.
* `--json`: Output as JSON * `--json`: Output as JSON
**`remote nuke`**: Delete every snapshot's metadata and every blob from the **`remote nuke`**: Delete every snapshot's metadata and every blob from the
+28 -8
View File
@@ -22,14 +22,34 @@ 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: Made a `file://` destination list through a symlinked
directly under the snapshot path directory ([issue #296](https://git.eeqj.de/sneak/vaultik/issues/296)).
([issue #303](https://git.eeqj.de/sneak/vaultik/issues/303)). A `metadata/` or `blobs/` directory that was a symlink to a directory
`**/node_modules` and `**/*.pyc`, the patterns `config.example.yml` elsewhere was listed as one file, so the destination store listed as
uses, excluded `sub/node_modules` and `sub/y.pyc` but backed up empty while backups and restores still worked through the link.
`node_modules/` and `x.pyc` at the top of the path, since the pattern `prune` then removed the local snapshot records and deleted every blob,
required a `/` before the name. As in gitignore, such a pattern now and `remote info` and `remote nuke` found no blobs. The listings now
matches at every depth, the top included. read through the link as `Get` and `Stat` do. A link whose target is
missing, such as one to a volume that is not mounted, or a link
pointing back up the tree makes them fail.
- 2026-10-13: Stopped `remote info` counting a file under `blobs/` that
is not a blob as an orphaned blob
([issue #295](https://git.eeqj.de/sneak/vaultik/issues/295)). A stray
file such as `.DS_Store` was reported as an orphaned blob and the
report advised running `prune`, which skips such a file and deleted
nothing. `remote info` now takes its blob list from the same listing
`prune` uses, in the table and in `--json`.
- 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
-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")
}
-8
View File
@@ -2013,14 +2013,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
+45 -10
View File
@@ -120,9 +120,10 @@ func (f *FileStorer) Delete(_ context.Context, key string) error {
return nil return nil
} }
// List returns all keys with the given prefix. It fails when the // List returns all keys with the given prefix, reading through symlinked
// destination directory is missing; a missing prefix under it is an // directories. It fails when the destination directory is missing or the
// empty listing. // prefix is a symlink whose target is missing; a missing prefix under the
// destination directory is an empty listing.
func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error) { func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error) {
var keys []string var keys []string
@@ -133,8 +134,7 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
basePath := f.fullPath(prefix) basePath := f.fullPath(prefix)
// Check if the prefix exists exists, err := f.prefixExists(basePath)
exists, err := afero.Exists(f.fs, basePath)
if err != nil { if err != nil {
return nil, fmt.Errorf("checking path: %w", err) return nil, fmt.Errorf("checking path: %w", err)
} }
@@ -143,7 +143,7 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
return keys, nil // Empty list for non-existent prefix return keys, nil // Empty list for non-existent prefix
} }
err = afero.Walk(f.fs, basePath, func(path string, info os.FileInfo, err error) error { err = f.walk(basePath, func(path string, info os.FileInfo, err error) error {
if err != nil { if err != nil {
return err return err
} }
@@ -176,7 +176,9 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
} }
// ListStream returns a channel of ObjectInfo for large result sets. Like // ListStream returns a channel of ObjectInfo for large result sets. Like
// List, it sends an error when the destination directory is missing. // List, it reads through symlinked directories and sends an error when
// the destination directory is missing or the prefix is a symlink whose
// target is missing.
func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan ObjectInfo { func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan ObjectInfo {
ch := make(chan ObjectInfo) ch := make(chan ObjectInfo)
@@ -192,8 +194,7 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
basePath := f.fullPath(prefix) basePath := f.fullPath(prefix)
// Check if the prefix exists exists, err := f.prefixExists(basePath)
exists, err := afero.Exists(f.fs, basePath)
if err != nil { if err != nil {
ch <- ObjectInfo{Err: fmt.Errorf("checking path: %w", err)} ch <- ObjectInfo{Err: fmt.Errorf("checking path: %w", err)}
@@ -204,7 +205,7 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
return // Empty channel for non-existent prefix return // Empty channel for non-existent prefix
} }
_ = afero.Walk(f.fs, basePath, func(path string, info os.FileInfo, err error) error { _ = f.walk(basePath, func(path string, info os.FileInfo, err error) error {
// Check context cancellation // Check context cancellation
select { select {
case <-ctx.Done(): case <-ctx.Done():
@@ -352,6 +353,40 @@ func (f *FileStorer) writeAtomic(
return nil return nil
} }
// walk is afero.Walk, except that it reads through a symlinked directory
// the way Get and Stat do, where afero.Walk reports the link as one file.
// afero.Walk uses Lstat, which does not follow a link, whenever the
// filesystem has one; the struct below carries only the afero.Fs methods,
// so the walk uses Stat instead. A link back up the tree is followed until
// the operating system refuses the path for too many levels of links, and
// the walk then fails.
func (f *FileStorer) walk(root string, fn filepath.WalkFunc) error {
return afero.Walk(struct{ afero.Fs }{f.fs}, root, fn)
}
// prefixExists is afero.Exists, except that it does not follow a symlink
// at path. A symlinked prefix whose target is missing, such as a link to
// a volume that is not mounted, therefore exists, and walk then fails on
// it; with afero.Exists it would list as empty, and prune would delete
// every blob.
func (f *FileStorer) prefixExists(path string) (bool, error) {
lstater, ok := f.fs.(afero.Lstater)
if !ok {
return afero.Exists(f.fs, path)
}
_, _, err := lstater.LstatIfPossible(path)
if err == nil {
return true, nil
}
if os.IsNotExist(err) {
return false, nil
}
return false, err
}
// fullPath returns the full filesystem path for a key. // fullPath returns the full filesystem path for a key.
func (f *FileStorer) fullPath(key string) string { func (f *FileStorer) fullPath(key string) string {
return filepath.Join(f.basePath, key) return filepath.Join(f.basePath, key)
+151
View File
@@ -4,7 +4,10 @@ import (
"context" "context"
"errors" "errors"
"io/fs" "io/fs"
"os"
"path/filepath" "path/filepath"
"reflect"
"strings"
"testing" "testing"
"sneak.berlin/go/vaultik/internal/storage" "sneak.berlin/go/vaultik/internal/storage"
@@ -60,3 +63,151 @@ func TestFileStorerListMissingDestination(t *testing.T) {
t.Errorf("ListStream error = %v, want a not-exist error", streamErr) t.Errorf("ListStream error = %v, want a not-exist error", streamErr)
} }
} }
// TestFileStorerListsThroughSymlinkedDirectory checks that List and
// ListStream read through a symlinked metadata/ or blobs/ directory, as Get
// and Stat do, instead of listing the link as one key. Listing the whole
// destination meets the link part-way through the walk; listing its prefix
// starts the walk at the link.
func TestFileStorerListsThroughSymlinkedDirectory(t *testing.T) {
t.Parallel()
for _, dir := range []string{"metadata", "blobs"} {
t.Run(dir, func(t *testing.T) {
t.Parallel()
base := t.TempDir()
key := dir + "/aa/bb/object"
s, err := storage.NewFileStorer(base)
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
err = s.Put(context.Background(), key, strings.NewReader("data"))
if err != nil {
t.Fatalf("Put: %v", err)
}
linkPath := filepath.Join(base, dir)
movedPath := filepath.Join(t.TempDir(), dir)
err = os.Rename(linkPath, movedPath)
if err != nil {
t.Fatalf("moving %s away: %v", dir, err)
}
err = os.Symlink(movedPath, linkPath)
if err != nil {
t.Fatalf("linking %s: %v", dir, err)
}
want := []string{key}
for _, prefix := range []string{"", dir + "/"} {
got := listSorted(t, s, prefix)
if !reflect.DeepEqual(got, want) {
t.Errorf("List(%q) = %v, want %v", prefix, got, want)
}
got = listStreamKeys(t, s, prefix)
if !reflect.DeepEqual(got, want) {
t.Errorf("ListStream(%q) = %v, want %v", prefix, got, want)
}
}
})
}
}
// TestFileStorerListFailsOnSymlinkWithMissingTarget checks that List and
// ListStream fail on a symlinked metadata/ or blobs/ directory whose target
// is missing, such as a link to a volume that is not mounted, as they do on
// a missing destination directory, instead of listing it as empty.
func TestFileStorerListFailsOnSymlinkWithMissingTarget(t *testing.T) {
t.Parallel()
ctx := context.Background()
for _, dir := range []string{"metadata", "blobs"} {
t.Run(dir, func(t *testing.T) {
t.Parallel()
base := t.TempDir()
s, err := storage.NewFileStorer(base)
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
unmounted := filepath.Join(t.TempDir(), "unmounted")
err = os.Symlink(unmounted, filepath.Join(base, dir))
if err != nil {
t.Fatalf("Symlink: %v", err)
}
for _, prefix := range []string{"", dir + "/"} {
keys, err := s.List(ctx, prefix)
if !errors.Is(err, fs.ErrNotExist) {
t.Errorf("List(%q) = %v, %v; want a not-exist error",
prefix, keys, err)
}
var streamErr error
for object := range s.ListStream(ctx, prefix) {
if object.Err != nil {
streamErr = object.Err
}
}
if !errors.Is(streamErr, fs.ErrNotExist) {
t.Errorf("ListStream(%q) error = %v, want a not-exist error",
prefix, streamErr)
}
}
})
}
}
// TestFileStorerListFailsOnSymlinkLoop checks that a symlink pointing back
// at the destination makes List and ListStream fail instead of walking it
// forever.
func TestFileStorerListFailsOnSymlinkLoop(t *testing.T) {
t.Parallel()
ctx := context.Background()
base := t.TempDir()
s, err := storage.NewFileStorer(base)
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
err = s.Put(ctx, testBlobKey, strings.NewReader("data"))
if err != nil {
t.Fatalf("Put: %v", err)
}
err = os.Symlink(base, filepath.Join(base, "blobs", "loop"))
if err != nil {
t.Fatalf("Symlink: %v", err)
}
keys, err := s.List(ctx, "blobs/")
if err == nil {
t.Errorf("List = %d keys and no error, want an error", len(keys))
}
var streamErr error
for object := range s.ListStream(ctx, "blobs/") {
if object.Err != nil {
streamErr = object.Err
}
}
if streamErr == nil {
t.Error("ListStream sent no error, want one")
}
}
+7 -18
View File
@@ -167,10 +167,6 @@ const (
// metadataKeyParts is the minimum "/"-separated segment count of a // metadataKeyParts is the minimum "/"-separated segment count of a
// metadata object key (metadata/<snapshot-id>/<filename>). // metadata object key (metadata/<snapshot-id>/<filename>).
metadataKeyParts = 3 metadataKeyParts = 3
// blobKeyParts is the minimum "/"-separated segment count of a blob
// object key (blobs/<aa>/<bb>/<hash>).
blobKeyParts = 4
) )
// SnapshotMetadataInfo contains information about a single snapshot's metadata // SnapshotMetadataInfo contains information about a single snapshot's metadata
@@ -454,23 +450,16 @@ func (v *Vaultik) scanRemoteBlobStorage(
v.stdoutf("Scanning blobs...\n") v.stdoutf("Scanning blobs...\n")
} }
blobCh := v.Storage.ListStream(v.ctx, "blobs/") // Use prune's listing, so a file under blobs/ that prune would never
allBlobs := make(map[string]int64) // delete is not counted as a blob, and so not as an orphaned one.
allBlobs, err := v.listAllRemoteBlobs()
for obj := range blobCh { if err != nil {
if obj.Err != nil { return err
return fmt.Errorf("listing blobs: %w", obj.Err)
} }
parts := strings.Split(obj.Key, "/") for _, size := range allBlobs {
if len(parts) < blobKeyParts {
continue
}
hash := parts[3]
allBlobs[hash] = obj.Size
result.TotalBlobCount++ result.TotalBlobCount++
result.TotalBlobSize += obj.Size result.TotalBlobSize += size
} }
// A blob named only by a manifest that could not be read, or by one // A blob named only by a manifest that could not be read, or by one
+4
View File
@@ -23,6 +23,10 @@ var errNukeRequiresForce = errors.New(
// metadata. // metadata.
const metadataDirName = "metadata" const metadataDirName = "metadata"
// blobKeyParts is the "/"-separated segment count of a blob object key
// (blobs/<aa>/<bb>/<hash>).
const blobKeyParts = 4
// NukeRemote deletes every snapshot's metadata and every blob from remote // NukeRemote deletes every snapshot's metadata and every blob from remote
// storage, along with any object an upload cut off part-way left under a // storage, along with any object an upload cut off part-way left under a
// temporary `.partial` name. After this returns successfully the bucket // temporary `.partial` name. After this returns successfully the bucket
+33
View File
@@ -185,6 +185,39 @@ func TestRemoteInfo_SkipsNonConformingMetadataName(t *testing.T) {
assert.NotContains(t, doc, "unreadable_manifests") assert.NotContains(t, doc, "unreadable_manifests")
} }
// TestRemoteInfo_DoesNotCountNonBlobFileUnderBlobs checks that a file
// under blobs/ whose name is not a blob hash is not counted as a blob.
// prune never deletes such a file, so counting it as an orphaned blob
// would advise a prune that removes nothing.
func TestRemoteInfo_DoesNotCountNonBlobFileUnderBlobs(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t)
env.addRemote(t, listRemoteID, time.Date(2026, 3, 2, 0, 0, 0, 0, time.UTC))
addBlob(t, env.store.testStorer, testBlobHashA)
require.NoError(t, env.store.Put(context.Background(),
"blobs/"+testBlobHashA[:2]+"/"+testBlobHashA[2:4]+"/.DS_Store",
bytes.NewReader([]byte("junk"))))
require.NoError(t, env.v.RemoteInfo(false))
text := env.stdout.String()
assert.Contains(t, text, "Total blobs on remote: 1 (")
assert.Contains(t, text, "Orphaned (unreferenced): 0 (")
assert.NotContains(t, text, "vaultik prune")
env.stdout.Reset()
require.NoError(t, env.v.RemoteInfo(true))
var doc map[string]any
require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &doc))
assert.InDelta(t, 1, doc["total_blob_count"], 0)
assert.InDelta(t, 0, doc["orphaned_blob_count"], 0)
assert.InDelta(t, 0, doc["orphaned_blob_size"], 0)
}
// TestRemoteInfo_DirectoryWithoutManifestLeavesOrphansKnown checks that // TestRemoteInfo_DirectoryWithoutManifestLeavesOrphansKnown checks that
// a directory under metadata/ holding no manifest.json.zst, such as one // a directory under metadata/ holding no manifest.json.zst, such as one
// left by a backup interrupted before its manifest upload, leaves the // left by a backup interrupted before its manifest upload, leaves the
+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()
+88 -29
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 " +
@@ -643,7 +644,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 +1154,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,12 +1167,14 @@ func (v *Vaultik) RemoveSnapshot(
return result, nil return result, nil
} }
if inLocalIndex {
log.Info("Removing snapshot from local database", "snapshot_id", snapshotID) 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 {
result.RemoteRemoved = v.removeSnapshotRemote(snapshotID, opts) result.RemoteRemoved = v.removeSnapshotRemote(snapshotID, opts)
@@ -1184,9 +1192,11 @@ func (v *Vaultik) RemoveSnapshot(
return result, v.outputRemoveJSON(result) return result, v.outputRemoveJSON(result)
} }
if inLocalIndex {
v.stdoutf("Removed snapshot '%s' from local database\n", snapshotID) 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 +1206,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 +1304,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 +1329,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 +1621,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 +1647,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 +1694,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)
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 {
err := v.Storage.Delete(v.ctx, key)
if err != nil { if err != nil {
return fmt.Errorf("removing %s: %w", key, err) return 0, err
}
for i, key := range objectsToDelete {
err = v.Storage.Delete(v.ctx, key)
if err != nil {
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,121 @@
package vaultik_test
import (
"context"
"io/fs"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// TestPruneDeletesNothingThroughSymlinkedDirectory checks that prune keeps
// every blob and the local snapshot record when the metadata/ or blobs/
// directory of a file:// destination is a symlink to a directory elsewhere
// (https://git.eeqj.de/sneak/vaultik/issues/296). A backup writes through
// such a link, so prune has to list through it too.
func TestPruneDeletesNothingThroughSymlinkedDirectory(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
for _, dir := range []string{"metadata", "blobs"} {
t.Run(dir, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, repos, _ := backUpToFileDestination(ctx, t, storeDir)
blobFiles := blobFilesIn(t, storeDir)
moveBehindSymlink(t, storeDir, dir)
require.NoError(t, v.Prune(&vaultik.PruneOptions{Force: true}))
// When blobs/ is the link, these paths run through it.
for _, blobFile := range blobFiles {
assert.FileExists(t, blobFile, "prune must delete no blob")
}
snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
require.NoError(t, err)
assert.Len(t, snapshots, 1, "prune must delete no local snapshot record")
})
}
}
// TestPruneFailsOnSymlinkedDirectoryWithMissingTarget checks that prune
// fails, deleting no blob and keeping the local snapshot record, when the
// metadata/ or blobs/ directory of a file:// destination is a symlink
// whose target is missing, as a link to a volume that is not mounted is.
func TestPruneFailsOnSymlinkedDirectoryWithMissingTarget(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
for _, dir := range []string{"metadata", "blobs"} {
t.Run(dir, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, repos, _ := backUpToFileDestination(ctx, t, storeDir)
blobFiles := blobFilesIn(t, storeDir)
targetPath := moveBehindSymlink(t, storeDir, dir)
// Unmount the volume the link points to, and mount it again
// once prune has run.
unmountedPath := targetPath + "-unmounted"
require.NoError(t, os.Rename(targetPath, unmountedPath))
err := v.Prune(&vaultik.PruneOptions{Force: true})
require.ErrorIs(t, err, fs.ErrNotExist)
require.NoError(t, os.Rename(unmountedPath, targetPath))
// When blobs/ is the link, these paths run through it.
for _, blobFile := range blobFiles {
assert.FileExists(t, blobFile, "prune must delete no blob")
}
snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
require.NoError(t, err)
assert.Len(t, snapshots, 1, "prune must delete no local snapshot record")
})
}
}
// blobFilesIn returns the paths of the blob files under storeDir/blobs, and
// fails the test when there are none.
func blobFilesIn(t *testing.T, storeDir string) []string {
t.Helper()
var blobFiles []string
err := filepath.WalkDir(filepath.Join(storeDir, "blobs"),
func(path string, entry fs.DirEntry, err error) error {
if err == nil && !entry.IsDir() {
blobFiles = append(blobFiles, path)
}
return err
})
require.NoError(t, err)
require.NotEmpty(t, blobFiles)
return blobFiles
}
// moveBehindSymlink moves storeDir/dir to a directory elsewhere, puts a
// symlink to it in its place, and returns where it went.
func moveBehindSymlink(t *testing.T, storeDir, dir string) string {
t.Helper()
linkPath := filepath.Join(storeDir, dir)
targetPath := filepath.Join(t.TempDir(), dir)
require.NoError(t, os.Rename(linkPath, targetPath))
require.NoError(t, os.Symlink(targetPath, linkPath))
return targetPath
}