diff --git a/README.md b/README.md index 72097c3..6a0892c 100644 --- a/README.md +++ b/README.md @@ -390,7 +390,9 @@ recipients, and local database statistics. **`remote info`**: Show storage backend type and location plus detailed remote storage inventory: per-snapshot metadata sizes, blob counts, and -orphaned blob detection. +orphaned blob detection. If a manifest cannot be read, the orphaned blob +figures are reported as unknown; `--json` gives them as `null` and lists +the remote key of each unreadable manifest in `unreadable_manifests`. * `--json`: Output as JSON **`remote nuke`**: Delete every snapshot's metadata and every blob from the diff --git a/TODO.md b/TODO.md index 3801fd1..e45eaa3 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,18 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-07: Made `remote info` stop reporting a snapshot's blobs as + orphaned when its manifest cannot be read, and stop printing raw + names from under `metadata/` + ([issue #228](https://git.eeqj.de/sneak/vaultik/issues/228)). A + manifest it failed to read was skipped, so that snapshot's blobs were + counted as orphaned and the report advised running `vaultik prune`. + The orphan figures are now unknown in that case, with no prune + advice, and `--json` gives them as `null` with the unreadable remote + keys in `unreadable_manifests`. A name under `metadata/` that is not + 64 lowercase hex characters is now skipped with a warning instead of + being printed, control characters included. + - 2026-10-07: Made taking the process-wide lock atomic ([issue #227](https://git.eeqj.de/sneak/vaultik/issues/227)). The lock read `vaultik.pid`, checked whether that PID was alive and then wrote diff --git a/internal/vaultik/info.go b/internal/vaultik/info.go index 53e6d44..6ac7b0d 100644 --- a/internal/vaultik/info.go +++ b/internal/vaultik/info.go @@ -206,9 +206,14 @@ type RemoteInfoResult struct { ReferencedBlobCount int `json:"referenced_blob_count"` ReferencedBlobSize int64 `json:"referenced_blob_size"` - // Orphaned blobs - OrphanedBlobCount int `json:"orphaned_blob_count"` - OrphanedBlobSize int64 `json:"orphaned_blob_size"` + // Orphaned blobs. Both stay nil (null in the JSON) when a manifest + // could not be read, since that snapshot's blobs would be counted + // as orphaned. + OrphanedBlobCount *int `json:"orphaned_blob_count"` + OrphanedBlobSize *int64 `json:"orphaned_blob_size"` + + // Remote key of each snapshot whose manifest could not be read + UnreadableManifests []string `json:"unreadable_manifests,omitempty"` } // RemoteInfo displays information about remote storage @@ -243,7 +248,9 @@ func (v *Vaultik) RemoteInfo(jsonOutput bool) error { v.stdoutf("Downloading %d manifest(s)...\n", len(snapshotIDs)) } - referencedBlobs := v.collectReferencedBlobsFromManifests(snapshotIDs, snapshotMetadata) + referencedBlobs, unreadableManifests := v.collectReferencedBlobsFromManifests( + snapshotIDs, snapshotMetadata) + result.UnreadableManifests = unreadableManifests v.populateRemoteInfoResult(result, snapshotMetadata, snapshotIDs, referencedBlobs) @@ -256,7 +263,7 @@ func (v *Vaultik) RemoteInfo(jsonOutput bool) error { "snapshots", result.TotalMetadataCount, "total_blobs", result.TotalBlobCount, "referenced_blobs", result.ReferencedBlobCount, - "orphaned_blobs", result.OrphanedBlobCount) + "unreadable_manifests", len(result.UnreadableManifests)) if jsonOutput { enc := json.NewEncoder(v.Stdout) @@ -292,6 +299,16 @@ func (v *Vaultik) collectSnapshotMetadata() ( snapshotID := parts[1] + // The name comes from the destination store, which is not + // trusted, and is printed in the report. Accept it only in the + // form of a remote key. + if !isBlobHash(snapshotID) { + log.Warn("Skipping non-conforming key under metadata/", + "key", obj.Key) + + continue + } + if _, exists := snapshotMetadata[snapshotID]; !exists { snapshotMetadata[snapshotID] = &SnapshotMetadataInfo{SnapshotID: snapshotID} } @@ -319,12 +336,15 @@ func (v *Vaultik) collectSnapshotMetadata() ( } // collectReferencedBlobsFromManifests downloads manifests and returns -// referenced blob hashes with sizes. +// referenced blob hashes with sizes, and the remote keys of the +// manifests it could not read. func (v *Vaultik) collectReferencedBlobsFromManifests( snapshotIDs []string, snapshotMetadata map[string]*SnapshotMetadataInfo, -) map[string]int64 { +) (map[string]int64, []string) { referencedBlobs := make(map[string]int64) + var unreadable []string + for _, snapshotID := range snapshotIDs { // snapshotIDs here are remote keys, taken straight from the // metadata/ listing. downloadManifestByKey is the single reader @@ -333,6 +353,8 @@ func (v *Vaultik) collectReferencedBlobsFromManifests( if err != nil { log.Warn("Failed to read manifest", "snapshot", snapshotID, "error", err) + unreadable = append(unreadable, snapshotID) + continue } @@ -349,7 +371,7 @@ func (v *Vaultik) collectReferencedBlobsFromManifests( info.BlobsSize = blobsSize } - return referencedBlobs + return referencedBlobs, unreadable } // populateRemoteInfoResult fills in the result's snapshot and @@ -378,8 +400,9 @@ func (v *Vaultik) populateRemoteInfoResult( } // scanRemoteBlobStorage lists all blobs on remote and computes orphan -// stats. showText is true only when the human report is being printed -// (not --json, not --quiet), gating the progress line. +// stats when every manifest was read. showText is true only when the +// human report is being printed (not --json, not --quiet), gating the +// progress line. func (v *Vaultik) scanRemoteBlobStorage( result *RemoteInfoResult, referencedBlobs map[string]int64, showText bool, ) error { @@ -406,13 +429,27 @@ func (v *Vaultik) scanRemoteBlobStorage( result.TotalBlobSize += obj.Size } + // A blob named only by a manifest that could not be read would be + // counted as orphaned, so the orphan figures stay unknown. + if len(result.UnreadableManifests) > 0 { + return nil + } + + var ( + orphanedCount int + orphanedSize int64 + ) + for hash, size := range allBlobs { if _, referenced := referencedBlobs[hash]; !referenced { - result.OrphanedBlobCount++ - result.OrphanedBlobSize += size + orphanedCount++ + orphanedSize += size } } + result.OrphanedBlobCount = &orphanedCount + result.OrphanedBlobSize = &orphanedSize + return nil } @@ -465,11 +502,20 @@ func (v *Vaultik) printRemoteInfoTable(result *RemoteInfoResult) { v.stdoutf("Referenced by snapshots: %s (%s)\n", humanize.Comma(int64(result.ReferencedBlobCount)), ubytes(result.ReferencedBlobSize)) - v.stdoutf("Orphaned (unreferenced): %s (%s)\n", - humanize.Comma(int64(result.OrphanedBlobCount)), - ubytes(result.OrphanedBlobSize)) - if result.OrphanedBlobCount > 0 { + if result.OrphanedBlobCount == nil { + v.stdoutf("Orphaned (unreferenced): unknown "+ + "(%d manifest(s) could not be read)\n", + len(result.UnreadableManifests)) + + return + } + + v.stdoutf("Orphaned (unreferenced): %s (%s)\n", + humanize.Comma(int64(*result.OrphanedBlobCount)), + ubytes(*result.OrphanedBlobSize)) + + if *result.OrphanedBlobCount > 0 { v.stdoutf("\nRun 'vaultik prune' to remove orphaned blobs.\n") } } diff --git a/internal/vaultik/remote_info_test.go b/internal/vaultik/remote_info_test.go new file mode 100644 index 0000000..3925f45 --- /dev/null +++ b/internal/vaultik/remote_info_test.go @@ -0,0 +1,88 @@ +package vaultik_test + +import ( + "bytes" + "context" + "encoding/json" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/snapshot" +) + +// TestRemoteInfo_UnreadableManifestLeavesOrphansUnknown checks that a +// manifest remote info cannot read makes the orphan figures unknown. A +// blob referenced only by that snapshot would otherwise be counted as +// orphaned, and the report would advise running prune. +func TestRemoteInfo_UnreadableManifestLeavesOrphansUnknown(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + const testBlobHashB = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + env := newListEnv(t) + + // The readable manifest references blob A only. + env.addRemote(t, listRemoteID, time.Date(2026, 3, 2, 0, 0, 0, 0, time.UTC)) + addBlob(t, env.store.testStorer, testBlobHashA) + addBlob(t, env.store.testStorer, testBlobHashB) + + // With every manifest readable, blob B is orphaned. + require.NoError(t, env.v.RemoteInfo(true)) + + var doc map[string]any + + require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &doc)) + assert.InDelta(t, 1, doc["orphaned_blob_count"], 0) + + // A second snapshot whose manifest cannot be decoded. Blob B may be + // one of its blobs. + unreadableKey := snapshot.RemoteSnapshotKey(listLocalID) + require.NoError(t, env.store.Put(context.Background(), + "metadata/"+unreadableKey+"/manifest.json.zst", + bytes.NewReader([]byte("not a valid manifest")))) + + env.stdout.Reset() + require.NoError(t, env.v.RemoteInfo(false)) + + text := env.stdout.String() + assert.Contains(t, text, + "Orphaned (unreferenced): unknown (1 manifest(s) could not be read)") + assert.NotContains(t, text, "vaultik prune") + + env.stdout.Reset() + require.NoError(t, env.v.RemoteInfo(true)) + + doc = nil + require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &doc)) + assert.Contains(t, doc, "orphaned_blob_count") + assert.Nil(t, doc["orphaned_blob_count"]) + assert.Contains(t, doc, "orphaned_blob_size") + assert.Nil(t, doc["orphaned_blob_size"]) + assert.Equal(t, []any{unreadableKey}, doc["unreadable_manifests"]) +} + +// TestRemoteInfo_SkipsNonConformingMetadataName checks that a directory +// under metadata/ whose name is not a remote key is left out of the +// report. The name comes from the destination store; printed raw, its +// control characters would reach the terminal. +func TestRemoteInfo_SkipsNonConformingMetadataName(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + env := newListEnv(t) + env.addRemote(t, listRemoteID, time.Date(2026, 3, 2, 0, 0, 0, 0, time.UTC)) + require.NoError(t, env.store.Put(context.Background(), + "metadata/\x1b[31mred/manifest.json.zst", + bytes.NewReader([]byte("not a valid manifest")))) + + require.NoError(t, env.v.RemoteInfo(false)) + + out := env.stdout.String() + assert.NotContains(t, out, "\x1b") + assert.Contains(t, out, "Total (1 snapshots)") +}