List a missing file:// destination directory as an error (closes #220)

The file backend listed a destination directory that does not exist as
an empty store. With the volume unplugged, snapshot list reported every
local snapshot as missing from the store, snapshot remove said it had
removed metadata it never reached, and prune dropped every local
snapshot record. List and ListStream now fail when the destination
directory is missing, so those commands take their existing path for a
store that cannot be listed. A missing prefix under an existing
directory is still an empty listing, and a first backup still creates
the directory.

Two test fixtures listed a file:// destination nothing had created; they
now create it.

Model: opus-5-5
This commit is contained in:
2026-10-06 04:22:54 +00:00
parent 713be502bd
commit 6488c54560
6 changed files with 238 additions and 13 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
+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, 0o755))
// 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.
+24 -8
View File
@@ -22,11 +22,12 @@ 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: a missing or unmounted
// destination cannot be listed, and 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 +120,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 +175,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, as on an unmounted
// volume, 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, 0o755))
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)
assert.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")
}