restore and verify report an unknown snapshot ID as not found (closes #426)
check / check (push) Successful in 13m30s

snapshot restore and snapshot verify given an ID that names no snapshot in the local index or at the destination failed with "object not found" on a hashed metadata path. They now look the ID up the way snapshot remove does and say that no such snapshot exists, before printing anything else.

Model: opus-5-5
This commit was merged in pull request #446.
This commit is contained in:
clawbot committed 2026-11-11 04:40:18 +01:00
1 parent 0d0c76fa24
commit 1df1d23094
5 files changed
+113 -50

No files matched your search

+10 -12
View File
@@ -128,10 +128,15 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
"paths", opts.Paths, "paths", opts.Paths,
) )
remoteKey, err := v.resolveSnapshotRemoteKey(opts.SnapshotID)
if err != nil {
return err
}
// Step 1: Download and decrypt the snapshot metadata database // Step 1: Download and decrypt the snapshot metadata database
log.Info("Downloading snapshot metadata...") log.Info("Downloading snapshot metadata...")
tempDB, tempDir, err := v.downloadSnapshotDB(opts.SnapshotID, identities) tempDB, tempDir, err := v.downloadSnapshotDB(opts.SnapshotID, remoteKey, identities)
if err != nil { if err != nil {
return fmt.Errorf("downloading snapshot database: %w", err) return fmt.Errorf("downloading snapshot database: %w", err)
} }
@@ -754,19 +759,12 @@ func (v *Vaultik) handleRestoreVerification(
return nil return nil
} }
// downloadSnapshotDB downloads and decrypts the snapshot metadata // downloadSnapshotDB downloads and decrypts the metadata database of the
// database. The identifier is resolved to the snapshot's remote key: a // snapshot at remoteKey, which resolveSnapshotRemoteKey returned for
// human ID is hashed, and a remote key (or its abbreviation, as printed // snapshotID.
// for a remote-only snapshot) is used as-is, so a host with no local
// index can restore the snapshots it can only see on the store.
func (v *Vaultik) downloadSnapshotDB( func (v *Vaultik) downloadSnapshotDB(
snapshotID string, identities []age.Identity, snapshotID, remoteKey string, identities []age.Identity,
) (*database.DB, string, error) { ) (*database.DB, string, error) {
remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
if err != nil {
return nil, "", err
}
// Download encrypted database from storage // Download encrypted database from storage
dbKey := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey) dbKey := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey)
+23 -23
View File
@@ -964,25 +964,22 @@ func (v *Vaultik) VerifySnapshotWithOptions(
Mode: "shallow", Mode: "shallow",
} }
v.printVerifyHeader(snapshotID, opts) // The key is kept so we can also check for the snapshot's encrypted
// database below.
// Resolve the identifier to the snapshot's remote key. A human ID is
// hashed; a remote key (or its abbreviation, as printed for a
// remote-only snapshot) is used as-is, so a host with no local index
// can verify a snapshot it can only see on the store. The key is kept
// so we can also check for the snapshot's encrypted database below.
remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID) remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
if err != nil { if err != nil {
if opts.JSON { if opts.JSON {
result.Status = verifyStatusFailed result.Status = verifyStatusFailed
result.ErrorMessage = fmt.Sprintf("resolving snapshot identifier: %v", err) result.ErrorMessage = err.Error()
return v.outputVerifyJSON(result) return v.outputVerifyJSON(result)
} }
return fmt.Errorf("resolving snapshot identifier: %w", err) return err
} }
v.printVerifyHeader(snapshotID, opts)
manifest, err := v.downloadManifestByKey(remoteKey) manifest, err := v.downloadManifestByKey(remoteKey)
if err != nil { if err != nil {
if opts.JSON { if opts.JSON {
@@ -1475,7 +1472,7 @@ func (v *Vaultik) RemoveSnapshot(
return result, err return result, err
} }
inLocalIndex, err := v.findSnapshotToRemove(snapshotID, opts) inLocalIndex, err := v.checkSnapshotExists(snapshotID, opts.LocalOnly)
if err != nil { if err != nil {
return result, err return result, err
} }
@@ -1538,17 +1535,20 @@ func (v *Vaultik) RemoveSnapshot(
return result, nil return result, nil
} }
// findSnapshotToRemove reports whether the local index has a row for // checkSnapshotExists returns errSnapshotNotFound when the local index
// snapshotID, and returns errSnapshotNotFound when there is nothing to // has no row for snapshotID and, unless localOnly is set, the
// remove: no row and, unless LocalOnly is set, no metadata on the // destination store has no metadata for it either. inLocalIndex reports
// destination store either. The destination store is consulted only // whether the local index has that row. The destination store is
// when the local index has no row, so a snapshot in the local index can // consulted only when the local index has no row, so a snapshot in the
// still be removed from it while the destination store is unreachable, // local index can still be removed from it while the destination store
// and only after checkDestinationMarker passes, so an unplugged volume's // is unreachable, and only after checkDestinationMarker passes, so an
// empty mount point is not taken for a store without the snapshot. // unplugged volume's empty mount point is not taken for a store without
func (v *Vaultik) findSnapshotToRemove( // the snapshot.
snapshotID string, opts *RemoveOptions, //
) (bool, error) { //nolint:nonamedreturns // the result name says what the bool means
func (v *Vaultik) checkSnapshotExists(
snapshotID string, localOnly bool,
) (inLocalIndex bool, err error) {
if v.Repositories != nil { if v.Repositories != nil {
snap, err := v.Repositories.Snapshots.GetByID(v.ctx, snapshotID) snap, err := v.Repositories.Snapshots.GetByID(v.ctx, snapshotID)
if err != nil { if err != nil {
@@ -1560,12 +1560,12 @@ func (v *Vaultik) findSnapshotToRemove(
} }
} }
if opts.LocalOnly { if localOnly {
return false, fmt.Errorf("%w in the local index: %s", return false, fmt.Errorf("%w in the local index: %s",
errSnapshotNotFound, snapshotID) errSnapshotNotFound, snapshotID)
} }
err := v.checkDestinationMarker() err = v.checkDestinationMarker()
if err != nil { if err != nil {
return false, err return false, err
} }
+8 -2
View File
@@ -28,8 +28,9 @@ var (
// Two forms are accepted, matching the two things a host can know: // Two forms are accepted, matching the two things a host can know:
// //
// - A human snapshot ID (hostname_name_timestamp), which a host holding // - A human snapshot ID (hostname_name_timestamp), which a host holding
// the local index has. It is hashed to its remote key; the store is // the local index has. It is hashed to its remote key. An ID that
// not consulted. // names no snapshot in the local index or on the destination store
// is an error (see checkSnapshotExists).
// - A remote key, or the leading part of one, which is all a host with // - A remote key, or the leading part of one, which is all a host with
// no local index can know — it is exactly what `snapshot list` prints // no local index can know — it is exactly what `snapshot list` prints
// for a remote-only snapshot (see formatRemoteOnlyID). It is resolved // for a remote-only snapshot (see formatRemoteOnlyID). It is resolved
@@ -41,6 +42,11 @@ var (
// RFC3339 timestamp). // RFC3339 timestamp).
func (v *Vaultik) resolveSnapshotRemoteKey(identifier string) (string, error) { func (v *Vaultik) resolveSnapshotRemoteKey(identifier string) (string, error) {
if !isRemoteKeyOrPrefix(identifier) { if !isRemoteKeyOrPrefix(identifier) {
_, err := v.checkSnapshotExists(identifier, false)
if err != nil {
return "", err
}
return snapshot.RemoteSnapshotKey(identifier), nil return snapshot.RemoteSnapshotKey(identifier), nil
} }
+62
View File
@@ -0,0 +1,62 @@
package vaultik_test
import (
"bytes"
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/config"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// TestUnknownSnapshotID checks that restore and verify, given an ID that
// names no snapshot in the local index or on the destination store, say
// so in the words `snapshot remove` uses and print nothing else.
func TestUnknownSnapshotID(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
const unknownID = "host1_home_2026-01-01T00:00:00Z"
commands := map[string]func(v *vaultik.Vaultik) error{
"restore": func(v *vaultik.Vaultik) error {
return v.Restore(&vaultik.RestoreOptions{
SnapshotID: unknownID,
TargetDir: t.TempDir(),
})
},
"verify": func(v *vaultik.Vaultik) error {
return v.VerifySnapshotWithOptions(unknownID, &vaultik.VerifyOptions{})
},
"verify --deep": func(v *vaultik.Vaultik) error {
return v.VerifySnapshotWithOptions(unknownID,
&vaultik.VerifyOptions{Deep: true})
},
}
for name, run := range commands {
t.Run(name, func(t *testing.T) {
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
// A backup wrote to this destination, so it carries the marker.
require.NoError(t, store.Put(context.Background(), markerFile,
bytes.NewReader(nil)))
tv := vaultik.NewForTesting(store)
tv.Config = &config.Config{AgeSecretKey: testAgeSecretKey}
tv.Repositories = newLocalIndex(t, "snapshot-001")
err := run(tv.Vaultik)
require.EqualError(t, err, "snapshot not found in the local "+
"index or on the destination store: "+unknownID)
assert.Empty(t, tv.Stdout.String())
})
}
}
+10 -13
View File
@@ -120,12 +120,17 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error {
log.Info("Starting snapshot verification", "snapshot_id", snapshotID, "mode", "deep") log.Info("Starting snapshot verification", "snapshot_id", snapshotID, "mode", "deep")
remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
if err != nil {
return v.deepVerifyFailure(result, opts, err.Error(), err)
}
if !opts.JSON && !v.UI.Quiet() { if !opts.JSON && !v.UI.Quiet() {
v.stdoutf("Deep verification of snapshot: %s\n\n", snapshotID) v.stdoutf("Deep verification of snapshot: %s\n\n", snapshotID)
} }
manifest, tempDB, dbBlobs, err := v.loadVerificationData( manifest, tempDB, dbBlobs, err := v.loadVerificationData(
snapshotID, opts, result, identities) snapshotID, remoteKey, opts, result, identities)
if err != nil { if err != nil {
return err return err
} }
@@ -170,21 +175,13 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error {
return nil return nil
} }
// loadVerificationData downloads manifest, database, and blob list for verification // loadVerificationData downloads manifest, database, and blob list for
// verification of the snapshot at remoteKey, which
// resolveSnapshotRemoteKey returned for snapshotID.
func (v *Vaultik) loadVerificationData( func (v *Vaultik) loadVerificationData(
snapshotID string, opts *VerifyOptions, result *VerifyResult, snapshotID, remoteKey string, opts *VerifyOptions, result *VerifyResult,
identities []age.Identity, identities []age.Identity,
) (*snapshot.Manifest, *tempDB, []snapshot.BlobInfo, error) { ) (*snapshot.Manifest, *tempDB, []snapshot.BlobInfo, error) {
// Resolve the identifier to the snapshot's remote key. A human ID is
// hashed; a remote key (or its abbreviation, as printed for a
// remote-only snapshot) is used as-is, so a host with no local index
// can verify a snapshot it can only see on the store.
remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
if err != nil {
return nil, nil, nil, v.deepVerifyFailure(result, opts,
fmt.Sprintf("resolving snapshot identifier: %v", err), err)
}
// Download manifest. downloadManifestByKey is the single reader for // Download manifest. downloadManifestByKey is the single reader for
// remote manifests; see its doc comment. // remote manifests; see its doc comment.
log.Info("Downloading manifest", "remote_key", remoteKey) log.Info("Downloading manifest", "remote_key", remoteKey)