diff --git a/.chloggen/main.yaml b/.chloggen/main.yaml new file mode 100755 index 0000000000..62e75201ca --- /dev/null +++ b/.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: ` + to the service account which contains secret name with image pull secret. diff --git a/controllers/reconcile_test.go b/controllers/reconcile_test.go index 211f7e7048..53385015c1 100644 --- a/controllers/reconcile_test.go +++ b/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, diff --git a/internal/manifests/mutate.go b/internal/manifests/mutate.go index 9ac2d04fd2..75c1a07804 100644 --- a/internal/manifests/mutate.go +++ b/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 } diff --git a/internal/manifests/mutate_test.go b/internal/manifests/mutate_test.go new file mode 100644 index 0000000000..c165c8535c --- /dev/null +++ b/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) +}