2 Commits
Author SHA1 Message Date
clawbot 19896120ff Bind the local index only after a backup writes the marker (closes #420)
check / check (push) Failing after 14m16s
A first backup that failed before writing anything (a mistyped bucket,
a refused credential) bound the local index to that destination, and
the corrected config was then refused.

A backup now checks for a different binding first, writes the
destination marker, and only then records the configured destination.
Purge, prune, snapshot remove and remote nuke still refuse a different
binding but no longer record one: only a backup puts data in the local
index, so an unbound local index holds nothing from any destination.

Model: opus-5-5
2026-11-11 02:26:35 +00:00
clawbot 88a2aa7953 Shorten internal/vaultik tests under the race detector (closes #416)
check / check (push) Successful in 9m11s
The internal/vaultik tests took 36 to 56 seconds under go test -race and ran past the 90-second timeout when the host was busy. Tests that built the same backup now copy one shared backup, and the interruption tests use smaller sources. No assertion is weakened and no test is removed. Measured back to back on next under the same load, the package takes 55% of its former time.

Model: opus-5-5
2026-11-11 03:25:41 +01:00
9 changed files with 308 additions and 77 deletions

No files matched your search

+14 -6
View File
@@ -51,7 +51,7 @@ func (v *Vaultik) checkDestinationMarker() error {
}
// Config is nil only in tests, which then record no binding (see
// EnsureStorageBinding).
// checkStorageBinding).
if v.Config != nil {
stored, configured, err := v.storageBinding()
if err != nil {
@@ -83,11 +83,19 @@ func (v *Vaultik) checkDestinationMarker() error {
return nil
}
// markDestination checks the backup destination store with
// checkDestinationMarker and then writes the marker, so a first backup
// marks its destination and a later one writes the same text again.
// markDestination refuses a local index bound to a different
// destination, checks the backup destination store for its marker,
// writes the marker, and then records the binding. A first backup marks
// its destination and a later one writes the same text again. The
// binding is recorded last, so a first backup that cannot write to the
// destination leaves the local index unbound.
func (v *Vaultik) markDestination() error {
err := v.checkDestinationMarker()
err := v.checkStorageBinding()
if err != nil {
return err
}
err = v.checkDestinationMarker()
if err != nil {
return err
}
@@ -99,5 +107,5 @@ func (v *Vaultik) markDestination() error {
destinationMarkerKey, err)
}
return nil
return v.EnsureStorageBinding()
}
+84 -12
View File
@@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"strings"
"sync"
"testing"
"github.com/spf13/afero"
@@ -24,19 +25,28 @@ import (
// 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.
// newFileDestinationVaultik returns a Vaultik that backs up the
// snapshot named "first" to a file:// destination at storeDir.
// newFileDestinationVaultik writes the fault source tree and returns a
// Vaultik that backs it up to a file:// destination at storeDir.
func newFileDestinationVaultik(
ctx context.Context, t *testing.T, storeDir string,
) (*vaultik.Vaultik, *database.Repositories) {
t.Helper()
osFs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "src")
dbPath := filepath.Join(tempDir, "index.sqlite")
writeFaultSourceTree(t, afero.NewOsFs(), dataDir)
writeFaultSourceTree(t, osFs, dataDir)
return openFileDestinationVaultik(ctx, t, storeDir, dataDir,
filepath.Join(tempDir, "index.sqlite"))
}
// openFileDestinationVaultik returns a Vaultik that backs up dataDir to a
// file:// destination at storeDir, with its local index at dbPath. The
// index is closed when the test ends.
func openFileDestinationVaultik(
ctx context.Context, t *testing.T, storeDir, dataDir, dbPath string,
) (*vaultik.Vaultik, *database.Repositories) {
t.Helper()
store, err := storage.NewFileStorer(storeDir)
require.NoError(t, err)
@@ -48,22 +58,84 @@ func newFileDestinationVaultik(
repos := database.NewRepositories(db)
cfg := changedFileConfig(dataDir, dbPath)
return newBackupVaultik(ctx, cfg, store, repos, db, osFs), repos
return newBackupVaultik(ctx, cfg, store, repos, db, afero.NewOsFs()), repos
}
// firstBackup is one backup of the snapshot named "first" to a file://
// destination. dir holds its source tree in src, its store in store and
// its closed local index in index/index.sqlite. The first test that
// needs it makes it, and TestMain removes it. Every test that copies it
// backs up the same source tree, so no test may change that tree.
//
//nolint:gochecknoglobals // made once and shared by the tests that copy it
var firstBackup struct {
once sync.Once
dir string
ready bool
}
// TestMain removes firstBackup once every test has run.
func TestMain(m *testing.M) {
code := m.Run()
_ = os.RemoveAll(firstBackup.dir)
os.Exit(code)
}
// makeFirstBackup writes the fault source tree and backs it up as the
// snapshot named "first", all in a new directory that it records in
// firstBackup.
func makeFirstBackup(ctx context.Context, t *testing.T) {
t.Helper()
dir, err := os.MkdirTemp("", "vaultik-first-backup-")
require.NoError(t, err)
firstBackup.dir = dir
storeDir := filepath.Join(dir, "store")
dataDir := filepath.Join(dir, "src")
indexDir := filepath.Join(dir, "index")
require.NoError(t, os.Mkdir(storeDir, 0o750))
require.NoError(t, os.Mkdir(indexDir, 0o750))
writeFaultSourceTree(t, afero.NewOsFs(), dataDir)
v, _ := openFileDestinationVaultik(ctx, t, storeDir, dataDir,
filepath.Join(indexDir, "index.sqlite"))
require.NoError(t, backUp(v, "first"))
require.NoError(t, v.DB.Close())
firstBackup.ready = true
}
// backUpToFileDestination creates the directory storeDir, as a user must
// before the first backup, and backs up the snapshot named "first" to a
// file:// destination there. Everything the returned Vaultik prints
// after the backup goes to the returned buffer.
// before the first backup, and returns a Vaultik whose file://
// destination there holds a backup of the snapshot named "first". The
// store and the local index are copies of firstBackup's, which is faster
// than backing up in every test. The returned Vaultik backs up a source
// tree shared with every other caller, so no test may change that tree.
// 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()
require.NoError(t, os.MkdirAll(storeDir, 0o750))
firstBackup.once.Do(func() { makeFirstBackup(ctx, t) })
require.True(t, firstBackup.ready, "another test failed to make firstBackup")
v, repos := newFileDestinationVaultik(ctx, t, storeDir)
require.NoError(t, backUp(v, "first"))
require.NoError(t, os.MkdirAll(storeDir, 0o750))
require.NoError(t, os.CopyFS(storeDir,
os.DirFS(filepath.Join(firstBackup.dir, "store"))))
indexDir := t.TempDir()
require.NoError(t, os.CopyFS(indexDir,
os.DirFS(filepath.Join(firstBackup.dir, "index"))))
v, repos := openFileDestinationVaultik(ctx, t, storeDir,
filepath.Join(firstBackup.dir, "src"),
filepath.Join(indexDir, "index.sqlite"))
out := &bytes.Buffer{}
v.Stdout = out
+2 -2
View File
@@ -68,7 +68,7 @@ func (v *Vaultik) NukeRemote(force bool) error {
// the same way, so the advice below must not follow these errors.
// The listing also fails on a destination that answers that it has
// no marker while the local index holds snapshots.
err := v.EnsureStorageBinding()
err := v.checkStorageBinding()
if err != nil {
return err
}
@@ -192,7 +192,7 @@ type PruneBlobsResult struct {
// PruneDatabase or PruneBlobs individually unless it specifically wants
// one half.
func (v *Vaultik) Prune(opts *PruneOptions) error {
err := v.EnsureStorageBinding()
err := v.checkStorageBinding()
if err != nil {
return err
}
+7 -7
View File
@@ -46,11 +46,11 @@ import (
// slice out a few KB — currently the dominant cost in restore.
//
// The test deliberately constructs an adversarial scenario: three
// blobs A/B/C of ~6 MB each, nine files distributed across them, and
// path-ordered names that interleave the blobs (a1, b1, c1, a2, b2,
// c2, …) so naive path-order processing would touch every blob before
// finishing any of them.
// localitySource is one 1 MiB fixture file used by the locality test.
// blobs A/B/C of five 64 KiB chunks each, nine files distributed across
// them, and path-ordered names that interleave the blobs (a1, b1, c1,
// a2, b2, c2, …) so naive path-order processing would touch every blob
// before finishing any of them.
// localitySource is one 64 KiB fixture file used by the locality test.
type localitySource struct {
path string
data []byte
@@ -65,10 +65,10 @@ type localityCopy struct {
// localitySourceBytes is the size of each source file of the locality
// fixture.
const localitySourceBytes = 1024 * 1024
const localitySourceBytes = 64 * 1024
// buildLocalityFixture writes the adversarial source layout described
// in TestRestoreLocalityAndReadAt: 15 sources of 1 MiB each (which the
// in TestRestoreLocalityAndReadAt: 15 sources of 64 KiB each (which the
// backup packs into 3 blobs of 5 chunks) plus 9 interleaved-name copies
// (3 per blob group).
func buildLocalityFixture(
@@ -27,10 +27,10 @@ import (
// TestRestoreSweeperEvictsBlobs exercises the reference-counted blob
// disk cache eviction during restore.
//
// The scenario: 30 unique 1 MB random files plus 10 duplicates of those
// (40 files total, 30 MB of unique content) get backed up with a 10 MB
// blob_size_limit. After backup the snapshot's encrypted blobs are
// restored through Vaultik.Restore, and per-key Get counts on the
// The scenario: 30 unique 128 KiB random files plus 10 duplicates of
// those (40 files total, 3.75 MiB of unique content) get backed up with
// a 1 MiB blob_size_limit. After backup the snapshot's encrypted blobs
// are restored through Vaultik.Restore, and per-key Get counts on the
// storage layer are recorded. Each blob in the snapshot MUST be
// downloaded exactly once — re-downloads would mean the sweeper either
// evicted a blob that was still needed (LRU regression) or that the
@@ -41,7 +41,7 @@ import (
// original AND every duplicate referencing its chunks have been
// restored.
// buildSweeperFixture writes the source layout for the sweeper test: 30
// unique 1 MB random files plus 10 duplicates of a random subset. The
// unique 128 KiB random files plus 10 duplicates of a random subset. The
// PRNG seed is fixed so failures are reproducible; the entropy is what
// matters here — the FastCDC chunker needs realistic-looking data to
// pick chunk boundaries naturally. Returns the expected content by
@@ -51,7 +51,7 @@ func buildSweeperFixture(
) map[string][]byte {
t.Helper()
const fileSize = 1 * 1024 * 1024
const fileSize = 128 * 1024
rng := rand.New(rand.NewSource(42)) //nolint:gosec // G404: fixture data only
@@ -159,7 +159,7 @@ func runSweeperBackup(
require.NoError(t, err)
require.Equal(t, uniqueFiles+duplicateFiles, scanResult.FilesScanned)
require.Greater(t, scanResult.BlobsCreated, 1,
"30 MB of unique data at 10 MB blob size should yield multiple blobs")
"3.75 MiB of unique data at 1 MiB blob size should yield multiple blobs")
require.NoError(t, sm.CompleteSnapshot(ctx, snapshotID))
require.NoError(t, sm.ExportSnapshotMetadata(ctx, dbPath, snapshotID))
@@ -199,8 +199,8 @@ func TestRestoreSweeperEvictsBlobs(t *testing.T) {
expected := buildSweeperFixture(t, fs, dataDir, uniqueFiles, duplicateFiles)
chunkSize := int64(64 * 1024)
maxBlobSize := int64(10 * 1024 * 1024)
chunkSize := int64(16 * 1024)
maxBlobSize := int64(1024 * 1024)
storer, err := storage.NewFileStorer(storeDir)
require.NoError(t, err)
+5 -10
View File
@@ -115,14 +115,9 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
"index_path", v.Config.IndexPath,
)
err := v.EnsureStorageBinding()
if err != nil {
return err
}
// Before PruneDatabase: it drops incomplete snapshots, which could
// leave the local index without one and so skip the marker check.
err = v.markDestination()
err := v.markDestination()
if err != nil {
return err
}
@@ -749,7 +744,7 @@ type SnapshotPurgeOptions struct {
// snapshot name, not the latest globally. This prevents `home` and `system`
// snapshots from cannibalizing each other.
func (v *Vaultik) PurgeSnapshotsWithOptions(opts *SnapshotPurgeOptions) error {
err := v.EnsureStorageBinding()
err := v.checkStorageBinding()
if err != nil {
return err
}
@@ -1285,7 +1280,7 @@ func (v *Vaultik) outputVerifyJSON(result *VerifyResult) error {
// jq` fail (issue #108). The narration is duplicated as log records,
// which go to stderr and so cannot corrupt the document.
func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error {
err := v.EnsureStorageBinding()
err := v.checkStorageBinding()
if err != nil {
return err
}
@@ -1475,7 +1470,7 @@ func (v *Vaultik) RemoveSnapshot(
SnapshotID: snapshotID,
}
err := v.EnsureStorageBinding()
err := v.checkStorageBinding()
if err != nil {
return result, err
}
@@ -1712,7 +1707,7 @@ func (v *Vaultik) removeSnapshotRemote(
// before it removes anything when the destination store cannot be
// listed or is not present by checkDestinationMarker.
func (v *Vaultik) RemoveAllSnapshots(opts *RemoveOptions) (*RemoveResult, error) {
err := v.EnsureStorageBinding()
err := v.checkStorageBinding()
if err != nil {
return nil, err
}
+30 -5
View File
@@ -187,6 +187,17 @@ func (e *listEnv) addRemoteRawTimestamp(
t.Helper()
remoteKey := snapshot.RemoteSnapshotKey(snapshotID)
e.putRemoteManifest(t, remoteKey,
encodeRemoteManifest(t, remoteKey, timestamp))
return remoteKey
}
// encodeRemoteManifest returns the encoded manifest of a one-blob
// snapshot whose remote key is remoteKey.
func encodeRemoteManifest(t *testing.T, remoteKey, timestamp string) []byte {
t.Helper()
manifest := &snapshot.Manifest{
// Note: the hashed key, never the human ID. That is precisely
// why a remote-only snapshot cannot be named.
@@ -202,11 +213,17 @@ func (e *listEnv) addRemoteRawTimestamp(
data, err := snapshot.EncodeManifest(manifest, 3)
require.NoError(t, err)
err = e.store.Put(context.Background(),
return data
}
// putRemoteManifest stores an encoded manifest on the destination store
// under remoteKey.
func (e *listEnv) putRemoteManifest(t *testing.T, remoteKey string, data []byte) {
t.Helper()
err := e.store.Put(context.Background(),
"metadata/"+remoteKey+"/manifest.json.zst", bytes.NewReader(data))
require.NoError(t, err)
return remoteKey
}
// Fixtures shared across the listing tests.
@@ -791,9 +808,17 @@ func TestListSnapshots_JSONReportsTruncation(t *testing.T) {
timestamp := time.Date(2026, 3, 2, 11, 22, 33, 0, time.UTC)
// One past the cap, so exactly one snapshot is omitted.
// One past the cap, so exactly one snapshot is omitted. The listing
// names a snapshot by the key its manifest is stored under, never by
// the manifest's own ID. So all of them share one encoded manifest:
// encoding 1001 took seconds.
data := encodeRemoteManifest(t,
snapshot.RemoteSnapshotKey("otherhost_bulk"),
timestamp.Format(time.RFC3339))
for i := range maxRemoteOnlyRowsForTest + 1 {
env.addRemote(t, fmt.Sprintf("otherhost_bulk_%04d", i), timestamp)
env.putRemoteManifest(t, snapshot.RemoteSnapshotKey(
fmt.Sprintf("otherhost_bulk_%04d", i)), data)
}
stderr := captureProcessStderr(t, func() {
+39 -26
View File
@@ -13,35 +13,30 @@ import (
var errStorageBindingMismatch = errors.New(
"local index is bound to a different backup destination")
// EnsureStorageBinding guarantees that the local index database is
// bound to the currently-configured storage destination. Every mutating
// command must call this before touching either the local index or the
// destination store, because the two live in lockstep: the local index
// records which chunks/blobs already exist at the destination, and
// mismatched destination + local index produces silent corruption (the
// scanner sees "known" chunks and skips uploads, then writes snapshots
// whose manifests reference blobs that aren't on the new destination).
// checkStorageBinding refuses when the local index database is bound to
// a different destination than the configured one, with an error that
// tells the user how to recover (revert the config, or run `vaultik
// database delete`). Every mutating command calls it before touching
// either the local index or the destination store, because the two live
// in lockstep: the local index records which chunks/blobs already exist
// at the destination, and a mismatched pair produces silent corruption
// (the scanner sees "known" chunks and skips uploads, then writes
// snapshots whose manifests reference blobs that aren't on the new
// destination).
//
// Behaviour:
// - On first use (empty stored value), record the configured
// destination (storage_url, or the s3.* endpoint, bucket and
// prefix) and log the binding.
// - When the stored value matches the configured destination, do
// nothing and return nil.
// - When the two differ, refuse with an error that tells the user
// how to recover (revert the config, or run `vaultik database
// delete` to discard the local index and rebuild against the new
// destination on the next backup).
// It records nothing. Only a backup binds the local index, through
// EnsureStorageBinding, and it does so before adding anything to it. An
// unbound local index therefore holds nothing from any destination.
//
// Read-only inspection commands (remote info, snapshot list, etc.)
// deliberately don't call this: they can be run against a bare
// destination store without any binding state.
func (v *Vaultik) EnsureStorageBinding() error {
func (v *Vaultik) checkStorageBinding() error {
if v.Repositories == nil || v.Config == nil {
// NewForTesting builds a Vaultik with no DB or config;
// there's nothing to bind and no binding to check. Callers
// exercising the bind path use a real DB and populate Config
// explicitly (see storage_bind_test.go).
// there's no binding to check. Callers exercising the bind
// path use a real DB and populate Config explicitly (see
// storage_bind_test.go).
return nil
}
@@ -50,6 +45,24 @@ func (v *Vaultik) EnsureStorageBinding() error {
return err
}
if stored == "" || stored == configured {
return nil
}
return fmt.Errorf("%w\n%s",
errStorageBindingMismatch, buildBindingMismatchMessage(stored, configured))
}
// EnsureStorageBinding records the configured destination (storage_url,
// or the s3.* endpoint, bucket and prefix) as the binding of an unbound
// local index. It refuses as checkStorageBinding does when the local
// index is bound to a different destination.
func (v *Vaultik) EnsureStorageBinding() error {
stored, configured, err := v.storageBinding()
if err != nil {
return err
}
if stored == "" {
err = v.Repositories.LocalMeta.Set(
v.ctx, database.LocalMetaKeyStorageURL, configured)
@@ -62,12 +75,12 @@ func (v *Vaultik) EnsureStorageBinding() error {
return nil
}
if stored == configured {
return nil
if stored != configured {
return fmt.Errorf("%w\n%s",
errStorageBindingMismatch, buildBindingMismatchMessage(stored, configured))
}
return fmt.Errorf("%w\n%s",
errStorageBindingMismatch, buildBindingMismatchMessage(stored, configured))
return nil
}
// storageBinding returns the destination the local index is bound to
+118
View File
@@ -2,6 +2,7 @@ package vaultik_test
import (
"context"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
@@ -9,9 +10,126 @@ import (
"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/storage/faultstore"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// storageBindingOf returns the destination the local index is bound to,
// or "" when it is not bound.
func storageBindingOf(
ctx context.Context, t *testing.T, repos *database.Repositories,
) string {
t.Helper()
got, err := repos.LocalMeta.Get(ctx, database.LocalMetaKeyStorageURL)
require.NoError(t, err)
return got
}
// TestFailedFirstBackupLeavesIndexUnbound checks that a first backup
// the destination refuses, as a mistyped bucket would, leaves the local
// index unbound, so a backup with the corrected destination succeeds.
func TestFailedFirstBackupLeavesIndexUnbound(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
storeDir := t.TempDir()
v, repos := newFileDestinationVaultik(ctx, t, storeDir)
working := v.Storage
refusing := faultstore.New(working)
refusing.OnPut = func(string) faultstore.PutAction { return faultstore.PutFail }
refusing.OnStat = func(string) bool { return true }
v.Storage = refusing
v.Config.StorageURL = "file://" + filepath.Join(t.TempDir(), "typo")
require.ErrorIs(t, backUp(v, "first"), faultstore.ErrInjectedUpload)
assert.Empty(t, storageBindingOf(ctx, t, repos))
v.Storage = working
v.Config.StorageURL = "file://" + storeDir
require.NoError(t, backUp(v, "first"))
assert.Equal(t, "file://"+storeDir, storageBindingOf(ctx, t, repos))
}
// TestBackupToOtherDestinationWritesNoMarker checks that a backup
// refuses a local index bound to a different destination before it
// writes the marker to the configured one.
func TestBackupToOtherDestinationWritesNoMarker(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
firstDir := t.TempDir()
v, _ := newFileDestinationVaultik(ctx, t, firstDir)
v.Config.StorageURL = "file://" + firstDir
require.NoError(t, backUp(v, "first"))
secondDir := t.TempDir()
secondStore, err := storage.NewFileStorer(secondDir)
require.NoError(t, err)
v.Storage = secondStore
v.Config.StorageURL = "file://" + secondDir
require.ErrorContains(t, backUp(v, "first"),
"local index is bound to a different backup destination")
assert.NoFileExists(t, filepath.Join(secondDir, markerFile))
}
// TestCommandsOtherThanBackupLeaveIndexUnbound checks that commands
// other than a backup do not bind an unbound local index, whether or not
// they fail. Each runs on a fresh local index.
func TestCommandsOtherThanBackupLeaveIndexUnbound(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
tests := []struct {
name string
run func(v *vaultik.Vaultik) error
}{
{"prune", func(v *vaultik.Vaultik) error {
return v.Prune(&vaultik.PruneOptions{Force: true})
}},
{"snapshot purge", func(v *vaultik.Vaultik) error {
return v.PurgeSnapshotsWithOptions(
&vaultik.SnapshotPurgeOptions{KeepLatest: true, Force: true})
}},
{"snapshot remove", func(v *vaultik.Vaultik) error {
_, err := v.RemoveSnapshot("host_first_2026-01-02T03:04:05Z",
&vaultik.RemoveOptions{Force: true})
return err
}},
{"snapshot remove --all", func(v *vaultik.Vaultik) error {
_, err := v.RemoveAllSnapshots(&vaultik.RemoveOptions{Force: true})
return err
}},
{"remote nuke", func(v *vaultik.Vaultik) error {
return v.NukeRemote(true)
}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
v, repos := newFileDestinationVaultik(ctx, t, t.TempDir())
_ = tt.run(v)
assert.Empty(t, storageBindingOf(ctx, t, repos))
})
}
}
// buildBindTestVaultik returns a minimal Vaultik wired with a real
// in-memory DB and a config carrying the given StorageURL. Enough to
// exercise EnsureStorageBinding without spinning up storage or fx.