Reconcile purge against remote by hashed key, not human ID (closes #160)
check / check (pull_request) Successful in 1m20s
check / check (push) Successful in 2m51s

syncWithRemote compared human snapshot IDs against the hashed metadata/<key>/ directory names, which never match, so it deleted every local snapshot record; the purge that followed then found nothing to remove remotely. Reconcile via listAllRemoteSnapshotKeys and RemoteSnapshotKey(id), matching CleanupLocalSnapshots, so a row still backed by remote metadata is kept.

The purge tests only passed because their stubs used the human-ID layout production never writes; they now write metadata under the hashed remote key. New tests prove remotely-backed local rows survive the reconcile and that a purge removes the local row and remote metadata together.

Model: opus-4-8
This commit was merged in pull request #183.
This commit is contained in:
2026-09-22 12:11:49 +02:00
parent d9f0220f94
commit 96ebcd40d7
3 changed files with 169 additions and 25 deletions
@@ -0,0 +1,146 @@
package vaultik_test
import (
"bytes"
"context"
"database/sql"
"strings"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/snapshot"
"sneak.berlin/go/vaultik/internal/types"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// setupConsistencyTest builds a Vaultik whose local database and mock
// remote both hold the given snapshots. Remote metadata is stored under
// the production layout, metadata/<RemoteSnapshotKey(id)>/manifest.json.zst.
// It returns the instance and the mock so a test can inspect the remote.
func setupConsistencyTest(
t *testing.T, snapshotIDs []string,
) (*vaultik.Vaultik, *MockStorer) {
t.Helper()
ctx := context.Background()
db, err := database.New(ctx, ":memory:")
require.NoError(t, err)
t.Cleanup(func() { _ = db.Close() })
repos := database.NewRepositories(db)
mockStorage := NewMockStorer()
for _, id := range snapshotIDs {
parts := strings.Split(id, "_")
startedAt, err := time.Parse(time.RFC3339, parts[len(parts)-1])
require.NoError(t, err, "parsing timestamp from snapshot ID %q", id)
completedAt := startedAt.Add(5 * time.Minute)
snap := &database.Snapshot{
ID: types.SnapshotID(id),
Hostname: testHostname,
VaultikVersion: testLabel,
StartedAt: startedAt,
CompletedAt: &completedAt,
}
err = repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
return repos.Snapshots.Create(ctx, tx, snap)
})
require.NoError(t, err, "creating snapshot %s", id)
metadataKey := "metadata/" + snapshot.RemoteSnapshotKey(id) +
"/manifest.json.zst"
err = mockStorage.Put(ctx, metadataKey, strings.NewReader("stub"))
require.NoError(t, err)
}
v := &vaultik.Vaultik{
Storage: mockStorage,
Repositories: repos,
DB: db,
Stdout: &bytes.Buffer{},
Stderr: &bytes.Buffer{},
Stdin: &bytes.Buffer{},
}
v.SetContext(ctx)
return v, mockStorage
}
func remoteHasSnapshot(t *testing.T, m *MockStorer, id string) bool {
t.Helper()
prefix := "metadata/" + snapshot.RemoteSnapshotKey(id) + "/"
keys, err := m.List(context.Background(), prefix)
require.NoError(t, err)
return len(keys) > 0
}
// TestPurgeKeepsRemotelyBackedLocalRows guards against issue #160
// (https://git.eeqj.de/sneak/vaultik/issues/160): purge reconciles local
// rows against the remote first, and that step compared human snapshot IDs
// against the hashed remote directory names, which never match — so it
// deleted every local record and the purge itself then removed nothing.
//
// With every snapshot still present remotely and nothing old enough to
// purge, all local rows must survive the reconcile untouched.
func TestPurgeKeepsRemotelyBackedLocalRows(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ids := []string{snapHomeT0, snapHomeT1, snapSystemT0}
v, _ := setupConsistencyTest(t, ids)
err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
// 100 years: nothing is old enough to delete, so the reconcile
// is the only thing that touches the rows.
OlderThan: "36500d",
Force: true,
})
require.NoError(t, err)
remaining := listRemainingSnapshots(t, v)
assert.Len(t, remaining, len(ids),
"remotely-backed local rows must survive the reconcile")
assert.Contains(t, remaining, snapHomeT0)
assert.Contains(t, remaining, snapHomeT1)
assert.Contains(t, remaining, snapSystemT0)
}
// TestPurgeRemovesLocalAndRemoteTogether proves the two halves stay
// consistent: a purged snapshot is gone both locally and remotely, while a
// retained one keeps both. Before the fix, the reconcile dropped every
// local row yet the remote metadata was left in place.
func TestPurgeRemovesLocalAndRemoteTogether(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ids := []string{snapHomeT0, snapHomeT1, snapSystemT0}
v, mock := setupConsistencyTest(t, ids)
err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
KeepLatest: true,
Force: true,
})
require.NoError(t, err)
// Keep latest per name: newest home and the lone system are kept.
remaining := listRemainingSnapshots(t, v)
assert.ElementsMatch(t, []string{snapHomeT1, snapSystemT0}, remaining)
// Local and remote agree: the older home snapshot is gone from both,
// the retained ones are present in both.
assert.False(t, remoteHasSnapshot(t, mock, snapHomeT0),
"purged snapshot must also be removed remotely")
assert.True(t, remoteHasSnapshot(t, mock, snapHomeT1),
"retained snapshot must remain remotely")
assert.True(t, remoteHasSnapshot(t, mock, snapSystemT0),
"retained snapshot must remain remotely")
}
+6 -2
View File
@@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/database" "sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/snapshot"
"sneak.berlin/go/vaultik/internal/types" "sneak.berlin/go/vaultik/internal/types"
"sneak.berlin/go/vaultik/internal/vaultik" "sneak.berlin/go/vaultik/internal/vaultik"
) )
@@ -60,8 +61,11 @@ func setupPurgeTest(t *testing.T, snapshotIDs []string) *vaultik.Vaultik {
}) })
require.NoError(t, err, "creating snapshot %s", id) require.NoError(t, err, "creating snapshot %s", id)
// Create remote metadata stub so syncWithRemote keeps it // Create the remote metadata stub under the production layout so
metadataKey := "metadata/" + id + "/manifest.json.zst" // syncWithRemote keeps the local row. Production stores metadata
// under the hashed remote key, not the human snapshot ID.
metadataKey := "metadata/" + snapshot.RemoteSnapshotKey(id) +
"/manifest.json.zst"
err = mockStorage.Put(ctx, metadataKey, strings.NewReader("stub")) err = mockStorage.Put(ctx, metadataKey, strings.NewReader("stub"))
require.NoError(t, err) require.NoError(t, err)
} }
+17 -23
View File
@@ -935,29 +935,23 @@ func (v *Vaultik) downloadManifestByKey(remoteKey string) (*snapshot.Manifest, e
func (v *Vaultik) syncWithRemote() error { func (v *Vaultik) syncWithRemote() error {
log.Info("Syncing with remote snapshots") log.Info("Syncing with remote snapshots")
// Get all remote snapshot IDs // Remote metadata lives under metadata/<remote-key>/, where the
remoteSnapshots := make(map[string]bool) // directory name is snapshot.RemoteSnapshotKey(id), not the human
objectCh := v.Storage.ListStream(v.ctx, "metadata/") // snapshot ID. Compare each local row's hashed key against that set
// so a row still backed by remote metadata is kept. Comparing human
for object := range objectCh { // IDs against the hashed directory names matches nothing and deletes
if object.Err != nil { // every local snapshot record (issue #160).
return fmt.Errorf("listing remote snapshots: %w", object.Err) remoteKeys, err := v.listAllRemoteSnapshotKeys()
} if err != nil {
return fmt.Errorf("listing remote snapshots: %w", err)
// Extract snapshot ID from paths like metadata/hostname-20240115-143052Z/
parts := strings.Split(object.Key, "/")
if len(parts) >= minSnapshotIDParts &&
parts[0] == metadataDirName && parts[1] != "" {
// Skip macOS resource fork files (._*) and other hidden files
if strings.HasPrefix(parts[1], ".") {
continue
}
remoteSnapshots[parts[1]] = true
}
} }
log.Debug("Found remote snapshots", "count", len(remoteSnapshots)) remoteKeySet := make(map[string]bool, len(remoteKeys))
for _, k := range remoteKeys {
remoteKeySet[k] = true
}
log.Debug("Found remote snapshots", "count", len(remoteKeySet))
// Get all local snapshots (use a high limit to get all) // Get all local snapshots (use a high limit to get all)
localSnapshots, err := v.Repositories.Snapshots.ListRecent(v.ctx, listRecentLimit) localSnapshots, err := v.Repositories.Snapshots.ListRecent(v.ctx, listRecentLimit)
@@ -965,12 +959,12 @@ func (v *Vaultik) syncWithRemote() error {
return fmt.Errorf("listing local snapshots: %w", err) return fmt.Errorf("listing local snapshots: %w", err)
} }
// Remove local snapshots that don't exist remotely // Remove local snapshots whose metadata is absent from the remote.
removedCount := 0 removedCount := 0
for _, snap := range localSnapshots { for _, snap := range localSnapshots {
snapshotIDStr := snap.ID.String() snapshotIDStr := snap.ID.String()
if !remoteSnapshots[snapshotIDStr] { if !remoteKeySet[snapshot.RemoteSnapshotKey(snapshotIDStr)] {
log.Info("Removing local snapshot not found in remote", log.Info("Removing local snapshot not found in remote",
"snapshot_id", snap.ID) "snapshot_id", snap.ID)