From b2ed2d19de6a3876571bdd40ca8f9e4b9872d1c4 Mon Sep 17 00:00:00 2001
From: Tom Pantelis <tompantelis@gmail.com>
Date: Wed, 6 Dec 2023 14:10:43 -0500
Subject: [PATCH] Ensure status field is deeply empty in CreateOrUpdate

...meaning any nested map fields are also empty. If a resource type
has a non-pointer struct field, it appears as an empty map when
converted to Unstructured so merely checking 'len' on the status map
doesn't suffice. Specifically this was observed with the LoadBalancer
field in the Service status. Added function to check if a map is deeply
empty.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
---
 pkg/util/create_or_update.go |  4 +-
 pkg/util/helper_test.go      | 78 ++++++++++++++++++++++++++++++++++++
 pkg/util/helpers.go          | 15 +++++++
 3 files changed, 95 insertions(+), 2 deletions(-)
 create mode 100644 pkg/util/helper_test.go

diff --git a/pkg/util/create_or_update.go b/pkg/util/create_or_update.go
index 4beac538..f4ef14fd 100644
--- a/pkg/util/create_or_update.go
+++ b/pkg/util/create_or_update.go
@@ -133,7 +133,7 @@ func maybeCreateOrUpdate[T runtime.Object](ctx context.Context, client resource.
 		origStatus := GetNestedField(origObj, StatusField)
 		newStatus, ok := GetNestedField(newObj, StatusField).(map[string]interface{})
 
-		if !ok || len(newStatus) == 0 {
+		if !ok || DeeplyEmpty(newStatus) {
 			unstructured.RemoveNestedField(origObj.Object, StatusField)
 			unstructured.RemoveNestedField(newObj.Object, StatusField)
 		} else if !equality.Semantic.DeepEqual(origStatus, newStatus) {
@@ -213,7 +213,7 @@ func createResource[T runtime.Object](ctx context.Context, client resource.Inter
 	}
 
 	status, ok := GetNestedField(resource.MustToUnstructuredUsingDefaultConverter(obj), StatusField).(map[string]interface{})
-	if ok && len(status) > 0 {
+	if ok && !DeeplyEmpty(status) {
 		// If the resource CRD has the status subresource the Create won't set the status field so we need to
 		// do a separate UpdateStatus call.
 		objMeta.SetResourceVersion(resource.MustToMeta(created).GetResourceVersion())
diff --git a/pkg/util/helper_test.go b/pkg/util/helper_test.go
new file mode 100644
index 00000000..4206238c
--- /dev/null
+++ b/pkg/util/helper_test.go
@@ -0,0 +1,78 @@
+/*
+SPDX-License-Identifier: Apache-2.0
+
+Copyright Contributors to the Submariner project.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util_test
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/submariner-io/admiral/pkg/resource"
+	"github.com/submariner-io/admiral/pkg/util"
+	corev1 "k8s.io/api/core/v1"
+)
+
+var _ = Describe("DeeplyEmpty", func() {
+	Specify("with an empty map should return true", func() {
+		Expect(util.DeeplyEmpty(map[string]interface{}{})).To(BeTrue())
+	})
+
+	Specify("with nested empty maps should return true", func() {
+		Expect(util.DeeplyEmpty(map[string]interface{}{
+			"nested": map[string]interface{}{},
+		})).To(BeTrue())
+
+		Expect(util.DeeplyEmpty(map[string]interface{}{
+			"nested1": map[string]interface{}{},
+			"nested2": map[string]interface{}{},
+		})).To(BeTrue())
+
+		Expect(util.DeeplyEmpty(map[string]interface{}{
+			"nested1": map[string]interface{}{
+				"nested2": map[string]interface{}{},
+			},
+		})).To(BeTrue())
+	})
+
+	Specify("with a non-empty map should return false", func() {
+		Expect(util.DeeplyEmpty(map[string]interface{}{"foo": "bar"})).To(BeFalse())
+	})
+
+	Specify("with nested non-empty maps should return false", func() {
+		Expect(util.DeeplyEmpty(map[string]interface{}{
+			"nested": map[string]interface{}{},
+			"foo":    "bar",
+		})).To(BeFalse())
+
+		Expect(util.DeeplyEmpty(map[string]interface{}{
+			"foo":    "bar",
+			"nested": map[string]interface{}{},
+		})).To(BeFalse())
+
+		Expect(util.DeeplyEmpty(map[string]interface{}{
+			"nested1": map[string]interface{}{
+				"nested2": map[string]interface{}{},
+				"foo":     "bar",
+			},
+		})).To(BeFalse())
+	})
+
+	Specify("an empty Service status should return true", func() {
+		Expect(util.DeeplyEmpty(util.GetNestedField(resource.MustToUnstructuredUsingDefaultConverter(&corev1.Service{}),
+			util.StatusField).(map[string]interface{}))).To(BeTrue())
+	})
+})
diff --git a/pkg/util/helpers.go b/pkg/util/helpers.go
index 3bd88508..53762e04 100644
--- a/pkg/util/helpers.go
+++ b/pkg/util/helpers.go
@@ -137,6 +137,21 @@ func CopyImmutableMetadata(from, to *unstructured.Unstructured) *unstructured.Un
 	return to
 }
 
+func DeeplyEmpty(m map[string]interface{}) bool {
+	for _, v := range m {
+		switch t := v.(type) {
+		case map[string]interface{}:
+			if !DeeplyEmpty(t) {
+				return false
+			}
+		default:
+			return false
+		}
+	}
+
+	return true
+}
+
 func AddCertificateErrorHandler(fatal bool) {
 	logCertificateError := logger.Errorf
 	if fatal {