From b50fb3f3efb26b61181e3a826040b1088fda412a Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Sat, 28 Dec 2024 13:07:45 -0700 Subject: [PATCH] Fix handling of net.IP during node merge Because net.IP is derived from []byte, it was being handled by mergo as a slice. This adds an additional transformer that handles net.IP specifically, and also watches for this type in field tracking. Signed-off-by: Jonathon Anderson --- internal/pkg/node/mergo.go | 21 ++++-- internal/pkg/node/mergo_test.go | 124 ++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 5 deletions(-) diff --git a/internal/pkg/node/mergo.go b/internal/pkg/node/mergo.go index f76e6e50..a06e21a6 100644 --- a/internal/pkg/node/mergo.go +++ b/internal/pkg/node/mergo.go @@ -1,6 +1,7 @@ package node import ( + "net" "reflect" "strings" @@ -58,10 +59,18 @@ func (config *NodesYaml) appendProfileProfiles(profiles []string, id string) []s return profiles } -type InterfaceTransformer struct{} +type Transformer struct{} -func (t InterfaceTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error { - if typ.Kind() == reflect.Interface { +func (t Transformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error { + if typ == reflect.TypeOf(net.IP{}) { + return func(dst, src reflect.Value) error { + if !src.IsValid() || !src.CanSet() { + return nil + } + dst.Set(src) + return nil + } + } else if typ.Kind() == reflect.Interface { return func(dst, src reflect.Value) error { if !src.IsValid() || src.IsZero() { return nil @@ -159,7 +168,7 @@ func (config *NodesYaml) MergeNode(id string) (node Node, fields fieldMap, err e // // Returns an error if the merging operation fails. func merge(dest, src interface{}, fields fieldMap, srcName string, multipleSrcName string) error { - if err := mergo.Merge(dest, src, mergo.WithAppendSlice, mergo.WithOverride, mergo.WithTransformers(InterfaceTransformer{})); err != nil { + if err := mergo.Merge(dest, src, mergo.WithAppendSlice, mergo.WithOverride, mergo.WithTransformers(Transformer{})); err != nil { return err } @@ -170,7 +179,9 @@ func merge(dest, src interface{}, fields fieldMap, srcName string, multipleSrcNa if prevSource != "" { switch value.Kind() { case reflect.Slice: - srcName = strings.Join([]string{prevSource, multipleSrcName}, ",") + if value.Type() != reflect.TypeOf(net.IP{}) { + srcName = strings.Join([]string{prevSource, multipleSrcName}, ",") + } case reflect.Interface: if _, ok := value.Interface().([]interface{}); ok { srcName = strings.Join([]string{prevSource, multipleSrcName}, ",") diff --git a/internal/pkg/node/mergo_test.go b/internal/pkg/node/mergo_test.go index 14410791..5b27c218 100644 --- a/internal/pkg/node/mergo_test.go +++ b/internal/pkg/node/mergo_test.go @@ -1,6 +1,8 @@ package node import ( + "fmt" + "net" "testing" "github.com/stretchr/testify/assert" @@ -823,6 +825,128 @@ nodes: } } +func Test_MergeNodeIP(t *testing.T) { + tests := map[string]struct { + nodesConf string + node string + netdev string + ipaddr net.IP + ipaddrStr string + source string + }{ + "empty": { + nodesConf: ` +nodes: + n1: + network devices: + default: {} +`, + node: "n1", + netdev: "default", + ipaddr: nil, + ipaddrStr: "", + source: "", + }, + "single node value": { + nodesConf: ` +nodes: + n1: + network devices: + default: + ipaddr: 192.168.0.1 +`, + node: "n1", + netdev: "default", + ipaddr: net.IPv4(192, 168, 0, 1), + ipaddrStr: "192.168.0.1", + source: "", + }, + "profile value": { + nodesConf: ` +nodeprofiles: + p1: + network devices: + default: + ipaddr: 192.168.0.1 +nodes: + n1: + profiles: + - p1 +`, + node: "n1", + netdev: "default", + ipaddr: net.IPv4(192, 168, 0, 1), + ipaddrStr: "192.168.0.1", + source: "p1", + }, + "multi-profile value": { + nodesConf: ` +nodeprofiles: + p1: + network devices: + default: + ipaddr: 192.168.0.1 + p2: + network devices: + default: + ipaddr: 192.168.1.1 +nodes: + n1: + profiles: + - p1 + - p2 +`, + node: "n1", + netdev: "default", + ipaddr: net.IPv4(192, 168, 1, 1), + ipaddrStr: "192.168.1.1", + source: "p2", + }, + "superseded": { + nodesConf: ` +nodeprofiles: + p1: + network devices: + default: + ipaddr: 192.168.0.1 + p2: + network devices: + default: + ipaddr: 192.168.1.1 +nodes: + n1: + profiles: + - p1 + - p2 + network devices: + default: + ipaddr: 192.168.2.1 +`, + node: "n1", + netdev: "default", + ipaddr: net.IPv4(192, 168, 2, 1), + ipaddrStr: "192.168.2.1", + source: "SUPERSEDED", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + env := testenv.New(t) + defer env.RemoveAll() + env.WriteFile("/etc/warewulf/nodes.conf", tt.nodesConf) + + registry, regErr := New() + assert.NoError(t, regErr) + node, fields, mergeErr := registry.MergeNode(tt.node) + assert.NoError(t, mergeErr) + assert.Equal(t, tt.ipaddr, node.NetDevs[tt.netdev].Ipaddr) + assert.Equal(t, tt.ipaddrStr, fields.Value(fmt.Sprintf("NetDevs[%s].Ipaddr", tt.netdev))) + assert.Equal(t, tt.source, fields.Source(fmt.Sprintf("NetDevs[%s].Ipaddr", tt.netdev))) + }) + } +} + func Test_MergeNodeKernel(t *testing.T) { var tests = map[string]struct { nodesConf string