Skip to content

Commit 0bb8a0e

Browse files
ozzywalshozzywalsh
authored
fix(target-allocator): scrapeClass tlsConfig fields silently dropped (#5101)
* fix(target-allocator): decode prom-operator types with json:",inline" embedded structs The TA config deserializer uses mapstructure with TagName:"yaml", which cannot squash embedded structs tagged with json:",inline" (e.g. TLSConfig embedding SafeTLSConfig and TLSFilesConfig). This silently drops fields like CAFile and InsecureSkipVerify when deserializing ScrapeClass TLS configuration from the ConfigMap. Add a generic decode hook (MapToMonitoringV1) that round-trips all monitoring/v1 types through sigs.k8s.io/yaml, which internally converts YAML to JSON and uses json.Unmarshal — correctly handling json:",inline". * Add e2e tests * Fix e2e * update e2e * Fix missing StorageConfig in scrape class test expected value * Fix e2e assertion and lint: service_monitor_selector is no longer null The json:",inline" fix causes empty LabelSelectors (e.g. serviceMonitorSelector: {}) to serialize correctly as objects instead of null. Update the e2e assertion to match. Also fix gci import ordering in config.go. * Add changelog entry for target allocator tlsConfig deserialization fix * Simplify changelog note and add config example in subtext
1 parent 6846a44 commit 0bb8a0e

9 files changed

Lines changed: 287 additions & 1 deletion

File tree

‎.chloggen/fix_ta-scrapeclass-tls-deserialization.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: bug_fix
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
5+
component: target allocator
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Fix scrapeClass tlsConfig fields being silently dropped in target allocator config.
9+
10+
# One or more tracking issues related to the change
11+
issues: [5101]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext: |
17+
scrapeClasses with tlsConfig like the following had their TLS fields silently dropped
18+
when passed to the target allocator:
19+
scrapeClasses:
20+
- name: tls-config
21+
tlsConfig:
22+
caFile: /scrapeclass-ca.pem
23+
insecureSkipVerify: true

‎cmd/otel-allocator/internal/config/config.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
ctrl "sigs.k8s.io/controller-runtime"
3434
"sigs.k8s.io/controller-runtime/pkg/certwatcher"
3535
"sigs.k8s.io/controller-runtime/pkg/log/zap"
36+
sigsyaml "sigs.k8s.io/yaml"
3637
)
3738

3839
const (
@@ -153,6 +154,42 @@ func MapToPromConfig() mapstructure.DecodeHookFuncType {
153154
}
154155
}
155156

157+
const monitoringV1PkgPath = "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
158+
159+
// MapToMonitoringV1 handles prom-operator types that use json:",inline" for embedded structs.
160+
// mapstructure with TagName:"yaml" can't squash these, so we round-trip through sigs.k8s.io/yaml
161+
// which converts YAML → JSON → json.Unmarshal, correctly handling json:",inline".
162+
func MapToMonitoringV1() mapstructure.DecodeHookFuncType {
163+
return func(
164+
f reflect.Type,
165+
t reflect.Type,
166+
data any,
167+
) (any, error) {
168+
if f.Kind() != reflect.Map {
169+
return data, nil
170+
}
171+
target := t
172+
if t.Kind() == reflect.Pointer {
173+
target = t.Elem()
174+
}
175+
if target.PkgPath() != monitoringV1PkgPath {
176+
return data, nil
177+
}
178+
yamlBytes, err := yaml.Marshal(data)
179+
if err != nil {
180+
return data, err
181+
}
182+
result := reflect.New(target)
183+
if err := sigsyaml.Unmarshal(yamlBytes, result.Interface()); err != nil {
184+
return data, err
185+
}
186+
if t.Kind() == reflect.Pointer {
187+
return result.Interface(), nil
188+
}
189+
return result.Elem().Interface(), nil
190+
}
191+
}
192+
156193
// MapToLabelSelector returns a DecodeHookFuncType that
157194
// provides a mechanism for decoding both matchLabels and matchExpressions from camelcase to lowercase
158195
// because we use yaml unmarshaling that supports lowercase field names if no `yaml` tag is defined
@@ -302,6 +339,7 @@ func unmarshal(cfg *Config, configFile string) error {
302339
DecodeHook: mapstructure.ComposeDecodeHookFunc(
303340
StringToModelOrTimeDurationHookFunc(),
304341
MapToPromConfig(),
342+
MapToMonitoringV1(),
305343
MapToLabelSelector(),
306344
),
307345
}

‎cmd/otel-allocator/internal/config/config_test.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"time"
1313

1414
"github.com/aws/smithy-go/ptr"
15+
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
1516
commonconfig "github.com/prometheus/common/config"
1617
"github.com/prometheus/common/model"
1718
promconfig "github.com/prometheus/prometheus/config"
@@ -548,6 +549,108 @@ func TestLoadFromFile(t *testing.T) {
548549
},
549550
wantErr: assert.NoError,
550551
},
552+
{
553+
name: "scrape class with camelCase tlsConfig fields",
554+
args: args{
555+
file: filepath.Join("testdata", "scrape_class_test.yaml"),
556+
},
557+
want: Config{
558+
ListenAddr: DefaultListenAddr,
559+
KubeConfigFilePath: DefaultKubeConfigFilePath,
560+
AllocationStrategy: DefaultAllocationStrategy,
561+
CollectorNamespace: "default",
562+
CollectorSelector: &metav1.LabelSelector{
563+
MatchLabels: map[string]string{
564+
"app.kubernetes.io/instance": "default.test",
565+
"app.kubernetes.io/managed-by": "opentelemetry-operator",
566+
},
567+
},
568+
FilterStrategy: DefaultFilterStrategy,
569+
PrometheusCR: PrometheusCRConfig{
570+
Enabled: true,
571+
ScrapeInterval: model.Duration(time.Second * 30),
572+
ScrapeClasses: []monitoringv1.ScrapeClass{
573+
{
574+
Name: "test-scrape-class",
575+
Default: ptr.Bool(false),
576+
TLSConfig: &monitoringv1.TLSConfig{
577+
TLSFilesConfig: monitoringv1.TLSFilesConfig{
578+
CAFile: "/etc/ca-bundle.pem",
579+
},
580+
SafeTLSConfig: monitoringv1.SafeTLSConfig{
581+
InsecureSkipVerify: ptr.Bool(true),
582+
},
583+
},
584+
},
585+
},
586+
ServiceMonitorNamespaceSelector: &metav1.LabelSelector{},
587+
PodMonitorNamespaceSelector: &metav1.LabelSelector{},
588+
ScrapeConfigNamespaceSelector: &metav1.LabelSelector{},
589+
ProbeNamespaceSelector: &metav1.LabelSelector{},
590+
ScrapeProtocols: defaultScrapeProtocolsCR,
591+
},
592+
HTTPS: HTTPSServerConfig{
593+
ListenAddr: ":8443",
594+
},
595+
PromConfig: &promconfig.Config{
596+
GlobalConfig: promconfig.GlobalConfig{
597+
ScrapeInterval: model.Duration(60 * time.Second),
598+
ScrapeTimeout: model.Duration(10 * time.Second),
599+
ScrapeNativeHistograms: ptr.Bool(false),
600+
ExtraScrapeMetrics: ptr.Bool(false),
601+
EvaluationInterval: model.Duration(60 * time.Second),
602+
MetricNameValidationScheme: model.UTF8Validation,
603+
MetricNameEscapingScheme: model.AllowUTF8,
604+
},
605+
Runtime: promconfig.DefaultRuntimeConfig,
606+
OTLPConfig: promconfig.DefaultOTLPConfig,
607+
StorageConfig: promconfig.StorageConfig{
608+
TSDBConfig: &promconfig.TSDBConfig{
609+
Retention: &promconfig.TSDBRetentionConfig{},
610+
},
611+
},
612+
ScrapeConfigs: []*promconfig.ScrapeConfig{
613+
{
614+
JobName: "prometheus",
615+
EnableCompression: true,
616+
HonorTimestamps: true,
617+
ScrapeInterval: model.Duration(60 * time.Second),
618+
ScrapeProtocols: promconfig.DefaultScrapeProtocols,
619+
ScrapeTimeout: model.Duration(10 * time.Second),
620+
MetricNameValidationScheme: model.UTF8Validation,
621+
MetricNameEscapingScheme: model.AllowUTF8,
622+
AlwaysScrapeClassicHistograms: ptr.Bool(false),
623+
ConvertClassicHistogramsToNHCB: ptr.Bool(false),
624+
ScrapeNativeHistograms: ptr.Bool(false),
625+
ExtraScrapeMetrics: ptr.Bool(false),
626+
MetricsPath: "/metrics",
627+
Scheme: "http",
628+
HTTPClientConfig: commonconfig.HTTPClientConfig{
629+
FollowRedirects: true,
630+
EnableHTTP2: true,
631+
},
632+
ServiceDiscoveryConfigs: []discovery.Config{
633+
discovery.StaticConfig{
634+
{
635+
Targets: []model.LabelSet{
636+
{model.AddressLabel: "prom.domain:9001"},
637+
{model.AddressLabel: "prom.domain:9002"},
638+
{model.AddressLabel: "prom.domain:9003"},
639+
},
640+
Labels: model.LabelSet{
641+
"my": "label",
642+
},
643+
Source: "0",
644+
},
645+
},
646+
},
647+
},
648+
},
649+
},
650+
CollectorNotReadyGracePeriod: 30 * time.Second,
651+
},
652+
wantErr: assert.NoError,
653+
},
551654
{
552655
name: "service monitor pod monitor selector with camelcase matchexpressions",
553656
args: args{

‎cmd/otel-allocator/internal/config/testdata/scrape_class_test.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
collector_namespace: default
2+
collector_selector:
3+
matchLabels:
4+
app.kubernetes.io/instance: default.test
5+
app.kubernetes.io/managed-by: opentelemetry-operator
6+
prometheus_cr:
7+
enabled: true
8+
scrape_interval: 30s
9+
scrape_classes:
10+
- name: test-scrape-class
11+
default: false
12+
tlsConfig:
13+
caFile: /etc/ca-bundle.pem
14+
insecureSkipVerify: true
15+
config:
16+
scrape_configs:
17+
- job_name: prometheus
18+
static_configs:
19+
- targets: ["prom.domain:9001", "prom.domain:9002", "prom.domain:9003"]
20+
labels:
21+
my: label

‎tests/e2e-targetallocator/targetallocator-prometheuscr/02-assert.yaml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ status:
1818
apiVersion: v1
1919
data:
2020
targetallocator.yaml:
21-
( contains(@, join(':', ['service_monitor_selector', ' null'])) ): true
21+
( contains(@, join(':', ['service_monitor_selector', ' null'])) ): false
2222
( contains(@, join(':', ['pod_monitor_selector', ' null'])) ): true
2323
( contains(@, join(':', ['probe_selector', ' null'])) ): false
2424
( contains(@, join(':', ['scrape_config_selector', ' null'])) ): false
@@ -63,3 +63,10 @@ metadata:
6363
name: check-ta-jobs-probes-v1beta1
6464
status:
6565
succeeded: 1
66+
---
67+
apiVersion: batch/v1
68+
kind: Job
69+
metadata:
70+
name: check-ta-scrape-config-with-scrape-class-v1beta1
71+
status:
72+
succeeded: 1

‎tests/e2e-targetallocator/targetallocator-prometheuscr/02-install.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ spec:
2727
evaluationInterval: 2m
2828
scrapeProtocols:
2929
- PrometheusText1.0.0
30+
serviceMonitorSelector: {}
3031
scrapeConfigSelector: {}
3132
probeSelector: {}
3233
scrapeClasses:

‎tests/e2e-targetallocator/targetallocator-prometheuscr/04-assert.yaml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
apiVersion: batch/v1
2+
kind: Job
3+
metadata:
4+
name: check-ta-smon-scrapeclass-config
5+
status:
6+
succeeded: 1
7+
---
8+
apiVersion: batch/v1
9+
kind: Job
10+
metadata:
11+
name: check-ta-smon-scrapeclass-tls
12+
status:
13+
succeeded: 1

‎tests/e2e-targetallocator/targetallocator-prometheuscr/04-install.yaml‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
---
2+
# ServiceMonitor that references the scrapeClass defined in step 02.
3+
# Verifies that scrapeClass tlsConfig is applied to ServiceMonitors,
4+
# not just Probes (which was the only CR type tested previously).
5+
apiVersion: v1
6+
kind: Service
7+
metadata:
8+
name: scrapeclass-svc
9+
labels:
10+
app: scrapeclass-test
11+
spec:
12+
selector:
13+
app: scrapeclass-test
14+
ports:
15+
- name: metrics
16+
port: 8080
17+
targetPort: 8080
18+
---
19+
apiVersion: monitoring.coreos.com/v1
20+
kind: ServiceMonitor
21+
metadata:
22+
name: scrapeclass-smon
23+
spec:
24+
selector:
25+
matchLabels:
26+
app: scrapeclass-test
27+
endpoints:
28+
- port: metrics
29+
interval: 10s
30+
scrapeClass: tls-config
31+
---
32+
# Check ServiceMonitor appears in TA scrape_configs
33+
apiVersion: batch/v1
34+
kind: Job
35+
metadata:
36+
name: check-ta-smon-scrapeclass-config
37+
spec:
38+
template:
39+
metadata:
40+
labels:
41+
checker: "true"
42+
spec:
43+
restartPolicy: OnFailure
44+
containers:
45+
- name: check-config
46+
image: mirror.gcr.io/curlimages/curl:latest
47+
args:
48+
- /bin/sh
49+
- -c
50+
- curl -s http://prometheus-cr-v1beta1-targetallocator/scrape_configs | grep "scrapeclass-smon"
51+
---
52+
# Check scrapeClass TLS caFile is applied specifically to the ServiceMonitor's scrape config,
53+
# not just to the Probe which also uses the same scrapeClass.
54+
apiVersion: batch/v1
55+
kind: Job
56+
metadata:
57+
name: check-ta-smon-scrapeclass-tls
58+
spec:
59+
template:
60+
metadata:
61+
labels:
62+
checker: "true"
63+
spec:
64+
restartPolicy: OnFailure
65+
containers:
66+
- name: check-tls
67+
image: mirror.gcr.io/curlimages/curl:latest
68+
args:
69+
- /bin/sh
70+
- -c
71+
- curl -s http://prometheus-cr-v1beta1-targetallocator/scrape_configs | grep "scrapeclass-smon.*scrapeclass-ca.pem"

‎tests/e2e-targetallocator/targetallocator-prometheuscr/chainsaw-test.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,12 @@ spec:
4848
file: 03-install.yaml
4949
- assert:
5050
file: 03-assert.yaml
51+
- name: step-04
52+
try:
53+
- apply:
54+
file: 04-install.yaml
55+
- assert:
56+
file: 04-assert.yaml
57+
catch:
58+
- podLogs:
59+
selector: checker=true

0 commit comments

Comments
 (0)