From b64ed2ccbbd3e2b52176f9391a33fe65fef9a0a2 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Wed, 23 Oct 2024 18:33:58 +0200 Subject: [PATCH 1/6] Add automatic RBAC creation for kubeletstats receiver Signed-off-by: Israel Blancas --- .chloggen/kubeletstats.yaml | 16 +++ Makefile | 5 +- internal/components/receivers/helpers.go | 4 +- internal/components/receivers/kubeletstats.go | 85 ++++++++++++++++ .../components/receivers/kubeletstats_test.go | 99 +++++++++++++++++++ internal/components/receivers/scraper_test.go | 1 - .../nodes-proxy.yaml | 10 ++ .../nodes-stats.yaml | 10 ++ .../extra-permissions-operator/nodes.yaml | 20 ++++ .../receiver-kubeletstats/00-install.yaml | 4 + .../receiver-kubeletstats/01-assert.yaml | 27 +++++ .../receiver-kubeletstats/01-install.yaml | 19 ++++ .../receiver-kubeletstats/02-assert.yaml | 30 ++++++ .../receiver-kubeletstats/02-install.yaml | 20 ++++ .../receiver-kubeletstats/03-assert.yaml | 30 ++++++ .../receiver-kubeletstats/03-install.yaml | 20 ++++ .../receiver-kubeletstats/chainsaw-test.yaml | 30 ++++++ 17 files changed, 427 insertions(+), 3 deletions(-) create mode 100755 .chloggen/kubeletstats.yaml create mode 100644 internal/components/receivers/kubeletstats.go create mode 100644 internal/components/receivers/kubeletstats_test.go create mode 100644 tests/e2e-automatic-rbac/extra-permissions-operator/nodes-proxy.yaml create mode 100644 tests/e2e-automatic-rbac/extra-permissions-operator/nodes-stats.yaml create mode 100644 tests/e2e-automatic-rbac/receiver-kubeletstats/00-install.yaml create mode 100644 tests/e2e-automatic-rbac/receiver-kubeletstats/01-assert.yaml create mode 100644 tests/e2e-automatic-rbac/receiver-kubeletstats/01-install.yaml create mode 100644 tests/e2e-automatic-rbac/receiver-kubeletstats/02-assert.yaml create mode 100644 tests/e2e-automatic-rbac/receiver-kubeletstats/02-install.yaml create mode 100644 tests/e2e-automatic-rbac/receiver-kubeletstats/03-assert.yaml create mode 100644 tests/e2e-automatic-rbac/receiver-kubeletstats/03-install.yaml create mode 100644 tests/e2e-automatic-rbac/receiver-kubeletstats/chainsaw-test.yaml diff --git a/.chloggen/kubeletstats.yaml b/.chloggen/kubeletstats.yaml new file mode 100755 index 0000000000..18d2979c32 --- /dev/null +++ b/.chloggen/kubeletstats.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# 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: "Add automatic RBAC creation for the `kubeletstats` receiver." + +# One or more tracking issues related to the change +issues: [3155] + +# (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: diff --git a/Makefile b/Makefile index 6652b06b86..97742ff360 100644 --- a/Makefile +++ b/Makefile @@ -204,9 +204,12 @@ add-image-opampbridge: add-rbac-permissions-to-operator: manifests kustomize # Kustomize only allows patches in the folder where the kustomization is located # This folder is ignored by .gitignore - cp -r tests/e2e-automatic-rbac/extra-permissions-operator/ config/rbac/extra-permissions-operator + mkdir -p config/rbac/extra-permissions-operator + cp -r tests/e2e-automatic-rbac/extra-permissions-operator/* config/rbac/extra-permissions-operator cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/namespaces.yaml cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/nodes.yaml + cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/nodes-stats.yaml + cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/nodes-proxy.yaml cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/rbac.yaml cd config/rbac && $(KUSTOMIZE) edit add patch --kind ClusterRole --name manager-role --path extra-permissions-operator/replicaset.yaml diff --git a/internal/components/receivers/helpers.go b/internal/components/receivers/helpers.go index 89a3cb6fe7..8438293a7a 100644 --- a/internal/components/receivers/helpers.go +++ b/internal/components/receivers/helpers.go @@ -136,8 +136,10 @@ var ( WithProtocol(corev1.ProtocolTCP). WithTargetPort(3100). MustBuild(), + components.NewBuilder[kubeletStatsConfig]().WithName("kubeletstats"). + WithRbacGen(generateKubeletStatsRbacRules). + MustBuild(), NewScraperParser("prometheus"), - NewScraperParser("kubeletstats"), NewScraperParser("sshcheck"), NewScraperParser("cloudfoundry"), NewScraperParser("vcenter"), diff --git a/internal/components/receivers/kubeletstats.go b/internal/components/receivers/kubeletstats.go new file mode 100644 index 0000000000..df810731db --- /dev/null +++ b/internal/components/receivers/kubeletstats.go @@ -0,0 +1,85 @@ +// 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 receivers + +import ( + "github.com/go-logr/logr" + rbacv1 "k8s.io/api/rbac/v1" +) + +type metricConfig struct { + Enabled bool `mapstructure:"enabled"` +} + +type metrics struct { + K8sContainerCPULimitUtilization metricConfig `mapstructure:"k8s.container.cpu_limit_utilization"` + K8sContainerCPURequestUtilization metricConfig `mapstructure:"k8s.container.cpu_request_utilization"` + K8sContainerMemoryLimitUtilization metricConfig `mapstructure:"k8s.container.memory_limit_utilization"` + K8sContainerMemoryRequestUtilization metricConfig `mapstructure:"k8s.container.memory_request_utilization"` + K8sPodCPULimitUtilization metricConfig `mapstructure:"k8s.pod.cpu_limit_utilization"` + K8sPodCPURequestUtilization metricConfig `mapstructure:"k8s.pod.cpu_request_utilization"` + K8sPodMemoryLimitUtilization metricConfig `mapstructure:"k8s.pod.memory_limit_utilization"` + K8sPodMemoryRequestUtilization metricConfig `mapstructure:"k8s.pod.memory_request_utilization"` +} + +// KubeletStatsConfig is a minimal struct needed for parsing a valid kubeletstats receiver configuration +// This only contains the fields necessary for parsing, other fields can be added in the future. +type kubeletStatsConfig struct { + ExtraMetadataLabels []string `mapstructure:"extra_metadata_labels"` + Metrics metrics `mapstructure:"metrics"` + AuthType string `mapstructure:"auth_type"` +} + +func generateKubeletStatsRbacRules(_ logr.Logger, config kubeletStatsConfig) ([]rbacv1.PolicyRule, error) { + // The Kubelet Stats Receiver needs get permissions on the nodes/stats resources always. + prs := []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"nodes/stats"}, + Verbs: []string{"get"}, + }, + } + + // Additionally, when using extra_metadata_labels or any of the {request|limit}_utilization metrics + // the processor also needs get permissions for nodes/proxy resources. + nodesProxyPr := rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"nodes/proxy"}, + Verbs: []string{"get"}, + } + + if len(config.ExtraMetadataLabels) > 0 { + prs = append(prs, nodesProxyPr) + return prs, nil + } + + metrics := []bool{ + config.Metrics.K8sContainerCPULimitUtilization.Enabled, + config.Metrics.K8sContainerCPURequestUtilization.Enabled, + config.Metrics.K8sContainerMemoryLimitUtilization.Enabled, + config.Metrics.K8sContainerMemoryRequestUtilization.Enabled, + config.Metrics.K8sPodCPULimitUtilization.Enabled, + config.Metrics.K8sPodCPURequestUtilization.Enabled, + config.Metrics.K8sPodMemoryLimitUtilization.Enabled, + config.Metrics.K8sPodMemoryRequestUtilization.Enabled, + } + for _, metric := range metrics { + if metric { + prs = append(prs, nodesProxyPr) + return prs, nil + } + } + return prs, nil +} diff --git a/internal/components/receivers/kubeletstats_test.go b/internal/components/receivers/kubeletstats_test.go new file mode 100644 index 0000000000..246aec5dee --- /dev/null +++ b/internal/components/receivers/kubeletstats_test.go @@ -0,0 +1,99 @@ +// 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 receivers + +import ( + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + rbacv1 "k8s.io/api/rbac/v1" +) + +func TestGenerateKubeletStatsRbacRules(t *testing.T) { + baseRule := rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"nodes/stats"}, + Verbs: []string{"get"}, + } + + proxyRule := rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"nodes/proxy"}, + Verbs: []string{"get"}, + } + + tests := []struct { + name string + config kubeletStatsConfig + expectedRules []rbacv1.PolicyRule + expectedErrMsg string + }{ + { + name: "Default config", + config: kubeletStatsConfig{}, + expectedRules: []rbacv1.PolicyRule{baseRule}, + }, + { + name: "Extra metadata labels", + config: kubeletStatsConfig{ + ExtraMetadataLabels: []string{"label1", "label2"}, + }, + expectedRules: []rbacv1.PolicyRule{baseRule, proxyRule}, + }, + { + name: "CPU limit utilization enabled", + config: kubeletStatsConfig{ + Metrics: metrics{ + K8sContainerCPULimitUtilization: metricConfig{Enabled: true}, + }, + }, + expectedRules: []rbacv1.PolicyRule{baseRule, proxyRule}, + }, + { + name: "Memory request utilization enabled", + config: kubeletStatsConfig{ + Metrics: metrics{ + K8sPodMemoryRequestUtilization: metricConfig{Enabled: true}, + }, + }, + expectedRules: []rbacv1.PolicyRule{baseRule, proxyRule}, + }, + { + name: "No extra permissions needed", + config: kubeletStatsConfig{ + Metrics: metrics{ + K8sContainerCPULimitUtilization: metricConfig{Enabled: false}, + }, + }, + expectedRules: []rbacv1.PolicyRule{baseRule}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rules, err := generateKubeletStatsRbacRules(logr.Logger{}, tt.config) + + if tt.expectedErrMsg != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedErrMsg) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectedRules, rules) + } + }) + } +} diff --git a/internal/components/receivers/scraper_test.go b/internal/components/receivers/scraper_test.go index 59ac2eec1f..ed7a4bbccd 100644 --- a/internal/components/receivers/scraper_test.go +++ b/internal/components/receivers/scraper_test.go @@ -29,7 +29,6 @@ func TestScraperParsers(t *testing.T) { defaultPort int }{ {"prometheus", "__prometheus", 0}, - {"kubeletstats", "__kubeletstats", 0}, {"sshcheck", "__sshcheck", 0}, {"cloudfoundry", "__cloudfoundry", 0}, {"vcenter", "__vcenter", 0}, diff --git a/tests/e2e-automatic-rbac/extra-permissions-operator/nodes-proxy.yaml b/tests/e2e-automatic-rbac/extra-permissions-operator/nodes-proxy.yaml new file mode 100644 index 0000000000..f5c1bd4393 --- /dev/null +++ b/tests/e2e-automatic-rbac/extra-permissions-operator/nodes-proxy.yaml @@ -0,0 +1,10 @@ +--- +- op: add + path: /rules/- + value: + apiGroups: + - "" + resources: + - nodes/proxy + verbs: + - get diff --git a/tests/e2e-automatic-rbac/extra-permissions-operator/nodes-stats.yaml b/tests/e2e-automatic-rbac/extra-permissions-operator/nodes-stats.yaml new file mode 100644 index 0000000000..2eb073fbed --- /dev/null +++ b/tests/e2e-automatic-rbac/extra-permissions-operator/nodes-stats.yaml @@ -0,0 +1,10 @@ +--- +- op: add + path: /rules/- + value: + apiGroups: + - "" + resources: + - nodes/stats + verbs: + - get diff --git a/tests/e2e-automatic-rbac/extra-permissions-operator/nodes.yaml b/tests/e2e-automatic-rbac/extra-permissions-operator/nodes.yaml index 3971ded1a4..12cd11bd9d 100644 --- a/tests/e2e-automatic-rbac/extra-permissions-operator/nodes.yaml +++ b/tests/e2e-automatic-rbac/extra-permissions-operator/nodes.yaml @@ -10,3 +10,23 @@ - get - list - watch +--- +- op: add + path: /rules/- + value: + apiGroups: + - "" + resources: + - nodes/proxy + verbs: + - get +--- +- op: add + path: /rules/- + value: + apiGroups: + - "" + resources: + - nodes/stats + verbs: + - get diff --git a/tests/e2e-automatic-rbac/receiver-kubeletstats/00-install.yaml b/tests/e2e-automatic-rbac/receiver-kubeletstats/00-install.yaml new file mode 100644 index 0000000000..919491411b --- /dev/null +++ b/tests/e2e-automatic-rbac/receiver-kubeletstats/00-install.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: chainsaw-kubeletstats \ No newline at end of file diff --git a/tests/e2e-automatic-rbac/receiver-kubeletstats/01-assert.yaml b/tests/e2e-automatic-rbac/receiver-kubeletstats/01-assert.yaml new file mode 100644 index 0000000000..8d0905900f --- /dev/null +++ b/tests/e2e-automatic-rbac/receiver-kubeletstats/01-assert.yaml @@ -0,0 +1,27 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: simplest-chainsaw-kubeletstats-cluster-role +rules: +- apiGroups: [""] + resources: ["nodes/stats"] + verbs: ["get"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: chainsaw-kubeletstats.simplest + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/name: simplest-chainsaw-kubeletstats-collector + app.kubernetes.io/part-of: opentelemetry + name: simplest-chainsaw-kubeletstats-collector +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: simplest-chainsaw-kubeletstats-cluster-role +subjects: +- kind: ServiceAccount + name: simplest-collector + namespace: chainsaw-kubeletstats diff --git a/tests/e2e-automatic-rbac/receiver-kubeletstats/01-install.yaml b/tests/e2e-automatic-rbac/receiver-kubeletstats/01-install.yaml new file mode 100644 index 0000000000..027d213129 --- /dev/null +++ b/tests/e2e-automatic-rbac/receiver-kubeletstats/01-install.yaml @@ -0,0 +1,19 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest + namespace: chainsaw-kubeletstats +spec: + config: | + receivers: + kubeletstats: + auth_type: "" + processors: + exporters: + debug: + service: + pipelines: + traces: + receivers: [kubeletstats] + processors: [] + exporters: [debug] diff --git a/tests/e2e-automatic-rbac/receiver-kubeletstats/02-assert.yaml b/tests/e2e-automatic-rbac/receiver-kubeletstats/02-assert.yaml new file mode 100644 index 0000000000..bf5b707020 --- /dev/null +++ b/tests/e2e-automatic-rbac/receiver-kubeletstats/02-assert.yaml @@ -0,0 +1,30 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: simplest-chainsaw-kubeletstats-cluster-role +rules: +- apiGroups: [""] + resources: ["nodes/stats"] + verbs: ["get"] +- apiGroups: [""] + resources: ["nodes/proxy"] + verbs: ["get"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: chainsaw-kubeletstats.simplest + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/name: simplest-chainsaw-kubeletstats-collector + app.kubernetes.io/part-of: opentelemetry + name: simplest-chainsaw-kubeletstats-collector +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: simplest-chainsaw-kubeletstats-cluster-role +subjects: +- kind: ServiceAccount + name: simplest-collector + namespace: chainsaw-kubeletstats diff --git a/tests/e2e-automatic-rbac/receiver-kubeletstats/02-install.yaml b/tests/e2e-automatic-rbac/receiver-kubeletstats/02-install.yaml new file mode 100644 index 0000000000..8452f05228 --- /dev/null +++ b/tests/e2e-automatic-rbac/receiver-kubeletstats/02-install.yaml @@ -0,0 +1,20 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest + namespace: chainsaw-kubeletstats +spec: + config: | + receivers: + kubeletstats: + extra_metadata_labels: + - container.id + processors: + exporters: + debug: + service: + pipelines: + traces: + receivers: [kubeletstats] + processors: [] + exporters: [debug] diff --git a/tests/e2e-automatic-rbac/receiver-kubeletstats/03-assert.yaml b/tests/e2e-automatic-rbac/receiver-kubeletstats/03-assert.yaml new file mode 100644 index 0000000000..bf5b707020 --- /dev/null +++ b/tests/e2e-automatic-rbac/receiver-kubeletstats/03-assert.yaml @@ -0,0 +1,30 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: simplest-chainsaw-kubeletstats-cluster-role +rules: +- apiGroups: [""] + resources: ["nodes/stats"] + verbs: ["get"] +- apiGroups: [""] + resources: ["nodes/proxy"] + verbs: ["get"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: chainsaw-kubeletstats.simplest + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/name: simplest-chainsaw-kubeletstats-collector + app.kubernetes.io/part-of: opentelemetry + name: simplest-chainsaw-kubeletstats-collector +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: simplest-chainsaw-kubeletstats-cluster-role +subjects: +- kind: ServiceAccount + name: simplest-collector + namespace: chainsaw-kubeletstats diff --git a/tests/e2e-automatic-rbac/receiver-kubeletstats/03-install.yaml b/tests/e2e-automatic-rbac/receiver-kubeletstats/03-install.yaml new file mode 100644 index 0000000000..8452f05228 --- /dev/null +++ b/tests/e2e-automatic-rbac/receiver-kubeletstats/03-install.yaml @@ -0,0 +1,20 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest + namespace: chainsaw-kubeletstats +spec: + config: | + receivers: + kubeletstats: + extra_metadata_labels: + - container.id + processors: + exporters: + debug: + service: + pipelines: + traces: + receivers: [kubeletstats] + processors: [] + exporters: [debug] diff --git a/tests/e2e-automatic-rbac/receiver-kubeletstats/chainsaw-test.yaml b/tests/e2e-automatic-rbac/receiver-kubeletstats/chainsaw-test.yaml new file mode 100644 index 0000000000..f693722ed3 --- /dev/null +++ b/tests/e2e-automatic-rbac/receiver-kubeletstats/chainsaw-test.yaml @@ -0,0 +1,30 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: receiver-kubeletstats +spec: + steps: + - name: create-namespace + try: + - apply: + file: 00-install.yaml + - name: default-config + try: + - apply: + file: 01-install.yaml + - assert: + file: 01-assert.yaml + - name: use-extra_metadata_labels + try: + - apply: + file: 02-install.yaml + - assert: + file: 02-assert.yaml + - name: k8snode-detector + try: + - apply: + file: 03-install.yaml + - assert: + file: 03-assert.yaml \ No newline at end of file From c0d7702659b0f6b0533d8867614e8bb199cb7ddd Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Thu, 24 Oct 2024 15:51:53 +0200 Subject: [PATCH 2/6] Inject K8S_NODE_NAME environment variable when using the kubeletstats receiver Signed-off-by: Israel Blancas --- ...779-kubeletstatsreiver-inject-en-vars.yaml | 16 ++++++++ apis/v1beta1/config.go | 40 +++++++++++++++++++ internal/components/builder.go | 7 +++- internal/components/component.go | 7 ++++ internal/components/generic_parser.go | 12 ++++++ internal/components/multi_endpoint.go | 4 ++ internal/components/receivers/helpers.go | 1 + internal/components/receivers/kubeletstats.go | 13 ++++++ internal/manifests/collector/container.go | 6 +++ 9 files changed, 105 insertions(+), 1 deletion(-) create mode 100755 .chloggen/2779-kubeletstatsreiver-inject-en-vars.yaml diff --git a/.chloggen/2779-kubeletstatsreiver-inject-en-vars.yaml b/.chloggen/2779-kubeletstatsreiver-inject-en-vars.yaml new file mode 100755 index 0000000000..eb48092056 --- /dev/null +++ b/.chloggen/2779-kubeletstatsreiver-inject-en-vars.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# 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: Inject environment K8S_NODE_NAME environment variable for the Kubelet Stats Receiver. + +# One or more tracking issues related to the change +issues: [2779] + +# (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: diff --git a/apis/v1beta1/config.go b/apis/v1beta1/config.go index 549aff9815..67c6a77d59 100644 --- a/apis/v1beta1/config.go +++ b/apis/v1beta1/config.go @@ -226,6 +226,42 @@ func (c *Config) getPortsForComponentKinds(logger logr.Logger, componentKinds .. return ports, nil } +// getEnvironmentVariablesForComponentKinds gets the environment variables for the given ComponentKind(s). +func (c *Config) getEnvironmentVariablesForComponentKinds(logger logr.Logger, componentKinds ...ComponentKind) ([]corev1.EnvVar, error) { + var envVars []corev1.EnvVar + enabledComponents := c.GetEnabledComponents() + for _, componentKind := range componentKinds { + var retriever components.ParserRetriever + var cfg AnyConfig + + switch componentKind { + case KindReceiver: + retriever = receivers.ReceiverFor + cfg = c.Receivers + case KindExporter: + continue + case KindProcessor: + continue + case KindExtension: + continue + } + for componentName := range enabledComponents[componentKind] { + parser := retriever(componentName) + if parsedEnvVars, err := parser.GetEnvironmentVariables(logger, cfg.Object[componentName]); err != nil { + return nil, err + } else { + envVars = append(envVars, parsedEnvVars...) + } + } + } + + sort.Slice(envVars, func(i, j int) bool { + return envVars[i].Name < envVars[j].Name + }) + + return envVars, nil +} + // applyDefaultForComponentKinds applies defaults to the endpoints for the given ComponentKind(s). func (c *Config) applyDefaultForComponentKinds(logger logr.Logger, componentKinds ...ComponentKind) error { if err := c.Service.ApplyDefaults(); err != nil { @@ -286,6 +322,10 @@ func (c *Config) GetAllPorts(logger logr.Logger) ([]corev1.ServicePort, error) { return c.getPortsForComponentKinds(logger, KindReceiver, KindExporter) } +func (c *Config) GetEnvironmentVariables(logger logr.Logger) ([]corev1.EnvVar, error) { + return c.getEnvironmentVariablesForComponentKinds(logger, KindReceiver) +} + func (c *Config) GetAllRbacRules(logger logr.Logger) ([]rbacv1.PolicyRule, error) { return c.getRbacRulesForComponentKinds(logger, KindReceiver, KindExporter, KindProcessor) } diff --git a/internal/components/builder.go b/internal/components/builder.go index 7abc174703..61a4126fc0 100644 --- a/internal/components/builder.go +++ b/internal/components/builder.go @@ -38,6 +38,7 @@ type Settings[ComponentConfigType any] struct { livenessGen ProbeGenerator[ComponentConfigType] readinessGen ProbeGenerator[ComponentConfigType] defaultsApplier Defaulter[ComponentConfigType] + envVarGen EnvVarGenerator[ComponentConfigType] } func NewEmptySettings[ComponentConfigType any]() *Settings[ComponentConfigType] { @@ -124,7 +125,11 @@ func (b Builder[ComponentConfigType]) WithReadinessGen(readinessGen ProbeGenerat o.readinessGen = readinessGen }) } - +func (b Builder[ComponentConfigType]) WithEnvVarGen(envVarGen EnvVarGenerator[ComponentConfigType]) Builder[ComponentConfigType] { + return append(b, func(o *Settings[ComponentConfigType]) { + o.envVarGen = envVarGen + }) +} func (b Builder[ComponentConfigType]) WithDefaultsApplier(defaultsApplier Defaulter[ComponentConfigType]) Builder[ComponentConfigType] { return append(b, func(o *Settings[ComponentConfigType]) { o.defaultsApplier = defaultsApplier diff --git a/internal/components/component.go b/internal/components/component.go index 3feb56d6a7..5c8975b9c2 100644 --- a/internal/components/component.go +++ b/internal/components/component.go @@ -49,6 +49,10 @@ type RBACRuleGenerator[ComponentConfigType any] func(logger logr.Logger, config // It's expected that type Config is the configuration used by a parser. type ProbeGenerator[ComponentConfigType any] func(logger logr.Logger, config ComponentConfigType) (*corev1.Probe, error) +// EnvVarGenerator is a function that generates a list of environment variables for a given config. +// It's expected that type Config is the configuration used by a parser. +type EnvVarGenerator[ComponentConfigType any] func(logger logr.Logger, config ComponentConfigType) ([]corev1.EnvVar, error) + // Defaulter is a function that applies given defaults to the passed Config. // It's expected that type Config is the configuration used by a parser. type Defaulter[ComponentConfigType any] func(logger logr.Logger, defaultAddr string, defaultPort int32, config ComponentConfigType) (map[string]interface{}, error) @@ -105,6 +109,9 @@ type Parser interface { // GetLivenessProbe returns a liveness probe set for the collector GetLivenessProbe(logger logr.Logger, config interface{}) (*corev1.Probe, error) + // GetEnvironmentVariables returns a list of environment variables for the collector + GetEnvironmentVariables(logger logr.Logger, config interface{}) ([]corev1.EnvVar, error) + // GetReadinessProbe returns a readiness probe set for the collector GetReadinessProbe(logger logr.Logger, config interface{}) (*corev1.Probe, error) diff --git a/internal/components/generic_parser.go b/internal/components/generic_parser.go index 02887a5892..a3a40e819d 100644 --- a/internal/components/generic_parser.go +++ b/internal/components/generic_parser.go @@ -34,6 +34,7 @@ type GenericParser[T any] struct { settings *Settings[T] portParser PortParser[T] rbacGen RBACRuleGenerator[T] + envVarGen EnvVarGenerator[T] livenessGen ProbeGenerator[T] readinessGen ProbeGenerator[T] defaultsApplier Defaulter[T] @@ -88,6 +89,17 @@ func (g *GenericParser[T]) GetRBACRules(logger logr.Logger, config interface{}) return g.rbacGen(logger, parsed) } +func (g *GenericParser[T]) GetEnvironmentVariables(logger logr.Logger, config interface{}) ([]corev1.EnvVar, error) { + if g.envVarGen == nil { + return nil, nil + } + var parsed T + if err := mapstructure.Decode(config, &parsed); err != nil { + return nil, err + } + return g.envVarGen(logger, parsed) +} + func (g *GenericParser[T]) Ports(logger logr.Logger, name string, config interface{}) ([]corev1.ServicePort, error) { if g.portParser == nil { return nil, nil diff --git a/internal/components/multi_endpoint.go b/internal/components/multi_endpoint.go index 39449cda2b..9c7019cb6d 100644 --- a/internal/components/multi_endpoint.go +++ b/internal/components/multi_endpoint.go @@ -116,6 +116,10 @@ func (m *MultiPortReceiver) GetRBACRules(logr.Logger, interface{}) ([]rbacv1.Pol return nil, nil } +func (m *MultiPortReceiver) GetEnvironmentVariables(logger logr.Logger, config interface{}) ([]corev1.EnvVar, error) { + return nil, nil +} + type MultiPortBuilder[ComponentConfigType any] []Builder[ComponentConfigType] func NewMultiPortReceiverBuilder(name string) MultiPortBuilder[*MultiProtocolEndpointConfig] { diff --git a/internal/components/receivers/helpers.go b/internal/components/receivers/helpers.go index 8438293a7a..7271fc5548 100644 --- a/internal/components/receivers/helpers.go +++ b/internal/components/receivers/helpers.go @@ -138,6 +138,7 @@ var ( MustBuild(), components.NewBuilder[kubeletStatsConfig]().WithName("kubeletstats"). WithRbacGen(generateKubeletStatsRbacRules). + WithEnvVarGen(generateKubeletStatsEnvVars). MustBuild(), NewScraperParser("prometheus"), NewScraperParser("sshcheck"), diff --git a/internal/components/receivers/kubeletstats.go b/internal/components/receivers/kubeletstats.go index df810731db..d9307b6faa 100644 --- a/internal/components/receivers/kubeletstats.go +++ b/internal/components/receivers/kubeletstats.go @@ -16,6 +16,7 @@ package receivers import ( "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" ) @@ -42,6 +43,18 @@ type kubeletStatsConfig struct { AuthType string `mapstructure:"auth_type"` } +func generateKubeletStatsEnvVars(_ logr.Logger, config kubeletStatsConfig) ([]corev1.EnvVar, error) { + // The documentation mentions that the K8S_NODE_NAME environment variable is required when using the serviceAccount auth type. + // Also, it mentions that it is a good idea to use it for the Read Only Endpoint. Added always to make it easier for users. + // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/kubeletstatsreceiver/README.md + if config.AuthType == "serviceAccount" { + return []corev1.EnvVar{ + {Name: "K8S_NODE_NAME", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{FieldPath: "spec.nodeName"}}}, + }, nil + } + return nil, nil +} + func generateKubeletStatsRbacRules(_ logr.Logger, config kubeletStatsConfig) ([]rbacv1.PolicyRule, error) { // The Kubelet Stats Receiver needs get permissions on the nodes/stats resources always. prs := []rbacv1.PolicyRule{ diff --git a/internal/manifests/collector/container.go b/internal/manifests/collector/container.go index 69afc65416..4687442cdd 100644 --- a/internal/manifests/collector/container.go +++ b/internal/manifests/collector/container.go @@ -177,6 +177,12 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme ) } + if envVars, err := otelcol.Spec.Config.GetEnvironmentVariables(logger); err != nil { + logger.Error(err, "could not get the environment variables from the config") + } else { + envVars = append(envVars, envVars...) + } + envVars = append(envVars, proxy.ReadProxyVarsFromEnv()...) return corev1.Container{ Name: naming.Container(), From b5f3fd1ebd8455e4437442020e3c220476a6e655 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Mon, 28 Oct 2024 18:03:08 +0100 Subject: [PATCH 3/6] Revert change Signed-off-by: Israel Blancas --- internal/components/receivers/scraper_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/components/receivers/scraper_test.go b/internal/components/receivers/scraper_test.go index ed7a4bbccd..59ac2eec1f 100644 --- a/internal/components/receivers/scraper_test.go +++ b/internal/components/receivers/scraper_test.go @@ -29,6 +29,7 @@ func TestScraperParsers(t *testing.T) { defaultPort int }{ {"prometheus", "__prometheus", 0}, + {"kubeletstats", "__kubeletstats", 0}, {"sshcheck", "__sshcheck", 0}, {"cloudfoundry", "__cloudfoundry", 0}, {"vcenter", "__vcenter", 0}, From 5b0cacc45faca6abfa40b684042ef6da830b42d0 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Tue, 29 Oct 2024 17:47:00 +0100 Subject: [PATCH 4/6] Fix lint Signed-off-by: Israel Blancas --- internal/manifests/collector/container.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/manifests/collector/container.go b/internal/manifests/collector/container.go index 4687442cdd..f499f08c55 100644 --- a/internal/manifests/collector/container.go +++ b/internal/manifests/collector/container.go @@ -177,10 +177,10 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme ) } - if envVars, err := otelcol.Spec.Config.GetEnvironmentVariables(logger); err != nil { + if configEnvVars, err := otelcol.Spec.Config.GetEnvironmentVariables(logger); err != nil { logger.Error(err, "could not get the environment variables from the config") } else { - envVars = append(envVars, envVars...) + envVars = append(envVars, configEnvVars...) } envVars = append(envVars, proxy.ReadProxyVarsFromEnv()...) From d94ca86ff648e555829a26f9bfecb0adb6944322 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Tue, 29 Oct 2024 19:27:20 +0100 Subject: [PATCH 5/6] Add missing tests Signed-off-by: Israel Blancas --- apis/v1beta1/config.go | 2 +- apis/v1beta1/config_test.go | 60 +++++++++++++++++++ internal/components/builder.go | 1 + internal/components/receivers/helpers.go | 3 + internal/components/receivers/kubeletstats.go | 9 +-- .../receiver-kubeletstats/01-assert.yaml | 21 +++++++ 6 files changed, 89 insertions(+), 7 deletions(-) diff --git a/apis/v1beta1/config.go b/apis/v1beta1/config.go index 67c6a77d59..82fd9a1325 100644 --- a/apis/v1beta1/config.go +++ b/apis/v1beta1/config.go @@ -228,7 +228,7 @@ func (c *Config) getPortsForComponentKinds(logger logr.Logger, componentKinds .. // getEnvironmentVariablesForComponentKinds gets the environment variables for the given ComponentKind(s). func (c *Config) getEnvironmentVariablesForComponentKinds(logger logr.Logger, componentKinds ...ComponentKind) ([]corev1.EnvVar, error) { - var envVars []corev1.EnvVar + var envVars []corev1.EnvVar = []corev1.EnvVar{} enabledComponents := c.GetEnabledComponents() for _, componentKind := range componentKinds { var retriever components.ParserRetriever diff --git a/apis/v1beta1/config_test.go b/apis/v1beta1/config_test.go index 44828d043b..b9c288f692 100644 --- a/apis/v1beta1/config_test.go +++ b/apis/v1beta1/config_test.go @@ -423,6 +423,66 @@ func TestConfig_GetEnabledComponents(t *testing.T) { } } +func TestConfig_getEnvironmentVariablesForComponentKinds(t *testing.T) { + tests := []struct { + name string + config *Config + componentKinds []ComponentKind + envVarsLen int + }{ + { + name: "no env vars", + config: &Config{ + Receivers: AnyConfig{ + Object: map[string]interface{}{ + "myreceiver": map[string]interface{}{ + "env": "test", + }, + }, + }, + Service: Service{ + Pipelines: map[string]*Pipeline{ + "test": { + Receivers: []string{"myreceiver"}, + }, + }, + }, + }, + componentKinds: []ComponentKind{KindReceiver}, + envVarsLen: 0, + }, + { + name: "kubeletstats env vars", + config: &Config{ + Receivers: AnyConfig{ + Object: map[string]interface{}{ + "kubeletstats": map[string]interface{}{}, + }, + }, + Service: Service{ + Pipelines: map[string]*Pipeline{ + "test": { + Receivers: []string{"kubeletstats"}, + }, + }, + }, + }, + componentKinds: []ComponentKind{KindReceiver}, + envVarsLen: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logger := logr.Discard() + envVars, err := tt.config.GetEnvironmentVariables(logger) + + assert.NoError(t, err) + assert.Len(t, envVars, tt.envVarsLen) + }) + } +} + func TestConfig_GetReceiverPorts(t *testing.T) { tests := []struct { name string diff --git a/internal/components/builder.go b/internal/components/builder.go index 61a4126fc0..a1c9b34bcc 100644 --- a/internal/components/builder.go +++ b/internal/components/builder.go @@ -146,6 +146,7 @@ func (b Builder[ComponentConfigType]) Build() (*GenericParser[ComponentConfigTyp name: o.name, portParser: o.portParser, rbacGen: o.rbacGen, + envVarGen: o.envVarGen, livenessGen: o.livenessGen, readinessGen: o.readinessGen, defaultsApplier: o.defaultsApplier, diff --git a/internal/components/receivers/helpers.go b/internal/components/receivers/helpers.go index 7271fc5548..251e910dcc 100644 --- a/internal/components/receivers/helpers.go +++ b/internal/components/receivers/helpers.go @@ -15,6 +15,8 @@ package receivers import ( + "fmt" + corev1 "k8s.io/api/core/v1" "github.com/open-telemetry/opentelemetry-operator/internal/components" @@ -176,5 +178,6 @@ var ( func init() { for _, parser := range componentParsers { Register(parser.ParserType(), parser) + fmt.Println(parser.ParserType()) } } diff --git a/internal/components/receivers/kubeletstats.go b/internal/components/receivers/kubeletstats.go index d9307b6faa..43f2be8697 100644 --- a/internal/components/receivers/kubeletstats.go +++ b/internal/components/receivers/kubeletstats.go @@ -47,12 +47,9 @@ func generateKubeletStatsEnvVars(_ logr.Logger, config kubeletStatsConfig) ([]co // The documentation mentions that the K8S_NODE_NAME environment variable is required when using the serviceAccount auth type. // Also, it mentions that it is a good idea to use it for the Read Only Endpoint. Added always to make it easier for users. // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/kubeletstatsreceiver/README.md - if config.AuthType == "serviceAccount" { - return []corev1.EnvVar{ - {Name: "K8S_NODE_NAME", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{FieldPath: "spec.nodeName"}}}, - }, nil - } - return nil, nil + return []corev1.EnvVar{ + {Name: "K8S_NODE_NAME", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{FieldPath: "spec.nodeName"}}}, + }, nil } func generateKubeletStatsRbacRules(_ logr.Logger, config kubeletStatsConfig) ([]rbacv1.PolicyRule, error) { diff --git a/tests/e2e-automatic-rbac/receiver-kubeletstats/01-assert.yaml b/tests/e2e-automatic-rbac/receiver-kubeletstats/01-assert.yaml index 8d0905900f..64e07dbd6d 100644 --- a/tests/e2e-automatic-rbac/receiver-kubeletstats/01-assert.yaml +++ b/tests/e2e-automatic-rbac/receiver-kubeletstats/01-assert.yaml @@ -25,3 +25,24 @@ subjects: - kind: ServiceAccount name: simplest-collector namespace: chainsaw-kubeletstats +--- +apiVersion: v1 +kind: Pod +metadata: + labels: + app.kubernetes.io/component: opentelemetry-collector + app.kubernetes.io/instance: chainsaw-kubeletstats.simplest + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/name: simplest-collector + app.kubernetes.io/part-of: opentelemetry + app.kubernetes.io/version: latest + namespace: chainsaw-kubeletstats +spec: + containers: + - name: otc-container + env: + - name: POD_NAME + - name: K8S_NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName From 319f15c832cd21f10c67a0ebfd6a3ce8804ee381 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Thu, 31 Oct 2024 09:47:19 +0100 Subject: [PATCH 6/6] Remove debug statement Signed-off-by: Israel Blancas --- internal/components/receivers/helpers.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/components/receivers/helpers.go b/internal/components/receivers/helpers.go index 251e910dcc..7271fc5548 100644 --- a/internal/components/receivers/helpers.go +++ b/internal/components/receivers/helpers.go @@ -15,8 +15,6 @@ package receivers import ( - "fmt" - corev1 "k8s.io/api/core/v1" "github.com/open-telemetry/opentelemetry-operator/internal/components" @@ -178,6 +176,5 @@ var ( func init() { for _, parser := range componentParsers { Register(parser.ParserType(), parser) - fmt.Println(parser.ParserType()) } }