1 Commits
Author SHA1 Message Date
sneak f2138dafc8 Name the S3 keys when storage_url moves to s3:// first (closes #428)
check / check (push) Successful in 10m27s
On a config that loads, `config set storage_url s3://...` is refused
until `s3.access_key_id` and `s3.secret_access_key` are set. The refusal
now says to set those two keys first, then `storage_url`.

Model: opus-5-5
2026-11-11 05:21:31 +00:00
21 changed files with 114 additions and 755 deletions

No files matched your search

+1 -1
View File
@@ -337,7 +337,7 @@ func runVaultikApp(
}, },
Mode: mode, Mode: mode,
}, op, func(err error) { }, op, func(err error) {
log.Info(failMsg, "error", err) log.Error(failMsg, "error", err)
ReportErrorf("%s: %v", failMsg, err) ReportErrorf("%s: %v", failMsg, err)
}) })
} }
+1 -1
View File
@@ -268,7 +268,7 @@ func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
{configWithoutRecipients, "snapshots.home.paths", "/srv", {configWithoutRecipients, "snapshots.home.paths", "/srv",
"into []string"}, "into []string"},
{configWithoutRecipients, "chunk_size", "fast", {configWithoutRecipients, "chunk_size", "fast",
`chunk_size: "fast" is not a size`}, "invalid size format"},
// Values of the right type that fail validation. // Values of the right type that fail validation.
{configWithoutRecipients, "compression_level", "99", {configWithoutRecipients, "compression_level", "99",
"compression_level must be between 1 and 19"}, "compression_level must be between 1 and 19"},
-4
View File
@@ -85,10 +85,6 @@ 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
@@ -1,205 +0,0 @@
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 once, // that cannot be decoded, and checks that the failure is reported as the
// as the prune's and not as a failed snapshot creation. // 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,9 +83,6 @@ 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,
@@ -96,8 +93,7 @@ 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. The error is printed once. See // creation. See https://git.eeqj.de/sneak/vaultik/issues/388.
// 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) {
@@ -118,9 +114,6 @@ 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")
@@ -131,8 +124,7 @@ 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. The error is printed once. See // older snapshot. See https://git.eeqj.de/sneak/vaultik/issues/395.
// 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(
@@ -167,9 +159,6 @@ 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.Info("Failed to show info", "error", err) log.Error("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.Info("Prune operation failed", "error", err) log.Error("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.Info("Failed to get remote info", "error", err) log.Error("Failed to get remote info", "error", err)
ReportErrorf("Failed to get remote info: %v", err) ReportErrorf("Failed to get remote info: %v", err)
}) })
}, },
+4 -4
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.Info("Post-backup prune failed", "error", err) log.Error("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.Info("Backup completed with an error", "error", err) log.Error("Backup completed with an error", "error", err)
ReportErrorf("Backup completed, but %v", err) ReportErrorf("Backup completed, but %v", err)
return return
} }
log.Info("Snapshot creation failed", "error", err) log.Error("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.Info("Verification failed", "error", err) log.Error("Verification failed", "error", err)
ReportErrorf("Verification failed: %v", err) ReportErrorf("Verification failed: %v", err)
}) })
}, },
+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.Info("Restore operation failed", "error", err) log.Error("Restore operation failed", "error", err)
ReportErrorf("Restore failed: %v", err) ReportErrorf("Restore failed: %v", err)
}) })
} }
+10 -30
View File
@@ -295,8 +295,7 @@ func Load(path string) (*Config, error) {
} }
// Parse reads YAML config data as Load does, without validating it. It // Parse reads YAML config data as Load does, without validating it. It
// refuses a key that no setting has and a size setting whose value is not a // refuses a key that no setting has, applies default values for optional
// size, naming the setting. It applies default values for optional
// fields, fills in ${...} references using smartconfig, and expands ~ in // fields, fills in ${...} references using smartconfig, and expands ~ in
// paths. // paths.
func Parse(data []byte) (*Config, error) { func Parse(data []byte) (*Config, error) {
@@ -321,9 +320,8 @@ func Parse(data []byte) (*Config, error) {
return nil, fmt.Errorf("failed to load config file: %w", err) return nil, fmt.Errorf("failed to load config file: %w", err)
} }
// A ${...} reference can bring in keys that are not in the file, and // A ${...} reference can bring in keys that are not in the file.
// is the value of a size setting only once it is filled in. err = checkFilledKeys(sc.Data())
err = checkFilledConfig(sc.Data())
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -396,8 +394,6 @@ 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
@@ -418,18 +414,18 @@ func (c *Config) Validate() error {
} }
} }
err := c.validateSnapshots() if len(c.Snapshots) == 0 {
if err != nil { return errNoSnapshots
return err
} }
err = c.validateExcludes() for name, snap := range c.Snapshots {
if err != nil { if len(snap.Paths) == 0 {
return err return fmt.Errorf("%w: %q", errSnapshotNoPaths, name)
}
} }
// Validate storage configuration // Validate storage configuration
err = c.validateStorage() err := c.validateStorage()
if err != nil { if err != nil {
return err return err
} }
@@ -468,22 +464,6 @@ 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
-39
View File
@@ -1,39 +0,0 @@
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]: %w", setting, i, err)
}
}
return nil
}
-65
View File
@@ -1,65 +0,0 @@
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)
}
})
}
}
+5 -11
View File
@@ -129,11 +129,10 @@ func checkKeys(node *yaml.Node, typ reflect.Type, path string,
return nil return nil
} }
// checkFilledConfig checks data, the config after smartconfig has filled in // checkFilledKeys is checkKeys for data, the config after smartconfig has
// its ${...} references: its keys with checkKeys, then its size settings // filled in its ${...} references. A key that a reference brings in is not
// with checkSizes. A key that a reference brings in is not in the file, and // in the file, and the copy that Encode makes has no line numbers.
// the copy that Encode makes has no line numbers. func checkFilledKeys(data map[string]any) error {
func checkFilledConfig(data map[string]any) error {
var filled yaml.Node var filled yaml.Node
err := filled.Encode(data) err := filled.Encode(data)
@@ -141,13 +140,8 @@ func checkFilledConfig(data map[string]any) error {
return fmt.Errorf("failed to marshal config data: %w", err) return fmt.Errorf("failed to marshal config data: %w", err)
} }
err = checkKeys(&filled, reflect.TypeFor[Config](), "", return checkKeys(&filled, reflect.TypeFor[Config](), "",
map[*yaml.Node]bool{}, map[nodeAndType]bool{}) map[*yaml.Node]bool{}, map[nodeAndType]bool{})
if err != nil {
return err
}
return checkSizes(&filled, nil)
} }
// checkMappingKeys is checkKeys for a map. // checkMappingKeys is checkKeys for a map.
+11 -77
View File
@@ -4,20 +4,13 @@ import (
"errors" "errors"
"fmt" "fmt"
"math" "math"
"reflect"
"slices"
"strconv"
"strings"
"github.com/dustin/go-humanize" "github.com/dustin/go-humanize"
"gopkg.in/yaml.v3"
) )
var ( var (
errSizeType = errors.New("size must be a number or string") errSizeType = errors.New("size must be a number or string")
errSizeTooLarge = errors.New("size exceeds maximum supported value") errSizeTooLarge = errors.New("size exceeds maximum supported value")
errNotASize = errors.New(
"is not a size; use a number of bytes or a unit such as 10MB")
) )
// Size represents a byte size that can be specified in configuration files. // Size represents a byte size that can be specified in configuration files.
@@ -39,15 +32,6 @@ func (s *Size) UnmarshalYAML(unmarshal func(any) error) error {
return nil return nil
} }
// A number too large for an int64 decodes as a float64. Its text can be
// 1e+20, which is not a size string.
var floatVal float64
err = unmarshal(&floatVal)
if err == nil && floatVal > math.MaxInt64 {
return errSizeTooLarge
}
// Try to unmarshal as string // Try to unmarshal as string
var strVal string var strVal string
@@ -56,66 +40,17 @@ func (s *Size) UnmarshalYAML(unmarshal func(any) error) error {
return errSizeType return errSizeType
} }
size, err := ParseSize(strVal) // Parse the string using go-humanize
bytes, err := humanize.ParseBytes(strVal)
if err != nil { if err != nil {
return err return fmt.Errorf("invalid size format: %w", err)
} }
*s = size if bytes > math.MaxInt64 {
return fmt.Errorf("%w: %s", errSizeTooLarge, strVal)
return nil
}
// checkSizes returns an error naming the first size setting in node whose
// value is not a size. node is the config, or the part of it at the dotted
// keys, after smartconfig has filled in its ${...} references.
func checkSizes(node *yaml.Node, keys []string) error {
typ, err := SettingType(keys)
if err != nil {
// Unknown keys are refused before this check, so keys is in one of
// two places. The env section holds no settings. A value of the
// wrong shape, such as a map given for hostname or inside exclude,
// is reported later by the YAML decoder.
return nil //nolint:nilerr // not an error here
} }
if typ == reflect.TypeFor[Size]() { *s = Size(bytes)
var size Size
err = node.Decode(&size)
if err != nil {
return fmt.Errorf("%s: %w", strings.Join(keys, "."), err)
}
return nil
}
switch node.Kind {
case yaml.DocumentNode:
for _, child := range node.Content {
err = checkSizes(child, keys)
if err != nil {
return err
}
}
case yaml.MappingNode:
for i := 0; i+1 < len(node.Content); i += 2 {
err = checkSizes(node.Content[i+1],
slices.Concat(keys, []string{node.Content[i].Value}))
if err != nil {
return err
}
}
case yaml.SequenceNode:
for i, child := range node.Content {
err = checkSizes(child, slices.Concat(keys, []string{strconv.Itoa(i)}))
if err != nil {
return err
}
}
case yaml.ScalarNode, yaml.AliasNode:
// A scalar holds no settings, and the filled config has no aliases.
}
return nil return nil
} }
@@ -135,17 +70,16 @@ func (s Size) String() string {
return humanize.Bytes(uint64(s)) return humanize.Bytes(uint64(s))
} }
// ParseSize parses a size string, such as "10MB", into a Size value. An // ParseSize parses a size string into a Size value
// error quotes s.
func ParseSize(s string) (Size, error) { func ParseSize(s string) (Size, error) {
bytes, err := humanize.ParseBigBytes(s) bytes, err := humanize.ParseBytes(s)
if err != nil { if err != nil {
return 0, fmt.Errorf("%q %w", s, errNotASize) return 0, fmt.Errorf("invalid size format: %w", err)
} }
if !bytes.IsInt64() { if bytes > math.MaxInt64 {
return 0, fmt.Errorf("%w: %s", errSizeTooLarge, s) return 0, fmt.Errorf("%w: %s", errSizeTooLarge, s)
} }
return Size(bytes.Int64()), nil return Size(bytes), nil
} }
-120
View File
@@ -1,120 +0,0 @@
package config //nolint:testpackage // checks errNotASize and errSizeTooLarge
import (
"errors"
"os"
"path/filepath"
"strings"
"testing"
)
// TestParseRefusesValueThatIsNotASize checks that a size setting whose
// value is not a size fails the parse with an error naming the setting and
// quoting the value, also when a ${...} reference fills the value in.
func TestParseRefusesValueThatIsNotASize(t *testing.T) {
t.Parallel()
sizePath := filepath.Join(t.TempDir(), "size")
err := os.WriteFile(sizePath, []byte("lots"), 0o600)
if err != nil {
t.Fatalf("write size: %v", err)
}
tests := []struct {
name string
yaml string
want string
}{
{"chunk_size", keysTestConfig + "chunk_size: banana\n",
`chunk_size: "banana" is not a size; ` +
"use a number of bytes or a unit such as 10MB"},
{"blob_size_limit", keysTestConfig + "blob_size_limit: 10XB\n",
`blob_size_limit: "10XB" is not a size; ` +
"use a number of bytes or a unit such as 10MB"},
{"s3.part_size", strings.Replace(keysTestConfig, " region:",
" part_size: big\n region:", 1),
`s3.part_size: "big" is not a size; ` +
"use a number of bytes or a unit such as 10MB"},
{"from a reference",
keysTestConfig + "chunk_size: ${FILE:" + sizePath + "}\n",
`chunk_size: "lots" is not a size; ` +
"use a number of bytes or a unit such as 10MB"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
_, err := Parse([]byte(tt.yaml))
if !errors.Is(err, errNotASize) {
t.Fatalf("Parse() error = %v, want errNotASize", err)
}
if err.Error() != tt.want {
t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
}
})
}
}
// TestParseRefusesSizeTooLarge checks that a size setting whose value is
// too large for a size fails the parse with an error naming the setting.
// YAML reads the second value as a float.
func TestParseRefusesSizeTooLarge(t *testing.T) {
t.Parallel()
tests := []struct {
value string
want string
}{
{"100EB", "chunk_size: size exceeds maximum supported value: 100EB"},
{"99999999999999999999",
"chunk_size: size exceeds maximum supported value"},
}
for _, tt := range tests {
t.Run(tt.value, func(t *testing.T) {
t.Parallel()
_, err := Parse([]byte(keysTestConfig + "chunk_size: " + tt.value + "\n"))
if !errors.Is(err, errSizeTooLarge) {
t.Fatalf("Parse() error = %v, want errSizeTooLarge", err)
}
if err.Error() != tt.want {
t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
}
})
}
}
// TestParseAcceptsSizes checks that size settings given as a number of
// bytes, as a unit, and through a ${...} reference still parse.
func TestParseAcceptsSizes(t *testing.T) {
t.Parallel()
sizePath := filepath.Join(t.TempDir(), "size")
err := os.WriteFile(sizePath, []byte("20MB"), 0o600)
if err != nil {
t.Fatalf("write size: %v", err)
}
data := strings.Replace(keysTestConfig, " region:",
" part_size: 6000000\n region:", 1) +
"chunk_size: ${FILE:" + sizePath + "}\n" +
"blob_size_limit: 1GB\n"
cfg, err := Parse([]byte(data))
if err != nil {
t.Fatalf("Parse() unexpected error: %v", err)
}
if cfg.ChunkSize != 20_000_000 || cfg.BlobSizeLimit != 1_000_000_000 ||
cfg.S3.PartSize != 6_000_000 {
t.Errorf("chunk_size, blob_size_limit, s3.part_size = %d, %d, %d, "+
"want 20000000, 1000000000, 6000000",
cfg.ChunkSize, cfg.BlobSizeLimit, cfg.S3.PartSize)
}
}
-77
View File
@@ -1,77 +0,0 @@
// 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("%q %w: %w",
pattern, 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
}
-73
View File
@@ -1,73 +0,0 @@
package exclude_test
import (
"slices"
"strings"
"testing"
"github.com/gobwas/glob"
"sneak.berlin/go/vaultik/internal/exclude"
)
// TestCompile checks the error, the anchoring and the globs that Compile
// returns for one exclude pattern.
func TestCompile(t *testing.T) {
t.Parallel()
tests := []struct {
name string
pattern string
wantErr string
wantAnchored bool
// wantMatches lists paths that one of the globs must match. When
// it is empty, Compile must return no globs.
wantMatches []string
}{
{name: "does not compile", pattern: "build[",
wantErr: `"build[" is not a valid exclude pattern`},
{name: "empty", pattern: ""},
{name: "leading slash anchors", pattern: "/build",
wantAnchored: true, wantMatches: []string{"build"}},
{name: "double star after the slash clears anchoring",
pattern: "/**/build", wantMatches: []string{"build"}},
{name: "double star in the middle", pattern: "a/**/*.log",
wantMatches: []string{"a/x.log", "a/b/x.log"}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
globs, anchored, err := exclude.Compile(tt.pattern)
if tt.wantErr == "" && err != nil {
t.Fatalf("Compile(%q) error = %v, want none", tt.pattern, err)
}
if tt.wantErr != "" &&
(err == nil || !strings.HasPrefix(err.Error(), tt.wantErr)) {
t.Fatalf("Compile(%q) error = %v, want one starting with %q",
tt.pattern, err, tt.wantErr)
}
if anchored != tt.wantAnchored {
t.Errorf("Compile(%q) anchored = %t, want %t",
tt.pattern, anchored, tt.wantAnchored)
}
if len(tt.wantMatches) == 0 && len(globs) != 0 {
t.Errorf("Compile(%q) returned %d globs, want none",
tt.pattern, len(globs))
}
for _, path := range tt.wantMatches {
matched := slices.ContainsFunc(globs, func(g glob.Glob) bool {
return g.Match(path)
})
if !matched {
t.Errorf("Compile(%q) globs do not match %q",
tt.pattern, path)
}
}
})
}
}
+65 -20
View File
@@ -20,7 +20,6 @@ import (
"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/database" "sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/exclude"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage" "sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/types" "sneak.berlin/go/vaultik/internal/types"
@@ -222,12 +221,8 @@ func NewScanner(cfg ScannerConfig) *Scanner {
progress = NewProgressReporter() progress = NewProgressReporter()
} }
compiledExclude, err := compileExcludePatterns(cfg.Exclude) // Compile exclude patterns
if err != nil { compiledExclude := compileExcludePatterns(cfg.Exclude)
log.Error("Failed to compile exclude patterns", "error", err)
return nil
}
uiw := cfg.UI uiw := cfg.UI
if uiw == nil { if uiw == nil {
@@ -1880,12 +1875,10 @@ 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 {
// The record is at info level: the returned error ends the backup // Only an interrupt cancels ctx, and the user is told about it,
// and is printed as its ERROR: line. Only an interrupt cancels // so an upload it stopped is not logged as a storage failure.
// 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.Info("Failed to upload blob", "hash", finishedBlob.Hash, "error", err) log.Error("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)
@@ -2323,18 +2316,51 @@ 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
// Config.Validate has already refused a pattern that does not compile. func compileExcludePatterns(patterns []string) []compiledPattern {
func compileExcludePatterns(patterns []string) ([]compiledPattern, error) {
var compiled []compiledPattern var compiled []compiledPattern
for _, p := range patterns { for _, p := range patterns {
globs, anchored, err := exclude.Compile(p) if p == "" {
if err != nil { continue
return nil, err
} }
for _, g := range globs { // 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
}
compiled = append(compiled, compiledPattern{ compiled = append(compiled, compiledPattern{
pattern: g, pattern: g,
anchored: anchored, anchored: anchored,
@@ -2343,7 +2369,26 @@ func compileExcludePatterns(patterns []string) ([]compiledPattern, error) {
} }
} }
return compiled, nil 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
} }
// 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, at the --verbose level. The logger writes to the os.Stderr // meanwhile. The logger writes to the os.Stderr it finds when it is
// it finds when it is initialized, so os.Stderr is pointed at a file // initialized, so os.Stderr is pointed at a file first. Not
// first. Not parallel-safe: os.Stderr and the logger are process-global. // 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{Verbose: true}) log.Initialize(log.Config{})
fn() fn()
@@ -107,10 +107,10 @@ func TestInterruptedUploadIsNotLoggedAsError(t *testing.T) {
} }
} }
// An upload that fails for any other reason is still logged. // An upload that fails for any other reason is still logged as an error.
// //
//nolint:paralleltest // captureLog replaces os.Stderr //nolint:paralleltest // captureLog replaces os.Stderr
func TestFailedUploadIsLogged(t *testing.T) { func TestFailedUploadIsLoggedAsError(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)
+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. A fatal failure is not logged here: the returned // dropped from the plan.
// 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)