Skip to content

Commit 2c5d057

Browse files
ItielOlenickItielOlenick
authored
Ensuring the collector behavior remains consistent when mTLS feature gate is enabled but TA is not deployed (#3496)
* Only mounting and using secret when TA is deployed * Typo * Fixed unit tests * Added changelog and tests * Added e2e test * Modified tests * Updated changelog * Readded changelog * Readded changelog * dummy commit
1 parent f26a319 commit 2c5d057

10 files changed

+210
-3
lines changed

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

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
change_type: bug_fix
2+
3+
component: collector
4+
5+
note: Prevent mounting secrets to collector when TA is not deployed and mTLS feature gate is enabled
6+
7+
issues: [3456]
8+
9+
subtext:

‎internal/manifests/collector/configmap.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) {
4545

4646
replaceCfgOpts := []ta.TAOption{}
4747

48-
if params.Config.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
48+
if params.OtelCol.Spec.TargetAllocator.Enabled && params.Config.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
4949
replaceCfgOpts = append(replaceCfgOpts, ta.WithTLSConfig(
5050
filepath.Join(constants.TACollectorTLSDirPath, constants.TACollectorCAFileName),
5151
filepath.Join(constants.TACollectorTLSDirPath, constants.TACollectorTLSCertFileName),

‎internal/manifests/collector/configmap_test.go

+55
Original file line numberDiff line numberDiff line change
@@ -182,4 +182,59 @@ service:
182182
assert.NoError(t, err)
183183

184184
})
185+
186+
t.Run("Should return expected collector config map without mTLS config", func(t *testing.T) {
187+
expectedData := map[string]string{
188+
"collector.yaml": `exporters:
189+
debug:
190+
receivers:
191+
prometheus:
192+
config:
193+
scrape_configs:
194+
- job_name: serviceMonitor/test/test/0
195+
static_configs:
196+
- targets: ["prom.domain:1001", "prom.domain:1002", "prom.domain:1003"]
197+
labels:
198+
my: label
199+
file_sd_configs:
200+
- files:
201+
- file2.json
202+
service:
203+
pipelines:
204+
metrics:
205+
exporters:
206+
- debug
207+
receivers:
208+
- prometheus
209+
`,
210+
}
211+
212+
param, err := newParams("test/test-img", "testdata/http_sd_config_servicemonitor_test.yaml", config.WithCertManagerAvailability(certmanager.Available))
213+
require.NoError(t, err)
214+
flgs := featuregate.Flags(colfg.GlobalRegistry())
215+
err = flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
216+
param.TargetAllocator = nil
217+
require.NoError(t, err)
218+
219+
hash, _ := manifestutils.GetConfigMapSHA(param.OtelCol.Spec.Config)
220+
expectedName := naming.ConfigMap("test", hash)
221+
222+
expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector"
223+
expectedLables["app.kubernetes.io/name"] = "test-collector"
224+
expectedLables["app.kubernetes.io/version"] = "latest"
225+
226+
actual, err := ConfigMap(param)
227+
228+
assert.NoError(t, err)
229+
assert.Equal(t, expectedName, actual.Name)
230+
assert.Equal(t, expectedLables, actual.Labels)
231+
assert.Equal(t, len(expectedData), len(actual.Data))
232+
for k, expected := range expectedData {
233+
assert.YAMLEq(t, expected, actual.Data[k])
234+
}
235+
236+
// Reset the value
237+
expectedLables["app.kubernetes.io/version"] = "0.47.0"
238+
assert.NoError(t, err)
239+
})
185240
}

‎internal/manifests/collector/container.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme
8585
})
8686
}
8787

88-
if cfg.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
88+
if otelcol.Spec.TargetAllocator.Enabled && cfg.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
8989
volumeMounts = append(volumeMounts,
9090
corev1.VolumeMount{
9191
Name: naming.TAClientCertificate(otelcol.Name),

‎internal/manifests/collector/container_test.go

+22
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,8 @@ func TestContainerWithCertManagerAvailable(t *testing.T) {
873873

874874
flgs := featuregate.Flags(colfg.GlobalRegistry())
875875
err := flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
876+
otelcol.Spec.TargetAllocator.Enabled = true
877+
876878
require.NoError(t, err)
877879

878880
// test
@@ -884,3 +886,23 @@ func TestContainerWithCertManagerAvailable(t *testing.T) {
884886
MountPath: constants.TACollectorTLSDirPath,
885887
})
886888
}
889+
890+
func TestContainerWithFeaturegateEnabledButTADisabled(t *testing.T) {
891+
otelcol := v1beta1.OpenTelemetryCollector{}
892+
893+
cfg := config.New(config.WithCertManagerAvailability(certmanager.Available))
894+
895+
flgs := featuregate.Flags(colfg.GlobalRegistry())
896+
err := flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
897+
898+
require.NoError(t, err)
899+
900+
// test
901+
c := Container(cfg, logger, otelcol, true)
902+
903+
// verify
904+
assert.NotContains(t, c.VolumeMounts, corev1.VolumeMount{
905+
Name: naming.TAClientCertificate(""),
906+
MountPath: constants.TACollectorTLSDirPath,
907+
})
908+
}

‎internal/manifests/collector/volume.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func Volumes(cfg config.Config, otelcol v1beta1.OpenTelemetryCollector) []corev1
4343
},
4444
}}
4545

46-
if cfg.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
46+
if otelcol.Spec.TargetAllocator.Enabled && cfg.CertManagerAvailability() == certmanager.Available && featuregate.EnableTargetAllocatorMTLS.IsEnabled() {
4747
volumes = append(volumes, corev1.Volume{
4848
Name: naming.TAClientCertificate(otelcol.Name),
4949
VolumeSource: corev1.VolumeSource{

‎internal/manifests/collector/volume_test.go

+22
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ func TestVolumeWithTargetAllocatorMTLS(t *testing.T) {
106106

107107
flgs := featuregate.Flags(colfg.GlobalRegistry())
108108
err := flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
109+
otelcol.Spec.TargetAllocator.Enabled = true
109110
require.NoError(t, err)
110111

111112
volumes := Volumes(cfg, otelcol)
@@ -140,4 +141,25 @@ func TestVolumeWithTargetAllocatorMTLS(t *testing.T) {
140141
volumes := Volumes(cfg, otelcol)
141142
assert.NotContains(t, volumes, corev1.Volume{Name: naming.TAClientCertificate(otelcol.Name)})
142143
})
144+
145+
t.Run("Feature gate enabled but TargetAllocator disabled", func(t *testing.T) {
146+
otelcol := v1beta1.OpenTelemetryCollector{}
147+
cfg := config.New(config.WithCertManagerAvailability(certmanager.Available))
148+
149+
flgs := featuregate.Flags(colfg.GlobalRegistry())
150+
err := flgs.Parse([]string{"--feature-gates=operator.targetallocator.mtls"})
151+
152+
require.NoError(t, err)
153+
154+
volumes := Volumes(cfg, otelcol)
155+
unexpectedVolume := corev1.Volume{
156+
Name: naming.TAClientCertificate(otelcol.Name),
157+
VolumeSource: corev1.VolumeSource{
158+
Secret: &corev1.SecretVolumeSource{
159+
SecretName: naming.TAClientCertificateSecretName(otelcol.Name),
160+
},
161+
},
162+
}
163+
assert.NotContains(t, volumes, unexpectedVolume)
164+
})
143165
}

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

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
apiVersion: apps/v1
2+
kind: StatefulSet
3+
metadata:
4+
name: prometheus-cr-collector
5+
status:
6+
readyReplicas: 1
7+
replicas: 1
8+
---
9+
apiVersion: v1
10+
data:
11+
collector.yaml: |
12+
receivers:
13+
jaeger:
14+
protocols:
15+
grpc:
16+
endpoint: 0.0.0.0:14250
17+
prometheus:
18+
config:
19+
global:
20+
scrape_interval: 30s
21+
scrape_protocols:
22+
- PrometheusProto
23+
- OpenMetricsText1.0.0
24+
- OpenMetricsText0.0.1
25+
- PrometheusText0.0.4
26+
scrape_configs:
27+
- job_name: otel-collector
28+
scrape_interval: 10s
29+
static_configs:
30+
- targets:
31+
- 0.0.0.0:8888
32+
exporters:
33+
debug: null
34+
service:
35+
telemetry:
36+
metrics:
37+
address: 0.0.0.0:8888
38+
pipelines:
39+
traces:
40+
exporters:
41+
- debug
42+
receivers:
43+
- jaeger
44+
kind: ConfigMap
45+
metadata:
46+
name: prometheus-cr-collector-7a42612e

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

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
apiVersion: opentelemetry.io/v1alpha1
2+
kind: OpenTelemetryCollector
3+
metadata:
4+
name: prometheus-cr
5+
spec:
6+
config: |
7+
receivers:
8+
jaeger:
9+
protocols:
10+
grpc:
11+
12+
# Collect own metrics
13+
prometheus:
14+
config:
15+
global:
16+
scrape_interval: 30s
17+
scrape_protocols: ['PrometheusProto','OpenMetricsText1.0.0','OpenMetricsText0.0.1','PrometheusText0.0.4']
18+
scrape_configs:
19+
- job_name: 'otel-collector'
20+
scrape_interval: 10s
21+
static_configs:
22+
- targets: [ '0.0.0.0:8888' ]
23+
24+
processors:
25+
26+
exporters:
27+
debug:
28+
service:
29+
pipelines:
30+
traces:
31+
receivers: [jaeger]
32+
exporters: [debug]
33+
mode: statefulset
34+
targetAllocator:
35+
enabled: false

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

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json
2+
apiVersion: chainsaw.kyverno.io/v1alpha1
3+
kind: Test
4+
metadata:
5+
creationTimestamp: null
6+
name: ta-disabled
7+
spec:
8+
steps:
9+
- name: step-00
10+
try:
11+
- apply:
12+
template: true
13+
file: 00-install.yaml
14+
- assert:
15+
file: 00-assert.yaml
16+
catch:
17+
- podLogs:
18+
selector: app.kubernetes.io/managed-by=opentelemetry-operator

0 commit comments

Comments
 (0)