From bad6ae7cb2f7a4eb3b242aa62bd96b8b4ea3e41d Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Fri, 3 Jun 2022 12:12:05 +0200 Subject: [PATCH] Add creation of ServiceAccount to the Target Allocator (#836) * Added the creation or linking of an existing service account to the target allocator * Fix test * Updated from feedback * Fixed test package --- apis/v1alpha1/opentelemetrycollector_types.go | 4 ++ ...ntelemetry.io_opentelemetrycollectors.yaml | 4 ++ ...ntelemetry.io_opentelemetrycollectors.yaml | 4 ++ docs/api.md | 7 +++ pkg/collector/reconcile/serviceaccount.go | 4 ++ .../reconcile/serviceaccount_test.go | 10 +++- pkg/naming/main.go | 5 ++ pkg/targetallocator/deployment.go | 5 +- pkg/targetallocator/serviceaccount.go | 47 +++++++++++++++ pkg/targetallocator/serviceaccount_test.go | 59 +++++++++++++++++++ .../e2e/smoke-targetallocator/02-assert.yaml | 6 +- 11 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 pkg/targetallocator/serviceaccount.go create mode 100644 pkg/targetallocator/serviceaccount_test.go diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 0fc9991882..25a0e60db7 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -134,6 +134,10 @@ type OpenTelemetryTargetAllocator struct { // +optional PrometheusCR OpenTelemetryTargetAllocatorPrometheusCR `json:"prometheusCR,omitempty"` + // ServiceAccount indicates the name of an existing service account to use with this instance. + // +optional + ServiceAccount string `json:"serviceAccount,omitempty"` + // Image indicates the container image to use for the OpenTelemetry TargetAllocator. // +optional Image string `json:"image,omitempty"` diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index b9eba01c87..173ea68836 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -692,6 +692,10 @@ spec: custom resources as targets or not. type: boolean type: object + serviceAccount: + description: ServiceAccount indicates the name of an existing + service account to use with this instance. + type: string type: object tolerations: description: Toleration to schedule OpenTelemetry Collector pods. diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 4cdacfcf29..60335aeff5 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -690,6 +690,10 @@ spec: custom resources as targets or not. type: boolean type: object + serviceAccount: + description: ServiceAccount indicates the name of an existing + service account to use with this instance. + type: string type: object tolerations: description: Toleration to schedule OpenTelemetry Collector pods. diff --git a/docs/api.md b/docs/api.md index b5213b0b6c..6e947a1127 100644 --- a/docs/api.md +++ b/docs/api.md @@ -2593,6 +2593,13 @@ TargetAllocator indicates a value which determines whether to spawn a target all PrometheusCR defines the configuration for the retrieval of PrometheusOperator CRDs ( servicemonitor.monitoring.coreos.com/v1 and podmonitor.monitoring.coreos.com/v1 ) retrieval. All CR instances which the ServiceAccount has access to will be retrieved. This includes other namespaces.
false + + serviceAccount + string + + ServiceAccount indicates the name of an existing service account to use with this instance.
+ + false diff --git a/pkg/collector/reconcile/serviceaccount.go b/pkg/collector/reconcile/serviceaccount.go index 0cb6cc0292..2aa55e50d9 100644 --- a/pkg/collector/reconcile/serviceaccount.go +++ b/pkg/collector/reconcile/serviceaccount.go @@ -26,6 +26,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" ) // +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;update;patch;delete @@ -36,6 +37,9 @@ func ServiceAccounts(ctx context.Context, params Params) error { if params.Instance.Spec.Mode != v1alpha1.ModeSidecar { desired = append(desired, collector.ServiceAccount(params.Instance)) } + if params.Instance.Spec.TargetAllocator.Enabled { + desired = append(desired, targetallocator.ServiceAccount(params.Instance)) + } // first, handle the create/update parts if err := expectedServiceAccounts(ctx, params, desired); err != nil { diff --git a/pkg/collector/reconcile/serviceaccount_test.go b/pkg/collector/reconcile/serviceaccount_test.go index 2886fae935..db310237c0 100644 --- a/pkg/collector/reconcile/serviceaccount_test.go +++ b/pkg/collector/reconcile/serviceaccount_test.go @@ -24,18 +24,24 @@ import ( "k8s.io/apimachinery/pkg/types" "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" ) func TestExpectedServiceAccounts(t *testing.T) { - t.Run("should create service account", func(t *testing.T) { + t.Run("should create multiple service accounts", func(t *testing.T) { desired := collector.ServiceAccount(params().Instance) - err := expectedServiceAccounts(context.Background(), params(), []v1.ServiceAccount{desired}) + allocatorDesired := targetallocator.ServiceAccount(params().Instance) + err := expectedServiceAccounts(context.Background(), params(), []v1.ServiceAccount{desired, allocatorDesired}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.ServiceAccount{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) assert.NoError(t, err) assert.True(t, exists) + allocatorExists, err := populateObjectIfExists(t, &v1.ServiceAccount{}, types.NamespacedName{Namespace: "default", Name: "test-targetallocator"}) + assert.NoError(t, err) + assert.True(t, allocatorExists) + }) t.Run("should update existing service account", func(t *testing.T) { diff --git a/pkg/naming/main.go b/pkg/naming/main.go index d81b9130c5..f1124fa616 100644 --- a/pkg/naming/main.go +++ b/pkg/naming/main.go @@ -83,3 +83,8 @@ func TAService(otelcol v1alpha1.OpenTelemetryCollector) string { func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) string { return DNSName(Truncate("%s-collector", 63, otelcol.Name)) } + +// TargetAllocatorServiceAccount returns the TargetAllocator service account resource name. +func TargetAllocatorServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) string { + return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) +} diff --git a/pkg/targetallocator/deployment.go b/pkg/targetallocator/deployment.go index 6a68e33bc6..f7be218dda 100644 --- a/pkg/targetallocator/deployment.go +++ b/pkg/targetallocator/deployment.go @@ -49,8 +49,9 @@ func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTele Annotations: otelcol.Spec.PodAnnotations, }, Spec: corev1.PodSpec{ - Containers: []corev1.Container{Container(cfg, logger, otelcol)}, - Volumes: Volumes(cfg, otelcol), + ServiceAccountName: ServiceAccountName(otelcol), + Containers: []corev1.Container{Container(cfg, logger, otelcol)}, + Volumes: Volumes(cfg, otelcol), }, }, }, diff --git a/pkg/targetallocator/serviceaccount.go b/pkg/targetallocator/serviceaccount.go new file mode 100644 index 0000000000..bd6d32df21 --- /dev/null +++ b/pkg/targetallocator/serviceaccount.go @@ -0,0 +1,47 @@ +// 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 targetallocator + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/pkg/naming" +) + +// ServiceAccountName returns the name of the existing or self-provisioned service account to use for the given instance. +func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string { + if len(instance.Spec.TargetAllocator.ServiceAccount) == 0 { + return naming.ServiceAccount(instance) + } + + return instance.Spec.TargetAllocator.ServiceAccount +} + +//ServiceAccount returns the service account for the given instance. +func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) corev1.ServiceAccount { + labels := Labels(otelcol) + labels["app.kubernetes.io/name"] = naming.TargetAllocatorServiceAccount(otelcol) + + return corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.TargetAllocatorServiceAccount(otelcol), + Namespace: otelcol.Namespace, + Labels: labels, + Annotations: otelcol.Annotations, + }, + } +} diff --git a/pkg/targetallocator/serviceaccount_test.go b/pkg/targetallocator/serviceaccount_test.go new file mode 100644 index 0000000000..a5f7e9fc44 --- /dev/null +++ b/pkg/targetallocator/serviceaccount_test.go @@ -0,0 +1,59 @@ +// 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 targetallocator + +import ( + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" +) + +func TestServiceAccountNewDefault(t *testing.T) { + // prepare + otelcol := v1alpha1.OpenTelemetryCollector{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-instance", + }, + } + + // test + sa := ServiceAccountName(otelcol) + + // verify + assert.Equal(t, "my-instance-collector", sa) +} + +func TestServiceAccountOverride(t *testing.T) { + // prepare + otelcol := v1alpha1.OpenTelemetryCollector{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-instance", + }, + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + TargetAllocator: v1alpha1.OpenTelemetryTargetAllocator{ + ServiceAccount: "my-special-sa", + }, + }, + } + + // test + sa := ServiceAccountName(otelcol) + + // verify + assert.Equal(t, "my-special-sa", sa) +} diff --git a/tests/e2e/smoke-targetallocator/02-assert.yaml b/tests/e2e/smoke-targetallocator/02-assert.yaml index 20e774ce99..7cddf56a54 100644 --- a/tests/e2e/smoke-targetallocator/02-assert.yaml +++ b/tests/e2e/smoke-targetallocator/02-assert.yaml @@ -18,4 +18,8 @@ apiVersion: v1 kind: ConfigMap metadata: name: stateful-targetallocator - \ No newline at end of file +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: stateful-targetallocator