From 718622bac9c9ab26b234cc2d897c8e32252ef3cb Mon Sep 17 00:00:00 2001 From: Craig Peterson <192540+captncraig@users.noreply.github.com> Date: Tue, 15 Aug 2023 10:47:51 -0600 Subject: [PATCH] prometheus.operator.*: fix panic when port not defined in CRD (#4813) * prometheus.operator.*: fix panic when port not defined in CRD * add tests * changelog * deprecated errors --- CHANGELOG.md | 2 ++ .../configgen/config_gen_podmonitor.go | 33 ++++++++--------- .../configgen/config_gen_podmonitor_test.go | 9 +---- .../configgen/config_gen_servicemonitor.go | 35 ++++++++++--------- .../config_gen_servicemonitor_test.go | 10 +----- 5 files changed, 39 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7e18ffca9fe..7b6eded37b9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,8 @@ Main (unreleased) - Fix issue on Windows where DNS short names were unresolvable. (@rfratto) +- Fix panic in `prometheus.operator.*` when no Port supplied in Monitor crds. (@captncraig) + v0.35.4 (2023-08-14) -------------------- diff --git a/component/prometheus/operator/configgen/config_gen_podmonitor.go b/component/prometheus/operator/configgen/config_gen_podmonitor.go index 8bd06f78ff2d..581a9018ad9d 100644 --- a/component/prometheus/operator/configgen/config_gen_podmonitor.go +++ b/component/prometheus/operator/configgen/config_gen_podmonitor.go @@ -182,28 +182,29 @@ func (cg *ConfigGenerator) GeneratePodMonitorConfig(m *promopv1.PodMonitor, ep p Regex: regex, }) } else if ep.TargetPort != nil { //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. - //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. - regex, err := relabel.NewRegexp(ep.TargetPort.String()) - if err != nil { - return nil, fmt.Errorf("parsing TargetPort as regex: %w", err) - } if ep.TargetPort.StrVal != "" { //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. + regex, err := relabel.NewRegexp(ep.TargetPort.String()) //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. + if err != nil { + return nil, fmt.Errorf("parsing TargetPort as regex: %w", err) + } + if ep.TargetPort.StrVal != "" { //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. + relabels.add(&relabel.Config{ + SourceLabels: model.LabelNames{"__meta_kubernetes_pod_container_port_name"}, + Action: "keep", + Regex: regex, + }) + } + } else if ep.TargetPort.IntVal != 0 { //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. + regex, err := relabel.NewRegexp(ep.TargetPort.String()) //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. + if err != nil { + return nil, fmt.Errorf("parsing TargetPort as regex: %w", err) + } relabels.add(&relabel.Config{ - SourceLabels: model.LabelNames{"__meta_kubernetes_pod_container_port_name"}, + SourceLabels: model.LabelNames{"__meta_kubernetes_pod_container_port_number"}, Action: "keep", Regex: regex, }) } - } else if ep.TargetPort.IntVal != 0 { //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. - regex, err := relabel.NewRegexp(ep.TargetPort.String()) //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. - if err != nil { - return nil, fmt.Errorf("parsing TargetPort as regex: %w", err) - } - relabels.add(&relabel.Config{ - SourceLabels: model.LabelNames{"__meta_kubernetes_pod_container_port_number"}, - Action: "keep", - Regex: regex, - }) } // Relabel namespace and pod and service labels into proper labels. diff --git a/component/prometheus/operator/configgen/config_gen_podmonitor_test.go b/component/prometheus/operator/configgen/config_gen_podmonitor_test.go index c9252bf5cd57..2ecb13d7309d 100644 --- a/component/prometheus/operator/configgen/config_gen_podmonitor_test.go +++ b/component/prometheus/operator/configgen/config_gen_podmonitor_test.go @@ -42,9 +42,7 @@ func TestGeneratePodMonitorConfig(t *testing.T) { Name: "podmonitor", }, }, - ep: promopv1.PodMetricsEndpoint{ - Port: "metrics", - }, + ep: promopv1.PodMetricsEndpoint{}, expectedRelabels: util.Untab(` - target_label: __meta_foo replacement: bar @@ -53,9 +51,6 @@ func TestGeneratePodMonitorConfig(t *testing.T) { - source_labels: [__meta_kubernetes_pod_phase] regex: (Failed|Succeeded) action: drop - - source_labels: [__meta_kubernetes_pod_container_port_name] - regex: metrics - action: keep - source_labels: [__meta_kubernetes_namespace] target_label: namespace - source_labels: [__meta_kubernetes_pod_container_name] @@ -64,8 +59,6 @@ func TestGeneratePodMonitorConfig(t *testing.T) { target_label: pod - target_label: job replacement: operator/podmonitor - - target_label: endpoint - replacement: metrics `), expected: &config.ScrapeConfig{ JobName: "podMonitor/operator/podmonitor/0", diff --git a/component/prometheus/operator/configgen/config_gen_servicemonitor.go b/component/prometheus/operator/configgen/config_gen_servicemonitor.go index e5cee99fa439..73ef43b8e404 100644 --- a/component/prometheus/operator/configgen/config_gen_servicemonitor.go +++ b/component/prometheus/operator/configgen/config_gen_servicemonitor.go @@ -168,28 +168,29 @@ func (cg *ConfigGenerator) GenerateServiceMonitorConfig(m *promopv1.ServiceMonit Regex: regex, }) } else if ep.TargetPort != nil { //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. - //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. - regex, err := relabel.NewRegexp(ep.TargetPort.String()) - if err != nil { - return nil, fmt.Errorf("parsing TargetPort as regex: %w", err) - } - if ep.TargetPort.StrVal != "" { + if ep.TargetPort.StrVal != "" { //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. + regex, err := relabel.NewRegexp(ep.TargetPort.String()) //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. + if err != nil { + return nil, fmt.Errorf("parsing TargetPort as regex: %w", err) + } + if ep.TargetPort.StrVal != "" { + relabels.add(&relabel.Config{ + SourceLabels: model.LabelNames{"__meta_kubernetes_pod_container_port_name"}, + Action: "keep", + Regex: regex, + }) + } + } else if ep.TargetPort.IntVal != 0 { //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. + regex, err := relabel.NewRegexp(ep.TargetPort.String()) //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. + if err != nil { + return nil, fmt.Errorf("parsing TargetPort as regex: %w", err) + } relabels.add(&relabel.Config{ - SourceLabels: model.LabelNames{"__meta_kubernetes_pod_container_port_name"}, + SourceLabels: model.LabelNames{"__meta_kubernetes_pod_container_port_number"}, Action: "keep", Regex: regex, }) } - } else if ep.TargetPort.IntVal != 0 { //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. - regex, err := relabel.NewRegexp(ep.TargetPort.String()) //nolint:staticcheck // Ignore SA1019 this field is marked as deprecated. - if err != nil { - return nil, fmt.Errorf("parsing TargetPort as regex: %w", err) - } - relabels.add(&relabel.Config{ - SourceLabels: model.LabelNames{"__meta_kubernetes_pod_container_port_number"}, - Action: "keep", - Regex: regex, - }) } sourceLabels := model.LabelNames{"__meta_kubernetes_endpoint_address_target_kind", "__meta_kubernetes_endpoint_address_target_name"} diff --git a/component/prometheus/operator/configgen/config_gen_servicemonitor_test.go b/component/prometheus/operator/configgen/config_gen_servicemonitor_test.go index 08f31ee7be3f..7c8a43017eae 100644 --- a/component/prometheus/operator/configgen/config_gen_servicemonitor_test.go +++ b/component/prometheus/operator/configgen/config_gen_servicemonitor_test.go @@ -42,17 +42,12 @@ func TestGenerateServiceMonitorConfig(t *testing.T) { Name: "svcmonitor", }, }, - ep: promopv1.Endpoint{ - Port: "metrics", - }, + ep: promopv1.Endpoint{}, expectedRelabels: util.Untab(` - target_label: __meta_foo replacement: bar - source_labels: [job] target_label: __tmp_prometheus_job_name - - source_labels: [__meta_kubernetes_endpoint_port_name] - regex: metrics - action: keep - source_labels: [__meta_kubernetes_endpoint_address_target_kind, __meta_kubernetes_endpoint_address_target_name] regex: Node;(.*) target_label: node @@ -76,9 +71,6 @@ func TestGenerateServiceMonitorConfig(t *testing.T) { - source_labels: [__meta_kubernetes_service_name] target_label: job replacement: ${1} - - target_label: endpoint - replacement: metrics - action: replace `), expected: &config.ScrapeConfig{ JobName: "serviceMonitor/operator/svcmonitor/0",