Compare commits

..
Author SHA1 Message Date
sneak fd336ee3e1 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`
created a map keyed `0` and wrote it, after which every command failed
to parse the config. A missing key is now created as a list when the
next path element is a number, and an index that list cannot take is an
error.

`config set` now decodes its result into the config type before
writing, and on failure refuses and leaves the file unchanged. It does
not validate: settings are set one at a time, and validation would
refuse a config still being filled in.

Judgement call: a `${...}` reference set on a setting that is not text
is now refused, since it decodes as text; `config edit` can still write
one.

Model: opus-5-5
2026-10-13 10:19:04 +00:00
7 changed files with 191 additions and 169 deletions
+8 -2
View File
@@ -271,8 +271,14 @@ 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. 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 and the file is left unchanged. Whether the
config is complete and its values are in range is checked when a command
loads it, not by `config set`, so settings can be set one at a time.
**`snapshot create`**: Perform incremental backup of configured snapshots.
* Optional snapshot names argument to create specific snapshots (default: all)
+45 -24
View File
@@ -44,6 +44,8 @@ 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(
"the config would no longer load, so the file was not changed")
)
const defaultConfigTemplate = `# vaultik configuration
@@ -385,8 +387,11 @@ 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. A change that gives a setting the wrong
type, such as a single value where a list belongs, is refused and the
file is left unchanged.
Examples:
vaultik config set storage_url "file:///mnt/backups"
@@ -426,6 +431,14 @@ func writeConfigSet(out *ui.Writer, path, key, value string) error {
return fmt.Errorf("marshaling config: %w", err)
}
// Refuse a result that config.Load could not decode. Load also
// validates the config; that is left out here because settings are set
// one at a time, so a config being filled in is incomplete in between.
err = yaml.Unmarshal(data, &config.Config{})
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)
@@ -553,8 +566,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 {
@@ -567,12 +580,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 {
@@ -641,14 +654,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
@@ -656,13 +671,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)
@@ -690,12 +700,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)
}
@@ -703,6 +708,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
+109 -5
View File
@@ -58,14 +58,15 @@ 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.
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 +76,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 +91,111 @@ 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)
}
}
// 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, and leaves the file as it
// was.
func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
t.Parallel()
tests := []struct {
key string
value string
}{
// Index 1 of the empty list the missing key gives.
{"age_recipients.1", testRecipient},
// A list set to a single value.
{"snapshots.home.paths", "/srv"},
{"compression_level", "fast"},
}
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.Errorf("config set %s %s succeeded, want an error",
tt.key, tt.value)
}
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read config: %v", err)
}
if string(data) != configWithoutRecipients {
t.Errorf("config set %s changed the file to:\n%s",
tt.key, data)
}
})
}
}
+1 -12
View File
@@ -309,22 +309,11 @@ func (v *Vaultik) restoreAllFiles(
return nil, err
}
plan, unplanned, err := newRestorePlan(
v.ctx, repos, files, chunkToBlobMap, blobIDToHash)
plan, 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 is left out of the restore plan with an error,
// and fails the chunk write with an error.
// has no blob_chunks row fails restore planning and the chunk write with
// an error.
func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
t.Parallel()
@@ -77,11 +77,9 @@ func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
chunkToBlobMap, err := v.buildChunkToBlobMap(ctx, repos)
require.NoError(t, err)
plan, unplanned, err := newRestorePlan(ctx, repos, []*database.File{file},
_, err = newRestorePlan(ctx, repos, []*database.File{file},
chunkToBlobMap, map[string]string{})
require.NoError(t, err)
require.ErrorIs(t, unplanned[file.ID], errPlanChunkMissing)
require.False(t, plan.hasPending())
require.ErrorIs(t, err, errPlanChunkMissing)
fileChunks, err := repos.FileChunks.GetByFileID(ctx, file.ID)
require.NoError(t, err)
+8 -17
View File
@@ -41,25 +41,20 @@ 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. 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.
// ready queue immediately.
func newRestorePlan(
ctx context.Context,
repos *database.Repositories,
files []*database.File,
chunkToBlobMap map[string]*database.BlobChunk,
blobIDToHash map[string]string,
) (*restorePlan, map[types.FileID]error, error) {
) (*restorePlan, 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.
@@ -71,7 +66,7 @@ nextFile:
fileChunks, err := repos.FileChunks.GetByFileID(ctx, f.ID)
if err != nil {
return nil, nil, fmt.Errorf("planning %s: %w", f.Path, err)
return nil, fmt.Errorf("planning %s: %w", f.Path, err)
}
blobs := make(map[string]struct{})
@@ -79,18 +74,14 @@ nextFile:
for _, fc := range fileChunks {
bc, ok := chunkToBlobMap[fc.ChunkHash.String()]
if !ok {
unplanned[f.ID] = fmt.Errorf("%w: %s",
errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
continue nextFile
return nil, fmt.Errorf("planning %s: %w: %s",
f.Path, errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
}
hash, ok := blobIDToHash[bc.BlobID.String()]
if !ok {
unplanned[f.ID] = fmt.Errorf("%w: %s",
errPlanBlobIDMissing, bc.BlobID)
continue nextFile
return nil, fmt.Errorf("planning %s: %w: %s",
f.Path, errPlanBlobIDMissing, bc.BlobID)
}
blobs[hash] = struct{}{}
@@ -112,7 +103,7 @@ nextFile:
}
}
return p, unplanned, nil
return p, nil
}
// markBlobCached records that the named blob is now resident in the
+16 -103
View File
@@ -28,8 +28,7 @@ const (
)
// missingBlobBackup is a snapshot of single-chunk files spread over two
// blobs, with one of those blobs lost: deleted from the store, or missing
// from the snapshot metadata.
// blobs, with one of those blobs deleted from the store.
type missingBlobBackup struct {
fs afero.Fs
cfg *config.Config
@@ -38,39 +37,20 @@ 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 lost blob.
// lost holds the source paths whose chunk is in the deleted blob.
lost map[string]bool
}
// TestRestoreSkipErrorsSkipsFilesOfMissingBlob restores with SkipErrors
// after one blob of a two-blob snapshot was deleted from the store.
// 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.
func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
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()
backup := backupThenDeleteOneBlob(ctx, t)
var out bytes.Buffer
@@ -92,14 +72,14 @@ func assertRestoreSkipsLostFiles(
if backup.lost[path] {
assert.Containsf(t, out.String(), path,
"%s needs the lost blob and must be reported", path)
"%s needs the deleted 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 lost blob", path)
require.NoErrorf(t, err, "%s does not need the deleted blob", path)
assert.Equalf(t, content, got, "%s restored with wrong content", path)
}
}
@@ -123,30 +103,6 @@ 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(
@@ -154,54 +110,6 @@ 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")
@@ -235,11 +143,16 @@ func backupSingleChunkFiles(
blobOfFile[path] = blobHashOfSingleChunkFile(ctx, t, repos, path)
}
lostBlob := blobOfFile[filepath.Join(dataDir, "file-00.bin")]
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)))
lost := make(map[string]bool)
for path, hash := range blobOfFile {
if hash == lostBlob {
if hash == deleted {
lost[path] = true
}
}
@@ -255,7 +168,7 @@ func backupSingleChunkFiles(
restoreDir: filepath.Join(tempDir, "restored"),
files: files,
lost: lost,
}, db, lostBlob
}
}
// blobHashOfSingleChunkFile returns the hash of the blob holding the one