From a1c8ef51c21a2be6878478cfc65310046e1ca466 Mon Sep 17 00:00:00 2001
From: Guillermo Gaston <gaslor@amazon.com>
Date: Thu, 10 Aug 2023 15:56:55 +0000
Subject: [PATCH] Fix AddAnnotations for unstructured.Unstructured

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.
---
 util/annotations/helpers.go      |  2 +-
 util/annotations/helpers_test.go | 36 +++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/util/annotations/helpers.go b/util/annotations/helpers.go
index 0ec9ef9388ac..47dc7fc6b77b 100644
--- a/util/annotations/helpers.go
+++ b/util/annotations/helpers.go
@@ -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 {
@@ -80,6 +79,7 @@ func AddAnnotations(o metav1.Object, desired map[string]string) bool {
 			hasChanged = true
 		}
 	}
+	o.SetAnnotations(annotations)
 	return hasChanged
 }
 
diff --git a/util/annotations/helpers_test.go b/util/annotations/helpers_test.go
index 9793fcf87369..de973cfb9c14 100644
--- a/util/annotations/helpers_test.go
+++ b/util/annotations/helpers_test.go
@@ -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
@@ -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 {