Skip to content

Commit

Permalink
fix: error with decoding config document with wrong apiVersion
Browse files Browse the repository at this point in the history
Fixes #8270

The base bug was that the registry will return `nil` `ConfigDocument` if
the version is not registered for a kind, which would result into weird
config decoding errors.

Add more unit-tests, while at it, also add more fuzzing samples.

Signed-off-by: Andrey Smirnov <[email protected]>
  • Loading branch information
smira committed Feb 8, 2024
1 parent 1e77bb1 commit 013e130
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 10 deletions.
2 changes: 0 additions & 2 deletions pkg/machinery/config/configloader/configloader_fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// 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/.

//go:build go1.18

package configloader_test

import (
Expand Down
2 changes: 2 additions & 0 deletions pkg/machinery/config/configloader/configloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ func callMethods(t testing.TB, obj reflect.Value, chain ...string) {
fallthrough
case "Doc":
fallthrough
case "APIUrl":
fallthrough
case "Endpoint":
// t.Logf("Skipping %v", nextChain)
continue
Expand Down
13 changes: 13 additions & 0 deletions pkg/machinery/config/configloader/internal/decoder/decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,19 @@ apiVersion: v1alpha3
omit: false
`),
},
{
name: "misspelled apiVersion",
source: []byte(`---
apiversion: v1alpha1
kind: ExtensionServicesConfig
config:
- name: nut-client
configFiles:
- content: MONITOR ${upsmonHost} 1 remote pass foo
mountPath: /usr/local/etc/nut/upsmon.conf
`),
expectedErr: "\"ExtensionServicesConfig\" \"\": not registered",
},
}

for _, tt := range tests {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("apiVersion: v1alpha1\nkind: SideroLinkConfig")
7 changes: 7 additions & 0 deletions pkg/machinery/config/configloader/testdata/multidoc2.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: v1alpha1
kind: ExtensionServicesConfig
config:
- name: foo
configFiles:
- content: hello
mountPath: /etc/foo
26 changes: 26 additions & 0 deletions pkg/machinery/config/configloader/testdata/multidoc3.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
apiVersion: v1alpha1
kind: NetworkDefaultActionConfig
ingress: block
---
apiVersion: v1alpha1
kind: NetworkRuleConfig
name: test
portSelector:
ports:
- 53
- 8000-9000
protocol: udp
ingress:
- subnet: 192.168.0.0/16
except: 192.168.0.3/32
- subnet: 2001::/16
---
apiVersion: v1alpha1
kind: NetworkRuleConfig
name: www
portSelector:
ports:
- 80
protocol: tcp
ingress:
- subnet: 192.168.0.0/16
29 changes: 21 additions & 8 deletions pkg/machinery/config/internal/registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,35 @@ var (
// NewDocumentFunc represents a function that creates a new document by version.
type NewDocumentFunc func(version string) config.Document

var registry = &Registry{
registered: map[string]NewDocumentFunc{},
}
var registry = NewRegistry()

// Registry represents the document kind/version registry.
//
// Global registry is available via top-level functions Register and New.
type Registry struct {
m sync.Mutex
registered map[string]NewDocumentFunc
}

// NewRegistry creates a new registry.
func NewRegistry() *Registry {
return &Registry{
registered: map[string]NewDocumentFunc{},
}
}

// Register registers a manifests with the registry.
func Register(kind string, f NewDocumentFunc) {
registry.register(kind, f)
registry.Register(kind, f)
}

// New creates a new instance of the requested manifest.
func New(kind, version string) (config.Document, error) {
return registry.new(kind, version)
return registry.New(kind, version)
}

func (r *Registry) register(kind string, f NewDocumentFunc) {
// Register registers a document kind with the registry.
func (r *Registry) Register(kind string, f NewDocumentFunc) {
r.m.Lock()
defer r.m.Unlock()

Expand All @@ -54,13 +62,18 @@ func (r *Registry) register(kind string, f NewDocumentFunc) {
r.registered[kind] = f
}

func (r *Registry) new(kind, version string) (config.Document, error) {
// New creates a new instance of the requested document.
func (r *Registry) New(kind, version string) (config.Document, error) {
r.m.Lock()
defer r.m.Unlock()

f, ok := r.registered[kind]
if ok {
return f(version), nil
doc := f(version)

if doc != nil {
return doc, nil
}
}

return nil, fmt.Errorf("%q %q: %w", kind, version, ErrNotRegistered)
Expand Down
77 changes: 77 additions & 0 deletions pkg/machinery/config/internal/registry/registry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// 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/.

package registry_test

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/siderolabs/talos/pkg/machinery/config/config"
"github.com/siderolabs/talos/pkg/machinery/config/internal/registry"
)

type mockDocument struct {
kind, version string
}

func (d mockDocument) Clone() config.Document {
return d
}

func (d mockDocument) Kind() string {
return d.kind
}

func (d mockDocument) APIVersion() string {
return d.version
}

func mockFactory(kind, version string) registry.NewDocumentFunc {
return func(requestedVersion string) config.Document {
if requestedVersion == version {
return mockDocument{kind, version}
}

return nil
}
}

func TestRegistry(t *testing.T) {
r := registry.NewRegistry()

// register document types
r.Register("kind1", mockFactory("kind1", "v1alpha1"))
r.Register("kind2", mockFactory("kind2", "v1alpha1"))

// register duplicate kind
assert.Panics(t, func() {
r.Register("kind1", mockFactory("kind1", "v1alpha3"))
})

// attempt to get unregistered kind
_, err := r.New("unknownKind", "unknownVersion")
require.Error(t, err)
assert.ErrorIs(t, err, registry.ErrNotRegistered)
assert.EqualError(t, err, "\"unknownKind\" \"unknownVersion\": not registered")

// successful creation of documents
d, err := r.New("kind1", "v1alpha1")
require.NoError(t, err)
assert.Equal(t, "kind1", d.Kind())
assert.Equal(t, "v1alpha1", d.APIVersion())

d, err = r.New("kind2", "v1alpha1")
require.NoError(t, err)
assert.Equal(t, "kind2", d.Kind())
assert.Equal(t, "v1alpha1", d.APIVersion())

// attempt get registered kind, but wrong version
_, err = r.New("kind1", "unknownVersion")
require.Error(t, err)
assert.ErrorIs(t, err, registry.ErrNotRegistered)
assert.EqualError(t, err, "\"kind1\" \"unknownVersion\": not registered")
}

0 comments on commit 013e130

Please sign in to comment.