From 92375f4ebba6ff945a78096348e6f8d2e4f2964a Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Fri, 5 Sep 2025 12:16:48 -0600 Subject: [PATCH] Explicit field names Fields now use the value of a `name` tag on a struct to determine the display name for a given field. This improves output for boolean-pointer fields, which otherwise have a P suffix and helper methods to retain backwards-compatibility. Signed-off-by: Jonathon Anderson --- internal/pkg/node/datastructure.go | 10 ++++----- internal/pkg/node/fields.go | 36 +++++++++++++++++++++++++----- internal/pkg/node/fields_test.go | 26 +++++++++++++++++++-- 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/internal/pkg/node/datastructure.go b/internal/pkg/node/datastructure.go index 4e7ed512..4fe1be96 100644 --- a/internal/pkg/node/datastructure.go +++ b/internal/pkg/node/datastructure.go @@ -94,7 +94,7 @@ Holds the disks of a node */ type Disk struct { id string `yaml:"-" json:"-"` - WipeTableP *bool `yaml:"wipe_table,omitempty" json:"wipe_table,omitempty" lopt:"diskwipe" comment:"whether or not the partition tables shall be wiped"` + WipeTableP *bool `yaml:"wipe_table,omitempty" json:"wipe_table,omitempty" lopt:"diskwipe" comment:"whether or not the partition tables shall be wiped" name:"WipeTable"` Partitions map[string]*Partition `yaml:"partitions,omitempty" json:"partitions,omitempty"` } @@ -109,9 +109,9 @@ type Partition struct { StartMiB string `yaml:"start_mib,omitempty" json:"start_mib,omitempty" comment:"the start of the partition" type:"uint"` TypeGuid string `yaml:"type_guid,omitempty" json:"type_guid,omitempty" lopt:"parttype" comment:"Set the partition type GUID"` Guid string `yaml:"guid,omitempty" json:"guid,omitempty" comment:"the GPT unique partition GUID"` - WipePartitionEntryP *bool `yaml:"wipe_partition_entry,omitempty" json:"wipe_partition_entry,omitempty" comment:"if true, Ignition will clobber an existing partition if it does not match the config"` - ShouldExistP *bool `yaml:"should_exist,omitempty" json:"should_exist,omitempty" lopt:"partcreate" comment:"Create partition if it does not exist"` - ResizeP *bool `yaml:"resize,omitempty" json:"resize,omitempty" comment:"whether or not the existing partition should be resize"` + WipePartitionEntryP *bool `yaml:"wipe_partition_entry,omitempty" json:"wipe_partition_entry,omitempty" comment:"if true, Ignition will clobber an existing partition if it does not match the config" name:"WipePartitionEntry"` + ShouldExistP *bool `yaml:"should_exist,omitempty" json:"should_exist,omitempty" lopt:"partcreate" comment:"Create partition if it does not exist" name:"ShouldExist"` + ResizeP *bool `yaml:"resize,omitempty" json:"resize,omitempty" comment:"whether or not the existing partition should be resize" name:"Resize"` } /* @@ -121,7 +121,7 @@ type FileSystem struct { id string `yaml:"-" json:"-"` Format string `yaml:"format,omitempty" json:"format,omitempty" lopt:"fsformat" comment:"format of the file system"` Path string `yaml:"path,omitempty" json:"path,omitempty" lopt:"fspath" comment:"the mount point of the file system"` - WipeFileSystemP *bool `yaml:"wipe_filesystem,omitempty" json:"wipe_filesystem,omitempty" lopt:"fswipe" comment:"wipe file system at boot"` + WipeFileSystemP *bool `yaml:"wipe_filesystem,omitempty" json:"wipe_filesystem,omitempty" lopt:"fswipe" comment:"wipe file system at boot" name:"WipeFileSystem"` Label string `yaml:"label,omitempty" json:"label,omitempty" comment:"the label of the filesystem"` Uuid string `yaml:"uuid,omitempty" json:"uuid,omitempty" comment:"the uuid of the filesystem"` Options []string `yaml:"options,omitempty" json:"options,omitempty" comment:"any additional options to be passed to the format-specific mkfs utility"` diff --git a/internal/pkg/node/fields.go b/internal/pkg/node/fields.go index a9992bd5..6de909ee 100644 --- a/internal/pkg/node/fields.go +++ b/internal/pkg/node/fields.go @@ -129,7 +129,9 @@ func getNestedFieldValue(obj interface{}, name string) (value reflect.Value, err err = fmt.Errorf("no value: %v", name) return } - value = value.FieldByName(fieldName) + // Find the actual field name if fieldName might be a display name + actualFieldName := findActualFieldName(value.Type(), fieldName) + value = value.FieldByName(actualFieldName) if key != "" { value = value.MapIndex(reflect.ValueOf(key)) if !value.IsValid() { @@ -213,6 +215,29 @@ func listFields(obj interface{}) (fields []string) { return listReflectedFields(reflect.TypeOf(obj), reflect.ValueOf(obj), "") } +// getFieldDisplayName returns the display name for a struct field. +// If the field has a "name" tag, it returns that value. Otherwise, it returns the field's actual name. +func getFieldDisplayName(field reflect.StructField) string { + if name := field.Tag.Get("name"); name != "" { + return name + } + return field.Name +} + +// findActualFieldName finds the actual struct field name given a display name. +// If no field matches the display name, it returns the original name (assuming it's already the actual field name). +func findActualFieldName(structType reflect.Type, displayName string) string { + for _, field := range reflect.VisibleFields(structType) { + if !field.IsExported() || field.Anonymous { + continue + } + if getFieldDisplayName(field) == displayName { + return field.Name + } + } + return displayName // fallback to original name +} + // 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. @@ -225,6 +250,7 @@ func listReflectedFields(t reflect.Type, v reflect.Value, prefix string) (fields } fieldType := field.Type fieldValue := reflect.Value{} + displayName := getFieldDisplayName(field) if v.IsValid() { fieldValue = v.FieldByName(field.Name) } @@ -233,7 +259,7 @@ func listReflectedFields(t reflect.Type, v reflect.Value, prefix string) (fields fieldValue = fieldValue.Elem() } if fieldType.Kind() == reflect.Struct { - fields = append(fields, listReflectedFields(fieldType, fieldValue, fmt.Sprintf("%v%v.", prefix, field.Name))...) + fields = append(fields, listReflectedFields(fieldType, fieldValue, fmt.Sprintf("%v%v.", prefix, displayName))...) } else if fieldType.Kind() == reflect.Map { if !fieldValue.IsValid() { continue @@ -250,13 +276,13 @@ func listReflectedFields(t reflect.Type, v reflect.Value, prefix string) (fields } } if elementType.Kind() == reflect.Struct { - fields = append(fields, listReflectedFields(elementType, elementValue, fmt.Sprintf("%v%v[%v].", prefix, field.Name, key.String()))...) + fields = append(fields, listReflectedFields(elementType, elementValue, fmt.Sprintf("%v%v[%v].", prefix, displayName, key.String()))...) } else { - fields = append(fields, fmt.Sprintf("%v%v[%v]", prefix, field.Name, key.String())) + fields = append(fields, fmt.Sprintf("%v%v[%v]", prefix, displayName, key.String())) } } } else { - fields = append(fields, prefix+field.Name) + fields = append(fields, prefix+displayName) } } return diff --git a/internal/pkg/node/fields_test.go b/internal/pkg/node/fields_test.go index f2244140..9bae729a 100644 --- a/internal/pkg/node/fields_test.go +++ b/internal/pkg/node/fields_test.go @@ -103,7 +103,7 @@ nodes: field: "Resources[fstab]", jsonValue: `[{"file":"/home","freq":0,"mntops":"defaults","passno":0,"spec":"warewulf:/home","vfstype":"nfs"}]`, }, - "disk partition": { + "disk partition (with name tag)": { nodesConf: ` nodes: n1: @@ -113,7 +113,7 @@ nodes: rootfs: resize: false`, node: "n1", - field: "Disks[/dev/vda].Partitions[rootfs].ResizeP", + field: "Disks[/dev/vda].Partitions[rootfs].Resize", value: "false", }, } @@ -256,6 +256,28 @@ func Test_listFields(t *testing.T) { "Resources[resource]", }, }, + "disk with name tags": { + object: Disk{ + Partitions: map[string]*Partition{ + "root": { + WipePartitionEntryP: new(bool), + ShouldExistP: new(bool), + ResizeP: new(bool), + }, + }, + }, + fields: []string{ + "WipeTable", + "Partitions[root].Number", + "Partitions[root].SizeMiB", + "Partitions[root].StartMiB", + "Partitions[root].TypeGuid", + "Partitions[root].Guid", + "Partitions[root].WipePartitionEntry", + "Partitions[root].ShouldExist", + "Partitions[root].Resize", + }, + }, } for name, tt := range tests {