Refuse config keys that no setting has (closes #419)
check / check (push) Failing after 19m39s
check / check (push) Failing after 19m39s
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. Keys that a
${...} reference brings in are checked once smartconfig has filled it
in, and named without a line. YAML merge keys and a top-level env
section, which smartconfig reads, 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
3a74896114
6 files changed
+341
-47
No files matched your search
@@ -9,6 +9,7 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -286,15 +287,36 @@ func Load(path string) (*Config, error) {
|
||||
}
|
||||
|
||||
// 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.
|
||||
// refuses a key that no setting has, applies default values for optional
|
||||
// fields, fills in ${...} references using smartconfig, and expands ~ in
|
||||
// paths.
|
||||
func Parse(data []byte) (*Config, error) {
|
||||
// smartconfig drops line numbers, so keys are checked in the data as
|
||||
// written.
|
||||
var root yaml.Node
|
||||
|
||||
err := yaml.Unmarshal(data, &root)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to parse config: %w", err)
|
||||
}
|
||||
|
||||
err = checkKeys(&root, reflect.TypeFor[Config](), "")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Load config using smartconfig for interpolation
|
||||
sc, err := smartconfig.NewFromReader(bytes.NewReader(data))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to load config file: %w", err)
|
||||
}
|
||||
|
||||
// A ${...} reference can bring in keys that are not in the file.
|
||||
err = checkFilledKeys(sc.Data())
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
cfg := &Config{
|
||||
// Set defaults
|
||||
BlobSizeLimit: defaultBlobSizeLimit,
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"reflect"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"gopkg.in/yaml.v3"
|
||||
)
|
||||
|
||||
// yamlMergeTag is the tag YAML gives a merge key (<<), which adds the keys
|
||||
// of the map it names to the map it sits in.
|
||||
const yamlMergeTag = "!!merge"
|
||||
|
||||
// envSection is the top-level key of a map of environment variables that
|
||||
// smartconfig sets while it loads the config. It is not a vaultik setting,
|
||||
// so its keys are not checked.
|
||||
const envSection = "env"
|
||||
|
||||
var errUnknownKey = errors.New("unknown config key")
|
||||
|
||||
// SettingType returns the type of the setting that the dotted key names in
|
||||
// Config, following the fields' yaml tags, as for s3.bucket or
|
||||
// snapshots.home.exclude.0. A snapshot name or a list index may be any
|
||||
// word. It returns false when no setting has that key.
|
||||
func SettingType(keys []string) (reflect.Type, bool) {
|
||||
typ := reflect.TypeFor[Config]()
|
||||
|
||||
for _, key := range keys {
|
||||
switch {
|
||||
case typ.Kind() == reflect.Map || typ.Kind() == reflect.Slice:
|
||||
typ = typ.Elem()
|
||||
case typ.Kind() == reflect.Struct:
|
||||
field, ok := yamlField(typ, key)
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
|
||||
typ = field.Type
|
||||
default:
|
||||
return nil, false
|
||||
}
|
||||
}
|
||||
|
||||
return typ, true
|
||||
}
|
||||
|
||||
// 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 && name != "-" {
|
||||
return field, true
|
||||
}
|
||||
}
|
||||
|
||||
return reflect.StructField{}, false
|
||||
}
|
||||
|
||||
// checkKeys returns an error naming the first key in node, a parsed config
|
||||
// or part of one at the dotted path, that no setting in typ has. The error
|
||||
// gives the key's line in the file, when it has one. A value of the wrong
|
||||
// type is left for the YAML decoder to report.
|
||||
func checkKeys(node *yaml.Node, typ reflect.Type, path string) error {
|
||||
switch node.Kind {
|
||||
case yaml.DocumentNode:
|
||||
for _, child := range node.Content {
|
||||
err := checkKeys(child, typ, path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
case yaml.AliasNode:
|
||||
return checkKeys(node.Alias, typ, path)
|
||||
case yaml.SequenceNode:
|
||||
if typ.Kind() != reflect.Slice {
|
||||
return nil
|
||||
}
|
||||
|
||||
for i, child := range node.Content {
|
||||
err := checkKeys(child, typ.Elem(), joinKey(path, strconv.Itoa(i)))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
case yaml.MappingNode:
|
||||
return checkMappingKeys(node, typ, path)
|
||||
case yaml.ScalarNode:
|
||||
// A scalar has no keys.
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// checkFilledKeys is checkKeys for data, the config after smartconfig has
|
||||
// filled in its ${...} references. A key that a reference brings in is not
|
||||
// in the file, and the copy that Encode makes has no line numbers.
|
||||
func checkFilledKeys(data map[string]any) error {
|
||||
var filled yaml.Node
|
||||
|
||||
err := filled.Encode(data)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to marshal config data: %w", err)
|
||||
}
|
||||
|
||||
return checkKeys(&filled, reflect.TypeFor[Config](), "")
|
||||
}
|
||||
|
||||
// checkMappingKeys is checkKeys for a map.
|
||||
func checkMappingKeys(node *yaml.Node, typ reflect.Type, path string) error {
|
||||
for i := 0; i+1 < len(node.Content); i += 2 {
|
||||
key, value := node.Content[i], node.Content[i+1]
|
||||
|
||||
if key.Tag == yamlMergeTag {
|
||||
// The value is one map or a list of maps.
|
||||
merged := []*yaml.Node{value}
|
||||
if value.Kind == yaml.SequenceNode {
|
||||
merged = value.Content
|
||||
}
|
||||
|
||||
for _, m := range merged {
|
||||
err := checkKeys(m, typ, path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
keyPath := joinKey(path, key.Value)
|
||||
if keyPath == envSection {
|
||||
continue
|
||||
}
|
||||
|
||||
var valueType reflect.Type
|
||||
|
||||
switch {
|
||||
case typ.Kind() == reflect.Map:
|
||||
valueType = typ.Elem()
|
||||
case typ.Kind() == reflect.Struct:
|
||||
field, ok := yamlField(typ, key.Value)
|
||||
if !ok && key.Line == 0 {
|
||||
// A key that a ${...} reference brought in has no line.
|
||||
return fmt.Errorf("%w %q", errUnknownKey, keyPath)
|
||||
}
|
||||
|
||||
if !ok {
|
||||
return fmt.Errorf("%w %q on line %d",
|
||||
errUnknownKey, keyPath, key.Line)
|
||||
}
|
||||
|
||||
valueType = field.Type
|
||||
default:
|
||||
return nil
|
||||
}
|
||||
|
||||
err := checkKeys(value, valueType, keyPath)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// joinKey appends key to the dotted path.
|
||||
func joinKey(path, key string) string {
|
||||
if path == "" {
|
||||
return key
|
||||
}
|
||||
|
||||
return path + "." + key
|
||||
}
|
||||
@@ -0,0 +1,115 @@
|
||||
package config //nolint:testpackage // checks the unexported errUnknownKey
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// keysTestConfig loads; each case below adds one line to it.
|
||||
const keysTestConfig = "storage_url: file:///tmp/vaultik-test-storage\n" +
|
||||
"s3:\n" +
|
||||
" region: us-east-1\n" +
|
||||
"snapshots:\n" +
|
||||
" test:\n" +
|
||||
" paths: [/tmp/vaultik-test-source]\n"
|
||||
|
||||
// TestParseRefusesUnknownKey checks that a key no setting has, at any
|
||||
// depth, fails the parse with an error naming the key and its line. A key
|
||||
// that a ${...} reference brings in is not in the file, so its error gives
|
||||
// no line.
|
||||
func TestParseRefusesUnknownKey(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
snapshotPath := filepath.Join(t.TempDir(), "snapshot.yml")
|
||||
|
||||
err := os.WriteFile(snapshotPath, []byte("other:\n"+
|
||||
" paths: [/tmp/vaultik-test-source]\n"+
|
||||
" exlude: [/tmp/cache]\n"), 0o600)
|
||||
if err != nil {
|
||||
t.Fatalf("write snapshot: %v", err)
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
yaml string
|
||||
want string
|
||||
}{
|
||||
{"top level", keysTestConfig + "compression_lvl: 9\n",
|
||||
`unknown config key "compression_lvl" on line 7`},
|
||||
{"in a snapshot", keysTestConfig + " exlude: [/tmp/cache]\n",
|
||||
`unknown config key "snapshots.test.exlude" on line 7`},
|
||||
{"in s3", strings.Replace(keysTestConfig, " region:",
|
||||
" bukcet: b\n region:", 1),
|
||||
`unknown config key "s3.bukcet" on line 3`},
|
||||
{"from a reference",
|
||||
keysTestConfig + " other: ${YAML:" + snapshotPath + ":other}\n",
|
||||
`unknown config key "snapshots.other.exlude"`},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
_, err := Parse([]byte(tt.yaml))
|
||||
if !errors.Is(err, errUnknownKey) {
|
||||
t.Fatalf("Parse() error = %v, want errUnknownKey", err)
|
||||
}
|
||||
|
||||
if err.Error() != tt.want {
|
||||
t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestParseAcceptsKnownKeys checks that a config with a ${...} reference
|
||||
// and a YAML merge key (<<) still parses.
|
||||
func TestParseAcceptsKnownKeys(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
levelPath := filepath.Join(t.TempDir(), "level")
|
||||
|
||||
err := os.WriteFile(levelPath, []byte("9"), 0o600)
|
||||
if err != nil {
|
||||
t.Fatalf("write level: %v", err)
|
||||
}
|
||||
|
||||
data := strings.Replace(keysTestConfig, " test:", " test: &test", 1) +
|
||||
" other:\n" +
|
||||
" <<: *test\n" +
|
||||
" exclude: [/tmp/vaultik-test-source/cache]\n" +
|
||||
"compression_level: ${FILE:" + levelPath + "}\n"
|
||||
|
||||
cfg, err := Parse([]byte(data))
|
||||
if err != nil {
|
||||
t.Fatalf("Parse() unexpected error: %v", err)
|
||||
}
|
||||
|
||||
if cfg.CompressionLevel != 9 {
|
||||
t.Errorf("compression_level = %d, want 9", cfg.CompressionLevel)
|
||||
}
|
||||
|
||||
other := cfg.Snapshots["other"]
|
||||
if len(other.Paths) != 1 || len(other.Exclude) != 1 {
|
||||
t.Errorf("snapshot other = %+v, want the paths of test and one exclude",
|
||||
other)
|
||||
}
|
||||
}
|
||||
|
||||
// TestParseAcceptsEnvSection checks that a config with a top-level env
|
||||
// section still parses. smartconfig sets the environment variables it
|
||||
// lists, so its keys are not settings.
|
||||
func TestParseAcceptsEnvSection(t *testing.T) {
|
||||
const name = "VAULTIK_TEST_ENV_SECTION"
|
||||
|
||||
// Parse sets the variable; t.Setenv restores it after the test.
|
||||
t.Setenv(name, "")
|
||||
|
||||
_, err := Parse([]byte(keysTestConfig + "env:\n " + name + ": set\n"))
|
||||
if err != nil {
|
||||
t.Fatalf("Parse() unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user