Author SHA1 Message Date
clawbot 59d7376c04 Refuse an exclude pattern that does not compile at config load (closes #455)
check / check (push) Successful in 9m29s
An exclude pattern that did not compile was skipped with a warning during the backup, which then stored what the user meant to leave out. Config.Validate now refuses it, naming the setting and quoting the pattern, for example exclude[2]: "build[" is not a valid exclude pattern: ...

The compile step moves to a new package, internal/exclude, as exclude.Compile; Validate and the scanner both call it. The scanner's skip-and-warn path is gone; a pattern that does not compile there now fails the scanner.

Model: opus-5-5
2026-11-11 09:02:09 +00:00
clawbot 6a8bb2f389 Print a failed command's error once on stderr (closes #433)
check / check (push) Failing after 9m31s
A failed command printed its error twice on stderr: as an ERROR log record and as the final ERROR: line the README documents. The command handlers, a restore failure that ends the command and a failed blob upload now log that error at info level or not at all, so without --verbose or --debug only the final line appears.

Model: opus-5-5
2026-11-11 07:38:11 +01:00
19 changed files with 495 additions and 404 deletions

No files matched your search

-2
View File
@@ -448,8 +448,6 @@ remove the snapshot's metadata from it (`vaultik prune` does not). For a
snapshot that is only on the destination store, that failure is an error. To
wipe everything on the destination in one go, use `vaultik remote nuke --force`.
- Accepts a snapshot ID, or — for a snapshot only on the destination store — its
remote key (or an unambiguous leading part of it) as shown by `snapshot list`.
- `--local-only`: Skip remote cleanup; only touch the local index
- `--dry-run`: Show what would be deleted without deleting
- `--force`: Skip confirmation prompt
+1 -1
View File
@@ -337,7 +337,7 @@ func runVaultikApp(
},
Mode: mode,
}, op, func(err error) {
log.Error(failMsg, "error", err)
log.Info(failMsg, "error", err)
ReportErrorf("%s: %v", failMsg, err)
})
}
+4
View File
@@ -85,6 +85,10 @@ func emitStartupBanner(args []string, w io.Writer) {
// 🛑 ERROR: format. Use it from goroutine error paths (where returning
// an error to cobra isn't an option) and anywhere else a CLI command
// must surface a failure outside the normal RunE return path.
//
// This line is the one report of the failure. A caller that also logs
// the failure logs it at info level, so the record appears only under
// --verbose or --debug and the error is not printed twice.
func ReportErrorf(format string, args ...any) {
ui.New(os.Stderr).Errorf(format, args...)
}
+205
View File
@@ -0,0 +1,205 @@
package cli //nolint:testpackage // shares the fixtures and capture helpers
import (
"context"
"encoding/json"
"fmt"
"io"
"os"
"path/filepath"
"strings"
"testing"
"filippo.io/age"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/ui"
)
const (
// errorLinePrefix starts the final line that reports a failed command.
errorLinePrefix = "ERROR: "
flagVerbose = "--verbose"
cmdRestore = "restore"
// blobsDir is the directory of a destination store that holds the
// blobs.
blobsDir = "blobs"
)
// TestEntryFailureIsPrintedOnce runs commands that fail after startup
// against a destination they cannot use. `info` is not among them: it
// cannot fail after startup.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntryFailureIsPrintedOnce(t *testing.T) {
for _, command := range [][]string{
{cmdSnapshot, cmdCreate},
{cmdSnapshot, cmdRemove, someSnapshotID},
{cmdSnapshot, cmdVerify, someSnapshotID},
{cmdRemote, cmdInfo},
{cmdPrune, flagJSON},
} {
for _, verbose := range []bool{false, true} {
name := fmt.Sprintf("%s verbose=%t", strings.Join(command, " "), verbose)
t.Run(name, func(t *testing.T) {
requireErrorPrintedOnce(t, verbose, append([]string{flagConfig,
writeUnusableDestinationConfig(t)}, command...)...)
})
}
}
}
// TestEntryBlobUploadFailureIsPrintedOnce backs up to a destination
// whose blobs directory is a file, so the blob upload fails.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntryBlobUploadFailureIsPrintedOnce(t *testing.T) {
for _, verbose := range []bool{false, true} {
t.Run(fmt.Sprintf("verbose=%t", verbose), func(t *testing.T) {
configPath, storeDir := writeBackupConfig(t)
require.NoError(t, os.WriteFile(
filepath.Join(storeDir, blobsDir), nil, 0o600))
requireErrorPrintedOnce(t, verbose,
flagConfig, configPath, cmdSnapshot, cmdCreate)
})
}
}
// TestEntryBlobDownloadFailureIsPrintedOnce backs up, deletes the blobs
// from the destination, and restores, so the blob download fails.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntryBlobDownloadFailureIsPrintedOnce(t *testing.T) {
for _, verbose := range []bool{false, true} {
t.Run(fmt.Sprintf("verbose=%t", verbose), func(t *testing.T) {
configPath, storeDir := writeBackupConfigWithPrivateKey(t)
code, _, _ := runEntry(t,
flagConfig, configPath, cmdSnapshot, cmdCreate)
require.Equal(t, 0, code)
require.NoError(t, os.RemoveAll(filepath.Join(storeDir, blobsDir)))
requireErrorPrintedOnce(t, verbose, flagConfig, configPath,
cmdSnapshot, cmdRestore, onlySnapshotID(t, configPath), t.TempDir())
})
}
}
// requireErrorPrintedOnce runs Entry with args, adding --verbose when
// verbose is set. The command must fail, and stderr must carry its error
// once, as the final ERROR: line. A log record carrying the same error
// appears only under --verbose.
func requireErrorPrintedOnce(t *testing.T, verbose bool, args ...string) {
t.Helper()
if verbose {
args = append(args, flagVerbose)
}
code, _, stderr := runEntry(t, args...)
require.Equal(t, 1, code)
errorLine, records := failureOutput(t, stderr)
if !verbose {
assert.Zero(t, records,
"without --verbose no log record repeats the error:\n%s", stderr)
return
}
assert.NotZero(t, records,
"--verbose must keep the log record of %q", errorLine)
}
// failureOutput requires stderr to hold exactly one ERROR: line and
// returns it, with the number of log records whose error attribute is
// the error that line reports. stderr is a pipe here, so every log
// record is a JSON object on its own line.
func failureOutput(t *testing.T, stderr string) (string, int) {
t.Helper()
var (
errorLines []string
records []map[string]any
)
for line := range strings.Lines(stderr) {
var record map[string]any
if json.Unmarshal([]byte(line), &record) == nil {
records = append(records, record)
continue
}
if strings.Contains(line, errorLinePrefix) {
errorLines = append(errorLines, strings.TrimSpace(line))
}
}
require.Len(t, errorLines, 1,
"the error must be printed once as an ERROR: line:\n%s", stderr)
// An error can run over several lines; the ERROR: line holds the
// first of them.
repeats := 0
for _, record := range records {
failure, ok := record["error"].(string)
firstLine, _, _ := strings.Cut(failure, "\n")
if ok && strings.Contains(errorLines[0], firstLine) {
repeats++
}
}
return errorLines[0], repeats
}
// writeBackupConfigWithPrivateKey builds what writeBackupConfig builds,
// with a generated key pair in place of its recipient. The config also
// holds the private key, so that restore can decrypt the backup. Returns
// the config path and the store directory.
func writeBackupConfigWithPrivateKey(t *testing.T) (string, string) {
t.Helper()
configPath, storeDir := writeBackupConfig(t)
identity, err := age.GenerateX25519Identity()
require.NoError(t, err)
out := ui.NewWithColor(io.Discard, false)
require.NoError(t, writeConfigSet(out, configPath,
"age_recipients.0", identity.Recipient().String()))
require.NoError(t, writeConfigSet(out, configPath,
"age_secret_key", identity.String()))
return configPath, storeDir
}
// onlySnapshotID returns the ID of the one snapshot in the local index
// that writeBackupConfig puts beside the config file.
func onlySnapshotID(t *testing.T, configPath string) string {
t.Helper()
ctx := context.Background()
db, err := database.New(ctx,
filepath.Join(filepath.Dir(configPath), "index.sqlite"))
require.NoError(t, err)
defer func() { require.NoError(t, db.Close()) }()
backedUp, err := database.NewRepositories(db).Snapshots.GetOnlySnapshot(ctx)
require.NoError(t, err)
return backedUp.ID.String()
}
@@ -61,8 +61,8 @@ func TestEntrySnapshotCreateRejectsKeepNewerThanBeforeBackup(t *testing.T) {
// 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.
// that cannot be decoded, and checks that the failure is reported once,
// 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) {
@@ -83,6 +83,9 @@ func TestEntrySnapshotCreateReportsPostBackupPruneFailure(t *testing.T) {
assert.Contains(t, stderr, "Backup completed, but post-backup prune failed")
assert.NotContains(t, stderr, "Snapshot creation failed")
_, repeats := failureOutput(t, stderr)
assert.Zero(t, repeats, "no log record may repeat the error:\n%s", stderr)
snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
require.NoError(t, err)
assert.Len(t, snapshots, 2,
@@ -93,7 +96,8 @@ func TestEntrySnapshotCreateReportsPostBackupPruneFailure(t *testing.T) {
// empties the source directory, as a volume that is not mounted leaves
// its mount point, and runs `snapshot create --prune`. The command exits
// non-zero and reports the backup as completed, not as a failed snapshot
// creation. See https://git.eeqj.de/sneak/vaultik/issues/388.
// creation. The error is printed once. See
// https://git.eeqj.de/sneak/vaultik/issues/388.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) {
@@ -114,6 +118,9 @@ func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) {
"Backup completed, but 1 source path(s) were empty")
assert.NotContains(t, stderr, "Snapshot creation failed")
_, repeats := failureOutput(t, stderr)
assert.Zero(t, repeats, "no log record may repeat the error:\n%s", stderr)
snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
require.NoError(t, err)
assert.Len(t, snapshots, 2, "the older snapshot is kept")
@@ -124,7 +131,8 @@ func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) {
// directory as a mount point, and runs `snapshot create --prune`, which
// does not find it as one. The command exits non-zero, reports the
// backup as completed, not as a failed snapshot creation, and keeps the
// older snapshot. See https://git.eeqj.de/sneak/vaultik/issues/395.
// older snapshot. The error is printed once. See
// https://git.eeqj.de/sneak/vaultik/issues/395.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotCreatePruneExitsNonZeroWhenMountPointIsMissing(
@@ -159,6 +167,9 @@ func TestEntrySnapshotCreatePruneExitsNonZeroWhenMountPointIsMissing(
"an earlier snapshot recorded were not found")
assert.NotContains(t, stderr, "Snapshot creation failed")
_, repeats := failureOutput(t, stderr)
assert.Zero(t, repeats, "no log record may repeat the error:\n%s", stderr)
snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
require.NoError(t, err)
assert.Len(t, snapshots, 2, "the older snapshot is kept")
+1 -1
View File
@@ -39,7 +39,7 @@ func NewInfoCommand() *cobra.Command {
}, func(v *vaultik.Vaultik) error {
return v.ShowInfo()
}, func(err error) {
log.Error("Failed to show info", "error", err)
log.Info("Failed to show info", "error", err)
ReportErrorf("Failed to show info: %v", err)
})
},
+1 -1
View File
@@ -60,7 +60,7 @@ referenced.`,
}, func(v *vaultik.Vaultik) error {
return v.Prune(opts)
}, func(err error) {
log.Error("Prune operation failed", "error", err)
log.Info("Prune operation failed", "error", err)
ReportErrorf("Prune failed: %v", err)
})
},
+1 -1
View File
@@ -98,7 +98,7 @@ func newRemoteInfoCommand() *cobra.Command {
}, func(v *vaultik.Vaultik) error {
return v.RemoteInfo(jsonOutput)
}, func(err error) {
log.Error("Failed to get remote info", "error", err)
log.Info("Failed to get remote info", "error", err)
ReportErrorf("Failed to get remote info: %v", err)
})
},
+4 -8
View File
@@ -109,7 +109,7 @@ on Linux), then /etc/vaultik/config.yml.`,
return v.CreateSnapshot(opts)
}, func(err error) {
if errors.Is(err, vaultik.ErrPostBackupPruneFailed) {
log.Error("Post-backup prune failed", "error", err)
log.Info("Post-backup prune failed", "error", err)
ReportErrorf("Backup completed, but %v", err)
return
@@ -118,13 +118,13 @@ on Linux), then /etc/vaultik/config.yml.`,
if errors.Is(err, vaultik.ErrFilesUnreadable) ||
errors.Is(err, vaultik.ErrSourcePathEmpty) ||
errors.Is(err, vaultik.ErrVolumeNotMounted) {
log.Error("Backup completed with an error", "error", err)
log.Info("Backup completed with an error", "error", err)
ReportErrorf("Backup completed, but %v", err)
return
}
log.Error("Snapshot creation failed", "error", err)
log.Info("Snapshot creation failed", "error", err)
ReportErrorf("Snapshot creation failed: %v", err)
})
},
@@ -258,7 +258,7 @@ func newSnapshotVerifyCommand() *cobra.Command {
return
}
log.Error("Verification failed", "error", err)
log.Info("Verification failed", "error", err)
ReportErrorf("Verification failed: %v", err)
})
},
@@ -288,10 +288,6 @@ know about), which is what 'vaultik prune' does. The command prints the
'vaultik prune' invocation to run as a follow-up only when it removed the
snapshot's metadata from the destination store, and never under --json.
The snapshot may be named by its ID or, on a host with no local index, by
the remote key that 'snapshot list' prints for a remote-only snapshot (an
unambiguous leading part is enough).
An ID that names no snapshot in the local index or on the destination
store is an error.
+1 -1
View File
@@ -104,7 +104,7 @@ func runRestore(cmd *cobra.Command, args []string, opts *RestoreOptions) error {
SkipErrors: rootFlags.SkipErrors,
})
}, func(err error) {
log.Error("Restore operation failed", "error", err)
log.Info("Restore operation failed", "error", err)
ReportErrorf("Restore failed: %v", err)
})
}
+25 -7
View File
@@ -396,6 +396,8 @@ func Parse(data []byte) (*Config, error) {
// never the value. An empty list is accepted, because only snapshot
// create needs a recipient and it checks for one itself
// - At least one snapshot must be configured with at least one path
// - Every exclude pattern, global or in a snapshot, must compile; errors
// name the setting and quote the pattern
// - Storage must be configured (either storage_url or s3.* fields)
// - The index path must be absolute: a relative one would resolve against
// the working directory, so each directory would get its own index
@@ -416,18 +418,18 @@ func (c *Config) Validate() error {
}
}
if len(c.Snapshots) == 0 {
return errNoSnapshots
err := c.validateSnapshots()
if err != nil {
return err
}
for name, snap := range c.Snapshots {
if len(snap.Paths) == 0 {
return fmt.Errorf("%w: %q", errSnapshotNoPaths, name)
}
err = c.validateExcludes()
if err != nil {
return err
}
// Validate storage configuration
err := c.validateStorage()
err = c.validateStorage()
if err != nil {
return err
}
@@ -466,6 +468,22 @@ func (c *Config) Validate() error {
return nil
}
// validateSnapshots refuses a config with no snapshot or with a snapshot
// that has no path.
func (c *Config) validateSnapshots() error {
if len(c.Snapshots) == 0 {
return errNoSnapshots
}
for name, snap := range c.Snapshots {
if len(snap.Paths) == 0 {
return fmt.Errorf("%w: %q", errSnapshotNoPaths, name)
}
}
return nil
}
// validateAgeRecipient parses one age_recipients entry with the age library
// and returns a value-free error on failure. A recipient string can be
// sensitive (an operator may paste a secret key by mistake), so neither the
+39
View File
@@ -0,0 +1,39 @@
package config
import (
"fmt"
"sneak.berlin/go/vaultik/internal/exclude"
)
// validateExcludes refuses an exclude pattern, global or in a snapshot,
// that does not compile, naming the setting and quoting the pattern.
func (c *Config) validateExcludes() error {
err := validateExcludeList("exclude", c.Exclude)
if err != nil {
return err
}
for _, name := range c.SnapshotNames() {
err = validateExcludeList("snapshots."+name+".exclude",
c.Snapshots[name].Exclude)
if err != nil {
return err
}
}
return nil
}
// validateExcludeList refuses the first pattern in patterns that does not
// compile. setting is the name of the list, as in "exclude".
func validateExcludeList(setting string, patterns []string) error {
for i, pattern := range patterns {
_, _, err := exclude.Compile(pattern)
if err != nil {
return fmt.Errorf("%s[%d]: %q %w", setting, i, pattern, err)
}
}
return nil
}
+65
View File
@@ -0,0 +1,65 @@
package config_test
import (
"os"
"path/filepath"
"strings"
"testing"
"sneak.berlin/go/vaultik/internal/config"
)
// TestLoadRefusesBadExcludePattern checks that an exclude pattern that does
// not compile, global or in a snapshot, fails at load with the setting and
// the pattern named, rather than the backup storing what it meant to leave
// out.
func TestLoadRefusesBadExcludePattern(t *testing.T) {
t.Parallel()
const storage = "storage_url: file:///tmp/vaultik-test-storage\n"
tests := []struct {
name string
config string
wantErr string
}{
{
name: "global",
config: "exclude: ['*.tmp', '.cache', 'build[']\n" +
"snapshots:\n" +
" home:\n" +
" paths: [/tmp/vaultik-test-source]\n" + storage,
wantErr: `exclude[2]: "build[" is not a valid exclude pattern`,
},
{
name: "snapshot",
config: "snapshots:\n" +
" home:\n" +
" paths: [/tmp/vaultik-test-source]\n" +
" exclude: ['/a/**/x[']\n" + storage,
wantErr: `snapshots.home.exclude[0]: "/a/**/x[" is not a valid exclude pattern`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
path := filepath.Join(t.TempDir(), "config.yml")
err := os.WriteFile(path, []byte(tt.config), 0o600)
if err != nil {
t.Fatalf("write config: %v", err)
}
_, err = config.Load(path)
if err == nil {
t.Fatalf("Load() returned nil, want an error containing %q", tt.wantErr)
}
if !strings.Contains(err.Error(), tt.wantErr) {
t.Fatalf("Load() error = %q, want it to contain %q", err, tt.wantErr)
}
})
}
}
+76
View File
@@ -0,0 +1,76 @@
// Package exclude compiles exclude patterns into globs.
package exclude
import (
"errors"
"fmt"
"strings"
"github.com/gobwas/glob"
)
var errBadExcludePattern = errors.New("is not a valid exclude pattern")
// Compile compiles one exclude pattern into the globs that match it.
// anchored reports whether the pattern matches only from the root of the
// source directory. An empty pattern yields no globs. Config.Validate and
// the scanner both compile patterns with it, so a pattern that loads is one
// the scanner can use.
func Compile(pattern string) ([]glob.Glob, bool, error) {
if pattern == "" {
return nil, false, nil
}
// A leading "/" anchors the pattern to the source directory.
trimmed, anchored := strings.CutPrefix(pattern, "/")
// A trailing "/" marks a directory and is not matched.
trimmed = strings.TrimSuffix(trimmed, "/")
// A leading "**/" demands a "/" before the name and so misses an
// entry directly under the source path. It means "at every depth",
// which is what an unanchored pattern already does, so drop the
// prefix and match unanchored: as in gitignore, "**/x" and
// "/**/x" then also match a top-level "x".
if rest, found := strings.CutPrefix(trimmed, "**/"); found {
trimmed = rest
anchored = false
}
// Likewise "/**/" inside a pattern demands two slashes, so
// "a/**/*.log" misses "a/x.log". As in gitignore it stands for
// zero or more directories, so also compile the pattern with it
// made a single "/". The glob library's {a,b} is no help: with a
// "**" inside it in mid-pattern, it misses paths it should match.
var globs []glob.Glob
for _, variant := range doubleStarVariants(trimmed) {
g, err := glob.Compile(variant, '/')
if err != nil {
return nil, false, fmt.Errorf("%w: %w", errBadExcludePattern, err)
}
globs = append(globs, g)
}
return globs, anchored, nil
}
// doubleStarVariants returns pattern once for every way of keeping each
// "/**/" in it or making it a single "/".
func doubleStarVariants(pattern string) []string {
const doubleStar = "/**/"
before, after, found := strings.Cut(pattern, doubleStar)
if !found {
return []string{pattern}
}
var variants []string
for _, rest := range doubleStarVariants(after) {
variants = append(variants, before+doubleStar+rest, before+"/"+rest)
}
return variants
}
+20 -65
View File
@@ -20,6 +20,7 @@ import (
"sneak.berlin/go/vaultik/internal/blob"
"sneak.berlin/go/vaultik/internal/chunker"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/exclude"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/types"
@@ -221,8 +222,12 @@ func NewScanner(cfg ScannerConfig) *Scanner {
progress = NewProgressReporter()
}
// Compile exclude patterns
compiledExclude := compileExcludePatterns(cfg.Exclude)
compiledExclude, err := compileExcludePatterns(cfg.Exclude)
if err != nil {
log.Error("Failed to compile exclude patterns", "error", err)
return nil
}
uiw := cfg.UI
if uiw == nil {
@@ -1875,10 +1880,12 @@ func (s *Scanner) uploadBlobIfNeeded(
err = s.storage.PutWithProgress(ctx, blobPath, blobWithReader.Reader,
finishedBlob.Compressed, progressCallback)
if err != nil {
// Only an interrupt cancels ctx, and the user is told about it,
// so an upload it stopped is not logged as a storage failure.
// The record is at info level: the returned error ends the backup
// and is printed as its ERROR: line. Only an interrupt cancels
// ctx, and the user is told about it, so an upload it stopped is
// not logged as a storage failure.
if ctx.Err() == nil {
log.Error("Failed to upload blob", "hash", finishedBlob.Hash, "error", err)
log.Info("Failed to upload blob", "hash", finishedBlob.Hash, "error", err)
}
return 0, false, fmt.Errorf("uploading blob to storage: %w", err)
@@ -2316,51 +2323,18 @@ func wrapPermissionError(path string, err error) error {
"with --skip-errors to continue past unreadable files)", path, err)
}
// compileExcludePatterns compiles the exclude patterns into glob matchers
func compileExcludePatterns(patterns []string) []compiledPattern {
// compileExcludePatterns compiles the exclude patterns into glob matchers.
// Config.Validate has already refused a pattern that does not compile.
func compileExcludePatterns(patterns []string) ([]compiledPattern, error) {
var compiled []compiledPattern
for _, p := range patterns {
if p == "" {
continue
globs, anchored, err := exclude.Compile(p)
if err != nil {
return nil, fmt.Errorf("%q %w", p, err)
}
// Check if pattern is anchored (starts with /)
anchored := strings.HasPrefix(p, "/")
pattern := p
if anchored {
pattern = p[1:] // Remove leading /
}
// Remove trailing slash if present (directory indicator)
pattern = strings.TrimSuffix(pattern, "/")
// A leading "**/" demands a "/" before the name and so misses an
// entry directly under the source path. It means "at every depth",
// which is what an unanchored pattern already does, so drop the
// prefix and match unanchored: as in gitignore, "**/x" and
// "/**/x" then also match a top-level "x".
if rest, found := strings.CutPrefix(pattern, "**/"); found {
pattern = rest
anchored = false
}
// Likewise "/**/" inside a pattern demands two slashes, so
// "a/**/*.log" misses "a/x.log". As in gitignore it stands for
// zero or more directories, so also compile the pattern with it
// made a single "/". The glob library's {a,b} is no help: with a
// "**" inside it in mid-pattern, it misses paths it should match.
for _, variant := range doubleStarVariants(pattern) {
// For patterns without path separators, we need to match them as components
// e.g., ".git" should match ".git" anywhere in the path
g, err := glob.Compile(variant, '/')
if err != nil {
log.Warn("Invalid exclude pattern, skipping", "pattern", p, "error", err)
break
}
for _, g := range globs {
compiled = append(compiled, compiledPattern{
pattern: g,
anchored: anchored,
@@ -2369,26 +2343,7 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
}
}
return compiled
}
// doubleStarVariants returns pattern once for every way of keeping each
// "/**/" in it or making it a single "/".
func doubleStarVariants(pattern string) []string {
const doubleStar = "/**/"
before, after, found := strings.Cut(pattern, doubleStar)
if !found {
return []string{pattern}
}
var variants []string
for _, rest := range doubleStarVariants(after) {
variants = append(variants, before+doubleStar+rest, before+"/"+rest)
}
return variants
return compiled, nil
}
// shouldExclude checks if a path should be excluded based on exclude patterns
+6 -6
View File
@@ -37,9 +37,9 @@ func uploadTestBlob(ctx context.Context, store storage.Storer) error {
}
// captureLog runs fn and returns what the logger wrote to stderr
// meanwhile. The logger writes to the os.Stderr it finds when it is
// initialized, so os.Stderr is pointed at a file first. Not
// parallel-safe: os.Stderr and the logger are process-global.
// meanwhile, at the --verbose level. The logger writes to the os.Stderr
// it finds when it is initialized, so os.Stderr is pointed at a file
// first. Not parallel-safe: os.Stderr and the logger are process-global.
func captureLog(t *testing.T, fn func()) string {
t.Helper()
@@ -53,7 +53,7 @@ func captureLog(t *testing.T, fn func()) string {
previous := os.Stderr
os.Stderr = stderrFile
log.Initialize(log.Config{})
log.Initialize(log.Config{Verbose: true})
fn()
@@ -107,10 +107,10 @@ func TestInterruptedUploadIsNotLoggedAsError(t *testing.T) {
}
}
// An upload that fails for any other reason is still logged as an error.
// An upload that fails for any other reason is still logged.
//
//nolint:paralleltest // captureLog replaces os.Stderr
func TestFailedUploadIsLoggedAsError(t *testing.T) {
func TestFailedUploadIsLogged(t *testing.T) {
inner, err := storage.NewFileStorer(t.TempDir())
if err != nil {
t.Fatal(err)
-198
View File
@@ -4,7 +4,6 @@ import (
"bytes"
"context"
"database/sql"
"encoding/json"
"io"
"strings"
"sync"
@@ -473,171 +472,6 @@ func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
assert.Contains(t, out, "Removed snapshot metadata from remote storage")
}
// TestRemoveSnapshot_RemoteOnlyByRemoteKey checks that a snapshot only
// on the destination store, on a host with no local index, is removed by
// the remote key that 'snapshot list' prints for such a snapshot, and by
// an unambiguous leading part of it — the same identifiers verify and
// restore accept.
func TestRemoveSnapshot_RemoteOnlyByRemoteKey(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
identifiers := map[string]string{
"full key": remoteKey,
"leading part": remoteKey[:8],
}
for name, identifier := range identifiers {
t.Run(name, func(t *testing.T) {
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
// No local index: a host that sees the snapshot only remotely.
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t)
result, err := tv.RemoveSnapshot(identifier,
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.True(t, result.RemoteRemoved)
assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
out := tv.Stdout.String()
assert.NotContains(t, out, "from local database")
assert.Contains(t, out, "Removed snapshot metadata from remote storage")
})
}
}
// TestRemoveSnapshot_LocalIndexByRemoteKey checks that a remote key (or
// its unambiguous leading part) that names a snapshot the local index
// holds removes that snapshot's local row and its remote metadata
// together. `snapshot list --json` prints a remote key for every
// snapshot, including local ones.
func TestRemoveSnapshot_LocalIndexByRemoteKey(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
identifiers := map[string]string{
"full key": remoteKey,
"leading part": remoteKey[:8],
}
for name, identifier := range identifiers {
t.Run(name, func(t *testing.T) {
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
// A backup wrote to this destination, so it carries the
// marker, and the local index holds the snapshot.
require.NoError(t, store.Put(context.Background(), markerFile,
bytes.NewReader(nil)))
tv := vaultik.NewForTesting(store)
repos := newLocalIndex(t, "snapshot-001")
tv.Repositories = repos
result, err := tv.RemoveSnapshot(identifier,
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.True(t, result.RemoteRemoved)
assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
row, err := repos.Snapshots.GetByID(
context.Background(), "snapshot-001")
require.NoError(t, err)
assert.Nil(t, row)
out := tv.Stdout.String()
assert.Contains(t, out,
"Removed snapshot 'snapshot-001' from local database")
assert.Contains(t, out, "Removed snapshot metadata from remote storage")
})
}
}
// TestRemoveSnapshot_DryRunByLeadingPart checks that a dry run by a
// leading part of a remote key names the snapshot that part resolved to:
// by its ID when the local index holds the snapshot, otherwise by its
// full remote key.
func TestRemoveSnapshot_DryRunByLeadingPart(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
for name, tc := range map[string]struct {
localIDs []string
shown string
}{
"in local index": {localIDs: []string{"snapshot-001"}, shown: "snapshot-001"},
"remote only": {localIDs: nil, shown: remoteKey},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
// A backup wrote to this destination, so it carries the marker.
require.NoError(t, store.Put(context.Background(), markerFile,
bytes.NewReader(nil)))
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t, tc.localIDs...)
_, err := tv.RemoveSnapshot(remoteKey[:8],
&vaultik.RemoveOptions{DryRun: true})
require.NoError(t, err)
assert.Contains(t, tv.Stdout.String(),
"Would remove snapshot: "+tc.shown+".")
assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
})
}
}
// TestRemoveSnapshot_JSONDryRunByLeadingPart checks that a JSON dry run
// by a leading part of a remote key prints the ID and the full remote key
// of the snapshot that part resolved to.
func TestRemoveSnapshot_JSONDryRunByLeadingPart(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
// A backup wrote to this destination, so it carries the marker.
require.NoError(t, store.Put(context.Background(), markerFile,
bytes.NewReader(nil)))
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t, "snapshot-001")
_, err := tv.RemoveSnapshot(remoteKey[:8],
&vaultik.RemoveOptions{DryRun: true, JSON: true})
require.NoError(t, err)
var printed map[string]any
require.NoError(t, json.Unmarshal(tv.Stdout.Bytes(), &printed))
assert.Equal(t, "snapshot-001", printed["snapshot_id"])
assert.Equal(t, remoteKey, printed["remote_key"])
assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
}
// TestRemoveSnapshot_NotInLocalIndexDeleteFails checks that removing a
// snapshot only the destination store holds fails when its metadata
// cannot be deleted there, under --force and under --json: nothing was
@@ -672,38 +506,6 @@ func TestRemoveSnapshot_NotInLocalIndexDeleteFails(t *testing.T) {
}
}
// TestRemoveSnapshot_LocalIndexByLeadingPartDeleteFails checks the
// warning when a snapshot the local index holds is removed by a leading
// part of its remote key and the delete on the destination store fails.
// The warning names the snapshot by its ID, not by the part given.
func TestRemoveSnapshot_LocalIndexByLeadingPartDeleteFails(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
// A backup wrote to this destination, so it carries the marker.
require.NoError(t, store.Put(context.Background(), markerFile,
bytes.NewReader(nil)))
faulty := faultstore.New(store)
faulty.OnDelete = func(string) bool { return true }
tv := vaultik.NewForTesting(faulty)
tv.Repositories = newLocalIndex(t, "snapshot-001")
result, err := tv.RemoveSnapshot(
snapshot.RemoteSnapshotKey("snapshot-001")[:8],
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.False(t, result.RemoteRemoved)
assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
assert.Contains(t, tv.Stdout.String(),
"Run 'vaultik snapshot remove snapshot-001' again")
}
func TestRemoveAllSnapshots_RequiresForce(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
+3 -3
View File
@@ -667,19 +667,19 @@ func (v *Vaultik) restoreProgressTick(
// handleRestoreFileError records a per-file restore failure: fatal unless
// --skip-errors is set, in which case the file is counted as failed and
// dropped from the plan.
// dropped from the plan. A fatal failure is not logged here: the returned
// error is printed as the command's ERROR: line.
func (v *Vaultik) handleRestoreFileError(
plan *restorePlan, opts *RestoreOptions, result *RestoreResult,
file *database.File, fileID types.FileID, err error,
) error {
log.Error("Failed to restore file", "path", file.Path, "error", err)
if !opts.SkipErrors {
return fmt.Errorf(
"restoring %s: %w (pass --skip-errors to continue past "+
"restore failures)", file.Path, err)
}
log.Error("Failed to restore file", "path", file.Path, "error", err)
v.UI.Errorf("Failed to restore %s: %v. Skipping (--skip-errors).",
v.UI.Path(file.Path.String()), err)
+28 -106
View File
@@ -1424,14 +1424,11 @@ type RemoveOptions struct {
LocalOnly bool // Skip remote cleanup; only touch the local index
}
// RemoveResult contains the result of a snapshot removal. For a single
// snapshot, SnapshotID is its ID, empty when the ID is not known, and
// RemoteKey is the full remote key of its metadata.
// RemoveResult contains the result of a snapshot removal
//
//nolint:tagliatelle // snake_case is the established JSON output format
type RemoveResult struct {
SnapshotID string `json:"snapshot_id,omitempty"`
RemoteKey string `json:"remote_key,omitempty"`
SnapshotsRemoved []string `json:"snapshots_removed,omitempty"`
RemoteRemoved bool `json:"remote_removed,omitempty"`
DryRun bool `json:"dry_run,omitempty"`
@@ -1461,35 +1458,25 @@ const snapshotRemoveCommandHint = "vaultik snapshot remove"
func (v *Vaultik) RemoveSnapshot(
snapshotID string, opts *RemoveOptions,
) (*RemoveResult, error) {
result := &RemoveResult{}
result := &RemoveResult{
SnapshotID: snapshotID,
}
err := v.checkStorageBinding()
if err != nil {
return result, err
}
inLocalIndex, resolvedID, remoteKey, err := v.findSnapshotToRemove(
snapshotID, opts.LocalOnly)
inLocalIndex, err := v.checkSnapshotExists(snapshotID, opts.LocalOnly)
if err != nil {
return result, err
}
result.SnapshotID = resolvedID
result.RemoteKey = remoteKey
// The dry run, the prompt and removeSnapshotRemote name the snapshot
// that snapshotID resolved to: by its ID, or by its full remote key
// when the ID is not known.
shown := resolvedID
if shown == "" {
shown = remoteKey
}
if opts.DryRun {
return v.removeSnapshotDryRun(result, shown, opts)
return v.removeSnapshotDryRun(result, snapshotID, opts)
}
if !v.confirmRemoveSnapshot(shown, opts) {
if !v.confirmRemoveSnapshot(snapshotID, opts) {
return result, ErrNotConfirmed
}
@@ -1502,16 +1489,16 @@ func (v *Vaultik) RemoveSnapshot(
}
if inLocalIndex {
log.Info("Removing snapshot from local database", "snapshot_id", resolvedID)
log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
err = v.deleteSnapshotFromLocalDB(resolvedID)
err = v.deleteSnapshotFromLocalDB(snapshotID)
if err != nil {
return result, fmt.Errorf("removing from local database: %w", err)
}
}
result.RemoteRemoved, err = v.removeSnapshotRemote(
shown, remoteKey, inLocalIndex, markerErr, opts)
snapshotID, inLocalIndex, markerErr, opts)
if err != nil {
return result, err
}
@@ -1530,7 +1517,7 @@ func (v *Vaultik) RemoveSnapshot(
if inLocalIndex {
v.UI.Infof("Removed snapshot '%s' from local database.",
v.UI.Snapshot(resolvedID))
v.UI.Snapshot(snapshotID))
}
if result.RemoteRemoved {
@@ -1543,52 +1530,6 @@ func (v *Vaultik) RemoveSnapshot(
return result, nil
}
// findSnapshotToRemove finds the snapshot that identifier names. It
// returns whether the local index holds that snapshot, its snapshot ID,
// and the remote key of its metadata on the destination store. The ID is
// known when identifier is a snapshot ID or the local index holds the
// snapshot; otherwise it is empty.
//
// A snapshot ID is looked up by checkSnapshotExists. Under localOnly the
// store is not consulted, so every identifier is looked up that way and
// a remote key is not found. Otherwise a remote key, or a leading part
// of one, is resolved by resolveSnapshotRemoteKey, as restore and verify
// do. That fails with errSnapshotKeyNotFound or errSnapshotKeyAmbiguous
// when the identifier matches no snapshot on the store, or more than
// one. The store is listed only after checkDestinationMarker passes, as
// in checkSnapshotExists. The local row is the one whose snapshot ID
// hashes to the resolved key, as in RemoveAllSnapshots, so removing by
// remote key removes that row too.
func (v *Vaultik) findSnapshotToRemove(
identifier string, localOnly bool,
) (bool, string, string, error) {
if localOnly || !isRemoteKeyOrPrefix(identifier) {
inLocalIndex, err := v.checkSnapshotExists(identifier, localOnly)
if err != nil {
return false, "", "", err
}
return inLocalIndex, identifier, snapshot.RemoteSnapshotKey(identifier), nil
}
err := v.checkDestinationMarker()
if err != nil {
return false, "", "", err
}
remoteKey, err := v.resolveSnapshotRemoteKey(identifier)
if err != nil {
return false, "", "", err
}
localID, err := v.localSnapshotIDForRemoteKey(remoteKey)
if err != nil {
return false, "", "", err
}
return localID != "", localID, remoteKey, nil
}
// checkSnapshotExists returns errSnapshotNotFound when the local index
// has no row for snapshotID and, unless localOnly is set, the
// destination store has no metadata for it either. inLocalIndex reports
@@ -1698,22 +1639,19 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
}
// removeSnapshotRemote strips the snapshot's metadata from the
// destination store. remoteKey is the resolved metadata directory name
// to delete, as returned by findSnapshotToRemove. shown names the
// snapshot in the log records and the warning: its ID, or its full
// remote key when the ID is not known. It returns true only when the
// delete finished without an error and removed at least one object;
// under LocalOnly it does nothing. markerErr is the result of
// checkDestinationMarker; when it is set, nothing is deleted and it is
// the failure. When the snapshot is only on the destination store, a
// failure is returned as an error, because that removal is all the
// command does. When the snapshot was in the local index, its removal
// there has already happened, so a failure is only a warning telling
// the user to run `vaultik snapshot remove` for the snapshot again once
// the destination store is reachable (`vaultik prune` never removes
// snapshot metadata that has its manifest).
// destination store. It returns true only when the delete finished
// without an error and removed at least one object; under LocalOnly it
// does nothing. markerErr is the result of checkDestinationMarker; when
// it is set, nothing is deleted and it is the failure. When the snapshot
// is only on the destination store, a failure is returned as an error,
// because that removal is all the command does. When the snapshot was in
// the local index, its removal there has already happened, so a failure
// is only a warning telling the user to run `vaultik snapshot remove`
// for the snapshot again once the destination store is reachable
// (`vaultik prune` never removes snapshot metadata that has its
// manifest).
func (v *Vaultik) removeSnapshotRemote(
shown, remoteKey string, inLocalIndex bool, markerErr error,
snapshotID string, inLocalIndex bool, markerErr error,
opts *RemoveOptions,
) (bool, error) {
if opts.LocalOnly {
@@ -1721,13 +1659,14 @@ func (v *Vaultik) removeSnapshotRemote(
}
log.Info("Removing snapshot metadata from remote storage",
"snapshot_id", shown)
"snapshot_id", snapshotID)
var deleted int
err := markerErr
if err == nil {
deleted, err = v.deleteRemoteSnapshotByKey(remoteKey)
deleted, err = v.deleteRemoteSnapshotByKey(
snapshot.RemoteSnapshotKey(snapshotID))
}
if err != nil && !inLocalIndex {
@@ -1739,14 +1678,14 @@ func (v *Vaultik) removeSnapshotRemote(
log.Warn("Could not remove snapshot metadata from remote storage; "+
"run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+
"again once the remote is reachable",
"snapshot_id", shown, "error", err)
"snapshot_id", snapshotID, "error", err)
// The UI writes to stdout, which under --json holds only the
// document; the log record above is the warning on stderr.
if v.UI != nil && !opts.JSON {
v.UI.Warningf("Could not remove snapshot metadata from remote: "+
"%v. Run '%s %s' again once the remote is reachable.",
err, snapshotRemoveCommandHint, shown)
err, snapshotRemoveCommandHint, snapshotID)
}
return false, nil
@@ -1841,23 +1780,6 @@ func (v *Vaultik) localSnapshotIDs() ([]string, error) {
return ids, nil
}
// localSnapshotIDForRemoteKey returns the human snapshot ID of the local
// index row whose RemoteSnapshotKey is key, or "" when no row has it.
func (v *Vaultik) localSnapshotIDForRemoteKey(key string) (string, error) {
ids, err := v.localSnapshotIDs()
if err != nil {
return "", fmt.Errorf("listing local snapshots: %w", err)
}
for _, id := range ids {
if snapshot.RemoteSnapshotKey(id) == key {
return id, nil
}
}
return "", nil
}
// listAllRemoteSnapshotKeys collects the hashed remote keys
// (subdirectories under metadata/) currently present in the
// destination store. Returns (nil, err) when the store cannot be