Merge annotations from desired with existing ones in the ServiceAccou…

…nts (#969)

* Merge annotations from desired with existing ones for ServiceAccount objects

Signed-off-by: Ruben Vargas <[email protected]>
Co-authored-by: Israel Blancas <[email protected]>

.chloggen/sa_fix.yaml

@@ -0,0 +1,25 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. operator, github action)
+component: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Modify SA annotations managed by the operator, preserve others.
+
+# One or more tracking issues related to the change
+issues: [970]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  This prevents other controllers that modified the SA from create an infinite loop where the other controller modifies something,
+  and tempo-operator removes it, the other controller detect the changes and add its and so on and so on.
+  
+  This is specific for OpenShift case, where the openshift-controller-manager annotates the SA with
+  openshift.io/internal-registry-pull-secret-ref.
+  
+  See https://github.com/openshift/openshift-controller-manager/pull/288/ and 
+  https://docs.openshift.com/container-platform/4.16/release_notes/ocp-4-16-release-notes.html section about 
+  "Legacy service account API token secrets are no longer generated for each service account"

internal/manifests/mutate.go

@@ -181,7 +181,6 @@ func mutateConfigMap(existing, desired *corev1.ConfigMap) {
 }
 
 func mutateServiceAccount(existing, desired *corev1.ServiceAccount) {
-	existing.Annotations = desired.Annotations
 	existing.Labels = desired.Labels
 }
 

internal/manifests/mutate_test.go

@@ -5,6 +5,7 @@ import (
 
 	routev1 "github.com/openshift/api/route/v1"
 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -1171,3 +1172,29 @@ func TestGetMutateFunc_MutateRoute(t *testing.T) {
 	require.Exactly(t, got.Annotations, want.Annotations)
 	require.Exactly(t, got.Spec, want.Spec)
 }
+
+func TestMutateServiceAccount(t *testing.T) {
+	existing := corev1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "simplest",
+			Annotations: map[string]string{
+				"config.openshift.io/serving-cert-secret-name": "my-secret",
+			},
+		},
+	}
+	desired := corev1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "simplest",
+		},
+	}
+
+	mutateFn := manifests.MutateFuncFor(&existing, &desired)
+	err := mutateFn()
+	require.NoError(t, err)
+	assert.Equal(t, corev1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "simplest",
+			Annotations: map[string]string{"config.openshift.io/serving-cert-secret-name": "my-secret"},
+		},
+	}, existing)
+}