chore: disallow duplicate documents on decoder level
Required for #9275 Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com>
This commit is contained in:
parent
bcaf63628b
commit
cd7c682662
@ -135,7 +135,7 @@ func ControlPlane(defaultAction nethelpers.DefaultAction, cidrs []netip.Prefix,
|
|||||||
panic(err)
|
panic(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return configpatcher.StrategicMergePatch{Provider: provider}
|
return configpatcher.NewStrategicMergePatch(provider)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Worker generates a default firewall for a worker node.
|
// Worker generates a default firewall for a worker node.
|
||||||
@ -187,5 +187,5 @@ func Worker(defaultAction nethelpers.DefaultAction, cidrs []netip.Prefix, gatewa
|
|||||||
panic(err)
|
panic(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return configpatcher.StrategicMergePatch{Provider: provider}
|
return configpatcher.NewStrategicMergePatch(provider)
|
||||||
}
|
}
|
||||||
|
@ -6,6 +6,7 @@
|
|||||||
package decoder
|
package decoder
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"cmp"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
@ -47,6 +48,12 @@ func NewDecoder() *Decoder {
|
|||||||
return &Decoder{}
|
return &Decoder{}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type documentID struct {
|
||||||
|
APIVersion string
|
||||||
|
Kind string
|
||||||
|
Name string
|
||||||
|
}
|
||||||
|
|
||||||
func parse(r io.Reader) (decoded []config.Document, err error) {
|
func parse(r io.Reader) (decoded []config.Document, err error) {
|
||||||
// Recover from yaml.v3 panics because we rely on machine configuration loading _a lot_.
|
// Recover from yaml.v3 panics because we rely on machine configuration loading _a lot_.
|
||||||
defer func() {
|
defer func() {
|
||||||
@ -61,6 +68,8 @@ func parse(r io.Reader) (decoded []config.Document, err error) {
|
|||||||
|
|
||||||
dec.KnownFields(true)
|
dec.KnownFields(true)
|
||||||
|
|
||||||
|
knownDocuments := map[documentID]struct{}{}
|
||||||
|
|
||||||
// Iterate through all defined documents.
|
// Iterate through all defined documents.
|
||||||
for {
|
for {
|
||||||
var manifests yaml.Node
|
var manifests yaml.Node
|
||||||
@ -78,6 +87,18 @@ func parse(r io.Reader) (decoded []config.Document, err error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
for _, manifest := range manifests.Content {
|
for _, manifest := range manifests.Content {
|
||||||
|
id := documentID{
|
||||||
|
APIVersion: findValue(manifest, ManifestAPIVersionKey, false),
|
||||||
|
Kind: cmp.Or(findValue(manifest, ManifestKindKey, false), "v1alpha1"),
|
||||||
|
Name: findValue(manifest, "name", false),
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, ok := knownDocuments[id]; ok {
|
||||||
|
return nil, fmt.Errorf("duplicate document %s/%s/%s is not allowed", id.APIVersion, id.Kind, id.Name)
|
||||||
|
}
|
||||||
|
|
||||||
|
knownDocuments[id] = struct{}{}
|
||||||
|
|
||||||
var target config.Document
|
var target config.Document
|
||||||
|
|
||||||
if target, err = decode(manifest); err != nil {
|
if target, err = decode(manifest); err != nil {
|
||||||
@ -146,3 +167,24 @@ func decode(manifest *yaml.Node) (target config.Document, err error) {
|
|||||||
|
|
||||||
return target, nil
|
return target, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func findValue(node *yaml.Node, key string, required bool) string {
|
||||||
|
if node.Kind != yaml.MappingNode {
|
||||||
|
panic(errors.New("expected a mapping node"))
|
||||||
|
}
|
||||||
|
|
||||||
|
for i := 0; i < len(node.Content)-1; i += 2 {
|
||||||
|
keyNode := node.Content[i]
|
||||||
|
val := node.Content[i+1]
|
||||||
|
|
||||||
|
if keyNode.Kind == yaml.ScalarNode && keyNode.Value == key {
|
||||||
|
return val.Value
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if required {
|
||||||
|
panic(fmt.Errorf("missing '%s'", key))
|
||||||
|
}
|
||||||
|
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
@ -6,10 +6,12 @@ package decoder_test
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
|
"io/fs"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/siderolabs/gen/xtesting/must"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
@ -345,6 +347,18 @@ func TestDecoderV1Alpha1Config(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDoubleV1Alpha1(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
files := os.DirFS("testdata/double").(fs.ReadFileFS) //nolint:errcheck
|
||||||
|
contents := must.Value(files.ReadFile("v1alpha1.yaml"))(t)
|
||||||
|
|
||||||
|
d := decoder.NewDecoder()
|
||||||
|
_, err := d.Decode(bytes.NewReader(contents))
|
||||||
|
require.Error(t, err)
|
||||||
|
require.ErrorContains(t, err, "not allowed")
|
||||||
|
}
|
||||||
|
|
||||||
func BenchmarkDecoderV1Alpha1Config(b *testing.B) {
|
func BenchmarkDecoderV1Alpha1Config(b *testing.B) {
|
||||||
b.ReportAllocs()
|
b.ReportAllocs()
|
||||||
|
|
||||||
|
7
pkg/machinery/config/configloader/internal/decoder/testdata/double/v1alpha1.yaml
vendored
Normal file
7
pkg/machinery/config/configloader/internal/decoder/testdata/double/v1alpha1.yaml
vendored
Normal file
@ -0,0 +1,7 @@
|
|||||||
|
version: v1alpha1
|
||||||
|
machine:
|
||||||
|
type: controlplane
|
||||||
|
---
|
||||||
|
version: v1alpha1
|
||||||
|
machine:
|
||||||
|
type: worker
|
@ -20,10 +20,10 @@ type patch []map[string]any
|
|||||||
|
|
||||||
// LoadPatch loads the strategic merge patch or JSON patch (JSON/YAML for JSON patch).
|
// LoadPatch loads the strategic merge patch or JSON patch (JSON/YAML for JSON patch).
|
||||||
func LoadPatch(in []byte) (Patch, error) {
|
func LoadPatch(in []byte) (Patch, error) {
|
||||||
// try configloader first, it is more strict about config format
|
// Try configloader first, as it is more strict about the config format
|
||||||
cfg, strategicErr := configloader.NewFromBytes(in)
|
cfg, strategicErr := configloader.NewFromBytes(in)
|
||||||
if strategicErr == nil {
|
if strategicErr == nil {
|
||||||
return StrategicMergePatch{cfg}, nil
|
return NewStrategicMergePatch(cfg), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
var (
|
var (
|
||||||
|
@ -70,7 +70,7 @@ func TestLoadStrategic(t *testing.T) {
|
|||||||
p, ok := raw.(configpatcher.StrategicMergePatch)
|
p, ok := raw.(configpatcher.StrategicMergePatch)
|
||||||
require.True(t, ok)
|
require.True(t, ok)
|
||||||
|
|
||||||
assert.Equal(t, "foo.bar", p.Machine().Network().Hostname())
|
assert.Equal(t, "foo.bar", p.Provider().Machine().Network().Hostname())
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestLoadJSONPatches(t *testing.T) {
|
func TestLoadJSONPatches(t *testing.T) {
|
||||||
@ -106,6 +106,6 @@ func TestLoadMixedPatches(t *testing.T) {
|
|||||||
require.Len(t, patchList, 3)
|
require.Len(t, patchList, 3)
|
||||||
|
|
||||||
assert.IsType(t, jsonpatch.Patch{}, patchList[0])
|
assert.IsType(t, jsonpatch.Patch{}, patchList[0])
|
||||||
assert.IsType(t, configpatcher.StrategicMergePatch{}, patchList[1])
|
assert.Implements(t, (*configpatcher.StrategicMergePatch)(nil), patchList[1])
|
||||||
assert.IsType(t, jsonpatch.Patch{}, patchList[2])
|
assert.IsType(t, jsonpatch.Patch{}, patchList[2])
|
||||||
}
|
}
|
||||||
|
@ -14,8 +14,9 @@ import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
// StrategicMergePatch is a strategic merge config patch.
|
// StrategicMergePatch is a strategic merge config patch.
|
||||||
type StrategicMergePatch struct {
|
type StrategicMergePatch interface {
|
||||||
coreconfig.Provider
|
Documents() []config.Document
|
||||||
|
Provider() coreconfig.Provider
|
||||||
}
|
}
|
||||||
|
|
||||||
// StrategicMerge performs strategic merge config patching.
|
// StrategicMerge performs strategic merge config patching.
|
||||||
@ -55,3 +56,20 @@ func StrategicMerge(cfg coreconfig.Provider, patch StrategicMergePatch) (corecon
|
|||||||
|
|
||||||
return container.New(left...)
|
return container.New(left...)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// NewStrategicMergePatch creates a new strategic merge patch. deleteSelectors is a list of delete selectors, can be empty.
|
||||||
|
func NewStrategicMergePatch(cfg coreconfig.Provider) StrategicMergePatch {
|
||||||
|
return strategicMergePatch{provider: cfg}
|
||||||
|
}
|
||||||
|
|
||||||
|
type strategicMergePatch struct {
|
||||||
|
provider coreconfig.Provider
|
||||||
|
}
|
||||||
|
|
||||||
|
func (s strategicMergePatch) Documents() []config.Document {
|
||||||
|
return s.provider.Documents()
|
||||||
|
}
|
||||||
|
|
||||||
|
func (s strategicMergePatch) Provider() coreconfig.Provider { return s.provider }
|
||||||
|
|
||||||
|
var _ StrategicMergePatch = strategicMergePatch{}
|
||||||
|
@ -91,7 +91,9 @@ func NewV1Alpha1(config *v1alpha1.Config) *Container {
|
|||||||
// Clone the container.
|
// Clone the container.
|
||||||
//
|
//
|
||||||
// Cloned container is not readonly.
|
// Cloned container is not readonly.
|
||||||
func (container *Container) Clone() coreconfig.Provider {
|
func (container *Container) Clone() coreconfig.Provider { return container.clone() }
|
||||||
|
|
||||||
|
func (container *Container) clone() *Container {
|
||||||
return &Container{
|
return &Container{
|
||||||
v1alpha1Config: container.v1alpha1Config.DeepCopy(),
|
v1alpha1Config: container.v1alpha1Config.DeepCopy(),
|
||||||
documents: xslices.Map(container.documents, config.Document.Clone),
|
documents: xslices.Map(container.documents, config.Document.Clone),
|
||||||
@ -304,7 +306,7 @@ func (container *Container) Validate(mode validation.RuntimeMode, opt ...validat
|
|||||||
|
|
||||||
// RedactSecrets returns a copy of the Provider with all secrets replaced with the given string.
|
// RedactSecrets returns a copy of the Provider with all secrets replaced with the given string.
|
||||||
func (container *Container) RedactSecrets(replacement string) coreconfig.Provider {
|
func (container *Container) RedactSecrets(replacement string) coreconfig.Provider {
|
||||||
clone := container.Clone().(*Container) //nolint:forcetypeassert,errcheck
|
clone := container.clone()
|
||||||
|
|
||||||
if clone.v1alpha1Config != nil {
|
if clone.v1alpha1Config != nil {
|
||||||
clone.v1alpha1Config.Redact(replacement)
|
clone.v1alpha1Config.Redact(replacement)
|
||||||
|
Loading…
x
Reference in New Issue
Block a user