1 Commits
Author SHA1 Message Date
sneak 89ce0f36f7 Make snapshot list report a changed destination instead of advising prune (closes #317)
check / check (push) Waiting to run
When the local index is bound to a different destination than the
configured one, snapshot list now gives the warning prune, snapshot
purge and snapshot remove refuse with: both destinations and the two
ways out (revert the configuration, or run vaultik database delete).
Below the table it replaces the local-only note and its prune advice.
With --json it goes to stderr through the logger, so stdout stays the
document, whose local rows show remote_present false.

Reading the binding moves out of EnsureStorageBinding into
storageBinding, which records nothing, so the read-only list can call
it without binding a fresh index.

Model: opus-5-5
2026-10-14 01:50:52 +00:00
11 changed files with 145 additions and 50 deletions
+10 -4
View File
@@ -323,7 +323,11 @@ read from the manifest.
Snapshots in the local index with no counterpart on the destination Snapshots in the local index with no counterpart on the destination
store are reported below the table as drift, with the `vaultik prune` store are reported below the table as drift, with the `vaultik prune`
invocation that reconciles them. invocation that reconciles them. If the local index is bound to a
different destination than the configured one (after a change to
`storage_url`, or `s3.endpoint`, `s3.bucket` or `s3.prefix`), the
command says so instead and gives the same two ways out that `prune`
gives: revert the configuration, or run `vaultik database delete`.
If the destination store cannot be listed (unmounted volume, If the destination store cannot be listed (unmounted volume,
permission denied, network down), the command warns, falls back to the permission denied, network down), the command warns, falls back to the
@@ -332,9 +336,11 @@ local index alone, and still exits zero.
(whether the snapshot is in the local index), `remote_key` (the full (whether the snapshot is in the local index), `remote_key` (the full
64-character storage key), and `remote_present` (whether it was seen 64-character storage key), and `remote_present` (whether it was seen
on the destination store, or `null` if the destination could not be on the destination store, or `null` if the destination could not be
listed). Warnings about an unlistable destination, unreadable listed). Warnings about an unlistable destination, a local index
manifests, and a truncated listing all go to stderr through the bound to a different destination (with both destinations and the
logger, so stdout stays a single parseable document. same two ways out), unreadable manifests, and a truncated listing all
go to stderr through the logger, so stdout stays a single parseable
document.
**`snapshot verify`**: Verify snapshot integrity. **`snapshot verify`**: Verify snapshot integrity.
* Default (shallow): checks that every blob the manifest lists is present in * Default (shallow): checks that every blob the manifest lists is present in
+2 -3
View File
@@ -104,8 +104,7 @@ func (v *Vaultik) FetchAndDecryptBlob(
// FetchBlob downloads a blob and returns a reader for the encrypted data. // FetchBlob downloads a blob and returns a reader for the encrypted data.
// The reader is closed as soon as ctx is cancelled, which ends a read // The reader is closed as soon as ctx is cancelled, which ends a read
// blocked inside it. A storage error is returned as is: restore and deep // blocked inside it.
// verify name the blob in their own errors.
func (v *Vaultik) FetchBlob( func (v *Vaultik) FetchBlob(
ctx context.Context, blobHash string, ctx context.Context, blobHash string,
) (io.ReadCloser, error) { ) (io.ReadCloser, error) {
@@ -120,7 +119,7 @@ func (v *Vaultik) FetchBlob(
rc, err := v.Storage.Get(ctx, blobPath) rc, err := v.Storage.Get(ctx, blobPath)
if err != nil { if err != nil {
return nil, err return nil, fmt.Errorf("downloading blob %s: %w", shortHash(blobHash), err)
} }
return newCloseOnCancelReader(ctx, rc), nil return newCloseOnCancelReader(ctx, rc), nil
+2 -9
View File
@@ -6,7 +6,6 @@ import (
"crypto/rand" "crypto/rand"
"io" "io"
"os" "os"
"path"
"path/filepath" "path/filepath"
"testing" "testing"
@@ -143,8 +142,7 @@ func TestDeepVerifyJSONCountsBlobs(t *testing.T) {
// TestDeepVerifyJSONCountsWrongSizeBlob checks that under --deep --json a // TestDeepVerifyJSONCountsWrongSizeBlob checks that under --deep --json a
// blob of the wrong size is counted in mismatched and fails the run, and // blob of the wrong size is counted in mismatched and fails the run, and
// that the check goes on past it to count a missing blob after it. The // that the check goes on past it to count a missing blob after it.
// store is on disk, so the message must not call it S3.
func TestDeepVerifyJSONCountsWrongSizeBlob(t *testing.T) { func TestDeepVerifyJSONCountsWrongSizeBlob(t *testing.T) {
log.Initialize(log.Config{}) log.Initialize(log.Config{})
t.Parallel() t.Parallel()
@@ -170,14 +168,11 @@ func TestDeepVerifyJSONCountsWrongSizeBlob(t *testing.T) {
require.Equal(t, 1, got.Mismatched) require.Equal(t, 1, got.Mismatched)
require.Equal(t, 1, got.Missing) require.Equal(t, 1, got.Missing)
require.Zero(t, got.Verified) require.Zero(t, got.Verified)
require.Contains(t, got.ErrorMessage, "with the wrong size")
require.NotContains(t, got.ErrorMessage, "S3")
} }
// TestDeepVerifyStopsOnBlobThatCannotBeChecked checks that a blob that is // TestDeepVerifyStopsOnBlobThatCannotBeChecked checks that a blob that is
// present in storage but cannot be checked is not counted as missing: // present in storage but cannot be checked is not counted as missing:
// deep verify stops and fails, saying it could not check the blob, with // deep verify stops and fails with the storage error.
// the storage error.
func TestDeepVerifyStopsOnBlobThatCannotBeChecked(t *testing.T) { func TestDeepVerifyStopsOnBlobThatCannotBeChecked(t *testing.T) {
log.Initialize(log.Config{}) log.Initialize(log.Config{})
t.Parallel() t.Parallel()
@@ -192,8 +187,6 @@ func TestDeepVerifyStopsOnBlobThatCannotBeChecked(t *testing.T) {
require.Error(t, err) require.Error(t, err)
require.Equal(t, "failed", got.Status) require.Equal(t, "failed", got.Status)
require.Zero(t, got.Missing) require.Zero(t, got.Missing)
require.Contains(t, got.ErrorMessage,
"could not check blob "+path.Base(keys[0]))
require.Contains(t, got.ErrorMessage, errStatDenied.Error()) require.Contains(t, got.ErrorMessage, errStatDenied.Error())
} }
+1 -2
View File
@@ -281,8 +281,7 @@ func TestPurgeOlderThan_TooLargeDeletesNothing(t *testing.T) {
OlderThan: "1000y", OlderThan: "1000y",
Force: true, Force: true,
}) })
require.EqualError(t, err, require.Error(t, err)
`invalid duration: "1000y" is longer than about 292 years`)
err = v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ err = v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
OlderThan: "292y", OlderThan: "292y",
+1 -4
View File
@@ -231,8 +231,7 @@ func (f trailingByteFile) Close() error {
} }
// TestRestoreMissingBlobAbortsWithoutSkipErrors checks that a deleted blob // TestRestoreMissingBlobAbortsWithoutSkipErrors checks that a deleted blob
// still ends the restore with an error when SkipErrors is not set, and // still ends the restore with an error when SkipErrors is not set.
// that the error names the blob once.
func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) { func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
log.Initialize(log.Config{}) log.Initialize(log.Config{})
t.Parallel() t.Parallel()
@@ -248,8 +247,6 @@ func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
}) })
require.ErrorIs(t, err, storage.ErrNotFound) require.ErrorIs(t, err, storage.ErrNotFound)
assert.Equal(t, 1, strings.Count(err.Error(), "downloading blob"),
err.Error())
} }
// TestRestoreChunkInNoBlobAbortsWithoutSkipErrors checks that a chunk held // TestRestoreChunkInNoBlobAbortsWithoutSkipErrors checks that a chunk held
+1 -7
View File
@@ -27,7 +27,7 @@ import (
// then grows one stored blob so its size no longer matches the manifest. // then grows one stored blob so its size no longer matches the manifest.
// Shallow verify must then fail, count the grown blob as a size mismatch, // Shallow verify must then fail, count the grown blob as a size mismatch,
// and drop it from the verified count rather than continuing to report it // and drop it from the verified count rather than continuing to report it
// as checked, and its error must not repeat the cli layer's prefix. // as checked.
func TestShallowVerifyDetectsWrongBlobSize(t *testing.T) { func TestShallowVerifyDetectsWrongBlobSize(t *testing.T) {
log.Initialize(log.Config{}) log.Initialize(log.Config{})
t.Parallel() t.Parallel()
@@ -96,12 +96,6 @@ func TestShallowVerifyDetectsWrongBlobSize(t *testing.T) {
require.Equal(t, 1, bad.Mismatched) require.Equal(t, 1, bad.Mismatched)
require.Equal(t, healthy.BlobCount-1, bad.Verified, require.Equal(t, healthy.BlobCount-1, bad.Verified,
"the wrong-sized blob must not be counted as verified") "the wrong-sized blob must not be counted as verified")
// The cli layer prints this error after its own "Verification failed:".
err = newVerifier(io.Discard).VerifySnapshotWithOptions(
snapshotID, &vaultik.VerifyOptions{})
require.EqualError(t, err,
"problems found in storage: 1 blobs have the wrong size")
} }
// TestShallowVerifyDetectsMissingDatabase backs up a real snapshot, // TestShallowVerifyDetectsMissingDatabase backs up a real snapshot,
+2 -5
View File
@@ -23,7 +23,6 @@ var (
errSnapshotNotInConfig = errors.New("snapshot not found in config") errSnapshotNotInConfig = errors.New("snapshot not found in config")
errNoSnapshotsInConfig = errors.New("no snapshots configured") errNoSnapshotsInConfig = errors.New("no snapshots configured")
errSnapshotVerifyFailed = errors.New("verification failed") errSnapshotVerifyFailed = errors.New("verification failed")
errProblemsInStorage = errors.New("problems found in storage")
errRemoveAllNeedsForce = errors.New("--all requires --force") errRemoveAllNeedsForce = errors.New("--all requires --force")
errSnapshotNotFound = errors.New("snapshot not found") errSnapshotNotFound = errors.New("snapshot not found")
errSnapshotRemoveFailed = errors.New("snapshot(s) could not be removed") errSnapshotRemoveFailed = errors.New("snapshot(s) could not be removed")
@@ -613,7 +612,7 @@ func selectSnapshotsToPurge(
case opts.OlderThan != "": case opts.OlderThan != "":
duration, err := parseDuration(opts.OlderThan) duration, err := parseDuration(opts.OlderThan)
if err != nil { if err != nil {
return nil, err return nil, fmt.Errorf("invalid duration: %w", err)
} }
cutoff := time.Now().UTC().Add(-duration) cutoff := time.Now().UTC().Add(-duration)
@@ -942,10 +941,8 @@ func (v *Vaultik) formatVerifyResult(
v.printVerifySummary(result, failure) v.printVerifySummary(result, failure)
} }
// Not errSnapshotVerifyFailed: the cli layer prints this after its own
// "Verification failed:".
if failure != "" { if failure != "" {
return fmt.Errorf("%w: %s", errProblemsInStorage, failure) return fmt.Errorf("%w: %s", errSnapshotVerifyFailed, failure)
} }
return nil return nil
+33 -4
View File
@@ -61,7 +61,9 @@ const tabPadding = 3
// attempt is made to recover or fabricate the human ID. // attempt is made to recover or fabricate the human ID.
// //
// Snapshots in the local index with no counterpart on the destination // Snapshots in the local index with no counterpart on the destination
// store are reported as drift below the table. // store are reported as drift below the table, unless the local index
// is bound to a different destination than the configured one: that is
// reported instead.
// //
// If remote listing fails (unmounted volume, permission denied, // If remote listing fails (unmounted volume, permission denied,
// network), we degrade to local-only with a warning. List never fails // network), we degrade to local-only with a warning. List never fails
@@ -103,7 +105,26 @@ func (v *Vaultik) ListSnapshots(jsonOutput bool) error {
return snapshots[i].Timestamp.After(snapshots[j].Timestamp) return snapshots[i].Timestamp.After(snapshots[j].Timestamp)
}) })
stored, configured, err := v.storageBinding()
if err != nil {
return err
}
// A local index bound to another destination describes that one, so
// its records are expected to be missing here, and prune refuses to
// run. The binding is reported instead of advising prune.
boundElsewhere := stored != "" && stored != configured
if jsonOutput { if jsonOutput {
// stdout holds only the document, so this goes to the logger.
if boundElsewhere {
log.Warn("Local index is bound to a different backup "+
"destination; to proceed, revert storage_url (or the s3.* "+
"endpoint, bucket and prefix) to the bound destination, "+
"or run 'vaultik database delete'",
"bound_to", stored, "configured", configured)
}
if remoteErr == nil { if remoteErr == nil {
v.reportJSONListingLimits(listing) v.reportJSONListingLimits(listing)
} }
@@ -125,8 +146,13 @@ func (v *Vaultik) ListSnapshots(jsonOutput bool) error {
} }
} }
if boundElsewhere {
v.UI.Warningf("%v\n%s", errStorageBindingMismatch,
buildBindingMismatchMessage(stored, configured))
}
if remoteErr == nil { if remoteErr == nil {
v.reportListDrift(snapshots, listing) v.reportListDrift(snapshots, listing, boundElsewhere)
} }
return nil return nil
@@ -432,7 +458,9 @@ func (v *Vaultik) snapshotInfoFromLocal(ls *database.Snapshot) SnapshotInfo {
// reportListDrift prints the reconciliation notes the merged table // reportListDrift prints the reconciliation notes the merged table
// cannot express on its own: local records with no counterpart on the // cannot express on its own: local records with no counterpart on the
// destination store, plus counts of remote snapshots that were // destination store, plus counts of remote snapshots that were
// unreadable or omitted. // unreadable or omitted. Local records are not reported when
// boundElsewhere is set: ListSnapshots has already said why they are
// missing.
// //
// This is what remains of the old reportRemoteDrift, and it no longer // This is what remains of the old reportRemoteDrift, and it no longer
// touches the destination store. Its remote-only half collapsed into // touches the destination store. Its remote-only half collapsed into
@@ -441,6 +469,7 @@ func (v *Vaultik) snapshotInfoFromLocal(ls *database.Snapshot) SnapshotInfo {
// so `snapshot list` lists the destination exactly once per invocation. // so `snapshot list` lists the destination exactly once per invocation.
func (v *Vaultik) reportListDrift( func (v *Vaultik) reportListDrift(
snapshots []SnapshotInfo, listing *remoteSnapshotListing, snapshots []SnapshotInfo, listing *remoteSnapshotListing,
boundElsewhere bool,
) { ) {
var localOnly []string var localOnly []string
@@ -450,7 +479,7 @@ func (v *Vaultik) reportListDrift(
} }
} }
if len(localOnly) > 0 { if len(localOnly) > 0 && !boundElsewhere {
v.UI.Warningf("%d local snapshot record(s) not found in backup "+ v.UI.Warningf("%d local snapshot record(s) not found in backup "+
"destination store:", len(localOnly)) "destination store:", len(localOnly))
+68
View File
@@ -421,6 +421,74 @@ func TestListSnapshots_LocalOnlyReportedAsDrift(t *testing.T) {
assert.NotContains(t, out, "snapshot cleanup") assert.NotContains(t, out, "snapshot cleanup")
} }
// TestListSnapshots_BoundElsewhereDoesNotAdvisePrune covers a changed
// storage_url: the local index is still bound to the old destination,
// so its records are missing from the new one. prune refuses to run in
// that state, so the listing must give the choices prune gives instead
// of advising it.
func TestListSnapshots_BoundElsewhereDoesNotAdvisePrune(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
const (
oldURL = "file:///mnt/backups/old"
newURL = "file:///mnt/backups/new"
)
env := newListEnv(t)
env.v.Config.StorageURL = newURL
require.NoError(t, env.v.Repositories.LocalMeta.Set(context.Background(),
database.LocalMetaKeyStorageURL, oldURL))
env.addLocal(t, listLocalID, time.Date(2026, 3, 1, 10, 0, 0, 0, time.UTC))
err := env.v.ListSnapshots(false)
require.NoError(t, err)
out := env.stdout.String()
assert.Contains(t, out, listLocalID)
assert.Contains(t, out,
"local index is bound to a different backup destination")
assert.Contains(t, out, oldURL)
assert.Contains(t, out, newURL)
assert.Contains(t, out, "revert storage_url")
assert.Contains(t, out, "vaultik database delete")
assert.NotContains(t, out, "vaultik prune")
}
// TestListSnapshots_JSONReportsBoundElsewhere is the --json side of the
// test above. Every local row comes out with remote_present false, which
// a script reads as stale local records unless it is told, on stderr,
// that the local index is bound to a different destination.
//
//nolint:paralleltest // captureProcessStderr replaces os.Stderr
func TestListSnapshots_JSONReportsBoundElsewhere(t *testing.T) {
const (
oldURL = "file:///mnt/backups/old"
newURL = "file:///mnt/backups/new"
)
env := newListEnv(t)
env.v.Config.StorageURL = newURL
require.NoError(t, env.v.Repositories.LocalMeta.Set(context.Background(),
database.LocalMetaKeyStorageURL, oldURL))
env.addLocal(t, listLocalID, time.Date(2026, 3, 1, 10, 0, 0, 0, time.UTC))
stderr := captureProcessStderr(t, func() {
require.NoError(t, env.v.ListSnapshots(true))
})
rows := decodeListJSON(t, env.stdout.String())
require.Len(t, rows, 1)
assert.Equal(t, listLocalID, rows[0].ID)
assert.Contains(t, stderr, "bound to a different backup destination")
assert.Contains(t, stderr, oldURL)
assert.Contains(t, stderr, newURL)
assert.Contains(t, stderr, "revert storage_url")
assert.Contains(t, stderr, "vaultik database delete")
}
// TestListSnapshots_UnreachableRemoteDegrades covers the promise in the // TestListSnapshots_UnreachableRemoteDegrades covers the promise in the
// doc comment: an unreachable destination is a warning plus local-only // doc comment: an unreachable destination is a warning plus local-only
// output, never a failure. // output, never a failure.
+23 -10
View File
@@ -45,17 +45,9 @@ func (v *Vaultik) EnsureStorageBinding() error {
return nil return nil
} }
configured := v.Config.StorageURL stored, configured, err := v.storageBinding()
if configured == "" {
// The s3.* form: bind on its endpoint, bucket and prefix,
// written the way storage_url would name the same destination.
s3 := v.Config.S3
configured = "s3://" + s3.Bucket + "/" + s3.Prefix + "?endpoint=" + s3.Endpoint
}
stored, err := v.Repositories.LocalMeta.Get(v.ctx, database.LocalMetaKeyStorageURL)
if err != nil { if err != nil {
return fmt.Errorf("reading local storage binding: %w", err) return err
} }
if stored == "" { if stored == "" {
@@ -78,6 +70,27 @@ func (v *Vaultik) EnsureStorageBinding() error {
errStorageBindingMismatch, buildBindingMismatchMessage(stored, configured)) errStorageBindingMismatch, buildBindingMismatchMessage(stored, configured))
} }
// storageBinding returns the destination the local index is bound to
// ("" when no binding is recorded yet), then the configured destination
// written the same way. It records nothing, so a read-only command can
// call it.
func (v *Vaultik) storageBinding() (string, string, error) {
configured := v.Config.StorageURL
if configured == "" {
// The s3.* form: bind on its endpoint, bucket and prefix,
// written the way storage_url would name the same destination.
s3 := v.Config.S3
configured = "s3://" + s3.Bucket + "/" + s3.Prefix + "?endpoint=" + s3.Endpoint
}
stored, err := v.Repositories.LocalMeta.Get(v.ctx, database.LocalMetaKeyStorageURL)
if err != nil {
return "", "", fmt.Errorf("reading local storage binding: %w", err)
}
return stored, configured, nil
}
// buildBindingMismatchMessage assembles the multi-line explanation // buildBindingMismatchMessage assembles the multi-line explanation
// shown when the local index is bound to a different destination than // shown when the local index is bound to a different destination than
// the currently-configured one (the first line lives in the // the currently-configured one (the first line lives in the
+2 -2
View File
@@ -680,7 +680,7 @@ func (v *Vaultik) verifyManifestAgainstDatabase(
func (v *Vaultik) verifyBlobExistenceFromDB( func (v *Vaultik) verifyBlobExistenceFromDB(
blobs []snapshot.BlobInfo, result *VerifyResult, blobs []snapshot.BlobInfo, result *VerifyResult,
) error { ) error {
log.Info("Verifying blob existence in storage", "blob_count", len(blobs)) log.Info("Verifying blob existence in S3", "blob_count", len(blobs))
for i, blob := range blobs { for i, blob := range blobs {
// The hash is read from the snapshot database, which is not // The hash is read from the snapshot database, which is not
@@ -700,7 +700,7 @@ func (v *Vaultik) verifyBlobExistenceFromDB(
result.Missing++ result.Missing++
result.MissingSize += blob.CompressedSize result.MissingSize += blob.CompressedSize
case err != nil: case err != nil:
return fmt.Errorf("could not check blob %s in storage: %w", return fmt.Errorf("checking blob %s in storage: %w",
blob.Hash, err) blob.Hash, err)
case stat.Size != blob.CompressedSize: case stat.Size != blob.CompressedSize:
log.Warn("Blob has the wrong size in storage", log.Warn("Blob has the wrong size in storage",