check if entry has real value in recursive getter

Signed-off-by: Christian Goll <[email protected]>
JasonYangShadow · Nov 30, 2023 · 8df43cc · 8df43cc
1 parent 25bb934
commit 8df43cc
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Fixed
+
+- Fixed a bug where profile tags were erroneously overridden by empty node
+  values. #884
+
 ## [4.4.1] 2023-06-03
 
 ### Fixed

diff --git a/internal/app/wwctl/node/set/main_test.go b/internal/app/wwctl/node/set/main_test.go
@@ -318,6 +318,43 @@ nodes:
     network devices:
       mynet:
         mtu: "1234"
+`},
+		{name: "single node set ipmitag",
+			args:    []string{"--tagadd", "nodetag1=nodevalue1", "n01"},
+			wantErr: false,
+			stdout:  "",
+			inDB: `WW_INTERNAL: 43
+nodeprofiles:
+  p1:
+    comment: testit 1
+    tags:
+      p1tag1: p1val1
+  p2:
+    comment: testit 1
+    tags:
+      p2tag2: p1val2
+nodes:
+  n01:
+    profiles:
+    - p1
+    - p2`,
+			outDb: `WW_INTERNAL: 43
+nodeprofiles:
+  p1:
+    comment: testit 1
+    tags:
+      p1tag1: p1val1
+  p2:
+    comment: testit 1
+    tags:
+      p2tag2: p1val2
+nodes:
+  n01:
+    profiles:
+    - p1
+    - p2
+    tags:
+      nodetag1: nodevalue1
 `},
 	}
 	conf_yml := `WW_INTERNAL: 0`

diff --git a/internal/pkg/node/transformers.go b/internal/pkg/node/transformers.go
@@ -74,8 +74,12 @@ func recursiveGetter(
 						// go over a simple map with strings
 						for sourceIter.Next() {
 							if !targetValue.Elem().Field(i).MapIndex(sourceIter.Key()).IsValid() {
-								str := getter((sourceIter.Value().Interface()).(*Entry))
-								targetValue.Elem().Field(i).SetMapIndex(sourceIter.Key(), reflect.ValueOf(str))
+								// Only write entries for which have real values. This matters for
+								// tags, as empty map elements can be created without this check
+								if ((sourceIter.Value().Interface()).(*Entry)).GotReal() {
+									str := getter((sourceIter.Value().Interface()).(*Entry))
+									targetValue.Elem().Field(i).SetMapIndex(sourceIter.Key(), reflect.ValueOf(str))
+								}
 							}
 						}
 					} else {
@@ -271,7 +275,7 @@ func recursiveSetter(source, target interface{}, nameArg string, setter func(*En
 					if targetValue.Elem().Field(i).IsZero() {
 						targetValue.Elem().Field(i).Set(reflect.MakeMap(targetType.Elem().Field(i).Type))
 					}
-					// delete a ap element which is only in the target
+					// delete a map element which is only in the target
 					if targetValue.Elem().Field(i).Len() > 0 && targetValue.Elem().Field(i).Len() < 0 {
 						sourceIter := sourceValueMatched.MapRange()
 						targetIter := targetValue.Elem().Field(i).MapRange()