1 Commits
Author SHA1 Message Date
sneak ecf36cbefa snapshot remove accepts a remote-only snapshot's remote key (closes #400)
check / check (push) In progress
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
2026-11-08 14:01:40 +00:00
4 changed files with 83 additions and 24 deletions

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 (`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 store, that failure is an error. To wipe everything on the
destination in one go, use `vaultik remote nuke --force`. 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 * `--local-only`: Skip remote cleanup; only touch the local index
* `--dry-run`: Show what would be deleted without deleting * `--dry-run`: Show what would be deleted without deleting
* `--force`: Skip confirmation prompt * `--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 '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. 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 An ID that names no snapshot in the local index or on the destination
store is an error. store is an error.
+42
View File
@@ -451,6 +451,48 @@ func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
assert.Contains(t, out, "Removed snapshot metadata from remote storage") 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_NotInLocalIndexDeleteFails checks that removing a // TestRemoveSnapshot_NotInLocalIndexDeleteFails checks that removing a
// snapshot only the destination store holds fails when its metadata // snapshot only the destination store holds fails when its metadata
// cannot be deleted there, under --force and under --json: nothing was // cannot be deleted there, under --force and under --json: nothing was
+34 -24
View File
@@ -1480,7 +1480,7 @@ func (v *Vaultik) RemoveSnapshot(
return result, err return result, err
} }
inLocalIndex, err := v.findSnapshotToRemove(snapshotID, opts) inLocalIndex, remoteKey, err := v.findSnapshotToRemove(snapshotID, opts)
if err != nil { if err != nil {
return result, err return result, err
} }
@@ -1511,7 +1511,7 @@ func (v *Vaultik) RemoveSnapshot(
} }
result.RemoteRemoved, err = v.removeSnapshotRemote( result.RemoteRemoved, err = v.removeSnapshotRemote(
snapshotID, inLocalIndex, markerErr, opts) snapshotID, remoteKey, inLocalIndex, markerErr, opts)
if err != nil { if err != nil {
return result, err return result, err
} }
@@ -1544,49 +1544,58 @@ func (v *Vaultik) RemoveSnapshot(
} }
// findSnapshotToRemove reports whether the local index has a row for // findSnapshotToRemove reports whether the local index has a row for
// snapshotID, and returns errSnapshotNotFound when there is nothing to // snapshotID, and returns the remote key to strip from the destination
// remove: no row and, unless LocalOnly is set, no metadata on the // store, or errSnapshotNotFound when there is nothing to remove: no row
// destination store either. The destination store is consulted only // and, unless LocalOnly is set, no metadata on the destination store
// when the local index has no row, so a snapshot in the local index can // either. The destination store is consulted only when the local index
// still be removed from it while the destination store is unreachable, // has no row, so a snapshot in the local index can still be removed from
// and only after checkDestinationMarker passes, so an unplugged volume's // it while the destination store is unreachable, and only after
// empty mount point is not taken for a store without the snapshot. // checkDestinationMarker passes, so an unplugged volume's empty mount
// point is not taken for a store without the snapshot. The identifier is
// resolved the way verify and restore do: a human ID is hashed; a remote
// key, or an unambiguous leading part of one, names a remote-only
// snapshot whose metadata directory is used as-is.
func (v *Vaultik) findSnapshotToRemove( func (v *Vaultik) findSnapshotToRemove(
snapshotID string, opts *RemoveOptions, snapshotID string, opts *RemoveOptions,
) (bool, error) { ) (bool, string, 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 {
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 { if snap != nil {
return true, nil return true, snapshot.RemoteSnapshotKey(snapshotID), nil
} }
} }
if opts.LocalOnly { 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) errSnapshotNotFound, snapshotID)
} }
err := v.checkDestinationMarker() err := v.checkDestinationMarker()
if err != nil { if err != nil {
return false, err return false, "", err
} }
objects, err := v.remoteSnapshotObjects(snapshot.RemoteSnapshotKey(snapshotID)) remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
if err != nil { 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 { if len(objects) == 0 {
return false, fmt.Errorf( return false, "", fmt.Errorf(
"%w in the local index or on the destination store: %s", "%w in the local index or on the destination store: %s",
errSnapshotNotFound, snapshotID) errSnapshotNotFound, snapshotID)
} }
return false, nil return false, remoteKey, nil
} }
// removeSnapshotDryRun reports what RemoveSnapshot would do without // removeSnapshotDryRun reports what RemoveSnapshot would do without
@@ -1649,9 +1658,11 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
} }
// removeSnapshotRemote strips the snapshot's metadata from the // removeSnapshotRemote strips the snapshot's metadata from the
// destination store. It returns true only when the delete finished // destination store. remoteKey is the resolved metadata directory name
// without an error and removed at least one object; under LocalOnly it // to delete, as returned by findSnapshotToRemove. It returns true only
// does nothing. markerErr is the result of checkDestinationMarker; when // 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 // 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, // 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 // because that removal is all the command does. When the snapshot was in
@@ -1661,7 +1672,7 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
// (`vaultik prune` never removes snapshot metadata that has its // (`vaultik prune` never removes snapshot metadata that has its
// manifest). // manifest).
func (v *Vaultik) removeSnapshotRemote( func (v *Vaultik) removeSnapshotRemote(
snapshotID string, inLocalIndex bool, markerErr error, snapshotID, remoteKey string, inLocalIndex bool, markerErr error,
opts *RemoveOptions, opts *RemoveOptions,
) (bool, error) { ) (bool, error) {
if opts.LocalOnly { if opts.LocalOnly {
@@ -1675,8 +1686,7 @@ func (v *Vaultik) removeSnapshotRemote(
err := markerErr err := markerErr
if err == nil { if err == nil {
deleted, err = v.deleteRemoteSnapshotByKey( deleted, err = v.deleteRemoteSnapshotByKey(remoteKey)
snapshot.RemoteSnapshotKey(snapshotID))
} }
if err != nil && !inLocalIndex { if err != nil && !inLocalIndex {