Skip to content

Commit

Permalink
feat: validate if extra fields present in the decoder
Browse files Browse the repository at this point in the history
This should address issues when the config is a valid yaml but contains
extra fields which may appear there if the indents got messed up somehow
for example.

Signed-off-by: Artem Chernyshev <artem.chernyshev@talos-systems.com>
  • Loading branch information
Unix4ever committed Aug 12, 2021
1 parent 5b57a98 commit 8ddbcc9
Show file tree
Hide file tree
Showing 3 changed files with 193 additions and 2 deletions.
7 changes: 7 additions & 0 deletions hack/release.toml
Expand Up @@ -99,6 +99,13 @@ Talos can be configued to use Kubernetes 1.21 or CAPI v0.4.x components can be u
description = """\
Added support for Equinix Metal IPs for the Talos virtual (shared) IP (option `equnixMetal` under `vip` in the machine configuration).
Talos automatically re-assigns IP using the Equinix Metal API when leadership changes.
"""

[notes.configuration]
title = "Machine Config Validation"
description = """\
Unknown keys in the machine config now make the config invalid,
so any attempt to apply/edit the configuration with the unknown keys will lead into an error.
"""

[make_deps]
Expand Down
98 changes: 97 additions & 1 deletion pkg/machinery/config/decoder/decoder.go
Expand Up @@ -13,6 +13,7 @@ import (
"gopkg.in/yaml.v3"

"github.com/talos-systems/talos/pkg/machinery/config"
"github.com/talos-systems/talos/pkg/machinery/config/encoder"
)

var (
Expand Down Expand Up @@ -95,7 +96,7 @@ func parse(source []byte) (decoded []interface{}, err error) {
}
}

//nolint:gocyclo
//nolint:gocyclo,cyclop
func decode(manifest *yaml.Node) (target interface{}, err error) {
var (
version string
Expand Down Expand Up @@ -136,6 +137,10 @@ func decode(manifest *yaml.Node) (target interface{}, err error) {
return nil, fmt.Errorf("deprecated decode: %w", err)
}

if err = validate(target, manifest); err != nil {
return nil, err
}

return target, nil
}
}
Expand Down Expand Up @@ -164,5 +169,96 @@ func decode(manifest *yaml.Node) (target interface{}, err error) {
return nil, fmt.Errorf("spec decode: %w", err)
}

if err = validate(target, spec); err != nil {
return nil, err
}

return target, nil
}

//nolint:gocyclo
func validate(target interface{}, spec *yaml.Node) error {
node, err := encoder.NewEncoder(target).Marshal()
if err != nil {
return err
}

src := map[string]interface{}{}
dst := map[string]interface{}{}

err = spec.Decode(src)
if err != nil {
return err
}

err = node.Decode(dst)
if err != nil {
return err
}

var checkUnknown func(interface{}, interface{}) interface{}

checkUnknown = func(left interface{}, right interface{}) interface{} {
switch v := left.(type) {
case map[string]interface{}:
r, ok := right.(map[string]interface{})
if !ok {
return "type mismatch"
}

unknownKeys := map[string]interface{}{}

for key, value := range v {
if _, ok := r[key]; !ok {
unknownKeys[key] = value

continue
}

if d := checkUnknown(value, r[key]); d != nil {
unknownKeys[key] = d
}
}

if len(unknownKeys) > 0 {
return unknownKeys
}
case []interface{}:
r, ok := right.([]interface{})
if !ok {
return "type mismatch"
}

if len(v) != len(r) {
return "slice length differs"
}

var unknownItems []interface{}

for i, item := range v {
if d := checkUnknown(item, r[i]); d != nil {
unknownItems = append(unknownItems, d)
}
}

if len(unknownItems) > 0 {
return unknownItems
}
}

return nil
}

diff := checkUnknown(src, dst)
if diff != nil {
var data []byte

if data, err = yaml.Marshal(diff); err != nil {
return fmt.Errorf("failed to marshal error summary %w", err)
}

return fmt.Errorf("unknown keys found during decoding:\n%s", string(data))
}

return nil
}
90 changes: 89 additions & 1 deletion pkg/machinery/config/decoder/decoder_test.go
Expand Up @@ -17,8 +17,17 @@ type Mock struct {
Test bool `yaml:"test"`
}

type MockV2 struct {
Slice []Mock `yaml:"slice"`
Map map[string]Mock `yaml:"map"`
}

func init() {
config.Register("mock", func(string) interface{} {
config.Register("mock", func(version string) interface{} {
if version == "v1alpha2" {
return &MockV2{}
}

return &Mock{}
})
}
Expand Down Expand Up @@ -137,6 +146,85 @@ spec:
want: nil,
wantErr: true,
},
{
name: "extra field",
fields: fields{
source: []byte(`---
kind: mock
version: v1alpha1
spec:
test: true
extra: fail
`),
},
want: nil,
wantErr: true,
},
{
name: "extra fields in map",
fields: fields{
source: []byte(`---
kind: mock
version: v1alpha2
spec:
map:
first:
test: true
extra: me
`),
},
want: nil,
wantErr: true,
},
{
name: "extra fields in slice",
fields: fields{
source: []byte(`---
kind: mock
version: v1alpha2
spec:
slice:
- test: true
not: working
more: extra
fields: here
`),
},
want: nil,
wantErr: true,
},
{
name: "valid nested",
fields: fields{
source: []byte(`---
kind: mock
version: v1alpha2
spec:
slice:
- test: true
map:
first:
test: true
second:
test: false
`),
},
want: []interface{}{
&MockV2{
Map: map[string]Mock{
"first": {
Test: true,
},
"second": {
Test: false,
},
},
Slice: []Mock{
{Test: true},
},
},
},
},
}

for _, tt := range tests {
Expand Down

0 comments on commit 8ddbcc9

Please sign in to comment.