Make remote nuke delete every object under metadata/ and blobs/ (closes #291)

remote nuke deleted only the snapshot metadata and blobs the snapshot
and blob listings recognise, so a snapshot's database whose manifest
was never uploaded, and any name under blobs/ that is not a blob hash,
survived while nuke reported the store empty.

After removing every snapshot, nuke now deletes every object listed
under metadata/ and blobs/, whatever its name, in place of the blob
prune step. A delete that fails is logged and the others still run. A
listing afterwards decides the result: nuke reports the store empty
only when it finds nothing, and otherwise names each object left and
fails.

Judgement call: nuke no longer prints the blob prune summary.

Model: opus-5-5

README.md

@@ -413,11 +413,12 @@ under `blobs/` whose name is not a blob hash is not counted as a blob,
 since `prune` never deletes it.
 * `--json`: Output as JSON
 
-**`remote nuke`**: Delete every snapshot's metadata and every blob from the
+**`remote nuke`**: Delete every object under `metadata/` and `blobs/` on
-backup destination store, leaving the bucket prefix empty. Destructive and
+the backup destination store, whatever its name, leaving the bucket prefix
-irreversible. This is the single supported way to wipe the entire
+empty. Destructive and irreversible. This is the single supported way to
-destination store. If anything cannot be deleted, the command exits
+wipe the entire destination store. If a listing afterwards still finds
-non-zero; running it again deletes what is left.
+anything there, the command names each object left and exits non-zero;
+running it again deletes what is left.
 * `--force`: Required to confirm destruction.
 
 **`database delete`**: Delete the local SQLite state database file

internal/vaultik/nuke_remote_test.go

@@ -18,7 +18,8 @@ import (
 // TestNukeRemoteLeavesNoFiles checks that remote nuke leaves no file
 // under a file:// destination, including the `.partial` files uploads
 // killed part-way leave next to a snapshot's metadata and next to where a
-// blob would have been.
+// blob would have been, a snapshot's database whose manifest was never
+// uploaded, and a file under blobs/ whose name is not a blob hash.
 func TestNukeRemoteLeavesNoFiles(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
@@ -38,6 +39,9 @@ func TestNukeRemoteLeavesNoFiles(t *testing.T) {
 			"db.zst.age-123456.partial"),
 		filepath.Join(storeDir, "blobs", hash[:2], hash[2:4],
 			hash+"-123456.partial"),
+		filepath.Join(storeDir, "metadata",
+			snapshot.RemoteSnapshotKey("no-manifest"), "db.zst.age"),
+		filepath.Join(storeDir, "blobs", "0b", "01", ".DS_Store"),
 	}
 
 	for _, leftover := range leftovers {
@@ -100,6 +104,34 @@ func TestNukeRemoteFailsWhenADeleteFails(t *testing.T) {
 	}
 }
 
+// TestNukeRemoteNamesWhatIsLeft checks that remote nuke, when an object
+// it does not recognise cannot be deleted, does not report the store
+// empty but names that object and fails.
+func TestNukeRemoteNamesWhatIsLeft(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	storeDir := filepath.Join(t.TempDir(), "store")
+	v, _, out := backUpToFileDestination(ctx, t, storeDir)
+
+	const leftKey = "blobs/0b/01/.DS_Store"
+
+	leftPath := filepath.Join(storeDir, filepath.FromSlash(leftKey))
+	require.NoError(t, os.MkdirAll(filepath.Dir(leftPath), 0o750))
+	require.NoError(t, os.WriteFile(leftPath, []byte("finder"), 0o600))
+
+	store := faultstore.New(v.Storage)
+	store.OnDelete = func(key string) bool { return key == leftKey }
+	v.Storage = store
+
+	require.Error(t, v.NukeRemote(true))
+
+	assert.Contains(t, out.String(), leftKey)
+	assert.NotContains(t, out.String(), "now empty")
+	assert.Contains(t, out.String(), "vaultik remote nuke --force")
+}
+
 // TestNukeRemoteRefusesAnotherDestinationWithoutRetryAdvice checks that
 // remote nuke, when the local index belongs to a different destination
 // than the configured one, fails without advising to run nuke again:

internal/vaultik/prune.go

@@ -22,6 +22,11 @@ var errNukeRequiresForce = errors.New(
 // errBlobDeleteFailed reports unreferenced blobs prune could not delete.
 var errBlobDeleteFailed = errors.New("blob(s) could not be deleted")
 
+// errObjectsLeft reports objects remote nuke still finds on the backup
+// destination store after deleting.
+var errObjectsLeft = errors.New(
+	"object(s) still on the backup destination store")
+
 // nukeCommandHint is the command suggested when a remote nuke could not
 // delete everything: running it again deletes what is left, which
 // `vaultik prune` does not do for snapshot metadata.
@@ -35,10 +40,11 @@ const metadataDirName = "metadata"
 // (blobs/<aa>/<bb>/<hash>).
 const blobKeyParts = 4
 
-// NukeRemote deletes every snapshot's metadata and every blob from remote
+// NukeRemote deletes every object under metadata/ and blobs/ in remote
-// storage, along with any object an upload cut off part-way left under a
+// storage, whatever its name, along with any object an upload cut off
-// temporary `.partial` name. After this returns successfully the bucket
+// part-way left under a temporary `.partial` name. It returns
-// prefix is empty and the next backup starts from scratch.
+// successfully only when a listing afterwards finds nothing left there,
+// and the next backup then starts from scratch.
 //
 // Refuses to run unless force is true. The caller is responsible for
 // confirming with the user.
@@ -68,7 +74,8 @@ func (v *Vaultik) NukeRemote(force bool) error {
 }
 
 // emptyDestinationStore does the deleting for NukeRemote and stops at
-// the first step that fails.
+// the first step that fails. It also fails when objects are still
+// listed at the end, naming each one.
 func (v *Vaultik) emptyDestinationStore() error {
 	v.UI.Beginf("Removing all snapshot metadata from backup destination store.")
 
@@ -77,25 +84,62 @@ func (v *Vaultik) emptyDestinationStore() error {
 		return fmt.Errorf("removing all snapshots: %w", err)
 	}
 
-	v.UI.Beginf("Removing any blobs still present in backup destination store.")
+	v.UI.Beginf("Removing everything still present in backup destination store.")
 
-	err = v.PruneBlobs(&PruneOptions{Force: true})
+	var left []string
-	if err != nil {
-		return fmt.Errorf("pruning blobs: %w", err)
-	}
 
-	// The file and rclone listings skip `.partial` objects, so the two
-	// steps above never delete them.
 	for _, prefix := range []string{"metadata/", "blobs/"} {
-		err = v.Storage.DeletePartialUploads(v.ctx, prefix)
+		keys, err := v.deleteEverythingUnder(prefix)
 		if err != nil {
-			return fmt.Errorf("deleting partial uploads: %w", err)
+			return err
 		}
+
+		left = append(left, keys...)
+	}
+
+	for _, key := range left {
+		v.UI.Warningf("Still on the backup destination store: %s", key)
+	}
+
+	if len(left) > 0 {
+		return fmt.Errorf("%d %w", len(left), errObjectsLeft)
 	}
 
 	return nil
 }
 
+// deleteEverythingUnder deletes every object under prefix, whatever its
+// name, and returns the keys a listing afterwards still finds. A delete
+// that fails is logged and the others still run; that object then
+// shows up in the listing afterwards.
+func (v *Vaultik) deleteEverythingUnder(prefix string) ([]string, error) {
+	keys, err := v.Storage.List(v.ctx, prefix)
+	if err != nil {
+		return nil, fmt.Errorf("listing %s: %w", prefix, err)
+	}
+
+	for _, key := range keys {
+		err = v.Storage.Delete(v.ctx, key)
+		if err != nil {
+			log.Error("Failed to delete object", "key", key, "error", err)
+		}
+	}
+
+	// The file and rclone listings skip `.partial` objects, so the
+	// deletes above never reach them.
+	err = v.Storage.DeletePartialUploads(v.ctx, prefix)
+	if err != nil {
+		return nil, fmt.Errorf("deleting partial uploads: %w", err)
+	}
+
+	left, err := v.Storage.List(v.ctx, prefix)
+	if err != nil {
+		return nil, fmt.Errorf("listing %s: %w", prefix, err)
+	}
+
+	return left, nil
+}
+
 // PruneBlobsResult contains the result of a blob prune operation
 //
 //nolint:tagliatelle // snake_case is the established JSON output format