Refactor GetNode merging with new MergeNode method

- Adds many tests for node+profile merging
- Ensures that tracked fields match mergo results

Signed-off-by: Jonathon Anderson <janderson@ciq.com>

internal/pkg/node/constructors.go

@@ -1,12 +1,9 @@
 package node
 
 import (
-	"bytes"
-	"encoding/gob"
 	"os"
 	"sort"
 
-	"dario.cat/mergo"
 	warewulfconf "github.com/warewulf/warewulf/internal/pkg/config"
 	"github.com/warewulf/warewulf/internal/pkg/wwlog"
 
@@ -51,51 +48,7 @@ func Parse(data []byte) (nodeList NodesYaml, err error) {
 Get a node with its merged in nodes
 */
 func (config *NodesYaml) GetNode(id string) (node Node, err error) {
-	if _, ok := config.Nodes[id]; !ok {
-		return node, ErrNotFound
-	}
-	node = EmptyNode()
-	// create a deep copy of the node, as otherwise pointers
-	// and not their contents is merged
-	var buf bytes.Buffer
-	enc := gob.NewEncoder(&buf)
-	dec := gob.NewDecoder(&buf)
-	err = enc.Encode(config.Nodes[id])
-	if err != nil {
-		return node, err
-	}
-	err = dec.Decode(&node)
-	if err != nil {
-		return node, err
-	}
-	for _, p := range cleanList(config.Nodes[id].Profiles) {
-		includedProfile, err := config.GetProfile(p)
-		if err != nil {
-			wwlog.Warn("profile not found: %s", p)
-			continue
-		}
-		err = mergo.Merge(&node.Profile, includedProfile, mergo.WithAppendSlice)
-		if err != nil {
-			return node, err
-		}
-	}
-	// finally set no exported values
-	node.id = id
-	node.valid = true
-	if netdev, ok := node.NetDevs[node.PrimaryNetDev]; ok {
-		netdev.primary = true
-	} else {
-		keys := make([]string, 0)
-		for k := range node.NetDevs {
-			keys = append(keys, k)
-		}
-		sort.Strings(keys)
-		if len(keys) > 0 {
-			wwlog.Debug("%s: no primary defined, sanitizing to: %s", id, keys[0])
-			node.NetDevs[keys[0]].primary = true
-			node.PrimaryNetDev = keys[0]
-		}
-	}
+	node, _, err = config.MergeNode(id)
 	wwlog.Debug("constructed node: %s", id)
 	return
 }

internal/pkg/node/constructors_test.go

@@ -111,12 +111,12 @@ func Test_Primary_Network(t *testing.T) {
 		}
 	})
 	t.Run("defined in profile", func(t *testing.T) {
-		assert.Equal(t, test_node5.NetDevs["override"].Device, "ib0")
-		assert.Equal(t, test_node5.NetDevs["override"].Type, "profile")
+		assert.Equal(t, "ib0", test_node5.NetDevs["override"].Device)
+		assert.Equal(t, "profile", test_node5.NetDevs["override"].Type)
 	})
 	t.Run("redefined in profile", func(t *testing.T) {
-		assert.Equal(t, test_node6.NetDevs["override"].Device, "ib1")
-		assert.Equal(t, test_node6.NetDevs["override"].Type, "profile")
+		assert.Equal(t, "ib1", test_node6.NetDevs["override"].Device)
+		assert.Equal(t, "profile", test_node6.NetDevs["override"].Type)
 	})
 }
 

internal/pkg/node/fields.go

@@ -0,0 +1,266 @@
+package node
+
+import (
+	"fmt"
+	"net"
+	"reflect"
+	"regexp"
+	"sort"
+	"strings"
+)
+
+// Field represents a single attribute field (typically to be used with a Node or a Profile),
+// including the field's name, the source of the value (such as a node or profile ID), and
+// the actual string value.
+//
+// It is primarily used to provide desired output for `wwctl <node|profile> list -a`.
+type Field struct {
+	Field  string
+	Source string
+	Value  string
+}
+
+// Set updates the field with the given source and value. If the value is empty, the operation
+// is skipped. If the field already has a source, and the new source is empty, the previous source
+// is marked as "SUPERSEDED" to indicate it was overridden (typically by a node's local
+// configuration).
+func (f *Field) Set(src, val string) {
+	if val == "" {
+		return
+	}
+	f.Value = val
+
+	if f.Source != "" && src == "" {
+		f.Source = "SUPERSEDED"
+	} else {
+		f.Source = src
+	}
+}
+
+// fieldMap maps field names to Field objects. This structure is used to track and manage
+// multiple fields, along with their sources and values, particularly by MergeNode.
+type fieldMap map[string]*Field
+
+// Set updates the correct field in the fieldMap with the given source and value.
+// If the field does not already exist in the fieldMap, it is created.
+func (fields fieldMap) Set(name, source, value string) {
+	if fields[name] == nil {
+		fields[name] = &Field{Field: name}
+	}
+	fields[name].Set(source, value)
+}
+
+// Source returns the source of the given field name if it exists in the map. If the field does
+// not exist, an empty string is returned.
+func (fields fieldMap) Source(name string) string {
+	if field, ok := fields[name]; ok {
+		return field.Source
+	}
+	return ""
+}
+
+// Value returns the value of the given field name if it exists in the map. If the field does
+// not exist, an empty string is returned.
+func (fields fieldMap) Value(name string) string {
+	if field, ok := fields[name]; ok {
+		return field.Value
+	}
+	return ""
+}
+
+// List returns a slice of Field structs for all fields that exist in the fieldMap, in the
+// order they are defined on the provided object. This ensures a consistent ordering of fields
+// for display purposes.
+func (fields fieldMap) List(obj interface{}) (output []Field) {
+	for _, name := range listFields(obj) {
+		if field, ok := fields[name]; ok {
+			output = append(output, *field)
+		}
+	}
+	return output
+}
+
+// GetFieldList extracts all fields from the provided object and returns them as a slice of Fields.
+// Each Field includes the field name and its string value. Fields that cannot be retrieved
+// or converted are skipped.
+func GetFieldList(obj interface{}) (fields []Field) {
+	for _, name := range listFields(obj) {
+		if value, err := getNestedFieldString(obj, name); err == nil {
+			fields = append(fields, Field{Field: name, Value: value})
+		}
+	}
+	return fields
+}
+
+var mapFieldElement *regexp.Regexp
+
+func init() {
+	// mapFieldElement matches map-indexed fields like "FieldName[Key]" to split into (FieldName, Key).
+	mapFieldElement = regexp.MustCompile(`^([^[]+)\[([^\]]+)\]$`)
+}
+
+// getNestedFieldValue retrieves the reflect.Value of a nested field.
+//
+// Supported syntax:
+// - Struct fields identified by a dotted path name (Struct.field)
+// - Map keys identified by square brackets (Map[key])
+//
+// Pointers are automatically dereferenced.
+//
+// If any element of the path does not exist, an error is returned.
+func getNestedFieldValue(obj interface{}, name string) (value reflect.Value, err error) {
+	value = reflect.ValueOf(obj)
+	if value.Kind() == reflect.Pointer {
+		value = value.Elem()
+	}
+
+	fieldNames := strings.Split(name, ".")
+	for _, fieldName := range fieldNames {
+		var key string
+		fieldName, key = parseMapField(fieldName)
+		if value.Kind() == reflect.Pointer {
+			if value.IsNil() {
+				err = fmt.Errorf("no value: %v", name)
+				return
+			}
+			value = value.Elem()
+		}
+		value = value.FieldByName(fieldName)
+		if key != "" {
+			value = value.MapIndex(reflect.ValueOf(key))
+			if !value.IsValid() {
+				err = fmt.Errorf("no value: %v", name)
+				return
+			}
+		}
+	}
+	return
+}
+
+// getNestedFieldString retrieves the string representation
+// of a nested field as returned by getNestedFieldValue.
+//
+// Returns an error if the field does not exist or cannot be retrieved.
+func getNestedFieldString(obj interface{}, name string) (string, error) {
+	if value, err := getNestedFieldValue(obj, name); err != nil {
+		return "", err
+	} else {
+		return valueStr(value), nil
+	}
+}
+
+// parseMapField extracts the map key if the field name represents a map access (e.g. "Fields[key]" returns "Fields", "key").
+// If there is no key specified, it simply returns the field name as is.
+func parseMapField(name string) (field, key string) {
+	if matches := mapFieldElement.FindStringSubmatch(name); matches != nil {
+		return matches[1], matches[2]
+	}
+	return name, ""
+}
+
+// listFields returns a slice of strings representing all exported, visible fields of the given
+// object's type, including nested fields in structs and keys in maps.
+//
+// Generated syntax:
+// - Struct fields identified by a dotted path name (Struct.field)
+// - Map keys identified by square brackets (Map[key])
+//
+// Pointers are transparently dereferenced and are not represented in the generated field name.
+func listFields(obj interface{}) (fields []string) {
+	return listReflectedFields(reflect.TypeOf(obj), reflect.ValueOf(obj), "")
+}
+
+// listReflectedFields recursively traverses the structure defined by reflect.Type and reflect.Value
+// to discover field paths. It supports struct fields, pointer fields, and map fields (with keys).
+// Fields are returned as their dotted paths. For map fields, keys are included as "[key]" segments.
+//
+// See listFields and getNestedFieldValue for more information.
+func listReflectedFields(t reflect.Type, v reflect.Value, prefix string) (fields []string) {
+	for _, field := range reflect.VisibleFields(t) {
+		if !field.IsExported() || field.Anonymous {
+			continue
+		}
+		fieldType := field.Type
+		fieldValue := reflect.Value{}
+		if v.IsValid() {
+			fieldValue = v.FieldByName(field.Name)
+		}
+		if fieldType.Kind() == reflect.Pointer {
+			fieldType = fieldType.Elem()
+			fieldValue = fieldValue.Elem()
+		}
+		if fieldType.Kind() == reflect.Struct {
+			fields = append(fields, listReflectedFields(fieldType, fieldValue, fmt.Sprintf("%v%v.", prefix, field.Name))...)
+		} else if fieldType.Kind() == reflect.Map {
+			if !fieldValue.IsValid() {
+				continue
+			}
+			keys := fieldValue.MapKeys()
+			sortValues(keys)
+			for _, key := range keys {
+				elementType := fieldType.Elem()
+				elementValue := fieldValue.MapIndex(key)
+				if elementType.Kind() == reflect.Pointer {
+					elementType = elementType.Elem()
+					if elementValue.IsValid() {
+						elementValue = elementValue.Elem()
+					}
+				}
+				if elementType.Kind() == reflect.Struct {
+					fields = append(fields, listReflectedFields(elementType, elementValue, fmt.Sprintf("%v%v[%v].", prefix, field.Name, key.String()))...)
+				} else {
+					fields = append(fields, fmt.Sprintf("%v%v[%v]", prefix, field.Name, key.String()))
+				}
+			}
+		} else {
+			fields = append(fields, prefix+field.Name)
+		}
+	}
+	return
+}
+
+// valueStr converts a reflect.Value into a string. For slices, it joins all elements into a comma-separated
+// string. For pointers, it returns an empty string if the pointer is nil. For other kinds, it returns the
+// formatted string representation of the value.
+func valueStr(value reflect.Value) (output string) {
+	if !value.IsValid() {
+		return ""
+	}
+	if value.Kind() == reflect.Pointer {
+		if value.IsZero() {
+			return ""
+		}
+		value = value.Elem()
+	}
+	if value.Type() == reflect.TypeOf(net.IP{}) {
+		if !value.IsZero() {
+			output = fmt.Sprintf("%s", value)
+		}
+	} else if value.Kind() == reflect.Slice {
+		var sliceStrs []string
+		for i := 0; i < value.Len(); i++ {
+			sliceStrs = append(sliceStrs, fmt.Sprintf("%v", value.Index(i)))
+		}
+		output = strings.Join(sliceStrs, ",")
+	} else {
+		output = fmt.Sprintf("%s", value)
+	}
+	return output
+}
+
+// sortValues sorts a slice of reflect.Values. Currently, it only supports sorting string values and
+// will panic if values of any other kind are encountered. Values of different kinds also cannot be sorted.
+func sortValues(values []reflect.Value) {
+	sort.Slice(values, func(i, j int) bool {
+		a, b := values[i], values[j]
+		if a.Kind() != b.Kind() {
+			panic(fmt.Sprintf("cannot sort values of different kinds: %s, %s", a.Kind(), b.Kind()))
+		}
+		switch a.Kind() {
+		case reflect.String:
+			return a.String() < b.String()
+		default:
+			panic(fmt.Sprintf("unsupported kind: %s", a.Kind()))
+		}
+	})
+}

internal/pkg/node/fields_test.go

@@ -0,0 +1,258 @@
+package node
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/warewulf/warewulf/internal/pkg/testenv"
+)
+
+func Test_getNestedFieldString(t *testing.T) {
+	var tests = map[string]struct {
+		nodesConf string
+		node      string
+		field     string
+		value     string
+	}{
+		"comment (simple)": {
+			nodesConf: `
+nodes:
+  n1:
+    comment: n1 comment`,
+			node:  "n1",
+			field: "Comment",
+			value: "n1 comment",
+		},
+		"kernel args (struct)": {
+			nodesConf: `
+nodes:
+  n1:
+    kernel:
+      args: n1 args`,
+			node:  "n1",
+			field: "Kernel.Args",
+			value: "n1 args",
+		},
+		"node tag (map)": {
+			nodesConf: `
+nodes:
+  n1:
+    tags:
+      tag: n1 tag`,
+			node:  "n1",
+			field: "Tags[tag]",
+			value: "n1 tag",
+		},
+		"system overlay (slice)": {
+			nodesConf: `
+nodes:
+  n1:
+    system overlay:
+    - no1
+    - no2`,
+			node:  "n1",
+			field: "SystemOverlay",
+			value: "no1,no2",
+		},
+		"netdev tag (map to struct)": {
+			nodesConf: `
+nodes:
+  n1:
+    network devices:
+      default:
+        tags:
+          tag: n1 netdev tag`,
+			node:  "n1",
+			field: "NetDevs[default].Tags[tag]",
+			value: "n1 netdev tag",
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			env := testenv.New(t)
+			defer env.RemoveAll(t)
+			env.WriteFile(t, "/etc/warewulf/nodes.conf", tt.nodesConf)
+
+			registry, regErr := New()
+			assert.NoError(t, regErr)
+			node := registry.Nodes[tt.node]
+			value, err := getNestedFieldString(node, tt.field)
+			assert.NoError(t, err)
+			assert.Equal(t, tt.value, value)
+		})
+	}
+}
+
+func Test_listFields(t *testing.T) {
+	var tests = map[string]struct {
+		object interface{}
+		fields []string
+	}{
+		"node": {
+			object: Node{
+				Profile: Profile{
+					Tags: map[string]string{
+						"tag": "value",
+					},
+					NetDevs: map[string]*NetDev{
+						"default": &NetDev{
+							Tags: map[string]string{
+								"nettag": "netvalue",
+							},
+						},
+					},
+				},
+			},
+			fields: []string{
+				"Discoverable",
+				"AssetKey",
+				"Profiles",
+				"Comment",
+				"ClusterName",
+				"ContainerName",
+				"Ipxe",
+				"RuntimeOverlay",
+				"SystemOverlay",
+				"Kernel.Version",
+				"Kernel.Args",
+				"Ipmi.UserName",
+				"Ipmi.Password",
+				"Ipmi.Ipaddr",
+				"Ipmi.Gateway",
+				"Ipmi.Netmask",
+				"Ipmi.Port",
+				"Ipmi.Interface",
+				"Ipmi.EscapeChar",
+				"Ipmi.Write",
+				"Ipmi.Template",
+				"Init",
+				"Root",
+				"NetDevs[default].Type",
+				"NetDevs[default].OnBoot",
+				"NetDevs[default].Device",
+				"NetDevs[default].Hwaddr",
+				"NetDevs[default].Ipaddr",
+				"NetDevs[default].Ipaddr6",
+				"NetDevs[default].Prefix",
+				"NetDevs[default].Netmask",
+				"NetDevs[default].Gateway",
+				"NetDevs[default].MTU",
+				"NetDevs[default].Tags[nettag]",
+				"Tags[tag]",
+				"PrimaryNetDev",
+			},
+		},
+		"profile": {
+			object: Profile{
+				Tags: map[string]string{
+					"tag": "value",
+				},
+				NetDevs: map[string]*NetDev{
+					"default": &NetDev{
+						Tags: map[string]string{
+							"nettag": "netvalue",
+						},
+					},
+				},
+			},
+			fields: []string{
+				"Comment",
+				"ClusterName",
+				"ContainerName",
+				"Ipxe",
+				"RuntimeOverlay",
+				"SystemOverlay",
+				"Kernel.Version",
+				"Kernel.Args",
+				"Ipmi.UserName",
+				"Ipmi.Password",
+				"Ipmi.Ipaddr",
+				"Ipmi.Gateway",
+				"Ipmi.Netmask",
+				"Ipmi.Port",
+				"Ipmi.Interface",
+				"Ipmi.EscapeChar",
+				"Ipmi.Write",
+				"Ipmi.Template",
+				"Init",
+				"Root",
+				"NetDevs[default].Type",
+				"NetDevs[default].OnBoot",
+				"NetDevs[default].Device",
+				"NetDevs[default].Hwaddr",
+				"NetDevs[default].Ipaddr",
+				"NetDevs[default].Ipaddr6",
+				"NetDevs[default].Prefix",
+				"NetDevs[default].Netmask",
+				"NetDevs[default].Gateway",
+				"NetDevs[default].MTU",
+				"NetDevs[default].Tags[nettag]",
+				"Tags[tag]",
+				"PrimaryNetDev",
+			},
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert.Equal(t, tt.fields, listFields(tt.object))
+		})
+	}
+}
+
+func Test_Field(t *testing.T) {
+	field := new(Field)
+	assert.Equal(t, "", field.Field)
+	assert.Equal(t, "", field.Source)
+	assert.Equal(t, "", field.Value)
+
+	field.Field = "test"
+	assert.Equal(t, "test", field.Field)
+
+	field.Set("", "value1")
+	assert.Equal(t, "", field.Source)
+	assert.Equal(t, "value1", field.Value)
+
+	field.Set("", "value2")
+	assert.Equal(t, "", field.Source)
+	assert.Equal(t, "value2", field.Value)
+
+	field.Set("source3", "value3")
+	assert.Equal(t, "source3", field.Source)
+	assert.Equal(t, "value3", field.Value)
+
+	field.Set("source4", "value4")
+	assert.Equal(t, "source4", field.Source)
+	assert.Equal(t, "value4", field.Value)
+
+	field.Set("", "value5")
+	assert.Equal(t, "SUPERSEDED", field.Source)
+	assert.Equal(t, "value5", field.Value)
+}
+
+func Test_fieldMap(t *testing.T) {
+	fieldMap := make(fieldMap)
+	assert.Equal(t, 0, len(fieldMap))
+
+	fieldMap.Set("field", "", "value1")
+	assert.Equal(t, "", fieldMap.Source("field"))
+	assert.Equal(t, "value1", fieldMap.Value("field"))
+
+	fieldMap.Set("field", "", "value2")
+	assert.Equal(t, "", fieldMap.Source("field"))
+	assert.Equal(t, "value2", fieldMap.Value("field"))
+
+	fieldMap.Set("field", "source3", "value3")
+	assert.Equal(t, "source3", fieldMap.Source("field"))
+	assert.Equal(t, "value3", fieldMap.Value("field"))
+
+	fieldMap.Set("field", "source4", "value4")
+	assert.Equal(t, "source4", fieldMap.Source("field"))
+	assert.Equal(t, "value4", fieldMap.Value("field"))
+
+	fieldMap.Set("field", "", "value5")
+	assert.Equal(t, "SUPERSEDED", fieldMap.Source("field"))
+	assert.Equal(t, "value5", fieldMap.Value("field"))
+}

internal/pkg/node/list.go

@@ -1,179 +0,0 @@
-package node
-
-import (
-	"fmt"
-	"net"
-	"reflect"
-	"sort"
-	"strconv"
-	"strings"
-)
-
-/*
-struct to hold the fields of GetFields
-*/
-type NodeFields struct {
-	Field  string
-	Source string
-	Value  string
-}
-
-func (f *NodeFields) Set(src, val string) {
-	if val == "" {
-		return
-	}
-	if f.Value == "" {
-		f.Value = val
-		f.Source = src
-	} else if f.Source != "" {
-		f.Value = val
-		if src == "" {
-			f.Source = "SUPERSEDED"
-		} else {
-			f.Source = src
-		}
-	}
-
-}
-
-type fieldMap map[string]*NodeFields
-
-/*
-Get all the info out of NodeConf. If emptyFields is set true, all fields are shown not only the ones with effective values
-*/
-func (nodeYml *NodesYaml) GetFields(node Node) (output []NodeFields) {
-	nodeMap := make(fieldMap)
-	for _, p := range node.Profiles {
-		if profile, ok := nodeYml.NodeProfiles[p]; ok {
-			nodeMap.importFields(profile, "", p)
-		}
-	}
-	rawNode, _ := nodeYml.GetNodeOnlyPtr(node.id)
-	nodeMap.importFields(rawNode, "", "")
-	for _, elem := range nodeMap {
-		if elem.Value != "" {
-			output = append(output, *elem)
-		}
-	}
-	sort.Slice(output, func(i, j int) bool {
-		return output[i].Field < output[j].Field
-	})
-	return output
-}
-
-/*
-Get all the info out of ProfileConf. If emptyFields is set true, all fields are shown not only the ones with effective values
-*/
-func (nodeYml *NodesYaml) GetFieldsProfile(profile Profile) (output []NodeFields) {
-	profileMap := make(fieldMap)
-	profileMap.importFields(&profile, "", "")
-	for _, elem := range profileMap {
-		if elem.Value != "" {
-			output = append(output, *elem)
-		}
-	}
-	sort.Slice(output, func(i, j int) bool {
-		return output[i].Field < output[j].Field
-	})
-	return output
-}
-
-/*
-Internal function which travels through all fields of a NodeConf and for this
-reason needs to be called via interface{}
-*/
-func (fieldMap fieldMap) importFields(obj interface{}, prefix string, source string) {
-	objValue := reflect.ValueOf(obj)
-	objType := reflect.TypeOf(obj)
-	if objValue.IsNil() {
-		return
-	}
-	for i := 0; i < objType.Elem().NumField(); i++ {
-		fieldValue := objValue.Elem().Field(i)
-		field := objType.Elem().Field(i)
-		if !fieldValue.IsValid() || !field.IsExported() {
-			continue
-		}
-		switch field.Type.Kind() {
-		case reflect.Map:
-			mapIter := fieldValue.MapRange()
-			for mapIter.Next() {
-				if mapIter.Value().Kind() == reflect.String {
-					key := fmt.Sprintf("%s%s[%s]", prefix, field.Name, mapIter.Key().String())
-					fieldMap[key] = &NodeFields{
-						Field:  key,
-						Source: source,
-						Value:  mapIter.Value().String(),
-					}
-				} else {
-					newPrefix := fmt.Sprintf("%s%s[%s].", prefix, field.Name, mapIter.Key().String())
-					fieldMap.importFields(mapIter.Value().Interface(), newPrefix, source)
-				}
-			}
-			if fieldValue.Len() == 0 {
-				key := fmt.Sprintf("%s%s[]", prefix, field.Name)
-				fieldMap[key] = &NodeFields{
-					Field: key,
-				}
-			}
-		case reflect.Struct: // inherited fields from a sub-entity
-			fieldMap.importFields(fieldValue.Addr().Interface(), "", source)
-		case reflect.Ptr:
-			if fieldValue.Addr().IsValid() {
-				if field.Type.Elem().Kind() == reflect.Bool {
-					fieldMap.importField(field, fieldValue, prefix, source)
-				} else {
-					newPrefix := fmt.Sprintf("%s%s.", prefix, field.Name)
-					fieldMap.importFields(fieldValue.Interface(), newPrefix, source)
-				}
-			}
-		default:
-			fieldMap.importField(field, fieldValue, prefix, source)
-		}
-	}
-}
-
-func (fieldMap fieldMap) importField(field reflect.StructField, fieldValue reflect.Value, prefix string, source string) {
-	key := prefix + field.Name
-	if _, ok := fieldMap[key]; !ok {
-		fieldMap[key] = &NodeFields{
-			Field:  key,
-			Source: source,
-		}
-	}
-
-	if field.Type == reflect.TypeOf([]string{}) {
-		vals := (fieldValue.Interface()).([]string)
-		src_str := source
-		if oldVal, ok := fieldMap[key]; ok {
-			if oldVal.Value != "" {
-				if len(vals) > 0 {
-					src_str = oldVal.Source + "+"
-				} else {
-					src_str = oldVal.Source
-				}
-				vals = append(vals, oldVal.Value)
-			} else {
-				src_str = oldVal.Source
-			}
-		}
-		fieldMap[key] = &NodeFields{
-			Field:  key,
-			Source: src_str,
-			Value:  strings.Join(vals, ","),
-		}
-	} else if field.Type == reflect.TypeOf(net.IP{}) {
-		val := (fieldValue.Interface()).(net.IP)
-		if val != nil {
-			fieldMap[key].Set(source, val.String())
-		}
-	} else if field.Type.Kind() == reflect.Bool {
-		fieldMap[key].Set(source, strconv.FormatBool(fieldValue.Bool()))
-	} else if field.Type.Kind() == reflect.Pointer && field.Type.Elem().Kind() == reflect.Bool {
-		if fieldValue.Elem().IsValid() {
-			fieldMap[key].Set(source, strconv.FormatBool(fieldValue.Elem().Bool()))
-		}
-	} else {
-		fieldMap[key].Set(source, fieldValue.String())
-	}
-}

internal/pkg/node/mergo.go

@@ -0,0 +1,86 @@
+package node
+
+import (
+	"bytes"
+	"encoding/gob"
+	"reflect"
+	"strings"
+
+	"dario.cat/mergo"
+
+	"github.com/warewulf/warewulf/internal/pkg/wwlog"
+)
+
+func copyProfile(this Profile) (Profile, error) {
+	var buf bytes.Buffer
+	enc := gob.NewEncoder(&buf)
+	dec := gob.NewDecoder(&buf)
+	profile := Profile{}
+	if err := enc.Encode(this); err != nil {
+		return profile, err
+	} else {
+		if err := dec.Decode(&profile); err != nil {
+			return profile, err
+		} else {
+			return profile, nil
+		}
+	}
+}
+
+func (config *NodesYaml) MergeNode(id string) (node Node, fields fieldMap, err error) {
+	node, err = config.GetNodeOnly(id)
+	if err != nil {
+		return node, fields, err
+	}
+	originalNode := node
+	node = EmptyNode()
+
+	fields = make(fieldMap)
+
+	for _, profileID := range cleanList(originalNode.Profiles) {
+		if profile, err := config.GetProfile(profileID); err != nil {
+			wwlog.Warn("profile not found: %s", profileID)
+			continue
+		} else if profile, err := copyProfile(profile); err != nil {
+			wwlog.Warn("error processing profile %s: %v", profileID, err)
+			continue
+		} else {
+			if err = mergo.Merge(&node.Profile, profile, mergo.WithAppendSlice, mergo.WithOverride); err != nil {
+				return node, fields, err
+			}
+			for _, fieldName := range listFields(profile) {
+				if value, err := getNestedFieldValue(profile, fieldName); err == nil && valueStr(value) != "" {
+					source := profileID
+					prevSource := fields.Source(fieldName)
+					if value.Kind() == reflect.Slice && prevSource != "" {
+						source = strings.Join([]string{prevSource, source}, ",")
+					}
+					if value, err := getNestedFieldString(node, fieldName); err == nil {
+						fields.Set(fieldName, source, value)
+					}
+				}
+			}
+		}
+	}
+
+	if err = mergo.Merge(&node, originalNode, mergo.WithAppendSlice, mergo.WithOverride); err != nil {
+		return node, fields, err
+	}
+	for _, fieldName := range listFields(originalNode) {
+		if value, err := getNestedFieldValue(originalNode, fieldName); err == nil && valueStr(value) != "" {
+			source := ""
+			prevSource := fields.Source(fieldName)
+			if value.Kind() == reflect.Slice && prevSource != "" {
+				source = strings.Join([]string{prevSource, id}, ",")
+			}
+			if value, err := getNestedFieldString(node, fieldName); err == nil {
+				fields.Set(fieldName, source, value)
+			}
+		}
+	}
+
+	node.id = id
+	node.valid = true
+	node.updatePrimaryNetDev()
+	return node, fields, nil
+}

internal/pkg/node/mergo_test.go

@@ -0,0 +1,575 @@
+package node
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/warewulf/warewulf/internal/pkg/testenv"
+)
+
+func Test_MergeNode(t *testing.T) {
+	var tests = map[string]struct {
+		nodesConf string
+		node      string
+		field     string
+		source    string
+		value     string
+		nodes     []string
+		fields    []string
+		sources   []string
+		values    []string
+	}{
+		"node comment": {
+			nodesConf: `
+nodes:
+  n1:
+    comment: n1 comment`,
+			node:   "n1",
+			field:  "Comment",
+			source: "",
+			value:  "n1 comment",
+		},
+		"profile comment": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+nodeprofiles:
+  p1:
+    comment: p1 comment`,
+			node:   "n1",
+			field:  "Comment",
+			source: "p1",
+			value:  "p1 comment",
+		},
+		"multiple profile comments": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    - p2
+nodeprofiles:
+  p1:
+    comment: p1 comment
+  p2:
+    comment: p2 comment`,
+			node:   "n1",
+			field:  "Comment",
+			source: "p2",
+			value:  "p2 comment",
+		},
+		"node comment supersedes profile comment": {
+			nodesConf: `
+nodes:
+  n1:
+    comment: n1 comment
+    profiles:
+    - p1
+nodeprofiles:
+  p1:
+    comment: p1 comment`,
+			node:   "n1",
+			field:  "Comment",
+			source: "SUPERSEDED",
+			value:  "n1 comment",
+		},
+		"node comment supersedes multiple profile comments": {
+			nodesConf: `
+nodes:
+  n1:
+    comment: n1 comment
+    profiles:
+    - p1
+    - p2
+nodeprofiles:
+  p1:
+    comment: p1 comment
+  p2:
+    comment: p2 comment`,
+			node:   "n1",
+			field:  "Comment",
+			source: "SUPERSEDED",
+			value:  "n1 comment",
+		},
+		"node kernel args": {
+			nodesConf: `
+nodes:
+  n1:
+    kernel:
+      args: n1 args`,
+			node:   "n1",
+			field:  "Kernel.Args",
+			source: "",
+			value:  "n1 args",
+		},
+		"profile kernel args": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+nodeprofiles:
+  p1:
+    kernel:
+      args: p1 args`,
+			node:   "n1",
+			field:  "Kernel.Args",
+			source: "p1",
+			value:  "p1 args",
+		},
+		"multiple profile kernel args": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    - p2
+nodeprofiles:
+  p1:
+    kernel:
+      args: p1 args
+  p2:
+    kernel:
+      args: p2 args`,
+			node:   "n1",
+			field:  "Kernel.Args",
+			source: "p2",
+			value:  "p2 args",
+		},
+		"node kernel args supersedes profile kernel args": {
+			nodesConf: `
+nodes:
+  n1:
+    kernel:
+      args: n1 args
+    profiles:
+    - p1
+nodeprofiles:
+  p1:
+    kernel:
+      args: p1 args`,
+			node:   "n1",
+			field:  "Kernel.Args",
+			source: "SUPERSEDED",
+			value:  "n1 args",
+		},
+		"node kernel args supersedes multiple profile kernel args": {
+			nodesConf: `
+nodes:
+  n1:
+    kernel:
+      args: n1 args
+    profiles:
+    - p1
+    - p2
+nodeprofiles:
+  p1:
+    kernel:
+      args: p1 args
+  p2:
+    kernel:
+      args: p2 args`,
+			node:   "n1",
+			field:  "Kernel.Args",
+			source: "SUPERSEDED",
+			value:  "n1 args",
+		},
+		"node tag": {
+			nodesConf: `
+nodes:
+  n1:
+    tags:
+      tag: n1 tag`,
+			node:   "n1",
+			field:  "Tags[tag]",
+			source: "",
+			value:  "n1 tag",
+		},
+		"profile tag": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+nodeprofiles:
+  p1:
+    tags:
+      tag: p1 tag`,
+			node:   "n1",
+			field:  "Tags[tag]",
+			source: "p1",
+			value:  "p1 tag",
+		},
+		"multiple profile tags": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    - p2
+nodeprofiles:
+  p1:
+    tags:
+      tag: p1 tag
+  p2:
+    tags:
+      tag: p2 tag`,
+			node:   "n1",
+			field:  "Tags[tag]",
+			source: "p2",
+			value:  "p2 tag",
+		},
+		"node tag supersedes profile tag": {
+			nodesConf: `
+nodes:
+  n1:
+    tags:
+      tag: n1 tag
+    profiles:
+    - p1
+nodeprofiles:
+  p1:
+    tags:
+      tag: p1 tag`,
+			node:   "n1",
+			field:  "Tags[tag]",
+			source: "SUPERSEDED",
+			value:  "n1 tag",
+		},
+		"node tag supersedes multiple profile tags": {
+			nodesConf: `
+nodes:
+  n1:
+    tags:
+      tag: n1 tag
+    profiles:
+    - p1
+    - p2
+nodeprofiles:
+  p1:
+    tags:
+      tag: p1 tag
+  p2:
+    tags:
+      tag: p2 tag`,
+			node:   "n1",
+			field:  "Tags[tag]",
+			source: "SUPERSEDED",
+			value:  "n1 tag",
+		},
+		"mixture of tags from nodes and profiles": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    - p2
+    tags:
+      n1: n1 tag
+nodeprofiles:
+  p1:
+    tags:
+      p1: p1 tag
+  p2:
+    tags:
+      p2: p2 tag`,
+			nodes: []string{
+				"n1",
+				"n1",
+				"n1",
+			},
+			fields: []string{
+				"Tags[n1]",
+				"Tags[p1]",
+				"Tags[p2]",
+			},
+			sources: []string{
+				"",
+				"p1",
+				"p2",
+			},
+			values: []string{
+				"n1 tag",
+				"p1 tag",
+				"p2 tag",
+			},
+		},
+		"node system overlay": {
+			nodesConf: `
+nodes:
+  n1:
+    system overlay:
+    - no1
+    - no2`,
+			node:   "n1",
+			field:  "SystemOverlay",
+			source: "",
+			value:  "no1,no2",
+		},
+		"profile system overlay": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+nodeprofiles:
+  p1:
+    system overlay:
+    - po1
+    - po2`,
+			node:   "n1",
+			field:  "SystemOverlay",
+			source: "p1",
+			value:  "po1,po2",
+		},
+		"multiple profile system overlays": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    - p2
+nodeprofiles:
+  p1:
+    system overlay:
+    - po1
+    - po2
+  p2:
+    system overlay:
+    - po3
+    - po4`,
+			node:   "n1",
+			field:  "SystemOverlay",
+			source: "p1,p2",
+			value:  "po1,po2,po3,po4",
+		},
+		"node system overlay adds to profile system overlay": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    system overlay:
+    - no1
+    - no2
+nodeprofiles:
+  p1:
+    system overlay:
+    - po1
+    - po2`,
+			node:   "n1",
+			field:  "SystemOverlay",
+			source: "p1,n1",
+			value:  "po1,po2,no1,no2",
+		},
+		"node system overlay adds to multiple profile system overlays": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    - p2
+    system overlay:
+    - no1
+    - no2
+nodeprofiles:
+  p1:
+    system overlay:
+    - po1
+    - po2
+  p2:
+    system overlay:
+    - po3
+    - po4`,
+			node:   "n1",
+			field:  "SystemOverlay",
+			source: "p1,p2,n1",
+			value:  "po1,po2,po3,po4,no1,no2",
+		},
+		"node netdev tag": {
+			nodesConf: `
+nodes:
+  n1:
+    network devices:
+      default:
+        tags:
+          tag: n1 netdev tag`,
+			node:   "n1",
+			field:  "NetDevs[default].Tags[tag]",
+			source: "",
+			value:  "n1 netdev tag",
+		},
+		"profile netdev tag": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+nodeprofiles:
+  p1:
+    network devices:
+      default:
+        tags:
+          tag: p1 netdev tag`,
+			node:   "n1",
+			field:  "NetDevs[default].Tags[tag]",
+			source: "p1",
+			value:  "p1 netdev tag",
+		},
+		"multiple profile netdev tags": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    - p2
+nodeprofiles:
+  p1:
+    network devices:
+      default:
+        tags:
+          tag: p1 netdev tag
+  p2:
+    network devices:
+      default:
+        tags:
+          tag: p2 netdev tag`,
+			node:   "n1",
+			field:  "NetDevs[default].Tags[tag]",
+			source: "p2",
+			value:  "p2 netdev tag",
+		},
+		"node supercededs profile netdev tag": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    network devices:
+      default:
+        tags:
+          tag: n1 netdev tag
+nodeprofiles:
+  p1:
+    network devices:
+      default:
+        tags:
+          tag: p1 netdev tag`,
+			node:   "n1",
+			field:  "NetDevs[default].Tags[tag]",
+			source: "SUPERSEDED",
+			value:  "n1 netdev tag",
+		},
+		"node supersedes multiple profile netdev tags": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    - p2
+    network devices:
+      default:
+        tags:
+          tag: n1 netdev tag
+nodeprofiles:
+  p1:
+    network devices:
+      default:
+        tags:
+          tag: p1 netdev tag
+  p2:
+    network devices:
+      default:
+        tags:
+          tag: p2 netdev tag`,
+			node:   "n1",
+			field:  "NetDevs[default].Tags[tag]",
+			source: "SUPERSEDED",
+			value:  "n1 netdev tag",
+		},
+		"mixture of netdev tags from nodes and profiles": {
+			nodesConf: `
+nodes:
+  n1:
+    profiles:
+    - p1
+    - p2
+    network devices:
+      default:
+        tags:
+          n1: n1 netdev tag
+nodeprofiles:
+  p1:
+    network devices:
+      default:
+        tags:
+          p1: p1 netdev tag
+  p2:
+    network devices:
+      default:
+        tags:
+          p2: p2 netdev tag`,
+			nodes: []string{
+				"n1",
+				"n1",
+				"n1",
+			},
+			fields: []string{
+				"NetDevs[default].Tags[n1]",
+				"NetDevs[default].Tags[p1]",
+				"NetDevs[default].Tags[p2]",
+			},
+			sources: []string{
+				"",
+				"p1",
+				"p2",
+			},
+			values: []string{
+				"n1 netdev tag",
+				"p1 netdev tag",
+				"p2 netdev tag",
+			},
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			env := testenv.New(t)
+			defer env.RemoveAll(t)
+			env.WriteFile(t, "/etc/warewulf/nodes.conf", tt.nodesConf)
+
+			registry, regErr := New()
+			assert.NoError(t, regErr)
+
+			if tt.node != "" {
+				node, fields, mergeErr := registry.MergeNode(tt.node)
+				assert.NoError(t, mergeErr)
+
+				value, valueErr := getNestedFieldString(node, tt.field)
+				assert.NoError(t, valueErr)
+				assert.Equal(t, tt.value, value)
+				assert.Equal(t, tt.value, fields.Value(tt.field))
+				assert.Equal(t, tt.source, fields.Source(tt.field))
+			}
+
+			var nodes []Node
+			for i, _ := range tt.nodes {
+				node, _, mergeErr := registry.MergeNode(tt.nodes[i])
+				assert.NoError(t, mergeErr)
+				nodes = append(nodes, node)
+			}
+
+			for i, _ := range tt.nodes {
+				_, fields, _ := registry.MergeNode(tt.nodes[i])
+				value, valueErr := getNestedFieldString(nodes[i], tt.fields[i])
+				assert.NoError(t, valueErr)
+				assert.Equal(t, tt.values[i], value)
+				assert.Equal(t, tt.values[i], fields.Value(tt.fields[i]))
+				assert.Equal(t, tt.sources[i], fields.Source(tt.fields[i]))
+			}
+		})
+	}
+}

internal/pkg/node/methods.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/warewulf/warewulf/internal/pkg/util"
+	"github.com/warewulf/warewulf/internal/pkg/wwlog"
 )
 
 type nodeList []Node
@@ -314,6 +315,23 @@ func (node *Node) Valid() bool {
 	return node.valid
 }
 
+func (node *Node) updatePrimaryNetDev() {
+	if netdev, ok := node.NetDevs[node.PrimaryNetDev]; ok {
+		netdev.primary = true
+	} else {
+		keys := make([]string, 0)
+		for k := range node.NetDevs {
+			keys = append(keys, k)
+		}
+		sort.Strings(keys)
+		if len(keys) > 0 {
+			wwlog.Debug("%s: no primary defined, sanitizing to: %s", node.id, keys[0])
+			node.NetDevs[keys[0]].primary = true
+			node.PrimaryNetDev = keys[0]
+		}
+	}
+}
+
 /*
 Check if the netdev is the primary one
 */