Skip to content

Commit

Permalink
fix: do not panic on invalid machine configs
Browse files Browse the repository at this point in the history 
Recover from yaml.v3 panics.
Refactor tests.

Signed-off-by: Alexey Palazhchenko <alexey.palazhchenko@talos-systems.com>
  • Loading branch information
@AlekSi
AlekSi committed Aug 26, 2021
1 parent c4048e2 commit 97da354
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 97 deletions.
21 changes: 13 additions & 8 deletions pkg/machinery/config/configloader/configloader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,21 @@ func TestSuite(t *testing.T) {
func (suite *Suite) SetupSuite() {}

func (suite *Suite) TestNew() {
for _, t := range []struct {
for _, tt := range []struct {
source []byte
errExpected bool
}{} {
_, err := newConfig(t.source)

if t.errExpected {
suite.Require().Error(err)
} else {
expectedErr string
}{
{
source: []byte(": \xea"),
expectedErr: "recovered: internal error: attempted to parse unknown event (please report): none",
},
} {
_, err := newConfig(tt.source)

if tt.expectedErr == "" {
suite.Require().NoError(err)
} else {
suite.Require().EqualError(err, tt.expectedErr)
}
}
}
7 changes: 7 additions & 0 deletions pkg/machinery/config/decoder/decoder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ func (d *Decoder) decode() ([]interface{}, error) {
}

func parse(source []byte) (decoded []interface{}, err error) {
// Recover from yaml.v3 panics because we rely on machine configuration loading _a lot_.
defer func() {
if p := recover(); p != nil {
err = fmt.Errorf("recovered: %v", p)
}
}()

decoded = []interface{}{}

r := bytes.NewReader(source)
Expand Down
159 changes: 70 additions & 89 deletions pkg/machinery/config/decoder/decoder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

//nolint:scopelint
package decoder_test

import (
"reflect"
"testing"

"github.com/stretchr/testify/assert"

"github.com/talos-systems/talos/pkg/machinery/config"
"github.com/talos-systems/talos/pkg/machinery/config/decoder"
"github.com/talos-systems/talos/pkg/machinery/config/types/v1alpha1"
Expand Down Expand Up @@ -44,138 +44,117 @@ func init() {
})
}

func TestDecoder_Decode(t *testing.T) {
type fields struct {
source []byte
}
func TestDecoder(t *testing.T) {
t.Parallel()

tests := []struct {
name string
fields fields
want []interface{}
wantErr bool
name string
source []byte
expected []interface{}
expectedErr string
}{
{
name: "valid",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha1
spec:
test: true
`),
},
want: []interface{}{
expected: []interface{}{
&Mock{
Test: true,
},
},
wantErr: false,
expectedErr: "",
},
{
name: "missing kind",
fields: fields{
source: []byte(`---
source: []byte(`---
version: v1alpha1
spec:
test: true
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "missing kind",
},
{
name: "empty kind",
fields: fields{
source: []byte(`---
source: []byte(`---
kind:
version: v1alpha1
spec:
test: true
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "missing kind",
},
{
name: "missing version",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
spec:
test: true
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "missing version",
},
{
name: "empty version",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version:
spec:
test: true
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "missing version",
},
{
name: "missing spec",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha1
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "missing spec",
},
{
name: "empty spec",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha1
spec:
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "missing spec content",
},
{
name: "tab instead of spaces",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha1
spec:
test: true
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "decode error: yaml: line 5: found character that cannot start any token",
},
{
name: "extra field",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha1
spec:
test: true
extra: fail
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "unknown keys found during decoding:\nextra: fail\n",
},
{
name: "extra fields in map",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha2
spec:
Expand All @@ -184,14 +163,12 @@ spec:
test: true
extra: me
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "unknown keys found during decoding:\nmap:\n first:\n extra: me\n",
},
{
name: "extra fields in slice",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha2
spec:
Expand All @@ -201,14 +178,12 @@ spec:
more: extra
fields: here
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "unknown keys found during decoding:\nslice:\n - fields: here\n more: extra\n not: working\n",
},
{
name: "extra zero fields in map",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha2
spec:
Expand All @@ -217,14 +192,12 @@ spec:
a:
b: {}
`),
},
want: nil,
wantErr: true,
expected: nil,
expectedErr: "unknown keys found during decoding:\nmap:\n second:\n a:\n b: {}\n",
},
{
name: "valid nested",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha2
spec:
Expand All @@ -236,8 +209,7 @@ spec:
second:
test: false
`),
},
want: []interface{}{
expected: []interface{}{
&MockV2{
Map: map[string]Mock{
"first": {
Expand All @@ -252,11 +224,11 @@ spec:
},
},
},
expectedErr: "",
},
{
name: "kubelet config",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: kubelet
version: v1alpha1
spec:
Expand All @@ -268,34 +240,43 @@ spec:
- rw
source: /var/local
`),
},
expected: nil,
expectedErr: "",
},
{
name: "omit empty test",
fields: fields{
source: []byte(`---
source: []byte(`---
kind: mock
version: v1alpha3
spec:
omit: false
`),
},
expected: nil,
expectedErr: "",
},
{
name: "internal error",
source: []byte(": \xea"),
expected: nil,
expectedErr: "recovered: internal error: attempted to parse unknown event (please report): none",
},
}

for _, tt := range tests {
tt := tt

t.Run(tt.name, func(t *testing.T) {
d := decoder.NewDecoder(tt.fields.source)
got, err := d.Decode()
if (err != nil) != tt.wantErr {
t.Errorf("Decoder.Decode() error = %v, wantErr %v", err, tt.wantErr)
t.Parallel()

return
d := decoder.NewDecoder(tt.source)
actual, err := d.Decode()
if tt.expected != nil {
assert.Equal(t, tt.expected, actual)
}
if tt.want != nil {
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Decoder.Decode() = %v, want %v", got, tt.want)
}
if tt.expectedErr == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, tt.expectedErr)
}
})
}
Expand Down

0 comments on commit 97da354

Please sign in to comment.