snapshot remove accepts a remote-only snapshot's remote key (closes #400)
check / check (push) Successful in 14m43s

A host with no local index could remove a remote-only snapshot only by
its plain snapshot ID, which it does not have. snapshot remove passed
its argument through RemoteSnapshotKey, rehashing a remote key so it
never matched. It now resolves the identifier with the same
resolveSnapshotRemoteKey that verify and restore use, so a remote key
or an unambiguous leading part of it works.

Model: deepseek-v4-pro
This commit is contained in:
sneak committed 2026-11-08 15:04:42 +00:00
1 parent 5abacbc5c1
commit a68a9ae0bd
4 files changed
+172 -36

No files matched your search

+3
View File
@@ -446,6 +446,9 @@ once the store is reachable to remove the snapshot's metadata from it
(`vaultik prune` does not). For a snapshot that is only on the
destination store, that failure is an error. To wipe everything on the
destination in one go, use `vaultik remote nuke --force`.
* Accepts a snapshot ID, or — for a snapshot only on the destination
store — its remote key (or an unambiguous leading part of it) as shown
by `snapshot list`.
* `--local-only`: Skip remote cleanup; only touch the local index
* `--dry-run`: Show what would be deleted without deleting
* `--force`: Skip confirmation prompt
+4
View File
@@ -288,6 +288,10 @@ know about), which is what 'vaultik prune' does. The command prints the
'vaultik prune' invocation to run as a follow-up only when it removed the
snapshot's metadata from the destination store, and never under --json.
The snapshot may be named by its ID or, on a host with no local index, by
the remote key that 'snapshot list' prints for a remote-only snapshot (an
unambiguous leading part is enough).
An ID that names no snapshot in the local index or on the destination
store is an error.
+94
View File
@@ -451,6 +451,100 @@ func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
assert.Contains(t, out, "Removed snapshot metadata from remote storage")
}
// TestRemoveSnapshot_RemoteOnlyByRemoteKey checks that a snapshot only
// on the destination store, on a host with no local index, is removed by
// the remote key that 'snapshot list' prints for such a snapshot, and by
// an unambiguous leading part of it — the same identifiers verify and
// restore accept. Snapshot remove used to hash the argument again, so
// the hashed key never matched and only the plain snapshot ID worked.
func TestRemoveSnapshot_RemoteOnlyByRemoteKey(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
identifiers := map[string]string{
"full key": remoteKey,
"leading part": remoteKey[:8],
}
for name, identifier := range identifiers {
t.Run(name, func(t *testing.T) {
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
// No local index: a host that sees the snapshot only remotely.
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t)
result, err := tv.RemoveSnapshot(identifier,
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.True(t, result.RemoteRemoved)
assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
out := tv.Stdout.String()
assert.NotContains(t, out, "from local database")
assert.Contains(t, out, "Removed snapshot metadata from remote storage")
})
}
}
// TestRemoveSnapshot_LocalIndexByRemoteKey checks that a remote key (or
// its unambiguous leading part) that names a snapshot the local index
// holds removes that snapshot's local row and its remote metadata
// together. `snapshot list --json` prints a remote key for every
// snapshot, including local ones.
func TestRemoveSnapshot_LocalIndexByRemoteKey(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
identifiers := map[string]string{
"full key": remoteKey,
"leading part": remoteKey[:8],
}
for name, identifier := range identifiers {
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, and the local index holds the snapshot.
require.NoError(t, store.Put(context.Background(), markerFile,
bytes.NewReader(nil)))
tv := vaultik.NewForTesting(store)
repos := newLocalIndex(t, "snapshot-001")
tv.Repositories = repos
result, err := tv.RemoveSnapshot(identifier,
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.True(t, result.RemoteRemoved)
assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
row, err := repos.Snapshots.GetByID(
context.Background(), "snapshot-001")
require.NoError(t, err)
assert.Nil(t, row)
out := tv.Stdout.String()
assert.Contains(t, out,
"Removed snapshot 'snapshot-001' from local database")
assert.Contains(t, out, "Removed snapshot metadata from remote storage")
})
}
}
// TestRemoveSnapshot_NotInLocalIndexDeleteFails checks that removing a
// snapshot only the destination store holds fails when its metadata
// cannot be deleted there, under --force and under --json: nothing was
+71 -36
View File
@@ -1480,7 +1480,7 @@ func (v *Vaultik) RemoveSnapshot(
return result, err
}
inLocalIndex, err := v.findSnapshotToRemove(snapshotID, opts)
inLocalIndex, localID, remoteKey, err := v.findSnapshotToRemove(snapshotID, opts)
if err != nil {
return result, err
}
@@ -1502,16 +1502,16 @@ func (v *Vaultik) RemoveSnapshot(
}
if inLocalIndex {
log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
log.Info("Removing snapshot from local database", "snapshot_id", localID)
err = v.deleteSnapshotFromLocalDB(snapshotID)
err = v.deleteSnapshotFromLocalDB(localID)
if err != nil {
return result, fmt.Errorf("removing from local database: %w", err)
}
}
result.RemoteRemoved, err = v.removeSnapshotRemote(
snapshotID, inLocalIndex, markerErr, opts)
snapshotID, remoteKey, inLocalIndex, markerErr, opts)
if err != nil {
return result, err
}
@@ -1530,7 +1530,7 @@ func (v *Vaultik) RemoveSnapshot(
if inLocalIndex {
v.UI.Infof("Removed snapshot '%s' from local database.",
v.UI.Snapshot(snapshotID))
v.UI.Snapshot(localID))
}
if result.RemoteRemoved {
@@ -1543,50 +1543,68 @@ func (v *Vaultik) RemoveSnapshot(
return result, nil
}
// findSnapshotToRemove reports whether the local index has a row for
// snapshotID, and returns errSnapshotNotFound when there is nothing to
// remove: no row and, unless LocalOnly is set, no metadata on the
// destination store either. The destination store is consulted only
// when the local index has no row, so a snapshot in the local index can
// still be removed from it while the destination store is unreachable,
// and only after checkDestinationMarker passes, so an unplugged volume's
// empty mount point is not taken for a store without the snapshot.
// findSnapshotToRemove resolves snapshotID, the identifier remove was
// given, and returns whether the local index holds that snapshot, the
// human ID of its row when it does, and the remote key to strip from
// the destination store. A human ID is looked up in the local index
// literally and hashed without consulting the store; a remote key, or
// an unambiguous leading part of one, is resolved against the store's
// metadata listing the way verify and restore do, and then matched back
// to the local row whose hashed ID equals it, so removing by such a key
// removes the local row too. The destination store is consulted only
// after checkDestinationMarker, so an unplugged volume's empty mount
// point is not taken for a store without the snapshot. The identifier
// naming nothing, on the local index or, unless LocalOnly is set, on
// the store, is errSnapshotNotFound; a remote-key form that matches
// nothing or more than one directory surfaces
// resolveSnapshotRemoteKey's errSnapshotKeyNotFound or
// errSnapshotKeyAmbiguous.
func (v *Vaultik) findSnapshotToRemove(
snapshotID string, opts *RemoveOptions,
) (bool, error) {
) (bool, string, string, error) {
if v.Repositories != nil {
snap, err := v.Repositories.Snapshots.GetByID(v.ctx, snapshotID)
if err != nil {
return false, fmt.Errorf("looking up snapshot in local index: %w", err)
return false, "", "", fmt.Errorf("looking up snapshot in local index: %w", err)
}
if snap != nil {
return true, nil
return true, snapshotID, snapshot.RemoteSnapshotKey(snapshotID), nil
}
}
if opts.LocalOnly {
return false, fmt.Errorf("%w in the local index: %s",
return false, "", "", fmt.Errorf("%w in the local index: %s",
errSnapshotNotFound, snapshotID)
}
err := v.checkDestinationMarker()
if err != nil {
return false, err
return false, "", "", err
}
objects, err := v.remoteSnapshotObjects(snapshot.RemoteSnapshotKey(snapshotID))
remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
if err != nil {
return false, fmt.Errorf("looking up snapshot on destination store: %w", err)
return false, "", "", err
}
objects, err := v.remoteSnapshotObjects(remoteKey)
if err != nil {
return false, "", "", fmt.Errorf("looking up snapshot on destination store: %w", err)
}
if len(objects) == 0 {
return false, fmt.Errorf(
return false, "", "", fmt.Errorf(
"%w in the local index or on the destination store: %s",
errSnapshotNotFound, snapshotID)
}
return false, nil
localID, err := v.localSnapshotIDForRemoteKey(remoteKey)
if err != nil {
return false, "", "", err
}
return localID != "", localID, remoteKey, nil
}
// removeSnapshotDryRun reports what RemoveSnapshot would do without
@@ -1649,19 +1667,20 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
}
// removeSnapshotRemote strips the snapshot's metadata from the
// destination store. It returns true only when the delete finished
// without an error and removed at least one object; under LocalOnly it
// does nothing. markerErr is the result of checkDestinationMarker; when
// it is set, nothing is deleted and it is the failure. When the snapshot
// is only on the destination store, a failure is returned as an error,
// because that removal is all the command does. When the snapshot was in
// the local index, its removal there has already happened, so a failure
// is only a warning telling the user to run `vaultik snapshot remove`
// for the snapshot again once the destination store is reachable
// (`vaultik prune` never removes snapshot metadata that has its
// manifest).
// destination store. remoteKey is the resolved metadata directory name
// to delete, as returned by findSnapshotToRemove. It returns true only
// when the delete finished without an error and removed at least one
// object; under LocalOnly it does nothing. markerErr is the result of
// checkDestinationMarker; when it is set, nothing is deleted and it is
// the failure. When the snapshot is only on the destination store, a
// failure is returned as an error, because that removal is all the
// command does. When the snapshot was in the local index, its removal
// there has already happened, so a failure is only a warning telling
// the user to run `vaultik snapshot remove` for the snapshot again once
// the destination store is reachable (`vaultik prune` never removes
// snapshot metadata that has its manifest).
func (v *Vaultik) removeSnapshotRemote(
snapshotID string, inLocalIndex bool, markerErr error,
snapshotID, remoteKey string, inLocalIndex bool, markerErr error,
opts *RemoveOptions,
) (bool, error) {
if opts.LocalOnly {
@@ -1675,8 +1694,7 @@ func (v *Vaultik) removeSnapshotRemote(
err := markerErr
if err == nil {
deleted, err = v.deleteRemoteSnapshotByKey(
snapshot.RemoteSnapshotKey(snapshotID))
deleted, err = v.deleteRemoteSnapshotByKey(remoteKey)
}
if err != nil && !inLocalIndex {
@@ -1790,6 +1808,23 @@ func (v *Vaultik) localSnapshotIDs() ([]string, error) {
return ids, nil
}
// localSnapshotIDForRemoteKey returns the human snapshot ID of the local
// index row whose RemoteSnapshotKey is key, or "" when no row has it.
func (v *Vaultik) localSnapshotIDForRemoteKey(key string) (string, error) {
ids, err := v.localSnapshotIDs()
if err != nil {
return "", fmt.Errorf("listing local snapshots: %w", err)
}
for _, id := range ids {
if snapshot.RemoteSnapshotKey(id) == key {
return id, nil
}
}
return "", nil
}
// listAllRemoteSnapshotKeys collects the hashed remote keys
// (subdirectories under metadata/) currently present in the
// destination store. Returns (nil, err) when the store cannot be