Allow custom annotations on service accounts (#3106)

* Allow custom annotations on service accounts

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Add extra annotation

Signed-off-by: Pavol Loffay <[email protected]>

---------

Signed-off-by: Pavol Loffay <[email protected]>

Author: pavolloffay

.chloggen/main.yaml

@@ -0,0 +1,18 @@
+# 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. collector, target allocator, auto-instrumentation, opamp, github action)
+component: collector
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Allow annotations on service account to prevent infinite reconciliation on OpenShift and creating infinite pull secrets.
+
+# One or more tracking issues related to the change
+issues: [3106]
+
+# (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: | 
+  On OpenShift 4.16 the platform automatically adds an annotation `openshift.io/internal-registry-pull-secret-ref: <simplest-collector-dockercfg-jwq66>`
+  to the service account which contains secret name with image pull secret.

controllers/reconcile_test.go

@@ -96,6 +96,9 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) {
 			},
 		},
 	}
+	deploymentExtraPorts.Annotations = map[string]string{
+		"new-annotation": "new-value",
+	}
 	ingressParams := testCollectorAssertNoErr(t, "test-ingress", "", testFileIngress)
 	ingressParams.Spec.Ingress.Type = "ingress"
 	updatedIngressParams := testCollectorAssertNoErr(t, "test-ingress", "", testFileIngress)
@@ -164,9 +167,15 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) {
 								"app.kubernetes.io/managed-by": "opentelemetry-operator",
 								"app.kubernetes.io/part-of":    "opentelemetry",
 							})
-							exists, err = populateObjectIfExists(t, &v1.ServiceAccount{}, namespacedObjectName(naming.ServiceAccount(params.Name), params.Namespace))
+							sa := &v1.ServiceAccount{}
+							exists, err = populateObjectIfExists(t, sa, namespacedObjectName(naming.ServiceAccount(params.Name), params.Namespace))
 							assert.NoError(t, err)
 							assert.True(t, exists)
+							assert.Equal(t, map[string]string{annotationName: "true"}, sa.Annotations)
+							saPatch := sa.DeepCopy()
+							saPatch.Annotations["user-defined-annotation"] = "value"
+							err = k8sClient.Patch(ctx, saPatch, client.MergeFrom(sa))
+							require.NoError(t, err)
 						},
 					},
 					wantErr:     assert.NoError,
@@ -198,6 +207,12 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) {
 								"app.kubernetes.io/managed-by": "opentelemetry-operator",
 								"app.kubernetes.io/part-of":    "opentelemetry",
 							})
+
+							sa := &v1.ServiceAccount{}
+							exists, err = populateObjectIfExists(t, sa, namespacedObjectName(naming.ServiceAccount(params.Name), params.Namespace))
+							assert.NoError(t, err)
+							assert.True(t, exists)
+							assert.Equal(t, map[string]string{annotationName: "true", "user-defined-annotation": "value", "new-annotation": "new-value"}, sa.Annotations)
 						},
 					},
 					wantErr:     assert.NoError,

internal/manifests/mutate.go

@@ -194,7 +194,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

@@ -0,0 +1,50 @@
+// Copyright The OpenTelemetry Authors
+//
+// 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 manifests
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+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 := 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)
+}