Check --keep-newer-than before the backup starts (closes #307)
check / check (push) Waiting to run
check / check (push) Waiting to run
snapshot create read --keep-newer-than only in the post-backup prune, so a value the duration parser rejects failed the run after the snapshot was uploaded: a nightly cron line added a snapshot each night, reported it as failed, and never deleted any. Without --prune the flag was ignored. CreateSnapshot now checks it first and fails, before anything is backed up, on an invalid duration or a missing --prune. A failed post-backup prune is now wrapped in ErrPostBackupPruneFailed and reported as "Backup completed, but post-backup prune failed: ..." rather than as a failed snapshot creation. The exit status stays 1. Model: opus-5-5
This commit is contained in:
@@ -0,0 +1,102 @@
|
||||
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"
|
||||
)
|
||||
|
||||
// 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{"--prune", "--keep-newer-than", "1.5d"},
|
||||
},
|
||||
{
|
||||
name: "unknown unit",
|
||||
args: []string{"--prune", "--keep-newer-than", "4x"},
|
||||
},
|
||||
{
|
||||
name: "without --prune",
|
||||
args: []string{"--keep-newer-than", "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, "--prune")
|
||||
|
||||
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")
|
||||
}
|
||||
@@ -97,6 +97,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)
|
||||
})
|
||||
|
||||
@@ -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
|
||||
@@ -53,6 +59,20 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
|
||||
return errNoAgeRecipients
|
||||
}
|
||||
|
||||
// Only the post-backup prune reads --keep-newer-than, after the
|
||||
// backup; check it here so a bad value stops the run before anything
|
||||
// is backed up.
|
||||
if opts.KeepNewerThan != "" {
|
||||
if !opts.Prune {
|
||||
return errKeepNewerThanNeedsPrune
|
||||
}
|
||||
|
||||
_, err := parseDuration(opts.KeepNewerThan)
|
||||
if err != nil {
|
||||
return fmt.Errorf("--keep-newer-than: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
overallStartTime := time.Now()
|
||||
|
||||
log.Info("Starting snapshot creation",
|
||||
@@ -123,7 +143,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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user