diff --git a/README.md b/README.md index 306b27d..2384391 100644 --- a/README.md +++ b/README.md @@ -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) diff --git a/internal/cli/config.go b/internal/cli/config.go index 545f81a..5350d88 100644 --- a/internal/cli/config.go +++ b/internal/cli/config.go @@ -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 ", 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 diff --git a/internal/cli/config_test.go b/internal/cli/config_test.go index f7b9eb2..7383307 100644 --- a/internal/cli/config_test.go +++ b/internal/cli/config_test.go @@ -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) + } + }) } }