kelvin/warewulf
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix boolean pointer handling

Browse Source 
- Refactor field reflection for clarity
- Update tests for clarity

Signed-off-by: Jonathon Anderson <janderson@ciq.com>
This commit is contained in:
Jonathon Anderson
2024-10-29 23:07:39 -06:00
parent e54ecfad95
commit fcc15eb8ef
3 changed files with 182 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files

155
internal/app/wwctl/node/list/main_test.go
View File

@@ -29,7 +29,7 @@ func Test_List(t *testing.T) {
args: []string{},
wantErr: false,
stdout: ` NODE NAME PROFILES NETWORK
n01 [default]
n01 [default]
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -44,9 +44,9 @@ nodes:
name: "multiple nodes list",
args: []string{},
wantErr: false,
stdout: ` NODE NAME PROFILES NETWORK
n01 [default]
n02 [default]
stdout: ` NODE NAME PROFILES NETWORK
n01 [default]
n02 [default]
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -64,9 +64,9 @@ nodes:
name: "node list returns multiple nodes",
args: []string{"n01,n02"},
wantErr: false,
stdout: ` NODE NAME PROFILES NETWORK
n01 [default]
n02 [default]
stdout: ` NODE NAME PROFILES NETWORK
n01 [default]
n02 [default]
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -84,9 +84,9 @@ nodes:
name: "node list returns multiple nodes (case 2)",
args: []string{"n01,n03"},
wantErr: false,
stdout: ` NODE NAME PROFILES NETWORK
n01 [default]
n03 [default]
stdout: ` NODE NAME PROFILES NETWORK
n01 [default]
n03 [default]
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -113,8 +113,8 @@ nodes:
name: "node list returns one node",
args: []string{"n01,"},
wantErr: false,
stdout: ` NODE NAME PROFILES NETWORK
n01 [default]
stdout: ` NODE NAME PROFILES NETWORK
n01 [default]
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -132,8 +132,8 @@ nodes:
name: "node list profile with network",
args: []string{},
wantErr: false,
stdout: ` NODE NAME PROFILES NETWORK
n01 [default] default
stdout: ` NODE NAME PROFILES NETWORK
n01 [default] default
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -150,9 +150,9 @@ nodes:
name: "node list profile with comment",
args: []string{"-a"},
wantErr: false,
stdout: `NODE FIELD PROFILE VALUE
n01 Comment default profilecomment
n01 Profiles default
stdout: ` NODE FIELD PROFILE VALUE
n01 Comment default profilecomment
n01 Profiles default
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -167,9 +167,9 @@ nodes:
name: "node list profile with comment superseded",
args: []string{"-a"},
wantErr: false,
stdout: `NODE FIELD PROFILE VALUE
n01 Comment SUPERSEDED nodecomment
n01 Profiles default
stdout: ` NODE FIELD PROFILE VALUE
n01 Comment SUPERSEDED nodecomment
n01 Profiles default
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -185,8 +185,8 @@ nodes:
name: "node list profile with ipmi user",
args: []string{"-i"},
wantErr: false,
stdout: `NODE IPMIIPADDR IPMIPORT IPMIUSERNAME IPMIINTERFACE
n01 <nil> admin
stdout: ` NODE IPMI IPADDR IPMI PORT IPMI USERNAME IPMI INTERFACE
n01 <nil> admin
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -202,8 +202,8 @@ nodes:
name: "node list profile with ipmi user superseded",
args: []string{"-i"},
wantErr: false,
stdout: `NODE IPMIIPADDR IPMIPORT IPMIUSERNAME IPMIINTERFACE
n01 <nil> user
stdout: ` NODE IPMI IPADDR IPMI PORT IPMI USERNAME IPMI INTERFACE
n01 <nil> user
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
@@ -218,23 +218,12 @@ nodes:
- default
`},
{
inDb: `WW_INTERNAL: 45
nodeprofiles:
p1: {}
p2: {}
nodes:
n01:
profiles:
- p1
- p2
`,
name: "multiple profiles list",
args: []string{},
wantErr: false,
stdout: ` NODE NAME PROFILES NETWORK
n01 [p1 p2]
`},
{
n01 [p1 p2]
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
p1: {}
@@ -244,14 +233,31 @@ nodes:
profiles:
- p1
- p2
`,
`},
{
name: "multiple profiles list all",
args: []string{"-a"},
wantErr: false,
stdout: `NODE FIELD PROFILE VALUE
n01 Profiles p1,p2
stdout: ` NODE FIELD PROFILE VALUE
n01 Profiles p1,p2
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
p1: {}
p2: {}
nodes:
n01:
profiles:
- p1
- p2
`},
{
name: "multiple overlays list long with negation",
args: []string{"-l"},
wantErr: false,
stdout: ` NODE NAME KERNEL OVERRIDE CONTAINER OVERLAYS (S/R)
n01 /rop1,rop2
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
p1:
@@ -262,14 +268,14 @@ nodes:
n01:
profiles:
- p1
`,
name: "multiple overlays list",
args: []string{"-l"},
wantErr: false,
stdout: `NODE NAME KERNEL OVERRIDE CONTAINER OVERLAYS (S/R)
n01 /rop1,rop2
`},
{
name: "multiple overlays list long",
args: []string{"-l"},
wantErr: false,
stdout: ` NODE NAME KERNEL OVERRIDE CONTAINER OVERLAYS (S/R)
n01 sop1/nop1,~rop1,rop1,rop2
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
p1:
@@ -285,14 +291,16 @@ nodes:
runtime overlay:
- nop1
- ~rop1
`,
name: "multiple overlays list",
args: []string{"-l"},
wantErr: false,
stdout: `NODE NAME KERNEL OVERRIDE CONTAINER OVERLAYS (S/R)
n01 sop1/nop1,~rop1,rop1,rop2
`},
{
name: "multiple overlays list all with negation",
args: []string{"-a"},
wantErr: false,
stdout: ` NODE FIELD PROFILE VALUE
n01 Profiles p1
n01 RuntimeOverlay p1+ nop1,~rop1,rop1,rop2
n01 SystemOverlay p1 sop1
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
p1:
@@ -308,14 +316,15 @@ nodes:
runtime overlay:
- nop1
- ~rop1
`,
`},
{
name: "multiple overlays list all",
args: []string{"-a"},
wantErr: false,
stdout: `NODE FIELD PROFILE VALUE
n01 Profiles p1
`},
{
stdout: ` NODE FIELD PROFILE VALUE
n01 Profiles p1
n01 RuntimeOverlay p1+ nop1,rop1,rop2
`,
inDb: `WW_INTERNAL: 45
nodeprofiles:
p1:
@@ -328,13 +337,32 @@ nodes:
- p1
runtime overlay:
- nop1
`,
name: "multiple overlays list all",
`},
{
name: "network onboot",
args: []string{"-a"},
wantErr: false,
stdout: `NODE FIELD PROFILE VALUE
n01 Profiles p1
n01 RuntimeOverlay p1+nop1,rop1,rop2
stdout: ` NODE FIELD PROFILE VALUE
n1 NetDevs[default].OnBoot true
`,
inDb: `WW_INTERNAL: 45
nodes:
n1:
network devices:
default:
onboot: true
`},
{
name: "empty network device",
args: []string{"-a"},
wantErr: false,
stdout: ` NODE FIELD PROFILE VALUE
`,
inDb: `WW_INTERNAL: 46
nodes:
wwnode1:
network devices:
default: {}
`},
}
@@ -371,8 +399,7 @@ n01 RuntimeOverlay p1+nop1,rop1,rop2
err := baseCmd.Execute()
assert.NoError(t, err)
assert.NotEmpty(t, buf, "output should not be empty")
assert.Contains(t, strings.ReplaceAll(buf.String(), " ", ""),
strings.ReplaceAll(tt.stdout, " ", ""))
assert.Equal(t, tt.stdout, buf.String())
})
}

167
internal/pkg/node/list.go
View File

@@ -1,6 +1,7 @@
package node
import (
"fmt"
"net"
"reflect"
"sort"
@@ -44,11 +45,11 @@ func (nodeYml *NodeYaml) GetFields(node NodeConf) (output []NodeFields) {
nodeMap := make(fieldMap)
for _, p := range node.Profiles {
if profile, ok := nodeYml.nodeProfiles[p]; ok {
nodeMap.recursiveFields(profile, "", p)
nodeMap.importFields(profile, "", p)
}
}
rawNode, _ := nodeYml.GetNodeOnlyPtr(node.id)
nodeMap.recursiveFields(rawNode, "", "")
nodeMap.importFields(rawNode, "", "")
for _, elem := range nodeMap {
if elem.Value != "" {
output = append(output, *elem)
@@ -65,7 +66,7 @@ Get all the info out of ProfileConf. If emptyFields is set true, all fields are
*/
func (nodeYml *NodeYaml) GetFieldsProfile(profile ProfileConf) (output []NodeFields) {
profileMap := make(fieldMap)
profileMap.recursiveFields(&profile, "", "")
profileMap.importFields(&profile, "", "")
for _, elem := range profileMap {
if elem.Value != "" {
output = append(output, *elem)
@@ -81,86 +82,98 @@ func (nodeYml *NodeYaml) GetFieldsProfile(profile ProfileConf) (output []NodeFie
Internal function which travels through all fields of a NodeConf and for this
reason needs to be called via interface{}
*/
func (fieldMap fieldMap) recursiveFields(obj interface{}, prefix string, source string) {
valObj := reflect.ValueOf(obj)
typeObj := reflect.TypeOf(obj)
if valObj.IsNil() {
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 < typeObj.Elem().NumField(); i++ {
if valObj.Elem().Field(i).IsValid() {
if !typeObj.Elem().Field(i).IsExported() {
continue
}
switch typeObj.Elem().Field(i).Type.Kind() {
case reflect.Map:
mapIter := valObj.Elem().Field(i).MapRange()
for mapIter.Next() {
if mapIter.Value().Kind() == reflect.String {
fieldMap[prefix+typeObj.Elem().Field(i).Name+"["+mapIter.Key().String()+"]"] = &NodeFields{
Field: prefix + typeObj.Elem().Field(i).Name + "[" + mapIter.Key().String() + "]",
Source: source,
Value: mapIter.Value().String(),
}
} else {
fieldMap.recursiveFields(mapIter.Value().Interface(), prefix+typeObj.Elem().Field(i).Name+"["+mapIter.Key().String()+"].", source)
}
}
if valObj.Elem().Field(i).Len() == 0 {
fieldMap[prefix+typeObj.Elem().Field(i).Name] = &NodeFields{
Field: prefix + typeObj.Elem().Field(i).Name + "[]",
}
}
case reflect.Struct:
fieldMap.recursiveFields(valObj.Elem().Field(i).Addr().Interface(), "", source)
case reflect.Ptr:
if valObj.Elem().Field(i).Addr().IsValid() {
fieldMap.recursiveFields(valObj.Elem().Field(i).Interface(), prefix+typeObj.Elem().Field(i).Name+".", source)
}
default:
if _, ok := fieldMap[prefix+typeObj.Elem().Field(i).Name]; !ok {
fieldMap[prefix+typeObj.Elem().Field(i).Name] = &NodeFields{
Field: prefix + typeObj.Elem().Field(i).Name,
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)
}
switch typeObj.Elem().Field(i).Type {
case reflect.TypeOf([]string{}):
vals := (valObj.Elem().Field(i).Interface()).([]string)
src_str := source
if oldVal, ok := fieldMap[prefix+typeObj.Elem().Field(i).Name]; 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[prefix+typeObj.Elem().Field(i).Name] = &NodeFields{
Field: prefix + typeObj.Elem().Field(i).Name,
Source: src_str,
Value: strings.Join(vals, ","),
}
case reflect.TypeOf(net.IP{}):
val := (valObj.Elem().Field(i).Interface()).(net.IP)
if val != nil {
fieldMap[prefix+typeObj.Elem().Field(i).Name].Set(source, val.String())
}
case reflect.TypeOf(true):
val := (valObj.Elem().Field(i).Interface()).(bool)
if val {
fieldMap[prefix+typeObj.Elem().Field(i).Name].Set(source, strconv.FormatBool(val))
}
default:
fieldMap[prefix+typeObj.Elem().Field(i).Name].Set(source, valObj.Elem().Field(i).String())
}
}
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())
}
}

2
internal/pkg/node/methods.go
View File

@@ -231,7 +231,7 @@ func UnmarshalConf(obj interface{}, excludeList []string) (lines []string) {
lines = append(lines, ymlStr...)
}
}
if field.Type.Kind() == reflect.Ptr && field.Tag.Get("yaml") != "" {
if field.Type.Kind() == reflect.Ptr && field.Type.Elem().Kind() == reflect.Struct && field.Tag.Get("yaml") != "" {
typeLine := field.Tag.Get("yaml")
if len(strings.Split(typeLine, ",")) > 1 {
typeLine = strings.Split(typeLine, ",")[0] + ":"