diff --git a/README.md b/README.md index 784afca..03eddda 100644 --- a/README.md +++ b/README.md @@ -339,7 +339,9 @@ local index alone, and still exits zero. **`snapshot purge`**: Remove old snapshots based on criteria. Retention is per-snapshot-name (`--keep-latest` keeps the latest of each name, not the -latest globally). +latest globally). If a snapshot cannot be removed from the local index, the +command stops there, leaves that snapshot's metadata on the destination store +and exits non-zero; `snapshot create --prune` fails the same way. * `--keep-latest`: Keep only the most recent snapshot of each name * `--older-than `: Remove snapshots older than duration (e.g. `30d`, `4w`, `6mo`, `1y`; `m` is minutes, `mo` is months) diff --git a/internal/cli/entry_snapshot_purge_test.go b/internal/cli/entry_snapshot_purge_test.go new file mode 100644 index 0000000..d20c9b4 --- /dev/null +++ b/internal/cli/entry_snapshot_purge_test.go @@ -0,0 +1,90 @@ +package cli //nolint:testpackage // shares runEntry and the hermetic config helpers + +import ( + "context" + "database/sql" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/snapshot" + "sneak.berlin/go/vaultik/internal/types" +) + +// The `snapshot purge` command word and flags used below. +const ( + cmdPurge = "purge" + flagKeepLatest = "--keep-latest" + flagForce = "--force" +) + +// TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails makes +// removing a snapshot from the local index fail and checks that +// `snapshot purge` exits non-zero and leaves that snapshot's metadata on +// the destination store. It used to log the error, delete the metadata +// anyway and exit 0. +// +//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals +func TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails(t *testing.T) { + ctx := context.Background() + configPath := writeHermeticPruneConfig(t, false) + + // writeHermeticPruneConfig puts the index and the store directory + // beside the config file. + dir := filepath.Dir(configPath) + + // Two snapshots of the same name: --keep-latest purges the older. + olderStart := time.Date(2026, 5, 1, 9, 0, 0, 0, time.UTC) + newerStart := olderStart.Add(time.Hour) + olderID := "test-host_test_" + olderStart.Format(time.RFC3339) + newerID := "test-host_test_" + newerStart.Format(time.RFC3339) + + db, err := database.New(ctx, filepath.Join(dir, "index.sqlite")) + require.NoError(t, err) + + repos := database.NewRepositories(db) + + manifestPaths := make(map[string]string) + + for id, startedAt := range map[string]time.Time{ + olderID: olderStart, + newerID: newerStart, + } { + completedAt := startedAt.Add(time.Minute) + snap := &database.Snapshot{ + ID: types.SnapshotID(id), + Hostname: "test-host", + VaultikVersion: "test", + 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) + + manifestPaths[id] = filepath.Join(dir, "store", "metadata", + snapshot.RemoteSnapshotKey(id), "manifest.json.zst") + require.NoError(t, os.MkdirAll(filepath.Dir(manifestPaths[id]), 0o750)) + require.NoError(t, os.WriteFile(manifestPaths[id], []byte("stub"), 0o600)) + } + + _, err = db.Conn().ExecContext(ctx, ` + CREATE TRIGGER refuse_snapshot_delete BEFORE DELETE ON snapshots + BEGIN SELECT RAISE(ABORT, 'simulated index error'); END`) + require.NoError(t, err) + require.NoError(t, db.Close()) + + code, _, stderr := runEntry(t, + flagConfig, configPath, cmdSnapshot, cmdPurge, flagKeepLatest, flagForce) + + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "simulated index error") + assert.FileExists(t, manifestPaths[olderID], + "a snapshot still in the local index keeps its metadata") +} diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 630e2ba..9f5c9e2 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -627,7 +627,8 @@ func selectSnapshotsToPurge( } // confirmAndExecutePurge shows deletion candidates, confirms with the -// user, and deletes snapshots. +// user, and deletes snapshots. It stops with an error at the first +// snapshot it cannot delete. func (v *Vaultik) confirmAndExecutePurge( toDelete []SnapshotInfo, force, quiet bool, ) error { @@ -669,10 +670,11 @@ func (v *Vaultik) confirmAndExecutePurge( snapshotID := snap.ID.String() log.Info("Deleting snapshot", "id", snapshotID) + // Stop before the remote delete: a snapshot still in the local + // index must keep its metadata on the destination store. err := v.deleteSnapshotFromLocalDB(snapshotID) if err != nil { - log.Error("Failed to delete from local database", - "snapshot_id", snapshotID, "error", err) + return fmt.Errorf("deleting snapshot %s from local index: %w", snapshotID, err) } _, err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))