Ensuring the collector behavior remains consistent when mTLS feature gate is enabled but TA is not deployed

.chloggen/Fix-Prevent-mounting-secrets-to-collector-when-TA-is-not-deployed-and-mTLS-feature-gate-is-enabled.yaml

@@ -0,0 +1,16 @@
+# 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: Prevent mounting secrets to collector when TA is not deployed and mTLS feature gate is enabled
+
+# One or more tracking issues related to the change
+issues: [3456]
+
+# (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:

internal/manifests/collector/configmap.go

@@ -45,7 +45,7 @@ func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) {
 
 	replaceCfgOpts := []ta.TAOption{}
 
-	if params.Config.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
+	if params.OtelCol.Spec.TargetAllocator.Enabled && params.Config.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
 		replaceCfgOpts = append(replaceCfgOpts, ta.WithTLSConfig(
 			filepath.Join(constants.TACollectorTLSDirPath, constants.TACollectorCAFileName),
 			filepath.Join(constants.TACollectorTLSDirPath, constants.TACollectorTLSCertFileName),

internal/manifests/collector/configmap_test.go

@@ -182,4 +182,59 @@ service:
 		assert.NoError(t, err)
 
 	})
+
+	t.Run("Should return expected collector config map without mTLS config", func(t *testing.T) {
+		expectedData := map[string]string{
+			"collector.yaml": `exporters:
+  debug:
+receivers:
+  prometheus:
+    config:
+      scrape_configs:
+      - job_name: serviceMonitor/test/test/0
+        static_configs:
+        - targets: ["prom.domain:1001", "prom.domain:1002", "prom.domain:1003"]
+          labels:
+            my: label
+        file_sd_configs:
+        - files:
+          - file2.json
+service:
+  pipelines:
+    metrics:
+      exporters:
+      - debug
+      receivers:
+      - prometheus
+`,
+		}
+
+		param, err := newParams("test/test-img", "testdata/http_sd_config_servicemonitor_test.yaml", config.WithCertManagerAvailability(certmanager.Available))
+		require.NoError(t, err)
+		flgs := featuregate.Flags(colfg.GlobalRegistry())
+		err = flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
+		param.TargetAllocator = nil
+		require.NoError(t, err)
+
+		hash, _ := manifestutils.GetConfigMapSHA(param.OtelCol.Spec.Config)
+		expectedName := naming.ConfigMap("test", hash)
+
+		expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector"
+		expectedLables["app.kubernetes.io/name"] = "test-collector"
+		expectedLables["app.kubernetes.io/version"] = "latest"
+
+		actual, err := ConfigMap(param)
+
+		assert.NoError(t, err)
+		assert.Equal(t, expectedName, actual.Name)
+		assert.Equal(t, expectedLables, actual.Labels)
+		assert.Equal(t, len(expectedData), len(actual.Data))
+		for k, expected := range expectedData {
+			assert.YAMLEq(t, expected, actual.Data[k])
+		}
+
+		// Reset the value
+		expectedLables["app.kubernetes.io/version"] = "0.47.0"
+		assert.NoError(t, err)
+	})
 }

internal/manifests/collector/container.go

@@ -85,7 +85,7 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme
 			})
 	}
 
-	if cfg.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
+	if otelcol.Spec.TargetAllocator.Enabled && cfg.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
 		volumeMounts = append(volumeMounts,
 			corev1.VolumeMount{
 				Name:      naming.TAClientCertificate(otelcol.Name),

internal/manifests/collector/container_test.go

@@ -873,6 +873,8 @@ func TestContainerWithCertManagerAvailable(t *testing.T) {
 
 	flgs := featuregate.Flags(colfg.GlobalRegistry())
 	err := flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
+	otelcol.Spec.TargetAllocator.Enabled = true
+
 	require.NoError(t, err)
 
 	// test
@@ -884,3 +886,23 @@ func TestContainerWithCertManagerAvailable(t *testing.T) {
 		MountPath: constants.TACollectorTLSDirPath,
 	})
 }
+
+func TestContainerWithFeaturegateEnabledButTADisabled(t *testing.T) {
+	otelcol := v1beta1.OpenTelemetryCollector{}
+
+	cfg := config.New(config.WithCertManagerAvailability(certmanager.Available))
+
+	flgs := featuregate.Flags(colfg.GlobalRegistry())
+	err := flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
+
+	require.NoError(t, err)
+
+	// test
+	c := Container(cfg, logger, otelcol, true)
+
+	// verify
+	assert.NotContains(t, c.VolumeMounts, corev1.VolumeMount{
+		Name:      naming.TAClientCertificate(""),
+		MountPath: constants.TACollectorTLSDirPath,
+	})
+}

internal/manifests/collector/volume.go

@@ -43,7 +43,7 @@ func Volumes(cfg config.Config, otelcol v1beta1.OpenTelemetryCollector) []corev1
 		},
 	}}
 
-	if cfg.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
+	if otelcol.Spec.TargetAllocator.Enabled && cfg.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
 		volumes = append(volumes, corev1.Volume{
 			Name: naming.TAClientCertificate(otelcol.Name),
 			VolumeSource: corev1.VolumeSource{

internal/manifests/collector/volume_test.go

@@ -106,6 +106,7 @@ func TestVolumeWithTargetAllocatorMTLS(t *testing.T) {
 
 		flgs := featuregate.Flags(colfg.GlobalRegistry())
 		err := flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
+		otelcol.Spec.TargetAllocator.Enabled = true
 		require.NoError(t, err)
 
 		volumes := Volumes(cfg, otelcol)
@@ -140,4 +141,25 @@ func TestVolumeWithTargetAllocatorMTLS(t *testing.T) {
 		volumes := Volumes(cfg, otelcol)
 		assert.NotContains(t, volumes, corev1.Volume{Name: naming.TAClientCertificate(otelcol.Name)})
 	})
+
+	t.Run("Feature gate enabled but TargetAllocator disabled", func(t *testing.T) {
+		otelcol := v1beta1.OpenTelemetryCollector{}
+		cfg := config.New(config.WithCertManagerAvailability(certmanager.Available))
+
+		flgs := featuregate.Flags(colfg.GlobalRegistry())
+		err := flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
+
+		require.NoError(t, err)
+
+		volumes := Volumes(cfg, otelcol)
+		unexpectedVolume := corev1.Volume{
+			Name: naming.TAClientCertificate(otelcol.Name),
+			VolumeSource: corev1.VolumeSource{
+				Secret: &corev1.SecretVolumeSource{
+					SecretName: naming.TAClientCertificateSecretName(otelcol.Name),
+				},
+			},
+		}
+		assert.NotContains(t, volumes, unexpectedVolume)
+	})
 }

tests/e2e-ta-collector-mtls/ta-disabled/00-assert.yaml

@@ -0,0 +1,33 @@
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: prometheus-cr-collector
+status:
+  readyReplicas: 1
+  replicas: 1
+---
+apiVersion: v1
+data:
+  collector.yaml: |
+    receivers:
+      otlp:
+        protocols:
+          grpc:
+            endpoint: 0.0.0.0:4317
+          http:
+            endpoint: 0.0.0.0:4318
+    exporters:
+      debug: null
+    service:
+      telemetry:
+        metrics:
+          address: 0.0.0.0:8888
+      pipelines:
+        traces:
+          exporters:
+            - debug
+          receivers:
+            - otlp
+kind: ConfigMap
+metadata:
+  name: prometheus-cr-collector-aec5aa11

tests/e2e-ta-collector-mtls/ta-disabled/00-install.yaml

@@ -0,0 +1,76 @@
+apiVersion: v1
+automountServiceAccountToken: true
+kind: ServiceAccount
+metadata:
+  name: collector
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: (join('-', ['collector', $namespace]))
+rules:
+  - apiGroups:
+      - ""
+    resources:
+      - pods
+      - nodes
+      - nodes/metrics
+      - services
+      - endpoints
+      - namespaces
+    verbs:
+      - get
+      - watch
+      - list
+  - apiGroups:
+      - networking.k8s.io
+    resources:
+      - ingresses
+    verbs:
+      - get
+      - watch
+      - list
+  - nonResourceURLs:
+      - /metrics
+      - /metrics/cadvisor
+    verbs:
+      - get
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: (join('-', ['collector', $namespace]))
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: (join('-', ['collector', $namespace]))
+subjects:
+  - kind: ServiceAccount
+    name: collector
+    namespace: ($namespace)
+---
+apiVersion: opentelemetry.io/v1alpha1
+kind: OpenTelemetryCollector
+metadata:
+  name: prometheus-cr
+spec:
+  config: |
+    receivers:
+      otlp:
+        protocols:
+          grpc:
+          http:
+    processors:
+
+    exporters:
+      debug:
+
+    service:
+      pipelines:
+        traces:
+          receivers: [otlp]
+          exporters: [debug]
+  mode: statefulset
+  serviceAccount: collector
+  targetAllocator:
+    enabled: false

tests/e2e-ta-collector-mtls/ta-disabled/chainsaw-test.yaml

@@ -0,0 +1,18 @@
+# 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: ta-disabled
+spec:
+  steps:
+  - name: step-00
+    try:
+    - apply:
+        template: true
+        file: 00-install.yaml
+    - assert:
+        file: 00-assert.yaml
+    catch:
+    - podLogs:
+        selector: app.kubernetes.io/managed-by=opentelemetry-operator