Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -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:
16 changes: 0 additions & 16 deletions .chloggen/operator32.yaml

This file was deleted.

2 changes: 1 addition & 1 deletion internal/manifests/collector/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
55 changes: 55 additions & 0 deletions internal/manifests/collector/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
2 changes: 1 addition & 1 deletion internal/manifests/collector/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
22 changes: 22 additions & 0 deletions internal/manifests/collector/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
})
}
2 changes: 1 addition & 1 deletion internal/manifests/collector/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
22 changes: 22 additions & 0 deletions internal/manifests/collector/volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
})
}
46 changes: 46 additions & 0 deletions tests/e2e-ta-collector-mtls/ta-disabled/00-assert.yaml
Original file line number Diff line number Diff line change
@@ -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
35 changes: 35 additions & 0 deletions tests/e2e-ta-collector-mtls/ta-disabled/00-install.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
name: prometheus-cr
spec:
config: |
receivers:
ItielOlenick marked this conversation as resolved.
Show resolved Hide resolved
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
18 changes: 18 additions & 0 deletions tests/e2e-ta-collector-mtls/ta-disabled/chainsaw-test.yaml
Original file line number Diff line number Diff line change
@@ -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
Loading