Skip to content

Commit cf65729

Browse files
fix: Fix ruler panic related to unset validation scheme (backport release-3.7.x) (#21401)
Co-authored-by: Paul Rogers <129207811+paul1r@users.noreply.github.com>
1 parent d11df0f commit cf65729

2 files changed

Lines changed: 77 additions & 2 deletions

File tree

‎pkg/ruler/base/notifier.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,9 @@ func buildNotifierConfig(amConfig *ruler_config.AlertManagerConfig, externalLabe
154154
promConfig := &config.Config{
155155
GlobalConfig: config.GlobalConfig{
156156
ExternalLabels: externalLabels,
157+
// Prometheus notifier ApplyConfig assigns this to alert relabel configs whose
158+
// scheme is unset; model.UnsetValidation panics during notify (see grafana/loki#21368).
159+
MetricNameValidationScheme: model.UTF8Validation,
157160
},
158161
AlertingConfig: config.AlertingConfig{
159162
AlertRelabelConfigs: amConfig.AlertRelabelConfigs,

‎pkg/ruler/base/notifier_test.go‎

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,16 @@ import (
55
"testing"
66
"time"
77

8+
"github.com/prometheus/client_golang/prometheus"
89
config_util "github.com/prometheus/common/config"
910
"github.com/prometheus/common/model"
11+
"github.com/prometheus/common/promslog"
1012
"github.com/prometheus/prometheus/config"
1113
"github.com/prometheus/prometheus/discovery"
1214
"github.com/prometheus/prometheus/discovery/dns"
1315
"github.com/prometheus/prometheus/model/labels"
1416
"github.com/prometheus/prometheus/model/relabel"
17+
"github.com/prometheus/prometheus/notifier"
1518
"github.com/stretchr/testify/require"
1619

1720
"github.com/grafana/dskit/flagext"
@@ -40,6 +43,9 @@ func TestBuildNotifierConfig(t *testing.T) {
4043
},
4144
},
4245
ncfg: &config.Config{
46+
GlobalConfig: config.GlobalConfig{
47+
MetricNameValidationScheme: model.UTF8Validation,
48+
},
4349
AlertingConfig: config.AlertingConfig{
4450
AlertmanagerConfigs: []*config.AlertmanagerConfig{
4551
{
@@ -68,6 +74,9 @@ func TestBuildNotifierConfig(t *testing.T) {
6874
},
6975
},
7076
ncfg: &config.Config{
77+
GlobalConfig: config.GlobalConfig{
78+
MetricNameValidationScheme: model.UTF8Validation,
79+
},
7180
AlertingConfig: config.AlertingConfig{
7281
AlertmanagerConfigs: []*config.AlertmanagerConfig{
7382
{
@@ -105,6 +114,9 @@ func TestBuildNotifierConfig(t *testing.T) {
105114
},
106115
},
107116
ncfg: &config.Config{
117+
GlobalConfig: config.GlobalConfig{
118+
MetricNameValidationScheme: model.UTF8Validation,
119+
},
108120
AlertingConfig: config.AlertingConfig{
109121
AlertmanagerConfigs: []*config.AlertmanagerConfig{
110122
{
@@ -143,6 +155,9 @@ func TestBuildNotifierConfig(t *testing.T) {
143155
},
144156
},
145157
ncfg: &config.Config{
158+
GlobalConfig: config.GlobalConfig{
159+
MetricNameValidationScheme: model.UTF8Validation,
160+
},
146161
AlertingConfig: config.AlertingConfig{
147162
AlertmanagerConfigs: []*config.AlertmanagerConfig{
148163
{
@@ -183,6 +198,9 @@ func TestBuildNotifierConfig(t *testing.T) {
183198
},
184199
},
185200
ncfg: &config.Config{
201+
GlobalConfig: config.GlobalConfig{
202+
MetricNameValidationScheme: model.UTF8Validation,
203+
},
186204
AlertingConfig: config.AlertingConfig{
187205
AlertmanagerConfigs: []*config.AlertmanagerConfig{
188206
{
@@ -218,6 +236,9 @@ func TestBuildNotifierConfig(t *testing.T) {
218236
},
219237
},
220238
ncfg: &config.Config{
239+
GlobalConfig: config.GlobalConfig{
240+
MetricNameValidationScheme: model.UTF8Validation,
241+
},
221242
AlertingConfig: config.AlertingConfig{
222243
AlertmanagerConfigs: []*config.AlertmanagerConfig{
223244
{
@@ -253,6 +274,9 @@ func TestBuildNotifierConfig(t *testing.T) {
253274
},
254275
},
255276
ncfg: &config.Config{
277+
GlobalConfig: config.GlobalConfig{
278+
MetricNameValidationScheme: model.UTF8Validation,
279+
},
256280
AlertingConfig: config.AlertingConfig{
257281
AlertmanagerConfigs: []*config.AlertmanagerConfig{
258282
{
@@ -291,6 +315,9 @@ func TestBuildNotifierConfig(t *testing.T) {
291315
},
292316
},
293317
ncfg: &config.Config{
318+
GlobalConfig: config.GlobalConfig{
319+
MetricNameValidationScheme: model.UTF8Validation,
320+
},
294321
AlertingConfig: config.AlertingConfig{
295322
AlertmanagerConfigs: []*config.AlertmanagerConfig{
296323
{
@@ -341,7 +368,8 @@ func TestBuildNotifierConfig(t *testing.T) {
341368
},
342369
},
343370
GlobalConfig: config.GlobalConfig{
344-
ExternalLabels: labels.FromStrings("region", "us-east-1"),
371+
ExternalLabels: labels.FromStrings("region", "us-east-1"),
372+
MetricNameValidationScheme: model.UTF8Validation,
345373
},
346374
},
347375
},
@@ -387,7 +415,8 @@ func TestBuildNotifierConfig(t *testing.T) {
387415
},
388416
},
389417
GlobalConfig: config.GlobalConfig{
390-
ExternalLabels: labels.FromStrings("region", "us-east-1"),
418+
ExternalLabels: labels.FromStrings("region", "us-east-1"),
419+
MetricNameValidationScheme: model.UTF8Validation,
391420
},
392421
},
393422
},
@@ -406,6 +435,49 @@ func TestBuildNotifierConfig(t *testing.T) {
406435
}
407436
}
408437

438+
// TestBuildNotifierConfig_SetsUTF8MetricNameValidationScheme fails if GlobalConfig leaves
439+
// MetricNameValidationScheme unset (model.UnsetValidation): notifier.ApplyConfig then copies
440+
// that onto alert_relabel_configs and alert delivery can panic (grafana/loki#21368).
441+
func TestBuildNotifierConfig_SetsUTF8MetricNameValidationScheme(t *testing.T) {
442+
ncfg, err := buildNotifierConfig(&ruler_config.AlertManagerConfig{
443+
AlertmanagerURL: "http://alertmanager.default.svc.cluster.local/alertmanager",
444+
}, labels.EmptyLabels())
445+
require.NoError(t, err)
446+
require.Equal(t, model.UTF8Validation, ncfg.GlobalConfig.MetricNameValidationScheme)
447+
}
448+
449+
// TestNotifierManagerSendWithAlertRelabelReplaceDoesNotPanic exercises the same relabel Replace
450+
// path as grafana/loki#21368; without UTF8 on synthetic GlobalConfig, ApplyConfig leaves relabel
451+
// configs unset and Send panics inside ValidationScheme.IsValidLabelName.
452+
func TestNotifierManagerSendWithAlertRelabelReplaceDoesNotPanic(t *testing.T) {
453+
ncfg, err := buildNotifierConfig(&ruler_config.AlertManagerConfig{
454+
AlertmanagerURL: "http://127.0.0.1:9093",
455+
AlertRelabelConfigs: []*relabel.Config{
456+
{
457+
Action: relabel.Replace,
458+
SourceLabels: model.LabelNames{"severity"},