IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2024-11-25 18:30:41 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Remove double quotes from matchers (#50038)

Browse Source 
* Alerting: Remove double quotes from matchers

With #38629 a new Alertmanager configuration object was introduced with `object_matchers`, it was meant to circumvent around the fact that Prometheus label names don't support a set of characters that Grafana needs to support for alerts, silences, matchers, etc. (with a common example being elasticsearch's `.`).
This new object does not include the label of sanitzation or validation that its Prometheus equivalent supports in `matchers` and therefore are semantically not equivalent.

This triggered the problem that when the migration is run, we use `matchers` as the object to populate in configuration for routing policies, but when the UI does its first save this object is transformed to `object_matchers`.

Matchers that were previously running just fine would immediately stop working as soon as the configuration is saved.

This problem surfaced with the introduction of #49952 where we stopped stripping double quotes from matchers (not just regex but _all_ of them).

* Add comment explaining rationale and future removal

Co-authored-by: Alex Weaver <weaver.alex.d@gmail.com>
This commit is contained in:
gotjosh 2022-06-01 22:05:24 +01:00 committed by GitHub
parent c63ebc887b
commit 1a50b0dbb7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 125 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
pkg/services/ngalert/CHANGELOG.md
View File

@ -54,11 +54,12 @@ Scopes must have an order to ensure consistency and ease of search, this helps u
- [FEATURE] Indicate whether routes are provisioned when GETting Alertmanager configuration #47857
- [FEATURE] Indicate whether contact point is provisioned when GETting Alertmanager configuration #48323
- [FEATURE] Indicate whether alert rule is provisioned when GETting the rule #48458
- [FEATURE] Alert rules with associated panels will take screenshots. #49293 #49338 #49374 #49377 #49378 #49379 #49381 #49385 #49439 #49445
- [BUGFIX] Migration: ignore alerts that do not belong to any existing organization\dashboard #49192
- [BUGFIX] Allow anonymous access to alerts #49203
- [BUGFIX] RBAC: replace create\update\delete actions for notification policies by alert.notifications:write #49185
- [BUGFIX] Fix access to alerts for Viewer role with editor permissions in folder #49270
- [FEATURE] Alert rules with associated panels will take screenshots. #49293 #49338 #49374 #49377 #49378 #49379 #49381 #49385 #49439 #49445
- [BUGFIX] Alerting: Remove double quotes from double quoted matchers #xxxx
- [ENHANCEMENT] Scheduler: ticker to support stopping #48142
## 8.5.3

24
pkg/services/ngalert/api/tooling/definitions/alertmanager.go
View File

@ -7,6 +7,7 @@ import (
"fmt"
"reflect"
"sort"
"strings"
"time"
"github.com/go-openapi/strfmt"
@ -727,7 +728,7 @@ func (r *Route) UnmarshalYAML(unmarshal func(interface{}) error) error {
return r.validateChild()
}
// Return an alertmanager route from a Grafana route. The ObjectMatchers are converted to Matchers.
// AsAMRoute returns an Alertmanager route from a Grafana route. The ObjectMatchers are converted to Matchers.
func (r *Route) AsAMRoute() *config.Route {
amRoute := &config.Route{
Receiver: r.Receiver,
@ -753,7 +754,7 @@ func (r *Route) AsAMRoute() *config.Route {
return amRoute
}
// Return a Grafana route from an alertmanager route. The Matchers are converted to ObjectMatchers.
// AsGrafanaRoute returns a Grafana route from an Alertmanager route. The Matchers are converted to ObjectMatchers.
func AsGrafanaRoute(r *config.Route) *Route {
gRoute := &Route{
Receiver: r.Receiver,
@ -1226,6 +1227,22 @@ func (m *ObjectMatchers) UnmarshalYAML(unmarshal func(interface{}) error) error
return fmt.Errorf("unsupported match type %q in matcher", rawMatcher[1])
}
// When Prometheus serializes a matcher, the value gets wrapped in quotes:
// https://github.com/prometheus/alertmanager/blob/main/pkg/labels/matcher.go#L77
// Remove these quotes so that we are matching against the right value.
//
// This is a stop-gap solution which will be superceded by https://github.com/grafana/grafana/issues/50040.
//
// The ngalert migration converts matchers into the Prom-style, quotes included.
// The UI then stores the quotes into ObjectMatchers without removing them.
// This approach allows these extra quotes to be stored in the database, and fixes them at read time.
// This works because the database stores matchers as JSON text.
//
// There is a subtle bug here, where users might intentionally add quotes to matchers. This method can remove such quotes.
// Since ObjectMatchers will be deprecated entirely, this bug will go away naturally with time.
rawMatcher[2] = strings.TrimPrefix(rawMatcher[2], "\"")
rawMatcher[2] = strings.TrimSuffix(rawMatcher[2], "\"")
matcher, err := labels.NewMatcher(matchType, rawMatcher[0], rawMatcher[2])
if err != nil {
return err
@ -1257,6 +1274,9 @@ func (m *ObjectMatchers) UnmarshalJSON(data []byte) error {
return fmt.Errorf("unsupported match type %q in matcher", rawMatcher[1])
}
rawMatcher[2] = strings.TrimPrefix(rawMatcher[2], "\"")
rawMatcher[2] = strings.TrimSuffix(rawMatcher[2], "\"")
matcher, err := labels.NewMatcher(matchType, rawMatcher[0], rawMatcher[2])
if err != nil {
return err

101
pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go
View File

@ -939,6 +939,107 @@ func Test_ReceiverMatchesBackend(t *testing.T) {
}
}
func TestObjectMatchers_UnmarshalJSON(t *testing.T) {
j := `{
"receiver": "autogen-contact-point-default",
"routes": [{
"receiver": "autogen-contact-point-1",
"object_matchers": [
[
"a",
"=",
"MFR3Gxrnk"
],
[
"b",
"=",
"\"MFR3Gxrnk\""
],
[
"c",
"=~",
"^[a-z0-9-]{1}[a-z0-9-]{0,30}$"
],
[
"d",
"=~",
"\"^[a-z0-9-]{1}[a-z0-9-]{0,30}$\""
]
],
"group_interval": "3s",
"repeat_interval": "10s"
}]
}`
var r Route
if err := json.Unmarshal([]byte(j), &r); err != nil {
require.NoError(t, err)
}
matchers := r.Routes[0].ObjectMatchers
// Without quotes.
require.Equal(t, matchers[0].Name, "a")
require.Equal(t, matchers[0].Value, "MFR3Gxrnk")
// With double quotes.
require.Equal(t, matchers[1].Name, "b")
require.Equal(t, matchers[1].Value, "MFR3Gxrnk")
// Regexp without quotes.
require.Equal(t, matchers[2].Name, "c")
require.Equal(t, matchers[2].Value, "^[a-z0-9-]{1}[a-z0-9-]{0,30}$")
// Regexp with quotes.
require.Equal(t, matchers[3].Name, "d")
require.Equal(t, matchers[3].Value, "^[a-z0-9-]{1}[a-z0-9-]{0,30}$")
}
func TestObjectMatchers_UnmarshalYAML(t *testing.T) {
y := `---
receiver: autogen-contact-point-default
routes:
- receiver: autogen-contact-point-1
object_matchers:
- - a
- "="
- MFR3Gxrnk
- - b
- "="
- '"MFR3Gxrnk"'
- - c
- "=~"
- "^[a-z0-9-]{1}[a-z0-9-]{0,30}$"
- - d
- "=~"
- '"^[a-z0-9-]{1}[a-z0-9-]{0,30}$"'
group_interval: 3s
repeat_interval: 10s
`
var r Route
if err := yaml.Unmarshal([]byte(y), &r); err != nil {
require.NoError(t, err)
}
matchers := r.Routes[0].ObjectMatchers
// Without quotes.
require.Equal(t, matchers[0].Name, "a")
require.Equal(t, matchers[0].Value, "MFR3Gxrnk")
// With double quotes.
require.Equal(t, matchers[1].Name, "b")
require.Equal(t, matchers[1].Value, "MFR3Gxrnk")
// Regexp without quotes.
require.Equal(t, matchers[2].Name, "c")
require.Equal(t, matchers[2].Value, "^[a-z0-9-]{1}[a-z0-9-]{0,30}$")
// Regexp with quotes.
require.Equal(t, matchers[3].Name, "d")
require.Equal(t, matchers[3].Value, "^[a-z0-9-]{1}[a-z0-9-]{0,30}$")
}
func Test_Marshaling_Validation(t *testing.T) {
jsonEncoded, err := ioutil.ReadFile("alertmanager_test_artifact.json")
require.Nil(t, err)