Author SHA1 Message Date
clawbot 5e33284ef5 Refuse an exclude pattern that does not compile at config load (closes #455)
check / check (push) Successful in 11m26s
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 config.CompileExcludePattern, which Validate and the scanner both call. 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 08:40:47 +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
18 changed files with 487 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 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`. 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 - `--local-only`: Skip remote cleanup; only touch the local index
- `--dry-run`: Show what would be deleted without deleting - `--dry-run`: Show what would be deleted without deleting
- `--force`: Skip confirmation prompt - `--force`: Skip confirmation prompt
+1 -1
View File
@@ -337,7 +337,7 @@ func runVaultikApp(
}, },
Mode: mode, Mode: mode,
}, op, func(err error) { }, op, func(err error) {
log.Error(failMsg, "error", err) log.Info(failMsg, "error", err)
ReportErrorf("%s: %v", failMsg, 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 // 🛑 ERROR: format. Use it from goroutine error paths (where returning
// an error to cobra isn't an option) and anywhere else a CLI command // an error to cobra isn't an option) and anywhere else a CLI command
// must surface a failure outside the normal RunE return path. // 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) { func ReportErrorf(format string, args ...any) {
ui.New(os.Stderr).Errorf(format, args...) 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 // TestEntrySnapshotCreateReportsPostBackupPruneFailure makes the
// post-backup prune fail after a good backup, by planting a manifest // 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 // that cannot be decoded, and checks that the failure is reported once,
// prune's and not as a failed snapshot creation. // as the prune's and not as a failed snapshot creation.
// //
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals //nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotCreateReportsPostBackupPruneFailure(t *testing.T) { 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.Contains(t, stderr, "Backup completed, but post-backup prune failed")
assert.NotContains(t, stderr, "Snapshot creation 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")) snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
require.NoError(t, err) require.NoError(t, err)
assert.Len(t, snapshots, 2, 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 // empties the source directory, as a volume that is not mounted leaves
// its mount point, and runs `snapshot create --prune`. The command exits // its mount point, and runs `snapshot create --prune`. The command exits
// non-zero and reports the backup as completed, not as a failed snapshot // 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 //nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) { func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) {
@@ -114,6 +118,9 @@ func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) {
"Backup completed, but 1 source path(s) were empty") "Backup completed, but 1 source path(s) were empty")
assert.NotContains(t, stderr, "Snapshot creation 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")) snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
require.NoError(t, err) require.NoError(t, err)
assert.Len(t, snapshots, 2, "the older snapshot is kept") 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 // directory as a mount point, and runs `snapshot create --prune`, which
// does not find it as one. The command exits non-zero, reports the // 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 // 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 //nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotCreatePruneExitsNonZeroWhenMountPointIsMissing( func TestEntrySnapshotCreatePruneExitsNonZeroWhenMountPointIsMissing(
@@ -159,6 +167,9 @@ func TestEntrySnapshotCreatePruneExitsNonZeroWhenMountPointIsMissing(
"an earlier snapshot recorded were not found") "an earlier snapshot recorded were not found")
assert.NotContains(t, stderr, "Snapshot creation 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")) snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
require.NoError(t, err) require.NoError(t, err)
assert.Len(t, snapshots, 2, "the older snapshot is kept") 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 { }, func(v *vaultik.Vaultik) error {
return v.ShowInfo() return v.ShowInfo()
}, func(err error) { }, 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) ReportErrorf("Failed to show info: %v", err)
}) })
}, },
+1 -1
View File
@@ -60,7 +60,7 @@ referenced.`,
}, func(v *vaultik.Vaultik) error { }, func(v *vaultik.Vaultik) error {
return v.Prune(opts) return v.Prune(opts)
}, func(err error) { }, func(err error) {
log.Error("Prune operation failed", "error", err) log.Info("Prune operation failed", "error", err)
ReportErrorf("Prune failed: %v", err) ReportErrorf("Prune failed: %v", err)
}) })
}, },
+1 -1
View File
@@ -98,7 +98,7 @@ func newRemoteInfoCommand() *cobra.Command {
}, func(v *vaultik.Vaultik) error { }, func(v *vaultik.Vaultik) error {
return v.RemoteInfo(jsonOutput) return v.RemoteInfo(jsonOutput)
}, func(err error) { }, 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) 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) return v.CreateSnapshot(opts)
}, func(err error) { }, func(err error) {
if errors.Is(err, vaultik.ErrPostBackupPruneFailed) { 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) ReportErrorf("Backup completed, but %v", err)
return return
@@ -118,13 +118,13 @@ on Linux), then /etc/vaultik/config.yml.`,
if errors.Is(err, vaultik.ErrFilesUnreadable) || if errors.Is(err, vaultik.ErrFilesUnreadable) ||
errors.Is(err, vaultik.ErrSourcePathEmpty) || errors.Is(err, vaultik.ErrSourcePathEmpty) ||
errors.Is(err, vaultik.ErrVolumeNotMounted) { 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) ReportErrorf("Backup completed, but %v", err)
return return
} }
log.Error("Snapshot creation failed", "error", err) log.Info("Snapshot creation failed", "error", err)
ReportErrorf("Snapshot creation failed: %v", err) ReportErrorf("Snapshot creation failed: %v", err)
}) })
}, },
@@ -258,7 +258,7 @@ func newSnapshotVerifyCommand() *cobra.Command {
return return
} }
log.Error("Verification failed", "error", err) log.Info("Verification failed", "error", err)
ReportErrorf("Verification failed: %v", 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 '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. 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 An ID that names no snapshot in the local index or on the destination
store is an error. 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, SkipErrors: rootFlags.SkipErrors,
}) })
}, func(err error) { }, func(err error) {
log.Error("Restore operation failed", "error", err) log.Info("Restore operation failed", "error", err)
ReportErrorf("Restore failed: %v", 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 // never the value. An empty list is accepted, because only snapshot
// create needs a recipient and it checks for one itself // create needs a recipient and it checks for one itself
// - At least one snapshot must be configured with at least one path // - 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) // - Storage must be configured (either storage_url or s3.* fields)
// - The index path must be absolute: a relative one would resolve against // - The index path must be absolute: a relative one would resolve against
// the working directory, so each directory would get its own index // 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 { err := c.validateSnapshots()
return errNoSnapshots if err != nil {
return err
} }
for name, snap := range c.Snapshots { err = c.validateExcludes()
if len(snap.Paths) == 0 { if err != nil {
return fmt.Errorf("%w: %q", errSnapshotNoPaths, name) return err
}
} }
// Validate storage configuration // Validate storage configuration
err := c.validateStorage() err = c.validateStorage()
if err != nil { if err != nil {
return err return err
} }
@@ -466,6 +468,22 @@ func (c *Config) Validate() error {
return nil 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 // validateAgeRecipient parses one age_recipients entry with the age library
// and returns a value-free error on failure. A recipient string can be // 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 // sensitive (an operator may paste a secret key by mistake), so neither the
+107
View File
@@ -0,0 +1,107 @@
package config
import (
"errors"
"fmt"
"strings"
"github.com/gobwas/glob"
)
var errBadExcludePattern = errors.New("is not a valid exclude pattern")
// CompileExcludePattern 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 CompileExcludePattern(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
}
// 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 := CompileExcludePattern(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)
}
})
}
}
+20 -65
View File
@@ -19,6 +19,7 @@ import (
"github.com/spf13/afero" "github.com/spf13/afero"
"sneak.berlin/go/vaultik/internal/blob" "sneak.berlin/go/vaultik/internal/blob"
"sneak.berlin/go/vaultik/internal/chunker" "sneak.berlin/go/vaultik/internal/chunker"
"sneak.berlin/go/vaultik/internal/config"
"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/storage" "sneak.berlin/go/vaultik/internal/storage"
@@ -221,8 +222,12 @@ func NewScanner(cfg ScannerConfig) *Scanner {
progress = NewProgressReporter() progress = NewProgressReporter()
} }
// Compile exclude patterns compiledExclude, err := compileExcludePatterns(cfg.Exclude)
compiledExclude := compileExcludePatterns(cfg.Exclude) if err != nil {
log.Error("Failed to compile exclude patterns", "error", err)
return nil
}
uiw := cfg.UI uiw := cfg.UI
if uiw == nil { if uiw == nil {
@@ -1875,10 +1880,12 @@ func (s *Scanner) uploadBlobIfNeeded(
err = s.storage.PutWithProgress(ctx, blobPath, blobWithReader.Reader, err = s.storage.PutWithProgress(ctx, blobPath, blobWithReader.Reader,
finishedBlob.Compressed, progressCallback) finishedBlob.Compressed, progressCallback)
if err != nil { if err != nil {
// Only an interrupt cancels ctx, and the user is told about it, // The record is at info level: the returned error ends the backup
// so an upload it stopped is not logged as a storage failure. // 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 { 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) 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) "with --skip-errors to continue past unreadable files)", path, err)
} }
// compileExcludePatterns compiles the exclude patterns into glob matchers // compileExcludePatterns compiles the exclude patterns into glob matchers.
func compileExcludePatterns(patterns []string) []compiledPattern { // Config.Validate has already refused a pattern that does not compile.
func compileExcludePatterns(patterns []string) ([]compiledPattern, error) {
var compiled []compiledPattern var compiled []compiledPattern
for _, p := range patterns { for _, p := range patterns {
if p == "" { globs, anchored, err := config.CompileExcludePattern(p)
continue if err != nil {
return nil, fmt.Errorf("%q %w", p, err)
} }
// Check if pattern is anchored (starts with /) for _, g := range globs {
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
}
compiled = append(compiled, compiledPattern{ compiled = append(compiled, compiledPattern{
pattern: g, pattern: g,
anchored: anchored, anchored: anchored,
@@ -2369,26 +2343,7 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
} }
} }
return compiled return compiled, 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
} }
// shouldExclude checks if a path should be excluded based on exclude patterns // 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 // captureLog runs fn and returns what the logger wrote to stderr
// meanwhile. The logger writes to the os.Stderr it finds when it is // meanwhile, at the --verbose level. The logger writes to the os.Stderr
// initialized, so os.Stderr is pointed at a file first. Not // it finds when it is initialized, so os.Stderr is pointed at a file
// parallel-safe: os.Stderr and the logger are process-global. // first. Not parallel-safe: os.Stderr and the logger are process-global.
func captureLog(t *testing.T, fn func()) string { func captureLog(t *testing.T, fn func()) string {
t.Helper() t.Helper()
@@ -53,7 +53,7 @@ func captureLog(t *testing.T, fn func()) string {
previous := os.Stderr previous := os.Stderr
os.Stderr = stderrFile os.Stderr = stderrFile
log.Initialize(log.Config{}) log.Initialize(log.Config{Verbose: true})
fn() 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 //nolint:paralleltest // captureLog replaces os.Stderr
func TestFailedUploadIsLoggedAsError(t *testing.T) { func TestFailedUploadIsLogged(t *testing.T) {
inner, err := storage.NewFileStorer(t.TempDir()) inner, err := storage.NewFileStorer(t.TempDir())
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
-198
View File
@@ -4,7 +4,6 @@ import (
"bytes" "bytes"
"context" "context"
"database/sql" "database/sql"
"encoding/json"
"io" "io"
"strings" "strings"
"sync" "sync"
@@ -473,171 +472,6 @@ func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
assert.Contains(t, out, "Removed snapshot metadata from remote storage") 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 // TestRemoveSnapshot_NotInLocalIndexDeleteFails checks that removing a
// snapshot only the destination store holds fails when its metadata // snapshot only the destination store holds fails when its metadata
// cannot be deleted there, under --force and under --json: nothing was // 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) { func TestRemoveAllSnapshots_RequiresForce(t *testing.T) {
log.Initialize(log.Config{}) log.Initialize(log.Config{})
t.Parallel() t.Parallel()
+3 -3
View File
@@ -667,19 +667,19 @@ func (v *Vaultik) restoreProgressTick(
// handleRestoreFileError records a per-file restore failure: fatal unless // handleRestoreFileError records a per-file restore failure: fatal unless
// --skip-errors is set, in which case the file is counted as failed and // --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( func (v *Vaultik) handleRestoreFileError(
plan *restorePlan, opts *RestoreOptions, result *RestoreResult, plan *restorePlan, opts *RestoreOptions, result *RestoreResult,
file *database.File, fileID types.FileID, err error, file *database.File, fileID types.FileID, err error,
) error { ) error {
log.Error("Failed to restore file", "path", file.Path, "error", err)
if !opts.SkipErrors { if !opts.SkipErrors {
return fmt.Errorf( return fmt.Errorf(
"restoring %s: %w (pass --skip-errors to continue past "+ "restoring %s: %w (pass --skip-errors to continue past "+
"restore failures)", file.Path, err) "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.Errorf("Failed to restore %s: %v. Skipping (--skip-errors).",
v.UI.Path(file.Path.String()), err) 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 LocalOnly bool // Skip remote cleanup; only touch the local index
} }
// RemoveResult contains the result of a snapshot removal. For a single // RemoveResult contains the result of a snapshot removal
// snapshot, SnapshotID is its ID, empty when the ID is not known, and
// RemoteKey is the full remote key of its metadata.
// //
//nolint:tagliatelle // snake_case is the established JSON output format //nolint:tagliatelle // snake_case is the established JSON output format
type RemoveResult struct { type RemoveResult struct {
SnapshotID string `json:"snapshot_id,omitempty"` SnapshotID string `json:"snapshot_id,omitempty"`
RemoteKey string `json:"remote_key,omitempty"`
SnapshotsRemoved []string `json:"snapshots_removed,omitempty"` SnapshotsRemoved []string `json:"snapshots_removed,omitempty"`
RemoteRemoved bool `json:"remote_removed,omitempty"` RemoteRemoved bool `json:"remote_removed,omitempty"`
DryRun bool `json:"dry_run,omitempty"` DryRun bool `json:"dry_run,omitempty"`
@@ -1461,35 +1458,25 @@ const snapshotRemoveCommandHint = "vaultik snapshot remove"
func (v *Vaultik) RemoveSnapshot( func (v *Vaultik) RemoveSnapshot(
snapshotID string, opts *RemoveOptions, snapshotID string, opts *RemoveOptions,
) (*RemoveResult, error) { ) (*RemoveResult, error) {
result := &RemoveResult{} result := &RemoveResult{
SnapshotID: snapshotID,
}
err := v.checkStorageBinding() err := v.checkStorageBinding()
if err != nil { if err != nil {
return result, err return result, err
} }
inLocalIndex, resolvedID, remoteKey, err := v.findSnapshotToRemove( inLocalIndex, err := v.checkSnapshotExists(snapshotID, opts.LocalOnly)
snapshotID, opts.LocalOnly)
if err != nil { if err != nil {
return result, err 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 { 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 return result, ErrNotConfirmed
} }
@@ -1502,16 +1489,16 @@ func (v *Vaultik) RemoveSnapshot(
} }
if inLocalIndex { 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 { if err != nil {
return result, fmt.Errorf("removing from local database: %w", err) return result, fmt.Errorf("removing from local database: %w", err)
} }
} }
result.RemoteRemoved, err = v.removeSnapshotRemote( result.RemoteRemoved, err = v.removeSnapshotRemote(
shown, remoteKey, inLocalIndex, markerErr, opts) snapshotID, inLocalIndex, markerErr, opts)
if err != nil { if err != nil {
return result, err return result, err
} }
@@ -1530,7 +1517,7 @@ func (v *Vaultik) RemoveSnapshot(
if inLocalIndex { if inLocalIndex {
v.UI.Infof("Removed snapshot '%s' from local database.", v.UI.Infof("Removed snapshot '%s' from local database.",
v.UI.Snapshot(resolvedID)) v.UI.Snapshot(snapshotID))
} }
if result.RemoteRemoved { if result.RemoteRemoved {
@@ -1543,52 +1530,6 @@ func (v *Vaultik) RemoveSnapshot(
return result, nil 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 // checkSnapshotExists returns errSnapshotNotFound when the local index
// has no row for snapshotID and, unless localOnly is set, the // has no row for snapshotID and, unless localOnly is set, the
// destination store has no metadata for it either. inLocalIndex reports // 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 // removeSnapshotRemote strips the snapshot's metadata from the
// destination store. remoteKey is the resolved metadata directory name // destination store. It returns true only when the delete finished
// to delete, as returned by findSnapshotToRemove. shown names the // without an error and removed at least one object; under LocalOnly it
// snapshot in the log records and the warning: its ID, or its full // does nothing. markerErr is the result of checkDestinationMarker; when
// remote key when the ID is not known. It returns true only when the // it is set, nothing is deleted and it is the failure. When the snapshot
// delete finished without an error and removed at least one object; // is only on the destination store, a failure is returned as an error,
// under LocalOnly it does nothing. markerErr is the result of // because that removal is all the command does. When the snapshot was in
// checkDestinationMarker; when it is set, nothing is deleted and it is // the local index, its removal there has already happened, so a failure
// the failure. When the snapshot is only on the destination store, a // is only a warning telling the user to run `vaultik snapshot remove`
// failure is returned as an error, because that removal is all the // for the snapshot again once the destination store is reachable
// command does. When the snapshot was in the local index, its removal // (`vaultik prune` never removes snapshot metadata that has its
// there has already happened, so a failure is only a warning telling // manifest).
// 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( func (v *Vaultik) removeSnapshotRemote(
shown, remoteKey string, inLocalIndex bool, markerErr error, snapshotID string, inLocalIndex bool, markerErr error,
opts *RemoveOptions, opts *RemoveOptions,
) (bool, error) { ) (bool, error) {
if opts.LocalOnly { if opts.LocalOnly {
@@ -1721,13 +1659,14 @@ func (v *Vaultik) removeSnapshotRemote(
} }
log.Info("Removing snapshot metadata from remote storage", log.Info("Removing snapshot metadata from remote storage",
"snapshot_id", shown) "snapshot_id", snapshotID)
var deleted int var deleted int
err := markerErr err := markerErr
if err == nil { if err == nil {
deleted, err = v.deleteRemoteSnapshotByKey(remoteKey) deleted, err = v.deleteRemoteSnapshotByKey(
snapshot.RemoteSnapshotKey(snapshotID))
} }
if err != nil && !inLocalIndex { if err != nil && !inLocalIndex {
@@ -1739,14 +1678,14 @@ func (v *Vaultik) removeSnapshotRemote(
log.Warn("Could not remove snapshot metadata from remote storage; "+ log.Warn("Could not remove snapshot metadata from remote storage; "+
"run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+ "run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+
"again once the remote is reachable", "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 // The UI writes to stdout, which under --json holds only the
// document; the log record above is the warning on stderr. // document; the log record above is the warning on stderr.
if v.UI != nil && !opts.JSON { if v.UI != nil && !opts.JSON {
v.UI.Warningf("Could not remove snapshot metadata from remote: "+ v.UI.Warningf("Could not remove snapshot metadata from remote: "+
"%v. Run '%s %s' again once the remote is reachable.", "%v. Run '%s %s' again once the remote is reachable.",
err, snapshotRemoveCommandHint, shown) err, snapshotRemoveCommandHint, snapshotID)
} }
return false, nil return false, nil
@@ -1841,23 +1780,6 @@ func (v *Vaultik) localSnapshotIDs() ([]string, error) {
return ids, nil 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 // listAllRemoteSnapshotKeys collects the hashed remote keys
// (subdirectories under metadata/) currently present in the // (subdirectories under metadata/) currently present in the
// destination store. Returns (nil, err) when the store cannot be // destination store. Returns (nil, err) when the store cannot be