diff --git a/.chloggen/Fix-Prevent-mounting-secrets-to-collector-when-TA-is-not-deployed-and-mTLS-feature-gate-is-enabled.yaml b/.chloggen/Fix-Prevent-mounting-secrets-to-collector-when-TA-is-not-deployed-and-mTLS-feature-gate-is-enabled.yaml new file mode 100755 index 0000000000..93cd9107e0 --- /dev/null +++ b/.chloggen/Fix-Prevent-mounting-secrets-to-collector-when-TA-is-not-deployed-and-mTLS-feature-gate-is-enabled.yaml @@ -0,0 +1,9 @@ +change_type: bug_fix + +component: collector + +note: Prevent mounting secrets to collector when TA is not deployed and mTLS feature gate is enabled + +issues: [3456] + +subtext: diff --git a/.chloggen/operator32.yaml b/.chloggen/operator32.yaml deleted file mode 100644 index 43c60adcb1..0000000000 --- a/.chloggen/operator32.yaml +++ /dev/null @@ -1,16 +0,0 @@ -# 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. operator, target allocator, github action) -component: operator - -# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Support for Kubernetes `1.32` version. - -# One or more tracking issues related to the change -issues: [ ] - -# (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/internal/manifests/collector/configmap.go b/internal/manifests/collector/configmap.go index b611dea178..3f68362354 100644 --- a/internal/manifests/collector/configmap.go +++ b/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), diff --git a/internal/manifests/collector/configmap_test.go b/internal/manifests/collector/configmap_test.go index a6469704ea..3adebcaedb 100644 --- a/internal/manifests/collector/configmap_test.go +++ b/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) + }) } diff --git a/internal/manifests/collector/container.go b/internal/manifests/collector/container.go index 5d96258f1d..769cc50ea1 100644 --- a/internal/manifests/collector/container.go +++ b/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), diff --git a/internal/manifests/collector/container_test.go b/internal/manifests/collector/container_test.go index 3f48fc26da..74ea4b1aaa 100644 --- a/internal/manifests/collector/container_test.go +++ b/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, + }) +} diff --git a/internal/manifests/collector/volume.go b/internal/manifests/collector/volume.go index f1bd201056..e59108b5c7 100644 --- a/internal/manifests/collector/volume.go +++ b/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{ diff --git a/internal/manifests/collector/volume_test.go b/internal/manifests/collector/volume_test.go index 03747d519e..4a3394dc8f 100644 --- a/internal/manifests/collector/volume_test.go +++ b/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) + }) } diff --git a/tests/e2e-ta-collector-mtls/ta-disabled/00-assert.yaml b/tests/e2e-ta-collector-mtls/ta-disabled/00-assert.yaml new file mode 100644 index 0000000000..548b61d96b --- /dev/null +++ b/tests/e2e-ta-collector-mtls/ta-disabled/00-assert.yaml @@ -0,0 +1,46 @@ +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: prometheus-cr-collector +status: + readyReplicas: 1 + replicas: 1 +--- +apiVersion: v1 +data: + collector.yaml: | + receivers: + jaeger: + protocols: + grpc: + endpoint: 0.0.0.0:14250 + prometheus: + config: + global: + scrape_interval: 30s + scrape_protocols: + - PrometheusProto + - OpenMetricsText1.0.0 + - OpenMetricsText0.0.1 + - PrometheusText0.0.4 + scrape_configs: + - job_name: otel-collector + scrape_interval: 10s + static_configs: + - targets: + - 0.0.0.0:8888 + exporters: + debug: null + service: + telemetry: + metrics: + address: 0.0.0.0:8888 + pipelines: + traces: + exporters: + - debug + receivers: + - jaeger +kind: ConfigMap +metadata: + name: prometheus-cr-collector-7a42612e diff --git a/tests/e2e-ta-collector-mtls/ta-disabled/00-install.yaml b/tests/e2e-ta-collector-mtls/ta-disabled/00-install.yaml new file mode 100644 index 0000000000..72c7665c9d --- /dev/null +++ b/tests/e2e-ta-collector-mtls/ta-disabled/00-install.yaml @@ -0,0 +1,35 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: prometheus-cr +spec: + config: | + receivers: + jaeger: + protocols: + grpc: + + # Collect own metrics + prometheus: + config: + global: + scrape_interval: 30s + scrape_protocols: ['PrometheusProto','OpenMetricsText1.0.0','OpenMetricsText0.0.1','PrometheusText0.0.4'] + scrape_configs: + - job_name: 'otel-collector' + scrape_interval: 10s + static_configs: + - targets: [ '0.0.0.0:8888' ] + + processors: + + exporters: + debug: + service: + pipelines: + traces: + receivers: [jaeger] + exporters: [debug] + mode: statefulset + targetAllocator: + enabled: false diff --git a/tests/e2e-ta-collector-mtls/ta-disabled/chainsaw-test.yaml b/tests/e2e-ta-collector-mtls/ta-disabled/chainsaw-test.yaml new file mode 100755 index 0000000000..614a1ed66d --- /dev/null +++ b/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