diff --git a/README.md b/README.md index 72097c3..cf9e39d 100644 --- a/README.md +++ b/README.md @@ -390,7 +390,13 @@ 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. A name under `metadata/` that is not a remote +key is skipped with a warning and is not printed. If a listed +`manifest.json.zst` cannot be read, or sits under a skipped name, the +orphaned blob figures are reported as unknown; `--json` gives them as +`null`, lists the remote key of each unreadable manifest in +`unreadable_manifests` and counts the manifests under skipped names in +`skipped_manifest_count`. * `--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 1e1fdaa..0964f46 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,22 @@ 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. A manifest under a + skipped name is then not read either, so it also leaves the orphan + figures unknown, and `--json` counts such manifests in + `skipped_manifest_count`. A directory with no manifest in it, as left + by an interrupted backup, leaves the figures known. + - 2026-10-07: Made `config set` keep a string that looks like a number ([issue #229](https://git.eeqj.de/sneak/vaultik/issues/229)). It wrote every value unquoted, and `config.Load` reads the file through untyped diff --git a/internal/vaultik/info.go b/internal/vaultik/info.go index 53e6d44..a39659d 100644 --- a/internal/vaultik/info.go +++ b/internal/vaultik/info.go @@ -183,6 +183,11 @@ type SnapshotMetadataInfo struct { TotalSize int64 `json:"total_size"` BlobCount int `json:"blob_count"` BlobsSize int64 `json:"blobs_size"` + + // Set when the listing holds this snapshot's manifest.json.zst. A + // backup interrupted before its manifest upload leaves a directory + // without one, which prune does not treat as a snapshot. + hasManifest bool } // RemoteInfoResult contains all remote storage information @@ -206,9 +211,20 @@ 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 + // was listed but not 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"` + + // Number of manifests not read because the name above them under + // metadata/ is not a remote key. The names themselves are not + // reported: they come from the destination store and may hold + // control characters. + SkippedManifestCount int `json:"skipped_manifest_count,omitempty"` } // RemoteInfo displays information about remote storage @@ -234,16 +250,28 @@ func (v *Vaultik) RemoteInfo(jsonOutput bool) error { v.stdoutf("Scanning snapshot metadata...\n") } - snapshotMetadata, snapshotIDs, err := v.collectSnapshotMetadata() + snapshotMetadata, snapshotIDs, skippedManifestCount, err := v.collectSnapshotMetadata() if err != nil { return err } + result.SkippedManifestCount = skippedManifestCount + if showText { - v.stdoutf("Downloading %d manifest(s)...\n", len(snapshotIDs)) + manifestCount := 0 + + for _, info := range snapshotMetadata { + if info.hasManifest { + manifestCount++ + } + } + + v.stdoutf("Downloading %d manifest(s)...\n", manifestCount) } - referencedBlobs := v.collectReferencedBlobsFromManifests(snapshotIDs, snapshotMetadata) + referencedBlobs, unreadableManifests := v.collectReferencedBlobsFromManifests( + snapshotIDs, snapshotMetadata) + result.UnreadableManifests = unreadableManifests v.populateRemoteInfoResult(result, snapshotMetadata, snapshotIDs, referencedBlobs) @@ -256,7 +284,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) @@ -273,16 +301,18 @@ func (v *Vaultik) RemoteInfo(jsonOutput bool) error { } // collectSnapshotMetadata scans remote metadata and returns -// per-snapshot info and sorted IDs. +// per-snapshot info, sorted IDs and the number of manifests it skipped +// because the name above them is not a remote key. func (v *Vaultik) collectSnapshotMetadata() ( - map[string]*SnapshotMetadataInfo, []string, error, + map[string]*SnapshotMetadataInfo, []string, int, error, ) { snapshotMetadata := make(map[string]*SnapshotMetadataInfo) + skippedManifestCount := 0 metadataCh := v.Storage.ListStream(v.ctx, "metadata/") for obj := range metadataCh { if obj.Err != nil { - return nil, nil, fmt.Errorf("listing metadata: %w", obj.Err) + return nil, nil, 0, fmt.Errorf("listing metadata: %w", obj.Err) } parts := strings.Split(obj.Key, "/") @@ -291,6 +321,22 @@ func (v *Vaultik) collectSnapshotMetadata() ( } snapshotID := parts[1] + filename := parts[2] + isManifest := filename == "manifest.json.zst" + + // 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) + + if isManifest { + skippedManifestCount++ + } + + continue + } if _, exists := snapshotMetadata[snapshotID]; !exists { snapshotMetadata[snapshotID] = &SnapshotMetadataInfo{SnapshotID: snapshotID} @@ -298,7 +344,10 @@ func (v *Vaultik) collectSnapshotMetadata() ( info := snapshotMetadata[snapshotID] - filename := parts[2] + if isManifest { + info.hasManifest = true + } + if strings.HasPrefix(filename, "manifest") { info.ManifestSize = obj.Size } else if strings.HasPrefix(filename, "db") { @@ -315,17 +364,25 @@ func (v *Vaultik) collectSnapshotMetadata() ( sort.Strings(snapshotIDs) - return snapshotMetadata, snapshotIDs, nil + return snapshotMetadata, snapshotIDs, skippedManifestCount, nil } -// collectReferencedBlobsFromManifests downloads manifests and returns -// referenced blob hashes with sizes. +// collectReferencedBlobsFromManifests downloads the listed manifests +// and returns 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 { + info := snapshotMetadata[snapshotID] + if !info.hasManifest { + continue + } + // snapshotIDs here are remote keys, taken straight from the // metadata/ listing. downloadManifestByKey is the single reader // for remote manifests; see its doc comment. @@ -333,10 +390,11 @@ func (v *Vaultik) collectReferencedBlobsFromManifests( if err != nil { log.Warn("Failed to read manifest", "snapshot", snapshotID, "error", err) + unreadable = append(unreadable, snapshotID) + continue } - info := snapshotMetadata[snapshotID] info.BlobCount = manifest.BlobCount var blobsSize int64 @@ -349,7 +407,7 @@ func (v *Vaultik) collectReferencedBlobsFromManifests( info.BlobsSize = blobsSize } - return referencedBlobs + return referencedBlobs, unreadable } // populateRemoteInfoResult fills in the result's snapshot and @@ -378,8 +436,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 listed 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 +465,28 @@ func (v *Vaultik) scanRemoteBlobStorage( result.TotalBlobSize += obj.Size } + // A blob named only by a manifest that could not be read, or by one + // under a skipped name, would be counted as orphaned, so the orphan + // figures stay unknown. + if len(result.UnreadableManifests) > 0 || result.SkippedManifestCount > 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 +539,21 @@ 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, "+ + "%d manifest(s) under a non-conforming name skipped)\n", + len(result.UnreadableManifests), result.SkippedManifestCount) + + 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..2c6c8da --- /dev/null +++ b/internal/vaultik/remote_info_test.go @@ -0,0 +1,159 @@ +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" +) + +// testBlobHashB is a blob that the manifest written by addRemote does +// not reference. +const testBlobHashB = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + +// 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() + + 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, "+ + "0 manifest(s) under a non-conforming name skipped)") + 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, and that the orphan figures are unknown when it holds a +// manifest. The name comes from the destination store; printed raw, its +// control characters would reach the terminal. Its manifest is not +// read, so a blob only it references would otherwise be counted as +// orphaned. +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)) + addBlob(t, env.store.testStorer, testBlobHashA) + addBlob(t, env.store.testStorer, testBlobHashB) + 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)) + + text := env.stdout.String() + assert.NotContains(t, text, "\x1b") + assert.NotContains(t, text, "31mred") + assert.Contains(t, text, "Total (1 snapshots)") + assert.Contains(t, text, "Orphaned (unreferenced): unknown "+ + "(0 manifest(s) could not be read, "+ + "1 manifest(s) under a non-conforming name skipped)") + assert.NotContains(t, text, "vaultik prune") + + env.stdout.Reset() + require.NoError(t, env.v.RemoteInfo(true)) + + out := env.stdout.String() + assert.NotContains(t, out, "31mred") + + var doc map[string]any + + require.NoError(t, json.Unmarshal([]byte(out), &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.InDelta(t, 1, doc["skipped_manifest_count"], 0) + assert.NotContains(t, doc, "unreadable_manifests") +} + +// TestRemoteInfo_DirectoryWithoutManifestLeavesOrphansKnown checks that +// a directory under metadata/ holding no manifest.json.zst, such as one +// left by a backup interrupted before its manifest upload, leaves the +// orphan figures known. prune does not treat such a directory as a +// snapshot and deletes the blobs the report lists as orphaned. +func TestRemoteInfo_DirectoryWithoutManifestLeavesOrphansKnown(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)) + addBlob(t, env.store.testStorer, testBlobHashA) + addBlob(t, env.store.testStorer, testBlobHashB) + + // One directory under a remote key and one under a non-conforming + // name, each holding only a database. + names := []string{snapshot.RemoteSnapshotKey(listLocalID), "\x1b[31mred"} + for _, name := range names { + require.NoError(t, env.store.Put(context.Background(), + "metadata/"+name+"/db.zst.age", + bytes.NewReader([]byte("not a valid database")))) + } + + require.NoError(t, env.v.RemoteInfo(false)) + + text := env.stdout.String() + assert.NotContains(t, text, "\x1b") + assert.Contains(t, text, "Downloading 1 manifest(s)...") + assert.Contains(t, text, "Orphaned (unreferenced): 1 (") + assert.Contains(t, text, "Run 'vaultik prune' to remove orphaned blobs.") + + env.stdout.Reset() + 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) + assert.NotContains(t, doc, "unreadable_manifests") + assert.NotContains(t, doc, "skipped_manifest_count") +}