List a missing file:// destination directory as an error #238

Merged
clawbot merged 1 commits from issue-220-missing-file-destination into next 2026-10-06 08:46:18 +02:00
7 changed files with 246 additions and 19 deletions
+10
View File
@@ -22,6 +22,16 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps # Completed Steps
- 2026-10-06: Made a `file://` destination whose directory is missing
count as one that cannot be listed
([issue #220](https://git.eeqj.de/sneak/vaultik/issues/220)). The file
backend listed a missing directory as an empty store, so with the
quickstart's USB stick unplugged `snapshot list` reported every local
snapshot as missing from the store, `snapshot remove` claimed to have
removed metadata it never reached, and `prune` dropped every local
snapshot record. Listing a missing destination directory is now an
error; a first backup still creates the directory.
- 2026-10-06: Made `--older-than` and `--keep-newer-than` reject a - 2026-10-06: Made `--older-than` and `--keep-newer-than` reject a
duration with characters outside its number-and-unit parts duration with characters outside its number-and-unit parts
([issue #215](https://git.eeqj.de/sneak/vaultik/issues/215)). The ([issue #215](https://git.eeqj.de/sneak/vaultik/issues/215)). The
+9 -6
View File
@@ -172,9 +172,9 @@ func TestBannerSuppressedInArgs(t *testing.T) {
// hermeticConfig is a complete, valid config that needs no network and // hermeticConfig is a complete, valid config that needs no network and
// no credentials: file:// storage is exempt from the S3 credential // no credentials: file:// storage is exempt from the S3 credential
// checks, and FileStorer over a directory that does not exist lists // checks. A test that lists the destination must create its directory
// zero objects without erroring. Chunk, blob and compression settings // first, because listing a directory that does not exist is an error.
// are filled in by config.Load. // Chunk, blob and compression settings are filled in by config.Load.
const hermeticConfig = `age_recipients: const hermeticConfig = `age_recipients:
- age1278m9q7dp3chsh2dcy82qk27v047zywyvtxwnj4cvt0z65jw6a7q5dqhfj - age1278m9q7dp3chsh2dcy82qk27v047zywyvtxwnj4cvt0z65jw6a7q5dqhfj
snapshots: snapshots:
@@ -196,22 +196,25 @@ hostname: test-host
// `snapshot list` is the command chosen because it is the only --json // `snapshot list` is the command chosen because it is the only --json
// command that reaches its document without a populated destination // command that reaches its document without a populated destination
// store: it reads the local index, streams `metadata/` (empty here), // store: it reads the local index, streams `metadata/` (empty here),
// and treats a barren destination as an empty list rather than a // and treats an empty destination directory as an empty list rather
// failure. // than a failure.
// //
// Not parallel: it replaces os.Args, os.Stdout and the xdg globals. // Not parallel: it replaces os.Args, os.Stdout and the xdg globals.
func TestEntryJSONStdoutIsExactlyOneDocument(t *testing.T) { func TestEntryJSONStdoutIsExactlyOneDocument(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
configPath := filepath.Join(dir, "config.yml") configPath := filepath.Join(dir, "config.yml")
storeDir := filepath.Join(dir, "store")
contents := fmt.Sprintf(hermeticConfig, contents := fmt.Sprintf(hermeticConfig,
filepath.Join(dir, "source"), filepath.Join(dir, "source"),
filepath.Join(dir, "store"), storeDir,
filepath.Join(dir, "index.sqlite")) filepath.Join(dir, "index.sqlite"))
require.NoError(t, require.NoError(t,
os.WriteFile(configPath, []byte(contents), configFileMode)) os.WriteFile(configPath, []byte(contents), configFileMode))
require.NoError(t, os.Mkdir(storeDir, 0o750))
// The PID lock lives under xdg.DataHome, which xdg resolves at // The PID lock lives under xdg.DataHome, which xdg resolves at
// package init; point it at the temp dir so the test neither // package init; point it at the temp dir so the test neither
// touches nor collides with the real one. // touches nor collides with the real one.
+10 -5
View File
@@ -98,25 +98,30 @@ func TestEntryPruneJSONStdoutIsExactlyOneDocument(t *testing.T) {
} }
} }
// writeHermeticPruneConfig builds a config over a temp directory and, if // writeHermeticPruneConfig builds a config over a temp directory with an
// seedStale is set, creates the index database up front with one // empty destination directory and, if seedStale is set, creates the
// snapshot record that has no counterpart on the destination store. // index database up front with one snapshot record that has no
// Returns the config path. // counterpart on the destination store. Returns the config path.
func writeHermeticPruneConfig(t *testing.T, seedStale bool) string { func writeHermeticPruneConfig(t *testing.T, seedStale bool) string {
t.Helper() t.Helper()
dir := t.TempDir() dir := t.TempDir()
configPath := filepath.Join(dir, "config.yml") configPath := filepath.Join(dir, "config.yml")
indexPath := filepath.Join(dir, "index.sqlite") indexPath := filepath.Join(dir, "index.sqlite")
storeDir := filepath.Join(dir, "store")
contents := fmt.Sprintf(hermeticConfig, contents := fmt.Sprintf(hermeticConfig,
filepath.Join(dir, "source"), filepath.Join(dir, "source"),
filepath.Join(dir, "store"), storeDir,
indexPath) indexPath)
require.NoError(t, require.NoError(t,
os.WriteFile(configPath, []byte(contents), configFileMode)) os.WriteFile(configPath, []byte(contents), configFileMode))
// prune fails on a destination directory that does not exist, so the
// empty store is created here rather than left to a first backup.
require.NoError(t, os.Mkdir(storeDir, 0o750))
// The PID lock lives under xdg.DataHome, which xdg resolves at // The PID lock lives under xdg.DataHome, which xdg resolves at
// package init; point it at the temp dir so the test neither // package init; point it at the temp dir so the test neither
// touches nor collides with the real one. // touches nor collides with the real one.
+23 -8
View File
@@ -22,11 +22,11 @@ type FileStorer struct {
// //
// Construction is intentionally cheap and does not touch the filesystem. // Construction is intentionally cheap and does not touch the filesystem.
// The basePath is recorded; the directory is created lazily on first // The basePath is recorded; the directory is created lazily on first
// write. Reads (Get/Stat/List) tolerate a missing basePath — a missing // write. Write operations (Put/PutWithProgress) call MkdirAll for the
// or unmounted destination during `snapshot list` should NOT block the
// command, it should degrade to "no remote snapshots reachable" with a
// warning. Write operations (Put/PutWithProgress) call MkdirAll for the
// per-blob parent directory, which also covers basePath on first use. // per-blob parent directory, which also covers basePath on first use.
// Get and Stat report a key under a missing basePath as ErrNotFound.
// List and ListStream fail on a missing basePath, because listing it as
// an empty store would make `prune` drop every local snapshot record.
// //
// Uses the real OS filesystem by default; call SetFilesystem to // Uses the real OS filesystem by default; call SetFilesystem to
// override for testing. // override for testing.
@@ -119,13 +119,20 @@ func (f *FileStorer) Delete(_ context.Context, key string) error {
return nil return nil
} }
// List returns all keys with the given prefix. // List returns all keys with the given prefix. It fails when the
// destination directory is missing; a missing prefix under it 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
_, err := f.fs.Stat(f.basePath)
if err != nil {
return nil, fmt.Errorf("checking destination directory: %w", err)
}
basePath := f.fullPath(prefix) basePath := f.fullPath(prefix)
// Check if base path exists // Check if the prefix exists
exists, err := afero.Exists(f.fs, 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)
@@ -167,16 +174,24 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
return keys, nil return keys, nil
} }
// ListStream returns a channel of ObjectInfo for large result sets. // ListStream returns a channel of ObjectInfo for large result sets. Like
// List, it sends an error when the destination directory 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)
go func() { go func() {
defer close(ch) defer close(ch)
_, err := f.fs.Stat(f.basePath)
if err != nil {
ch <- ObjectInfo{Err: fmt.Errorf("checking destination directory: %w", err)}
return
}
basePath := f.fullPath(prefix) basePath := f.fullPath(prefix)
// Check if base path exists // Check if the prefix exists
exists, err := afero.Exists(f.fs, 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)}
+35
View File
@@ -1,6 +1,10 @@
package storage_test package storage_test
import ( import (
"context"
"errors"
"io/fs"
"path/filepath"
"testing" "testing"
"sneak.berlin/go/vaultik/internal/storage" "sneak.berlin/go/vaultik/internal/storage"
@@ -25,3 +29,34 @@ func TestFileStorer(t *testing.T) {
t.Parallel() t.Parallel()
runStorerConformance(t, newFileStorer) runStorerConformance(t, newFileStorer)
} }
// TestFileStorerListMissingDestination checks that List and ListStream
// fail when the destination directory does not exist, instead of
// reporting an empty store.
func TestFileStorerListMissingDestination(t *testing.T) {
t.Parallel()
ctx := context.Background()
s, err := storage.NewFileStorer(filepath.Join(t.TempDir(), "unmounted"))
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
keys, err := s.List(ctx, "metadata/")
if !errors.Is(err, fs.ErrNotExist) {
t.Errorf("List = %v, %v; want a not-exist error", keys, err)
}
var streamErr error
for object := range s.ListStream(ctx, "metadata/") {
if object.Err != nil {
streamErr = object.Err
}
}
if !errors.Is(streamErr, fs.ErrNotExist) {
t.Errorf("ListStream error = %v, want a not-exist error", streamErr)
}
}
+4
View File
@@ -301,6 +301,10 @@ func TestInterruptedBlobUploadRecordsNoUploadedBlob(t *testing.T) {
writeFaultSourceTree(t, fs, dataDir) writeFaultSourceTree(t, fs, dataDir)
// No upload succeeds, so nothing creates the destination directory.
// It must exist for the listing below to show that no blob survived.
require.NoError(t, os.Mkdir(storeDir, 0o750))
inner, err := storage.NewFileStorer(storeDir) inner, err := storage.NewFileStorer(storeDir)
require.NoError(t, err) require.NoError(t, err)
@@ -0,0 +1,155 @@
package vaultik_test
import (
"bytes"
"context"
"io/fs"
"os"
"path/filepath"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"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/220: a
// file:// destination whose directory is missing, such as a USB stick
// that is not plugged in, cannot be listed. It is not an empty store, so
// no command may conclude from it that the local snapshots are gone.
// backUpToFileDestination backs up the snapshot named "first" to a
// file:// destination at storeDir, which need not exist yet. Everything
// the returned Vaultik prints after the backup goes to the returned
// buffer.
func backUpToFileDestination(
ctx context.Context, t *testing.T, storeDir string,
) (*vaultik.Vaultik, *database.Repositories, *bytes.Buffer) {
t.Helper()
osFs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "src")
dbPath := filepath.Join(tempDir, "index.sqlite")
writeFaultSourceTree(t, osFs, dataDir)
store, err := storage.NewFileStorer(storeDir)
require.NoError(t, err)
db, err := database.New(ctx, dbPath)
require.NoError(t, err)
t.Cleanup(func() { _ = db.Close() })
repos := database.NewRepositories(db)
cfg := changedFileConfig(dataDir, dbPath)
v := newBackupVaultik(ctx, cfg, store, repos, db, osFs)
require.NoError(t, backUp(v, "first"))
out := &bytes.Buffer{}
v.Stdout = out
v.UI = ui.NewWithColor(out, false)
return v, repos, out
}
// backUpThenUnplug backs up to a file:// destination, then moves the
// destination directory away, as unplugging the volume it lives on would.
func backUpThenUnplug(
ctx context.Context, t *testing.T,
) (*vaultik.Vaultik, *database.Repositories, *bytes.Buffer) {
t.Helper()
storeDir := filepath.Join(t.TempDir(), "usbstick")
v, repos, out := backUpToFileDestination(ctx, t, storeDir)
require.NoError(t, os.Rename(storeDir, storeDir+"-unplugged"))
return v, repos, out
}
// TestFirstBackupCreatesDestinationDirectory checks that a first backup
// to a destination directory that does not exist yet creates it, and
// that the destination can be listed afterwards.
//
//nolint:paralleltest // installs the global logger via log.Initialize
func TestFirstBackupCreatesDestinationDirectory(t *testing.T) {
log.Initialize(log.Config{})
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "volume", "backup")
v, _, out := backUpToFileDestination(ctx, t, storeDir)
require.NoError(t, v.ListSnapshots(false))
assert.NotContains(t, out.String(), "Could not list backup destination store")
assert.NotContains(t, out.String(), "not found in backup destination store")
}
// TestListSnapshotsWarnsWhenDestinationMissing checks that snapshot list
// warns and shows the local index alone, without reporting the local
// snapshot as missing from the destination.
//
//nolint:paralleltest // installs the global logger via log.Initialize
func TestListSnapshotsWarnsWhenDestinationMissing(t *testing.T) {
log.Initialize(log.Config{})
ctx := context.Background()
v, repos, out := backUpThenUnplug(ctx, t)
id := localSnapshotID(ctx, t, repos, "first")
require.NoError(t, v.ListSnapshots(false))
assert.Contains(t, out.String(), "Could not list backup destination store")
assert.Contains(t, out.String(), "Showing snapshots from the local index only.")
assert.Contains(t, out.String(), id)
assert.NotContains(t, out.String(), "not found in backup destination store")
}
// TestRemoveSnapshotWarnsWhenDestinationMissing checks that snapshot
// remove warns that the metadata could not be removed from the
// destination, instead of reporting that it was.
//
//nolint:paralleltest // installs the global logger via log.Initialize
func TestRemoveSnapshotWarnsWhenDestinationMissing(t *testing.T) {
log.Initialize(log.Config{})
ctx := context.Background()
v, repos, out := backUpThenUnplug(ctx, t)
result, err := v.RemoveSnapshot(localSnapshotID(ctx, t, repos, "first"),
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.False(t, result.RemoteRemoved)
assert.Contains(t, out.String(),
"Could not remove snapshot metadata from remote")
assert.NotContains(t, out.String(),
"Removed snapshot metadata from remote storage")
}
// TestPruneKeepsLocalRecordsWhenDestinationMissing checks that prune
// fails on a destination it cannot list and deletes no local snapshot
// record.
//
//nolint:paralleltest // installs the global logger via log.Initialize
func TestPruneKeepsLocalRecordsWhenDestinationMissing(t *testing.T) {
log.Initialize(log.Config{})
ctx := context.Background()
v, repos, _ := backUpThenUnplug(ctx, t)
err := v.Prune(&vaultik.PruneOptions{Force: true})
require.ErrorIs(t, err, fs.ErrNotExist)
require.ErrorContains(t, err, "listing remote snapshots")
snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
require.NoError(t, err)
assert.Len(t, snapshots, 1, "prune must delete no local snapshot record")
}