Refuse config keys that no setting has (closes #419)
check / check (push) Failing after 21m21s
check / check (push) Failing after 21m21s
config.Parse walks the file as written and fails on the first key that
matches no setting, naming its dotted path and line, so a typo in
exclude or prune_grace_period no longer falls back silently. ${...}
references and YAML merge keys still load. config set refuses such a key
before writing. The key-to-field lookup moves from internal/cli to
config.SettingType so both use it.
Model: opus-5-5
This commit is contained in:
1 parent
aae7fa17ae
commit
ba343c30e2
6 files changed
+283
-47
No files matched your search
+13
-37
@@ -46,6 +46,7 @@ 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")
|
||||
errNoSuchSetting = errors.New("no setting has this key")
|
||||
errWouldNotLoad = errors.New(
|
||||
"vaultik would not load the config after this change, " +
|
||||
"so the file was not changed")
|
||||
@@ -439,15 +440,15 @@ func newConfigSetCommand() *cobra.Command {
|
||||
Long: `Sets a scalar config value addressed by dotted YAML path and writes
|
||||
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.
|
||||
number, otherwise as a map. A key that no setting has is refused.
|
||||
|
||||
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, such as the one config init writes before
|
||||
storage_url is set or one with a ${ENV:...} reference to a variable
|
||||
not set here, is checked one setting at a time: only a change after
|
||||
which the changed setting cannot be read, such as one giving it the
|
||||
wrong type, is refused.
|
||||
not set here, is checked one setting at a time: a change after which
|
||||
the changed setting cannot be read, such as one giving it the wrong
|
||||
type, is refused.
|
||||
|
||||
Examples:
|
||||
vaultik config set storage_url "file:///mnt/backups"
|
||||
@@ -484,6 +485,11 @@ func writeConfigSet(out *ui.Writer, path, key, value string) error {
|
||||
|
||||
keys := strings.Split(key, ".")
|
||||
|
||||
_, ok := config.SettingType(keys)
|
||||
if !ok {
|
||||
return fmt.Errorf("%w: %s", errNoSuchSetting, key)
|
||||
}
|
||||
|
||||
err = yamlPathSet(root, keys, value)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -739,41 +745,11 @@ func yamlPathSet(root *yaml.Node, keys []string, value string) error {
|
||||
}
|
||||
|
||||
// configKeyIsString reports whether the dotted key names a string in
|
||||
// config.Config, following the fields' yaml tags, as s3.access_key_id and
|
||||
// snapshots.home.exclude.0 do.
|
||||
// config.Config, as s3.access_key_id and snapshots.home.exclude.0 do.
|
||||
func configKeyIsString(keys []string) bool {
|
||||
typ := reflect.TypeFor[config.Config]()
|
||||
typ, ok := config.SettingType(keys)
|
||||
|
||||
for _, key := range keys {
|
||||
switch {
|
||||
case typ.Kind() == reflect.Map || typ.Kind() == reflect.Slice:
|
||||
// The key is a snapshot name or a list index.
|
||||
typ = typ.Elem()
|
||||
case typ.Kind() == reflect.Struct:
|
||||
field, ok := yamlField(typ, key)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
typ = field.Type
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
return typ.Kind() == reflect.String
|
||||
}
|
||||
|
||||
// yamlField returns the field of struct type typ whose yaml tag names key.
|
||||
func yamlField(typ reflect.Type, key string) (reflect.StructField, bool) {
|
||||
for field := range typ.Fields() {
|
||||
name, _, _ := strings.Cut(field.Tag.Get("yaml"), ",")
|
||||
if name == key {
|
||||
return field, true
|
||||
}
|
||||
}
|
||||
|
||||
return reflect.StructField{}, false
|
||||
return ok && typ.Kind() == reflect.String
|
||||
}
|
||||
|
||||
// yamlSetInMapping resolves (creating if needed) the value node for the
|
||||
|
||||
@@ -237,6 +237,14 @@ func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
|
||||
value string
|
||||
reason string
|
||||
}{
|
||||
// Keys that no setting has, on a config that loads and on one
|
||||
// that does not.
|
||||
{configWithoutRecipients, "nosuchkey", "1",
|
||||
"no setting has this key: nosuchkey"},
|
||||
{configWithoutRecipients, "snapshots.home.exlude", "/tmp",
|
||||
"no setting has this key: snapshots.home.exlude"},
|
||||
{defaultConfigText(macOS), "compression_lvl", "9",
|
||||
"no setting has this key: compression_lvl"},
|
||||
// Index 1 of the empty list the missing key gives.
|
||||
{configWithoutRecipients, "age_recipients.1", testRecipient,
|
||||
"index out of range"},
|
||||
|
||||
Reference in new issue
Block a user