diff --git a/README.md b/README.md index 06a531b..179fb06 100644 --- a/README.md +++ b/README.md @@ -448,6 +448,8 @@ 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 diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index 8996d35..7a3b402 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -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. diff --git a/internal/vaultik/remove_snapshot_test.go b/internal/vaultik/remove_snapshot_test.go index ea88183..939bf44 100644 --- a/internal/vaultik/remove_snapshot_test.go +++ b/internal/vaultik/remove_snapshot_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "database/sql" + "encoding/json" "io" "strings" "sync" @@ -472,6 +473,171 @@ 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. +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_DryRunByLeadingPart checks that a dry run by a +// leading part of a remote key names the snapshot that part resolved to: +// by its ID when the local index holds the snapshot, otherwise by its +// full remote key. +func TestRemoveSnapshot_DryRunByLeadingPart(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + remoteKey := snapshot.RemoteSnapshotKey("snapshot-001") + + for name, tc := range map[string]struct { + localIDs []string + shown string + }{ + "in local index": {localIDs: []string{"snapshot-001"}, shown: "snapshot-001"}, + "remote only": {localIDs: nil, shown: remoteKey}, + } { + 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.Repositories = newLocalIndex(t, tc.localIDs...) + + _, err := tv.RemoveSnapshot(remoteKey[:8], + &vaultik.RemoveOptions{DryRun: true}) + + require.NoError(t, err) + assert.Contains(t, tv.Stdout.String(), + "Would remove snapshot: "+tc.shown+".") + assert.True(t, store.hasKey(remoteKeyPath("snapshot-001"))) + }) + } +} + +// TestRemoveSnapshot_JSONDryRunByLeadingPart checks that a JSON dry run +// by a leading part of a remote key prints the ID and the full remote key +// of the snapshot that part resolved to. +func TestRemoveSnapshot_JSONDryRunByLeadingPart(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + remoteKey := snapshot.RemoteSnapshotKey("snapshot-001") + + 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.Repositories = newLocalIndex(t, "snapshot-001") + + _, err := tv.RemoveSnapshot(remoteKey[:8], + &vaultik.RemoveOptions{DryRun: true, JSON: true}) + require.NoError(t, err) + + var printed map[string]any + + require.NoError(t, json.Unmarshal(tv.Stdout.Bytes(), &printed)) + assert.Equal(t, "snapshot-001", printed["snapshot_id"]) + assert.Equal(t, remoteKey, printed["remote_key"]) + assert.True(t, store.hasKey(remoteKeyPath("snapshot-001"))) +} + // 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 @@ -506,6 +672,38 @@ func TestRemoveSnapshot_NotInLocalIndexDeleteFails(t *testing.T) { } } +// TestRemoveSnapshot_LocalIndexByLeadingPartDeleteFails checks the +// warning when a snapshot the local index holds is removed by a leading +// part of its remote key and the delete on the destination store fails. +// The warning names the snapshot by its ID, not by the part given. +func TestRemoveSnapshot_LocalIndexByLeadingPartDeleteFails(t *testing.T) { + log.Initialize(log.Config{}) + 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))) + + faulty := faultstore.New(store) + faulty.OnDelete = func(string) bool { return true } + + tv := vaultik.NewForTesting(faulty) + tv.Repositories = newLocalIndex(t, "snapshot-001") + + result, err := tv.RemoveSnapshot( + snapshot.RemoteSnapshotKey("snapshot-001")[:8], + &vaultik.RemoveOptions{Force: true}) + + require.NoError(t, err) + assert.False(t, result.RemoteRemoved) + assert.True(t, store.hasKey(remoteKeyPath("snapshot-001"))) + assert.Contains(t, tv.Stdout.String(), + "Run 'vaultik snapshot remove snapshot-001' again") +} + func TestRemoveAllSnapshots_RequiresForce(t *testing.T) { log.Initialize(log.Config{}) t.Parallel() diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 34c7827..61e6fc7 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -1424,11 +1424,14 @@ type RemoveOptions struct { LocalOnly bool // Skip remote cleanup; only touch the local index } -// RemoveResult contains the result of a snapshot removal +// RemoveResult contains the result of a snapshot removal. For a single +// snapshot, SnapshotID is its ID, empty when the ID is not known, and +// RemoteKey is the full remote key of its metadata. // //nolint:tagliatelle // snake_case is the established JSON output format type RemoveResult struct { SnapshotID string `json:"snapshot_id,omitempty"` + RemoteKey string `json:"remote_key,omitempty"` SnapshotsRemoved []string `json:"snapshots_removed,omitempty"` RemoteRemoved bool `json:"remote_removed,omitempty"` DryRun bool `json:"dry_run,omitempty"` @@ -1458,25 +1461,35 @@ const snapshotRemoveCommandHint = "vaultik snapshot remove" func (v *Vaultik) RemoveSnapshot( snapshotID string, opts *RemoveOptions, ) (*RemoveResult, error) { - result := &RemoveResult{ - SnapshotID: snapshotID, - } + result := &RemoveResult{} err := v.checkStorageBinding() if err != nil { return result, err } - inLocalIndex, err := v.checkSnapshotExists(snapshotID, opts.LocalOnly) + inLocalIndex, resolvedID, remoteKey, err := v.findSnapshotToRemove( + snapshotID, opts.LocalOnly) if err != nil { return result, err } - if opts.DryRun { - return v.removeSnapshotDryRun(result, snapshotID, opts) + result.SnapshotID = resolvedID + result.RemoteKey = remoteKey + + // The dry run, the prompt and removeSnapshotRemote name the snapshot + // that snapshotID resolved to: by its ID, or by its full remote key + // when the ID is not known. + shown := resolvedID + if shown == "" { + shown = remoteKey } - if !v.confirmRemoveSnapshot(snapshotID, opts) { + if opts.DryRun { + return v.removeSnapshotDryRun(result, shown, opts) + } + + if !v.confirmRemoveSnapshot(shown, opts) { return result, ErrNotConfirmed } @@ -1489,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", resolvedID) - err = v.deleteSnapshotFromLocalDB(snapshotID) + err = v.deleteSnapshotFromLocalDB(resolvedID) if err != nil { return result, fmt.Errorf("removing from local database: %w", err) } } result.RemoteRemoved, err = v.removeSnapshotRemote( - snapshotID, inLocalIndex, markerErr, opts) + shown, remoteKey, inLocalIndex, markerErr, opts) if err != nil { return result, err } @@ -1517,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(resolvedID)) } if result.RemoteRemoved { @@ -1530,6 +1543,52 @@ func (v *Vaultik) RemoveSnapshot( return result, nil } +// findSnapshotToRemove finds the snapshot that identifier names. It +// returns whether the local index holds that snapshot, its snapshot ID, +// and the remote key of its metadata on the destination store. The ID is +// known when identifier is a snapshot ID or the local index holds the +// snapshot; otherwise it is empty. +// +// A snapshot ID is looked up by checkSnapshotExists. Under localOnly the +// store is not consulted, so every identifier is looked up that way and +// a remote key is not found. Otherwise a remote key, or a leading part +// of one, is resolved by resolveSnapshotRemoteKey, as restore and verify +// do. That fails with errSnapshotKeyNotFound or errSnapshotKeyAmbiguous +// when the identifier matches no snapshot on the store, or more than +// one. The store is listed only after checkDestinationMarker passes, as +// in checkSnapshotExists. The local row is the one whose snapshot ID +// hashes to the resolved key, as in RemoveAllSnapshots, so removing by +// remote key removes that row too. +func (v *Vaultik) findSnapshotToRemove( + identifier string, localOnly bool, +) (bool, string, string, error) { + if localOnly || !isRemoteKeyOrPrefix(identifier) { + inLocalIndex, err := v.checkSnapshotExists(identifier, localOnly) + if err != nil { + return false, "", "", err + } + + return inLocalIndex, identifier, snapshot.RemoteSnapshotKey(identifier), nil + } + + err := v.checkDestinationMarker() + if err != nil { + return false, "", "", err + } + + remoteKey, err := v.resolveSnapshotRemoteKey(identifier) + if err != nil { + return false, "", "", err + } + + localID, err := v.localSnapshotIDForRemoteKey(remoteKey) + if err != nil { + return false, "", "", err + } + + return localID != "", localID, remoteKey, nil +} + // checkSnapshotExists returns errSnapshotNotFound when the local index // has no row for snapshotID and, unless localOnly is set, the // destination store has no metadata for it either. inLocalIndex reports @@ -1639,19 +1698,22 @@ 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. shown names the +// snapshot in the log records and the warning: its ID, or its full +// remote key when the ID is not known. 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, + shown, remoteKey string, inLocalIndex bool, markerErr error, opts *RemoveOptions, ) (bool, error) { if opts.LocalOnly { @@ -1659,14 +1721,13 @@ func (v *Vaultik) removeSnapshotRemote( } log.Info("Removing snapshot metadata from remote storage", - "snapshot_id", snapshotID) + "snapshot_id", shown) var deleted int err := markerErr if err == nil { - deleted, err = v.deleteRemoteSnapshotByKey( - snapshot.RemoteSnapshotKey(snapshotID)) + deleted, err = v.deleteRemoteSnapshotByKey(remoteKey) } if err != nil && !inLocalIndex { @@ -1678,14 +1739,14 @@ func (v *Vaultik) removeSnapshotRemote( log.Warn("Could not remove snapshot metadata from remote storage; "+ "run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+ "again once the remote is reachable", - "snapshot_id", snapshotID, "error", err) + "snapshot_id", shown, "error", err) // The UI writes to stdout, which under --json holds only the // document; the log record above is the warning on stderr. if v.UI != nil && !opts.JSON { v.UI.Warningf("Could not remove snapshot metadata from remote: "+ "%v. Run '%s %s' again once the remote is reachable.", - err, snapshotRemoveCommandHint, snapshotID) + err, snapshotRemoveCommandHint, shown) } return false, nil @@ -1780,6 +1841,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