Skip to content

Commit

Permalink
Ensure status field is deeply empty in CreateOrUpdate
Browse files Browse the repository at this point in the history
...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 <[email protected]>
  • Loading branch information
tpantelis authored and aswinsuryan committed Dec 8, 2023
1 parent c126e0e commit b2ed2d1
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 2 deletions.
4 changes: 2 additions & 2 deletions pkg/util/create_or_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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())
Expand Down
78 changes: 78 additions & 0 deletions pkg/util/helper_test.go
Original file line number Diff line number Diff line change
@@ -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())
})
})
15 changes: 15 additions & 0 deletions pkg/util/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit b2ed2d1

Please sign in to comment.