Skip to content

Commit

Permalink
Fix AddAnnotations for unstructured.Unstructured
Browse files Browse the repository at this point in the history
The previous implementation worked well for most Object implementations
(definitely all embedding metav1.ObjectMeta) because it relies on:
* The fact that GetAnnotations doesn't return a copy of the
annotations but the underlying map storing them.
* When calling set annotations, the input map is used for storage in the
  Object implementation, hence when adding entries to that map from the
  outside, it changes the content of the Object's annotations.
This is not necessarily part of the contract specified by metav1.Object
so we can't really guarantee all implementations will satisfy these 2
conditions. It turns out, unstructured.Unstructured doesn't.
  • Loading branch information
g-gaston committed Aug 10, 2023
1 parent 3a37397 commit da8cb15
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
2 changes: 1 addition & 1 deletion util/annotations/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ func AddAnnotations(o metav1.Object, desired map[string]string) bool {
annotations := o.GetAnnotations()
if annotations == nil {
annotations = make(map[string]string)
o.SetAnnotations(annotations)
}
hasChanged := false
for k, v := range desired {
Expand All @@ -80,6 +79,7 @@ func AddAnnotations(o metav1.Object, desired map[string]string) bool {
hasChanged = true
}
}
o.SetAnnotations(annotations)
return hasChanged
}

Expand Down
36 changes: 35 additions & 1 deletion util/annotations/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ import (
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

func TestAddAnnotations(t *testing.T) {
g := NewWithT(t)

var testcases = []struct {
testcases := []struct {
name string
obj metav1.Object
input map[string]string
Expand Down Expand Up @@ -141,6 +142,39 @@ func TestAddAnnotations(t *testing.T) {
},
changed: true,
},
{
name: "should add annotations to an empty unstructured",
obj: &unstructured.Unstructured{},
input: map[string]string{
"foo": "buzz",
},
expected: map[string]string{
"foo": "buzz",
},
changed: true,
},
{
name: "should add annotations to a non empty unstructured",
obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
"foo": "bar",
},
},
},
},
input: map[string]string{
"thing1": "thing2",
"buzz": "blah",
},
expected: map[string]string{
"foo": "bar",
"thing1": "thing2",
"buzz": "blah",
},
changed: true,
},
}

for _, tc := range testcases {
Expand Down

0 comments on commit da8cb15

Please sign in to comment.