Compare commits

..
Author SHA1 Message Date
sneak 661adafd93 Make config set create a list for an index under a missing key (closes #308)
check / check (push) Waiting to run
`config set age_recipients.0 age1...` on a file without `age_recipients`
wrote a map keyed `0`, after which no command could load the config. A
missing key is now created as a list when the next path element is a
number.

`config set` checks its result with the config loader before writing and
refuses a failure the change brings in: a config that loaded must still
load, and one still being filled in (failing validation until complete)
must still decode into the config type. The new `config.Parse`, split out
of `config.Load`, decodes config data, filling in `${...}` references,
without validating.

Rule suppressed: gosec G304 where `config.Load` and the new tests read a
file.

Model: opus-5-5
2026-10-13 15:06:20 +00:00
clawbot e48bafccef Restore the other files when a chunk is in no blob under --skip-errors (closes #326)
check / check (push) Waiting to run
Restore planning stopped at the first file needing a chunk that no blob
holds, so --skip-errors never saw that file and nothing was restored.
The planner now leaves such a file out of the plan and returns it with
the reason, and each one goes through handleRestoreFileError like any
other failed file: skipped and counted under --skip-errors, fatal
without it. The fatal error now suggests --skip-errors, as other restore
failures do.

The skip-errors tests now share one backup helper; the new case removes
a blob from the snapshot metadata and exports it again.

Model: opus-5-5
2026-10-13 16:53:05 +02:00
12 changed files with 473 additions and 395 deletions
+10 -2
View File
@@ -271,8 +271,16 @@ existing file. Created with mode `0600` since it will contain credentials.
**`config set`**: Set a scalar config value by dotted YAML path
(e.g. `vaultik config set compression_level 9`,
`vaultik config set storage_url "file:///mnt/backups"`). Comments and
formatting in the file are preserved; intermediate maps are created as
needed.
formatting in the file are preserved. Missing keys along the path are
created: as a list when the next path element is a number (so
`vaultik config set age_recipients.0 age1...` works on a file without
`age_recipients`), otherwise as a map. If the config loads, a change
after which it would not, such as `vaultik config set compression_level 99`,
is refused and the file is left unchanged. A config that does not load
yet, such as the one `config init` writes before `storage_url` is set,
can be filled in one setting at a time: only a change that gives a
setting the wrong type, such as a single value where a list belongs or a
word where a number belongs, is refused.
**`snapshot create`**: Perform incremental backup of configured snapshots.
* Optional snapshot names argument to create specific snapshots (default: all)
+74 -24
View File
@@ -44,6 +44,9 @@ var (
errNeedNumericIndex = errors.New("key is a list; use a numeric index")
errIndexOutOfRange = errors.New("index out of range")
errNotMapOrList = errors.New("key is not a map or list")
errWouldNotLoad = errors.New(
"vaultik would not load the config after this change, " +
"so the file was not changed")
)
const defaultConfigTemplate = `# vaultik configuration
@@ -391,8 +394,15 @@ func newConfigSetCommand() *cobra.Command {
Use: "set <key> <value>",
Short: "Set a config value by dotted path (e.g. compression_level 5)",
Long: `Sets a scalar config value addressed by dotted YAML path and writes
the file back, preserving comments and formatting. Intermediate maps
are created as needed.
the file back, preserving comments and formatting. Missing keys along
the path are created: as a list when the next path element is a
number, otherwise as a map.
If the config loads, a change after which it would not, such as
compression_level 99, is refused and the file is left unchanged. A
config that does not load yet, such as the one config init writes
before storage_url is set, can be filled in one setting at a time:
only a change that gives a setting the wrong type is refused.
Examples:
vaultik config set storage_url "file:///mnt/backups"
@@ -422,6 +432,11 @@ func writeConfigSet(out *ui.Writer, path, key, value string) error {
return err
}
oldData, err := marshalConfigYAML(root)
if err != nil {
return fmt.Errorf("marshaling config: %w", err)
}
err = yamlPathSet(root, strings.Split(key, "."), value)
if err != nil {
return err
@@ -432,6 +447,11 @@ func writeConfigSet(out *ui.Writer, path, key, value string) error {
return fmt.Errorf("marshaling config: %w", err)
}
err = checkConfigChange(oldData, data)
if err != nil {
return fmt.Errorf("%w: %w", errWouldNotLoad, err)
}
err = os.WriteFile(path, data, configFileMode)
if err != nil {
return fmt.Errorf("writing config file: %w", err)
@@ -453,6 +473,28 @@ func writeConfigSet(out *ui.Writer, path, key, value string) error {
return nil
}
// checkConfigChange returns the reason vaultik would not load newData, the
// config after a change, if the change brings that failure in. If oldData,
// the config before it, decodes into the config type, the new one must
// too; if the old one also passes validation, the new one must pass it. A
// config still being filled in fails validation until it is complete, so
// only decoding is checked for it.
func checkConfigChange(oldData, newData []byte) error {
oldCfg, oldErr := config.Parse(oldData)
if oldErr == nil {
newCfg, err := config.Parse(newData)
if err != nil {
return err
}
if oldCfg.Validate() == nil {
return newCfg.Validate()
}
}
return nil
}
// marshalConfigYAML renders a config document tree with 2-space indentation,
// matching defaultConfigTemplate. yaml.Marshal defaults to 4 spaces, which
// would reindent the whole file on the first `config set` despite the promise
@@ -559,8 +601,8 @@ func yamlPathGet(root *yaml.Node, keys []string) (*yaml.Node, error) {
return node, nil
}
// yamlPathSet navigates a dotted key path, creating intermediate maps as
// needed, and sets the final key to the given scalar value. Numeric path
// yamlPathSet navigates a dotted key path, creating missing keys as needed,
// and sets the final key to the given scalar value. Numeric path
// components index into sequences; an index equal to the sequence length
// appends a new element (e.g. "age_recipients.1" on a 1-element list).
func yamlPathSet(root *yaml.Node, keys []string, value string) error {
@@ -573,12 +615,12 @@ func yamlPathSet(root *yaml.Node, keys []string, value string) error {
node = node.Content[0]
}
for i, key := range keys {
for i := range keys {
last := i == len(keys)-1
switch node.Kind {
case yaml.MappingNode:
node = yamlSetInMapping(node, key, value, last)
node = yamlSetInMapping(node, keys, i, value, last)
case yaml.SequenceNode:
next, err := yamlSetInSequence(node, keys, i, value, last)
if err != nil {
@@ -647,14 +689,16 @@ func yamlField(typ reflect.Type, key string) (reflect.StructField, bool) {
return reflect.StructField{}, false
}
// yamlSetInMapping resolves (creating if needed) the value node for key
// within a mapping node, setting it to value when it is the final path
// element, and returns the node to descend into.
func yamlSetInMapping(node *yaml.Node, key, value string, last bool) *yaml.Node {
// yamlSetInMapping resolves (creating if needed) the value node for the
// path element keys[i] within a mapping node, setting it to value when it
// is the final path element, and returns the node to descend into.
func yamlSetInMapping(
node *yaml.Node, keys []string, i int, value string, last bool,
) *yaml.Node {
var valueNode *yaml.Node
for j := 0; j+1 < len(node.Content); j += 2 {
if node.Content[j].Value == key {
if node.Content[j].Value == keys[i] {
valueNode = node.Content[j+1]
break
@@ -662,13 +706,8 @@ func yamlSetInMapping(node *yaml.Node, key, value string, last bool) *yaml.Node
}
if valueNode == nil {
keyNode := &yaml.Node{Kind: yaml.ScalarNode, Value: key}
valueNode = &yaml.Node{Kind: yaml.MappingNode}
if last {
valueNode = &yaml.Node{Kind: yaml.ScalarNode, Value: value}
}
keyNode := &yaml.Node{Kind: yaml.ScalarNode, Value: keys[i]}
valueNode = newPathNode(keys, i, value)
node.Content = append(node.Content, keyNode, valueNode)
} else if last {
setScalar(valueNode, value)
@@ -696,12 +735,7 @@ func yamlSetInSequence(
}
if idx == len(node.Content) {
newNode := &yaml.Node{Kind: yaml.MappingNode}
if last {
newNode = &yaml.Node{Kind: yaml.ScalarNode, Value: value}
}
node.Content = append(node.Content, newNode)
node.Content = append(node.Content, newPathNode(keys, i, value))
} else if last {
setScalar(node.Content[idx], value)
}
@@ -709,6 +743,22 @@ func yamlSetInSequence(
return node.Content[idx], nil
}
// newPathNode returns the node to create for the missing path element
// keys[i]: the value when it is the final path element, otherwise an empty
// list when the next path element is a number and an empty map when not.
func newPathNode(keys []string, i int, value string) *yaml.Node {
if i == len(keys)-1 {
return &yaml.Node{Kind: yaml.ScalarNode, Value: value}
}
_, err := strconv.Atoi(keys[i+1])
if err == nil {
return &yaml.Node{Kind: yaml.SequenceNode}
}
return &yaml.Node{Kind: yaml.MappingNode}
}
// setScalar overwrites a node in place with a plain scalar value.
func setScalar(n *yaml.Node, value string) {
n.Kind = yaml.ScalarNode
+186 -5
View File
@@ -58,14 +58,17 @@ func TestDefaultConfigTemplateParses(t *testing.T) {
}
}
// testRecipient is a valid age public key.
const testRecipient = "age1278m9q7dp3chsh2dcy82qk27v047zywyvtxwnj4cvt0z65jw6a7q5dqhfj"
// TestConfigSetRecipientOnFreshConfig follows the README quickstart: on the
// file `config init` writes, `config set age_recipients.0` and
// `config set storage_url` give a config that loads with that recipient.
// Neither is refused, although the config does not load until both are set;
// once it loads, a recipient that is not a valid key is.
func TestConfigSetRecipientOnFreshConfig(t *testing.T) {
t.Parallel()
const recipient = "age1278m9q7dp3chsh2dcy82qk27v047zywyvtxwnj4cvt0z65jw6a7q5dqhfj"
path := filepath.Join(t.TempDir(), "config.yml")
err := os.WriteFile(path, []byte(defaultConfigTemplate), configFileMode)
@@ -75,7 +78,7 @@ func TestConfigSetRecipientOnFreshConfig(t *testing.T) {
out := ui.NewWithColor(&bytes.Buffer{}, false)
err = writeConfigSet(out, path, "age_recipients.0", recipient)
err = writeConfigSet(out, path, "age_recipients.0", testRecipient)
if err != nil {
t.Fatalf("config set age_recipients.0: %v", err)
}
@@ -90,8 +93,186 @@ func TestConfigSetRecipientOnFreshConfig(t *testing.T) {
t.Fatalf("config.Load: %v", err)
}
if len(cfg.AgeRecipients) != 1 || cfg.AgeRecipients[0] != recipient {
t.Errorf("age_recipients = %v, want [%s]", cfg.AgeRecipients, recipient)
if len(cfg.AgeRecipients) != 1 || cfg.AgeRecipients[0] != testRecipient {
t.Errorf("age_recipients = %v, want [%s]", cfg.AgeRecipients, testRecipient)
}
err = writeConfigSet(out, path, "age_recipients.0", "notakey")
if err == nil || !strings.Contains(err.Error(), "not a valid recipient") {
t.Errorf("config set age_recipients.0 notakey gives error %v, "+
"want one saying it is not a valid recipient", err)
}
_, err = config.Load(path)
if err != nil {
t.Errorf("config.Load after the refused change: %v", err)
}
}
// configWithoutRecipients loads, but has no age_recipients key and no
// snapshot named work.
const configWithoutRecipients = `storage_url: file:///mnt/backups
snapshots:
home:
paths:
- "~"
`
// TestConfigSetIndexUnderMissingKey checks that a list index under a key
// the file does not have creates a list, and that the result loads.
func TestConfigSetIndexUnderMissingKey(t *testing.T) {
t.Parallel()
tests := []struct {
key string
value string
field func(cfg *config.Config) []string
}{
{"age_recipients.0", testRecipient,
func(cfg *config.Config) []string { return cfg.AgeRecipients }},
{"snapshots.work.paths.0", "/srv/work",
func(cfg *config.Config) []string { return cfg.Snapshots["work"].Paths }},
}
for _, tt := range tests {
t.Run(tt.key, func(t *testing.T) {
t.Parallel()
path := filepath.Join(t.TempDir(), "config.yml")
err := os.WriteFile(path, []byte(configWithoutRecipients), configFileMode)
if err != nil {
t.Fatalf("write config: %v", err)
}
err = writeConfigSet(ui.NewWithColor(&bytes.Buffer{}, false),
path, tt.key, tt.value)
if err != nil {
t.Fatalf("config set %s: %v", tt.key, err)
}
cfg, err := config.Load(path)
if err != nil {
t.Fatalf("config.Load: %v", err)
}
got := tt.field(cfg)
if len(got) != 1 || got[0] != tt.value {
t.Errorf("%s gives %v, want [%s]", tt.key, got, tt.value)
}
})
}
}
// TestConfigSetKeepsFileThatWouldNotLoad checks that config set refuses a
// change after which the config would not load, with an error giving the
// reason, and leaves the file as it was. On the file `config init` writes,
// which does not load until storage_url is set, only a value of the wrong
// type is refused.
func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
t.Parallel()
tests := []struct {
config string
key string
value string
reason string
}{
// Index 1 of the empty list the missing key gives.
{configWithoutRecipients, "age_recipients.1", testRecipient,
"index out of range"},
// A list set to a single value.
{configWithoutRecipients, "snapshots.home.paths", "/srv",
"into []string"},
{configWithoutRecipients, "chunk_size", "fast",
"invalid size format"},
// Values of the right type that fail validation.
{configWithoutRecipients, "compression_level", "99",
"compression_level must be between 1 and 19"},
{configWithoutRecipients, "chunk_size", "1KB",
"chunk_size must be at least 1MB"},
// The type check on a config that does not load yet.
{defaultConfigTemplate, "snapshots.apps.paths", "/srv",
"into []string"},
}
for _, tt := range tests {
t.Run(tt.key+" "+tt.value, func(t *testing.T) {
t.Parallel()
path := filepath.Join(t.TempDir(), "config.yml")
err := os.WriteFile(path, []byte(tt.config), configFileMode)
if err != nil {
t.Fatalf("write config: %v", err)
}
err = writeConfigSet(ui.NewWithColor(&bytes.Buffer{}, false),
path, tt.key, tt.value)
if err == nil || !strings.Contains(err.Error(), tt.reason) {
t.Errorf("config set %s %s gives error %v, want one saying %q",
tt.key, tt.value, err, tt.reason)
}
data, err := os.ReadFile(path) //nolint:gosec // G304: test-controlled path
if err != nil {
t.Fatalf("read config: %v", err)
}
if string(data) != tt.config {
t.Errorf("config set %s changed the file to:\n%s",
tt.key, data)
}
})
}
}
// TestConfigSetRefusesOnlyItsOwnFailure checks that setting another key
// works on a config with a ${...} reference on compression_level, which
// loads because loading fills the reference in, and on one whose
// compression_level already fails to load.
func TestConfigSetRefusesOnlyItsOwnFailure(t *testing.T) {
t.Parallel()
levelPath := filepath.Join(t.TempDir(), "level")
err := os.WriteFile(levelPath, []byte("9"), configFileMode)
if err != nil {
t.Fatalf("write level: %v", err)
}
configs := map[string]string{
"reference": configWithoutRecipients +
"compression_level: ${FILE:" + levelPath + "}\n",
"failing": configWithoutRecipients + "compression_level: fast\n",
}
for name, configYAML := range configs {
t.Run(name, func(t *testing.T) {
t.Parallel()
path := filepath.Join(t.TempDir(), "config.yml")
err := os.WriteFile(path, []byte(configYAML), configFileMode)
if err != nil {
t.Fatalf("write config: %v", err)
}
err = writeConfigSet(ui.NewWithColor(&bytes.Buffer{}, false),
path, "hostname", "myhost")
if err != nil {
t.Fatalf("config set hostname: %v", err)
}
data, err := os.ReadFile(path) //nolint:gosec // G304: test-controlled path
if err != nil {
t.Fatalf("read config: %v", err)
}
if !strings.Contains(string(data), "hostname: myhost") {
t.Errorf("config set hostname wrote:\n%s", data)
}
})
}
}
+41 -24
View File
@@ -4,6 +4,7 @@
package config
import (
"bytes"
"errors"
"fmt"
"os"
@@ -233,14 +234,48 @@ func New(path Path) (*Config, error) {
return cfg, nil
}
// Load reads and parses the configuration file from the specified path.
// It applies default values for optional fields, performs environment variable
// substitution using smartconfig, and validates the configuration.
// The configuration file should be in YAML format. Returns an error if the file
// cannot be read, parsed, or if validation fails.
// Load reads the configuration file at path, parses it with Parse, warns
// when others can read the file, and validates the configuration.
// Returns an error if the file cannot be read, parsed, or if validation
// fails.
func Load(path string) (*Config, error) {
data, err := os.ReadFile(path) //nolint:gosec // G304: config path is operator-supplied
if err != nil {
return nil, fmt.Errorf("failed to load config file: %w", err)
}
cfg, err := Parse(data)
if err != nil {
return nil, err
}
// Check config file permissions (warn if world or group readable)
//nolint:gosec // G703: config path is operator-supplied by design
info, statErr := os.Stat(path)
if statErr == nil {
mode := info.Mode().Perm()
if mode&0044 != 0 { // group or world readable
log.Warn(cfg.readableByOthersWarning(),
"path", path,
"mode", fmt.Sprintf("%04o", mode),
"recommendation", "chmod 600 "+path)
}
}
err = cfg.Validate()
if err != nil {
return nil, fmt.Errorf("invalid config: %w", err)
}
return cfg, nil
}
// Parse reads YAML config data as Load does, without validating it. It
// applies default values for optional fields, fills in ${...} references
// using smartconfig, and expands ~ in paths.
func Parse(data []byte) (*Config, error) {
// Load config using smartconfig for interpolation
sc, err := smartconfig.NewFromConfigPath(path)
sc, err := smartconfig.NewFromReader(bytes.NewReader(data))
if err != nil {
return nil, fmt.Errorf("failed to load config file: %w", err)
}
@@ -302,24 +337,6 @@ func Load(path string) (*Config, error) {
cfg.S3.Region = "us-east-1"
}
// Check config file permissions (warn if world or group readable)
//nolint:gosec // G703: config path is operator-supplied by design
info, statErr := os.Stat(path)
if statErr == nil {
mode := info.Mode().Perm()
if mode&0044 != 0 { // group or world readable
log.Warn(cfg.readableByOthersWarning(),
"path", path,
"mode", fmt.Sprintf("%04o", mode),
"recommendation", "chmod 600 "+path)
}
}
err = cfg.Validate()
if err != nil {
return nil, fmt.Errorf("invalid config: %w", err)
}
return cfg, nil
}
@@ -161,14 +161,13 @@ func TestVerifyManifestBlobsRejectsShortHash(t *testing.T) {
Blobs: []snapshot.BlobInfo{{Hash: "abc", CompressedSize: 1}},
}
verified, missing, mismatched, missingSize, unchecked, err :=
verified, missing, mismatched, missingSize, err :=
v.verifyManifestBlobs(manifest, &VerifyOptions{JSON: true})
require.ErrorIs(t, err, errInvalidBlobHash)
require.Zero(t, verified)
require.Zero(t, missing)
require.Zero(t, mismatched)
require.Zero(t, missingSize)
require.Empty(t, unchecked)
}
// TestVerifyBlobChunksRejectsNegativeLength proves a blob_chunks row with a
+12 -1
View File
@@ -309,11 +309,22 @@ func (v *Vaultik) restoreAllFiles(
return nil, err
}
plan, err := newRestorePlan(v.ctx, repos, files, chunkToBlobMap, blobIDToHash)
plan, unplanned, err := newRestorePlan(
v.ctx, repos, files, chunkToBlobMap, blobIDToHash)
if err != nil {
return nil, fmt.Errorf("building restore plan: %w", err)
}
// Ranging over files rather than the map reports them in a stable order.
for _, file := range files {
if planErr, ok := unplanned[file.ID]; ok {
err = v.handleRestoreFileError(plan, opts, result, file, file.ID, planErr)
if err != nil {
return nil, err
}
}
}
filesByID, totalBytesExpected := indexRestoreFiles(files)
v.UI.Beginf("Restoring %s files (%s) to %s.",
@@ -61,8 +61,8 @@ func craftedFile(
}
// TestRestoreShortChunkHashInNoBlob proves a file whose short chunk hash
// has no blob_chunks row fails restore planning and the chunk write with
// an error.
// has no blob_chunks row is left out of the restore plan with an error,
// and fails the chunk write with an error.
func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
t.Parallel()
@@ -77,9 +77,11 @@ func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
chunkToBlobMap, err := v.buildChunkToBlobMap(ctx, repos)
require.NoError(t, err)
_, err = newRestorePlan(ctx, repos, []*database.File{file},
plan, unplanned, err := newRestorePlan(ctx, repos, []*database.File{file},
chunkToBlobMap, map[string]string{})
require.ErrorIs(t, err, errPlanChunkMissing)
require.NoError(t, err)
require.ErrorIs(t, unplanned[file.ID], errPlanChunkMissing)
require.False(t, plan.hasPending())
fileChunks, err := repos.FileChunks.GetByFileID(ctx, file.ID)
require.NoError(t, err)
+17 -8
View File
@@ -41,20 +41,25 @@ type restorePlan struct {
// newRestorePlan builds the file→blob index for the given files. Files
// whose chunks reference no blobs (symlinks, directories) start in the
// ready queue immediately.
// ready queue immediately. A file with a chunk that no blob holds cannot
// be restored: it is left out of the plan and returned, by ID, with the
// reason.
func newRestorePlan(
ctx context.Context,
repos *database.Repositories,
files []*database.File,
chunkToBlobMap map[string]*database.BlobChunk,
blobIDToHash map[string]string,
) (*restorePlan, error) {
) (*restorePlan, map[types.FileID]error, error) {
p := &restorePlan{
fileBlobs: make(map[types.FileID]map[string]struct{}, len(files)),
blobFiles: make(map[string]map[types.FileID]struct{}),
ready: make([]types.FileID, 0, len(files)),
cached: make(map[string]struct{}),
}
unplanned := make(map[types.FileID]error)
nextFile:
for _, f := range files {
if f.IsSymlink() || f.Mode&uint32(os.ModeDir) != 0 {
// No chunks to fetch — restore can run immediately.
@@ -66,7 +71,7 @@ func newRestorePlan(
fileChunks, err := repos.FileChunks.GetByFileID(ctx, f.ID)
if err != nil {
return nil, fmt.Errorf("planning %s: %w", f.Path, err)
return nil, nil, fmt.Errorf("planning %s: %w", f.Path, err)
}
blobs := make(map[string]struct{})
@@ -74,14 +79,18 @@ func newRestorePlan(
for _, fc := range fileChunks {
bc, ok := chunkToBlobMap[fc.ChunkHash.String()]
if !ok {
return nil, fmt.Errorf("planning %s: %w: %s",
f.Path, errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
unplanned[f.ID] = fmt.Errorf("%w: %s",
errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
continue nextFile
}
hash, ok := blobIDToHash[bc.BlobID.String()]
if !ok {
return nil, fmt.Errorf("planning %s: %w: %s",
f.Path, errPlanBlobIDMissing, bc.BlobID)
unplanned[f.ID] = fmt.Errorf("%w: %s",
errPlanBlobIDMissing, bc.BlobID)
continue nextFile
}
blobs[hash] = struct{}{}
@@ -103,7 +112,7 @@ func newRestorePlan(
}
}
return p, nil
return p, unplanned, nil
}
// markBlobCached records that the named blob is now resident in the
+103 -16
View File
@@ -28,7 +28,8 @@ const (
)
// missingBlobBackup is a snapshot of single-chunk files spread over two
// blobs, with one of those blobs deleted from the store.
// blobs, with one of those blobs lost: deleted from the store, or missing
// from the snapshot metadata.
type missingBlobBackup struct {
fs afero.Fs
cfg *config.Config
@@ -37,20 +38,39 @@ type missingBlobBackup struct {
restoreDir string
// files holds the original content by source path.
files map[string][]byte
// lost holds the source paths whose chunk is in the deleted blob.
// lost holds the source paths whose chunk is in the lost blob.
lost map[string]bool
}
// TestRestoreSkipErrorsSkipsFilesOfMissingBlob restores with SkipErrors
// after one blob of a two-blob snapshot was deleted. Every file stored in
// that blob must be reported as failed and left absent, every other file
// must be restored intact, and Restore must still return an error.
// after one blob of a two-blob snapshot was deleted from the store.
func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
backup := backupThenDeleteOneBlob(ctx, t)
assertRestoreSkipsLostFiles(ctx, t, backupThenDeleteOneBlob(ctx, t))
}
// TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob restores with
// SkipErrors a snapshot whose metadata lists chunks that no blob holds, as
// earlier backup defects left behind.
func TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
assertRestoreSkipsLostFiles(ctx, t, backupThenDropOneBlobFromMetadata(ctx, t))
}
// assertRestoreSkipsLostFiles restores backup with SkipErrors. Every file
// stored in the lost blob must be reported as failed and left absent,
// every other file must be restored intact, and Restore must still return
// an error.
func assertRestoreSkipsLostFiles(
ctx context.Context, t *testing.T, backup *missingBlobBackup,
) {
t.Helper()
var out bytes.Buffer
@@ -72,14 +92,14 @@ func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) {
if backup.lost[path] {
assert.Containsf(t, out.String(), path,
"%s needs the deleted blob and must be reported", path)
"%s needs the lost blob and must be reported", path)
assert.NoFileExists(t, restored)
continue
}
got, err := afero.ReadFile(backup.fs, restored)
require.NoErrorf(t, err, "%s does not need the deleted blob", path)
require.NoErrorf(t, err, "%s does not need the lost blob", path)
assert.Equalf(t, content, got, "%s restored with wrong content", path)
}
}
@@ -103,6 +123,30 @@ func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
require.ErrorIs(t, err, storage.ErrNotFound)
}
// TestRestoreChunkInNoBlobAbortsWithoutSkipErrors checks that a chunk held
// by no blob still ends the restore, before any file is written, when
// SkipErrors is not set.
func TestRestoreChunkInNoBlobAbortsWithoutSkipErrors(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
backup := backupThenDropOneBlobFromMetadata(ctx, t)
v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, backup.fs)
err := v.Restore(&vaultik.RestoreOptions{
SnapshotID: backup.snapshotID,
TargetDir: backup.restoreDir,
})
require.ErrorContains(t, err, "chunk missing from blob map")
for path := range backup.files {
assert.NoFileExists(t, filepath.Join(backup.restoreDir, path))
}
}
// backupThenDeleteOneBlob backs up missingBlobFileCount single-chunk
// files, then deletes from the store the blob holding the first of them.
func backupThenDeleteOneBlob(
@@ -110,6 +154,54 @@ func backupThenDeleteOneBlob(
) *missingBlobBackup {
t.Helper()
backup, db, lostBlob := backupSingleChunkFiles(ctx, t)
require.NoError(t, db.Close())
require.NoError(t, backup.storer.Delete(ctx, fmt.Sprintf(
"blobs/%s/%s/%s", lostBlob[:2], lostBlob[2:4], lostBlob)))
return backup
}
// backupThenDropOneBlobFromMetadata backs up missingBlobFileCount
// single-chunk files, then removes the blob holding the first of them from
// the snapshot and exports the snapshot metadata again. The exported
// metadata then lists chunks that no blob holds.
func backupThenDropOneBlobFromMetadata(
ctx context.Context, t *testing.T,
) *missingBlobBackup {
t.Helper()
backup, db, lostBlob := backupSingleChunkFiles(ctx, t)
// The uploads row refers to the blob row, so it would keep the export
// from deleting it.
for _, query := range []string{
"DELETE FROM snapshot_blobs WHERE blob_hash = ?",
"DELETE FROM uploads WHERE blob_hash = ?",
} {
_, err := db.Conn().ExecContext(ctx, query, lostBlob)
require.NoError(t, err)
}
sm := newFaultSnapshotManager(
backup.fs, backup.storer, backup.cfg, database.NewRepositories(db))
require.NoError(t,
sm.ExportSnapshotMetadata(ctx, db.Path(), backup.snapshotID))
require.NoError(t, db.Close())
return backup
}
// backupSingleChunkFiles backs up missingBlobFileCount single-chunk files.
// It returns the backup, with lost naming the files stored in the same
// blob as the first of them, the still open local index, and the hash of
// that blob.
func backupSingleChunkFiles(
ctx context.Context, t *testing.T,
) (*missingBlobBackup, *database.DB, string) {
t.Helper()
fs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "src")
@@ -143,16 +235,11 @@ func backupThenDeleteOneBlob(
blobOfFile[path] = blobHashOfSingleChunkFile(ctx, t, repos, path)
}
require.NoError(t, db.Close())
deleted := blobOfFile[filepath.Join(dataDir, "file-00.bin")]
require.NoError(t, storer.Delete(ctx, fmt.Sprintf(
"blobs/%s/%s/%s", deleted[:2], deleted[2:4], deleted)))
lostBlob := blobOfFile[filepath.Join(dataDir, "file-00.bin")]
lost := make(map[string]bool)
for path, hash := range blobOfFile {
if hash == deleted {
if hash == lostBlob {
lost[path] = true
}
}
@@ -168,7 +255,7 @@ func backupThenDeleteOneBlob(
restoreDir: filepath.Join(tempDir, "restored"),
files: files,
lost: lost,
}
}, db, lostBlob
}
// blobHashOfSingleChunkFile returns the hash of the blob holding the one
-223
View File
@@ -4,20 +4,15 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"os"
"path"
"path/filepath"
"slices"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/snapshot"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/ui"
"sneak.berlin/go/vaultik/internal/vaultik"
)
@@ -156,224 +151,6 @@ func TestShallowVerifyDetectsMissingDatabase(t *testing.T) {
"shallow verify must fail when db.zst.age is absent")
}
// errStatDenied stands in for any storage error other than not-found: a
// permission error, an S3 403, a dropped connection.
var errStatDenied = errors.New("injected stat failure")
// statFailingStorer fails Stat with errStatDenied for the keys in
// failKeys and passes every other call to the wrapped Storer.
type statFailingStorer struct {
storage.Storer
failKeys []string
}
func (s statFailingStorer) Stat(
ctx context.Context, key string,
) (*storage.ObjectInfo, error) {
if slices.Contains(s.failKeys, key) {
return nil, fmt.Errorf("stat %q: %w", key, errStatDenied)
}
return s.Storer.Stat(ctx, key)
}
// TestShallowVerifyReportsStatErrorsApartFromMissing backs up a real
// snapshot, deletes one blob, and makes storage fail Stat with an error
// other than not-found for a second blob and for the encrypted database.
// Only the deleted blob may be counted as missing. The second blob and
// the database must be reported as could not be checked, with the error
// storage returned, and verify must fail, in the text output and under
// --json.
func TestShallowVerifyReportsStatErrorsApartFromMissing(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
fs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "source")
storeDir := filepath.Join(tempDir, "remote")
dbPath := filepath.Join(tempDir, "index.sqlite")
chunkSize := int64(32 * 1024)
maxBlobSize := int64(128 * 1024)
// Random content, so the snapshot spans several blobs.
require.NoError(t, fs.MkdirAll(dataDir, 0o755))
appendRandomBytes(t, fs, map[string][]byte{},
filepath.Join(dataDir, "data.bin"), maxBlobSize*4)
ctx := context.Background()
cfg, storer, snapshotID := runFileStorageBackup(
ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize)
blobKeys := storedBlobKeys(t, storeDir)
require.GreaterOrEqual(t, len(blobKeys), 2, "expected at least two blobs")
missingKey, uncheckedKey := blobKeys[0], blobKeys[1]
missingHash, uncheckedHash := path.Base(missingKey), path.Base(uncheckedKey)
require.NoError(t, os.Remove(filepath.Join(storeDir, missingKey)))
dbKey := "metadata/" + snapshot.RemoteSnapshotKey(snapshotID) + "/db.zst.age"
failing := statFailingStorer{
Storer: storer,
failKeys: []string{uncheckedKey, dbKey},
}
newVerifier := func(out io.Writer) *vaultik.Vaultik {
v := &vaultik.Vaultik{
Config: cfg,
Storage: failing,
Fs: fs,
Stdout: out,
Stderr: io.Discard,
UI: ui.NewWithColor(io.Discard, false),
}
v.SetContext(ctx)
return v
}
var out bytes.Buffer
err := newVerifier(&out).VerifySnapshotWithOptions(
snapshotID, &vaultik.VerifyOptions{JSON: true})
require.Error(t, err)
result := decodeVerifyResult(t, out.Bytes())
require.Equal(t, "failed", result.Status)
require.Equal(t, 1, result.Missing, "only the deleted blob is missing")
require.Equal(t, result.BlobCount-2, result.Verified)
require.Len(t, result.Unchecked, 1)
require.Equal(t, uncheckedHash, result.Unchecked[0].Hash)
require.Contains(t, result.Unchecked[0].Error, errStatDenied.Error())
require.False(t, result.DatabaseMissing,
"a database that could not be checked is not missing")
require.Contains(t, result.DatabaseCheckError, errStatDenied.Error())
out.Reset()
err = newVerifier(&out).VerifySnapshotWithOptions(
snapshotID, &vaultik.VerifyOptions{})
require.Error(t, err)
text := out.String()
require.Contains(t, text, "Missing: "+missingHash)
require.NotContains(t, text, "Missing: "+uncheckedHash)
require.Contains(t, text, "Could not check: "+uncheckedHash)
require.Contains(t, text, "Encrypted database: could not be checked")
require.Contains(t, text, errStatDenied.Error())
require.Contains(t, text, "FAILED")
}
// TestShallowVerifyQuietFailsOnStatErrorAlone backs up a healthy snapshot,
// then makes storage fail Stat, with an error other than not-found, for
// one blob alone and then for the encrypted database alone. Each run must
// fail verify, and under --quiet, which drops the report, the error
// storage returned must still reach stderr.
//
//nolint:paralleltest // captureProcessStderr replaces os.Stderr
func TestShallowVerifyQuietFailsOnStatErrorAlone(t *testing.T) {
log.Initialize(log.Config{})
fs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "source")
storeDir := filepath.Join(tempDir, "remote")
dbPath := filepath.Join(tempDir, "index.sqlite")
chunkSize := int64(32 * 1024)
maxBlobSize := int64(128 * 1024)
require.NoError(t, fs.MkdirAll(dataDir, 0o755))
require.NoError(t, afero.WriteFile(fs,
filepath.Join(dataDir, "data.bin"),
bytesPattern("shallow-quiet-", int(maxBlobSize*2)), 0o644))
ctx := context.Background()
cfg, storer, snapshotID := runFileStorageBackup(
ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize)
newQuietVerifier := func(s storage.Storer) *vaultik.Vaultik {
v := &vaultik.Vaultik{
Config: cfg,
Storage: s,
Fs: fs,
Stdout: io.Discard,
Stderr: io.Discard,
UI: ui.NewWithColor(io.Discard, false),
}
v.SetContext(ctx)
v.UI.SetQuiet(true)
return v
}
require.NoError(t,
newQuietVerifier(storer).VerifySnapshotWithOptions(
snapshotID, &vaultik.VerifyOptions{}),
"shallow verify should pass on a healthy snapshot")
blobKeys := storedBlobKeys(t, storeDir)
require.NotEmpty(t, blobKeys)
dbKey := "metadata/" + snapshot.RemoteSnapshotKey(snapshotID) + "/db.zst.age"
for _, failKey := range []string{blobKeys[0], dbKey} {
v := newQuietVerifier(statFailingStorer{
Storer: storer,
failKeys: []string{failKey},
})
var err error
stderr := captureProcessStderr(t, func() {
err = v.VerifySnapshotWithOptions(
snapshotID, &vaultik.VerifyOptions{})
})
require.Error(t, err,
"verify must fail when %s could not be checked", failKey)
require.Contains(t, stderr, errStatDenied.Error(),
"under --quiet the error for %s must still reach stderr", failKey)
}
}
// storedBlobKeys returns the storage key of every blob file under
// storeDir, sorted.
func storedBlobKeys(t *testing.T, storeDir string) []string {
t.Helper()
var keys []string
err := filepath.Walk(filepath.Join(storeDir, "blobs"),
func(p string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() {
return nil
}
rel, err := filepath.Rel(storeDir, p)
if err != nil {
return err
}
keys = append(keys, filepath.ToSlash(rel))
return nil
})
require.NoError(t, err)
return keys
}
// growOneBlob appends bytes to the first blob file found under blobsDir,
// changing its on-disk size so it no longer matches the manifest.
func growOneBlob(t *testing.T, fs afero.Fs, blobsDir string) {
+21 -70
View File
@@ -14,7 +14,6 @@ import (
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/snapshot"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/types"
)
@@ -778,10 +777,18 @@ func (v *Vaultik) VerifySnapshotWithOptions(
v.stdoutf("Checking blob presence and sizes...\n")
}
v.checkSnapshotDatabase(remoteKey, result)
// A snapshot is only restorable if its encrypted database is present
// alongside the blobs. Shallow verify checks that the object exists; it
// does not decrypt it (that is deep verify's job).
dbPath := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey)
result.Verified, result.Missing, result.Mismatched, result.MissingSize,
result.Unchecked, err = v.verifyManifestBlobs(manifest, opts)
_, dbErr := v.Storage.Stat(v.ctx, dbPath)
if dbErr != nil {
result.DatabaseMissing = true
}
result.Verified, result.Missing, result.Mismatched, result.MissingSize, err =
v.verifyManifestBlobs(manifest, opts)
if err != nil {
if opts.JSON {
result.Status = verifyStatusFailed
@@ -818,46 +825,25 @@ func (v *Vaultik) printVerifyHeader(snapshotID string, opts *VerifyOptions) {
}
}
// checkSnapshotDatabase records in result whether the snapshot's
// encrypted database is missing or could not be checked. A snapshot is
// only restorable if its database is present alongside the blobs.
// Shallow verify checks that the object exists; it does not decrypt it
// (that is deep verify's job).
func (v *Vaultik) checkSnapshotDatabase(remoteKey string, result *VerifyResult) {
dbPath := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey)
_, err := v.Storage.Stat(v.ctx, dbPath)
switch {
case errors.Is(err, storage.ErrNotFound):
result.DatabaseMissing = true
case err != nil:
result.DatabaseCheckError = err.Error()
}
}
// verifyManifestBlobs checks that each blob in the manifest is present in
// storage with the size the manifest records, returning the counts of
// blobs that were present with the right size, absent, and present but the
// wrong size, the total bytes of the absent blobs, and the blobs it could
// not check because storage returned an error other than not-found. It
// does not read blob contents; deep verification (RunDeepVerify) does
// that. The size comparison matches the deep path (see
// verifyBlobExistenceFromDB).
// wrong size, plus the total bytes of the absent blobs. It does not read
// blob contents; deep verification (RunDeepVerify) does that. The size
// comparison matches the deep path (see verifyBlobExistenceFromDB).
func (v *Vaultik) verifyManifestBlobs(
manifest *snapshot.Manifest, opts *VerifyOptions,
) (int, int, int, int64, []UncheckedBlob, error) {
) (int, int, int, int64, error) {
var (
verified, missing, mismatched int
missingSize int64
unchecked []UncheckedBlob
)
for _, blob := range manifest.Blobs {
// The manifest is unauthenticated, so its blob hashes are checked
// before being spliced into a storage path.
if !isBlobHash(blob.Hash) {
return 0, 0, 0, 0, nil, fmt.Errorf("%w: %s",
return 0, 0, 0, 0, fmt.Errorf("%w: %s",
errInvalidBlobHash, shortHash(blob.Hash))
}
@@ -866,7 +852,7 @@ func (v *Vaultik) verifyManifestBlobs(
stat, err := v.Storage.Stat(v.ctx, blobPath)
switch {
case errors.Is(err, storage.ErrNotFound):
case err != nil:
if !opts.JSON && !v.UI.Quiet() {
v.stdoutf(" Missing: %s (%s)\n",
blob.Hash, ubytes(blob.CompressedSize))
@@ -874,13 +860,6 @@ func (v *Vaultik) verifyManifestBlobs(
missing++
missingSize += blob.CompressedSize
case err != nil:
if !opts.JSON && !v.UI.Quiet() {
v.stdoutf(" Could not check: %s (%v)\n", blob.Hash, err)
}
unchecked = append(unchecked,
UncheckedBlob{Hash: blob.Hash, Error: err.Error()})
case stat.Size != blob.CompressedSize:
if !opts.JSON && !v.UI.Quiet() {
v.stdoutf(" Wrong size: %s (store has %s, manifest lists %s)\n",
@@ -893,7 +872,7 @@ func (v *Vaultik) verifyManifestBlobs(
}
}
return verified, missing, mismatched, missingSize, unchecked, nil
return verified, missing, mismatched, missingSize, nil
}
// formatVerifyResult outputs the final verification results as JSON or
@@ -914,19 +893,9 @@ func (v *Vaultik) formatVerifyResult(
return v.outputVerifyJSON(result)
}
// --quiet drops the report but not the errors storage returned, which
// are logged on stderr. A failure is still returned either way, and
// the cli layer prints it on stderr.
if v.UI.Quiet() {
for _, blob := range result.Unchecked {
log.Warn("Could not check blob", "blob", blob.Hash, "error", blob.Error)
}
if result.DatabaseCheckError != "" {
log.Warn("Could not check the encrypted database",
"snapshot", result.SnapshotID, "error", result.DatabaseCheckError)
}
} else {
// Under --quiet a failure is still returned, and the cli layer
// prints it on stderr.
if !v.UI.Quiet() {
v.printVerifySummary(result, failure)
}
@@ -952,19 +921,10 @@ func (v *Vaultik) printVerifySummary(result *VerifyResult, failure string) {
v.stdoutf(" Wrong size: %d blobs\n", result.Mismatched)
}
if len(result.Unchecked) > 0 {
v.stdoutf(" Could not check: %d blobs\n", len(result.Unchecked))
}
if result.DatabaseMissing {
v.stdoutf(" Encrypted database: missing\n")
}
if result.DatabaseCheckError != "" {
v.stdoutf(" Encrypted database: could not be checked (%s)\n",
result.DatabaseCheckError)
}
v.stdoutf(" Status: ")
if failure != "" {
@@ -992,19 +952,10 @@ func shallowVerifyFailure(result *VerifyResult) string {
fmt.Sprintf("%d blobs have the wrong size", result.Mismatched))
}
if len(result.Unchecked) > 0 {
parts = append(parts, fmt.Sprintf("%d blobs could not be checked",
len(result.Unchecked)))
}
if result.DatabaseMissing {
parts = append(parts, "the encrypted database is missing")
}
if result.DatabaseCheckError != "" {
parts = append(parts, "the encrypted database could not be checked")
}
return strings.Join(parts, "; ")
}
+2 -16
View File
@@ -57,25 +57,11 @@ type VerifyResult struct {
Missing int `json:"missing"`
MissingSize int64 `json:"missing_size,omitempty"`
Mismatched int `json:"mismatched,omitempty"`
// Unchecked lists the blobs shallow verify could not check, because
// storage answered with an error other than not-found, so whether
// they are present is unknown.
Unchecked []UncheckedBlob `json:"unchecked,omitempty"`
// DatabaseMissing is set by shallow verify when the snapshot's
// encrypted database (metadata/<key>/db.zst.age) is absent, which
// makes the snapshot unrestorable regardless of the blobs.
DatabaseMissing bool `json:"database_missing,omitempty"`
// DatabaseCheckError is set by shallow verify to the error storage
// returned when the encrypted database could not be checked.
DatabaseCheckError string `json:"database_check_error,omitempty"`
ErrorMessage string `json:"error,omitempty"`
}
// UncheckedBlob is a blob shallow verify could not check, with the error
// storage returned for it.
type UncheckedBlob struct {
Hash string `json:"hash"`
Error string `json:"error"`
DatabaseMissing bool `json:"database_missing,omitempty"`
ErrorMessage string `json:"error,omitempty"`
}
// deepVerifyFailure records a failure in the result and returns it appropriately