From 539002088ccc4a412714f95b33eae10bd208e2c2 Mon Sep 17 00:00:00 2001
From: Christian Goll <cgoll@suse.com>
Date: Fri, 6 Oct 2023 14:59:12 +0200
Subject: [PATCH] check if entry has real value in recursive getter

Signed-off-by: Christian Goll <cgoll@suse.com>
---
 CHANGELOG.md                             |  2 ++
 internal/app/wwctl/node/set/main_test.go | 37 ++++++++++++++++++++++++
 internal/pkg/node/transformers.go        | 10 +++++--
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ab76ff74..696a4965 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -41,6 +41,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix build for API.
 - Bindd mounts for `wwctl exec` are now on the same fs
 - Fixed the ability to set MTU with wwctl #947
+- Fixed a bug where profile tags were erroneously overridden by empty node
+  values. #884
 
 ### Changed
 
diff --git a/internal/app/wwctl/node/set/main_test.go b/internal/app/wwctl/node/set/main_test.go
index 188c465c..48ee49e4 100644
--- a/internal/app/wwctl/node/set/main_test.go
+++ b/internal/app/wwctl/node/set/main_test.go
@@ -371,6 +371,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
 `},
 	}
 	for _, tt := range tests {
diff --git a/internal/pkg/node/transformers.go b/internal/pkg/node/transformers.go
index 1ea8fb7a..feabcc7c 100644
--- a/internal/pkg/node/transformers.go
+++ b/internal/pkg/node/transformers.go
@@ -72,8 +72,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 {
@@ -175,7 +179,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()