diff --git a/README.md b/README.md index 1451d0a..306b27d 100644 --- a/README.md +++ b/README.md @@ -286,7 +286,9 @@ needed. remove orphaned blobs from remote storage. By default keeps only the latest snapshot per name; use `--keep-newer-than` for a rolling window. * `--keep-newer-than `: With `--prune`, keep snapshots newer than - this duration instead of only the latest (e.g. `4w`, `30d`, `6mo`, `1y`) + this duration instead of only the latest (e.g. `4w`, `30d`, `6mo`, `1y`). + Without `--prune`, or with a value that is not a valid duration, the + command fails before the backup starts. **`snapshot list`**: Show every snapshot known to this host — the union of the local index and the backup destination store — with timestamps diff --git a/internal/cli/entry_snapshot_create_prune_test.go b/internal/cli/entry_snapshot_create_prune_test.go new file mode 100644 index 0000000..2a7b229 --- /dev/null +++ b/internal/cli/entry_snapshot_create_prune_test.go @@ -0,0 +1,108 @@ +package cli //nolint:testpackage // shares runEntry and the hermetic config helpers + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Flags of `snapshot create` used to build argument vectors below. +const ( + flagPrune = "--prune" + flagKeepNewerThan = "--keep-newer-than" +) + +// TestEntrySnapshotCreateRejectsKeepNewerThanBeforeBackup checks that a +// --keep-newer-than the post-backup prune could not use stops `snapshot +// create` before the backup: the command fails and the destination +// store stays empty. It used to be read only by the prune, after the +// snapshot was uploaded, and without --prune it was ignored. +// +//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals +func TestEntrySnapshotCreateRejectsKeepNewerThanBeforeBackup(t *testing.T) { + for _, testCase := range []struct { + name string + args []string + }{ + { + name: "decimal in a day count", + args: []string{flagPrune, flagKeepNewerThan, "1.5d"}, + }, + { + name: "unknown unit", + args: []string{flagPrune, flagKeepNewerThan, "4x"}, + }, + { + name: "without --prune", + args: []string{flagKeepNewerThan, "4w"}, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + configPath, storeDir := writeBackupConfig(t) + + code, _, _ := runEntry(t, append( + []string{flagConfig, configPath, cmdSnapshot, cmdCreate}, + testCase.args...)...) + + assert.Equal(t, 1, code) + + stored, err := os.ReadDir(storeDir) + require.NoError(t, err) + assert.Empty(t, stored, "nothing may be uploaded") + }) + } +} + +// TestEntrySnapshotCreateReportsPostBackupPruneFailure makes the +// post-backup prune fail after a good backup, by planting a manifest +// that cannot be decoded, and checks that the failure is reported as the +// prune's and not as a failed snapshot creation. +// +//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals +func TestEntrySnapshotCreateReportsPostBackupPruneFailure(t *testing.T) { + configPath, storeDir := writeBackupConfig(t) + + // A remote key is 64 lowercase hex characters. The prune reads every + // manifest on the store and fails on one it cannot decode. + plantedDir := filepath.Join(storeDir, "metadata", strings.Repeat("a", 64)) + require.NoError(t, os.MkdirAll(plantedDir, 0o750)) + require.NoError(t, os.WriteFile( + filepath.Join(plantedDir, "manifest.json.zst"), + []byte("not a valid manifest"), 0o600)) + + code, _, stderr := runEntry(t, + flagConfig, configPath, cmdSnapshot, cmdCreate, flagPrune) + + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "Backup completed, but post-backup prune failed") + assert.NotContains(t, stderr, "Snapshot creation failed") + + snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata")) + require.NoError(t, err) + assert.Len(t, snapshots, 2, + "the new snapshot is on the store beside the planted one") +} + +// writeBackupConfig builds a hermetic config with one small file in its +// source directory and an empty destination store directory. Returns the +// config path and the store directory. +func writeBackupConfig(t *testing.T) (string, string) { + t.Helper() + + configPath := writeHermeticPruneConfig(t, false) + + // writeHermeticPruneConfig puts the source and store directories + // beside the config file. + dir := filepath.Dir(configPath) + sourceDir := filepath.Join(dir, "source") + + require.NoError(t, os.Mkdir(sourceDir, 0o750)) + require.NoError(t, os.WriteFile( + filepath.Join(sourceDir, "file.txt"), []byte("hello"), 0o600)) + + return configPath, filepath.Join(dir, "store") +} diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index 1dc6811..f157102 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -71,6 +71,11 @@ otherwise from the platform config directory (~/.config/vaultik/config.yml on Linux), then /etc/vaultik/config.yml.`, Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { + err := opts.Validate() + if err != nil { + return err + } + // Pass snapshot names from args opts.Snapshots = args // --skip-errors is a global flag on the root command. @@ -97,6 +102,13 @@ on Linux), then /etc/vaultik/config.yml.`, }, func(v *vaultik.Vaultik) error { return v.CreateSnapshot(opts) }, func(err error) { + if errors.Is(err, vaultik.ErrPostBackupPruneFailed) { + log.Error("Post-backup prune failed", "error", err) + ReportErrorf("Backup completed, but %v", err) + + return + } + log.Error("Snapshot creation failed", "error", err) ReportErrorf("Snapshot creation failed: %v", err) }) diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 2d65a0c..9b767df 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -28,8 +28,14 @@ var ( errNoAgeRecipients = errors.New( "creating a snapshot needs at least one public key in " + "age_recipients (generate a keypair with: age-keygen)") + errKeepNewerThanNeedsPrune = errors.New("--keep-newer-than requires --prune") ) +// ErrPostBackupPruneFailed wraps a failure of the post-backup prune, +// which runs only after every snapshot was created and uploaded, so the +// caller can report it apart from a failed backup. +var ErrPostBackupPruneFailed = errors.New("post-backup prune failed") + // listRecentLimit caps how many snapshot rows are fetched from the // local index for listing and purge operations. const listRecentLimit = 10000 @@ -45,6 +51,26 @@ type SnapshotCreateOptions struct { Snapshots []string // Optional list of snapshot names to process (empty = all) } +// Validate fails when KeepNewerThan is set without Prune or is not a +// valid duration. Only the post-backup prune reads KeepNewerThan, after +// the backup, so the caller checks it before starting one. +func (opts *SnapshotCreateOptions) Validate() error { + if opts.KeepNewerThan == "" { + return nil + } + + if !opts.Prune { + return errKeepNewerThanNeedsPrune + } + + _, err := parseDuration(opts.KeepNewerThan) + if err != nil { + return fmt.Errorf("--keep-newer-than: %w", err) + } + + return nil +} + // CreateSnapshot executes the snapshot creation operation func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error { // config.Load accepts an empty list, since listing, verifying and @@ -123,7 +149,7 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error { if opts.Prune { err = v.runPostBackupPrune(snapshotNames, opts.KeepNewerThan) if err != nil { - return fmt.Errorf("post-backup prune: %w", err) + return fmt.Errorf("%w: %w", ErrPostBackupPruneFailed, err) } }