From f8c33ab47d973cb339ce0366d00197f697fb4cdf Mon Sep 17 00:00:00 2001 From: Koki Kato Date: Sat, 26 Dec 2020 17:52:10 +0900 Subject: [PATCH 1/4] api: check silence matching by string comparison in getSilences Signed-off-by: Koki Kato --- api/v2/api.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/api/v2/api.go b/api/v2/api.go index 24f308d542..53c5f40412 100644 --- a/api/v2/api.go +++ b/api/v2/api.go @@ -638,13 +638,28 @@ func sortSilences(sils open_api_models.GettableSilences) { }) } +// gettableSilenceMatchesFilterLabels returns true if +// a given silence matches a list of matchers. +// A silence matches a filter (list of matchers) if +// for all matchers in the filter, there exists a matcher in the silence +// such that their names, types, and values are equivalent. func gettableSilenceMatchesFilterLabels(s open_api_models.GettableSilence, matchers []*labels.Matcher) bool { - sms := make(map[string]string) - for _, m := range s.Matchers { - sms[*m.Name] = *m.Value + for _, matcher := range matchers { + found := false + for _, m := range s.Matchers { + if matcher.Name == *m.Name && + (matcher.Type == labels.MatchEqual && !*m.IsRegex || matcher.Type == labels.MatchRegexp && *m.IsRegex) && + matcher.Value == *m.Value { + found = true + break + } + } + if !found { + return false + } } - return matchFilterLabels(matchers, sms) + return true } func (api *API) getSilenceHandler(params silence_ops.GetSilenceParams) middleware.Responder { From 72ce7fd71f75bc14a80f50b8f8cfd24bbdf475a5 Mon Sep 17 00:00:00 2001 From: Koki Kato Date: Sat, 26 Dec 2020 17:52:36 +0900 Subject: [PATCH 2/4] api: add test for gettableSilenceMatchesFilterLabels Signed-off-by: Koki Kato --- api/v2/api_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/api/v2/api_test.go b/api/v2/api_test.go index ca55c59c18..ca8dc0280d 100644 --- a/api/v2/api_test.go +++ b/api/v2/api_test.go @@ -130,6 +130,82 @@ func TestGetSilencesHandler(t *testing.T) { } } +func createModelMatcher(name string, value string, isRegex bool) *open_api_models.Matcher { + return &open_api_models.Matcher{ + Name: &name, + Value: &value, + IsRegex: &isRegex, + } +} + +func createLabelMatcher(name string, value string, matchType labels.MatchType) *labels.Matcher { + matcher, _ := labels.NewMatcher(matchType, name, value) + return matcher +} + +func TestGettableSilenceMatchesFilterLabels(t *testing.T) { + type test struct { + silenceMatchers []*open_api_models.Matcher + filterMatchers []*labels.Matcher + expected bool + } + + tests := []test{ + { + []*open_api_models.Matcher{createModelMatcher("label", "value", false)}, + []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchEqual)}, + true, + }, + { + []*open_api_models.Matcher{createModelMatcher("label", "value", false)}, + []*labels.Matcher{createLabelMatcher("label", "novalue", labels.MatchEqual)}, + false, + }, + { + []*open_api_models.Matcher{createModelMatcher("label", "(foo|bar)", true)}, + []*labels.Matcher{createLabelMatcher("label", "(foo|bar)", labels.MatchRegexp)}, + true, + }, + { + []*open_api_models.Matcher{createModelMatcher("label", "foo", true)}, + []*labels.Matcher{createLabelMatcher("label", "(foo|bar)", labels.MatchRegexp)}, + false, + }, + + { + []*open_api_models.Matcher{createModelMatcher("label", "value", false)}, + []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchRegexp)}, + false, + }, + { + []*open_api_models.Matcher{createModelMatcher("label", "value", true)}, + []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchEqual)}, + false, + }, + + { + []*open_api_models.Matcher{ + createModelMatcher("label", "(foo|bar)", true), + createModelMatcher("label", "value", false), + }, + []*labels.Matcher{createLabelMatcher("label", "(foo|bar)", labels.MatchRegexp)}, + true, + }, + } + + for _, test := range tests { + silence := open_api_models.GettableSilence{ + Silence: open_api_models.Silence{ + Matchers: test.silenceMatchers, + }, + } + actual := gettableSilenceMatchesFilterLabels(silence, test.filterMatchers) + if test.expected != actual { + t.Fatal("unexpected match result between silence and filter. expected:", test.expected, ", actual:", actual) + } + } +} + func convertDateTime(ts time.Time) *strfmt.DateTime { dt := strfmt.DateTime(ts) return &dt From b9aae07a73007752276519d97f207816e526e146 Mon Sep 17 00:00:00 2001 From: Koki Kato Date: Tue, 9 Feb 2021 22:53:00 +0900 Subject: [PATCH 3/4] Check silence matching with Protobuf models Signed-off-by: Koki Kato To support negative matchers. --- api/v2/api.go | 19 +++++++++++-------- api/v2/api_test.go | 41 ++++++++++++++++++++--------------------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/api/v2/api.go b/api/v2/api.go index 53c5f40412..3394206e9d 100644 --- a/api/v2/api.go +++ b/api/v2/api.go @@ -584,14 +584,14 @@ func (api *API) getSilencesHandler(params silence_ops.GetSilencesParams) middlew sils := open_api_models.GettableSilences{} for _, ps := range psils { + if !checkSilenceMatchesFilterLabels(ps, matchers) { + continue + } silence, err := gettableSilenceFromProto(ps) if err != nil { level.Error(logger).Log("msg", "Failed to unmarshal silence from proto", "err", err) return silence_ops.NewGetSilencesInternalServerError().WithPayload(err.Error()) } - if !gettableSilenceMatchesFilterLabels(silence, matchers) { - continue - } sils = append(sils, &silence) } @@ -638,18 +638,21 @@ func sortSilences(sils open_api_models.GettableSilences) { }) } -// gettableSilenceMatchesFilterLabels returns true if +// checkSilenceMatchesFilterLabels returns true if // a given silence matches a list of matchers. // A silence matches a filter (list of matchers) if // for all matchers in the filter, there exists a matcher in the silence // such that their names, types, and values are equivalent. -func gettableSilenceMatchesFilterLabels(s open_api_models.GettableSilence, matchers []*labels.Matcher) bool { +func checkSilenceMatchesFilterLabels(s *silencepb.Silence, matchers []*labels.Matcher) bool { for _, matcher := range matchers { found := false for _, m := range s.Matchers { - if matcher.Name == *m.Name && - (matcher.Type == labels.MatchEqual && !*m.IsRegex || matcher.Type == labels.MatchRegexp && *m.IsRegex) && - matcher.Value == *m.Value { + if matcher.Name == m.Name && + (matcher.Type == labels.MatchEqual && m.Type == silencepb.Matcher_EQUAL || + matcher.Type == labels.MatchRegexp && m.Type == silencepb.Matcher_REGEXP || + matcher.Type == labels.MatchNotEqual && m.Type == silencepb.Matcher_NOT_EQUAL || + matcher.Type == labels.MatchNotRegexp && m.Type == silencepb.Matcher_NOT_REGEXP) && + matcher.Value == m.Pattern { found = true break } diff --git a/api/v2/api_test.go b/api/v2/api_test.go index ca8dc0280d..e58ee020be 100644 --- a/api/v2/api_test.go +++ b/api/v2/api_test.go @@ -26,6 +26,7 @@ import ( general_ops "github.com/prometheus/alertmanager/api/v2/restapi/operations/general" "github.com/prometheus/alertmanager/config" "github.com/prometheus/alertmanager/pkg/labels" + "github.com/prometheus/alertmanager/silence/silencepb" "github.com/prometheus/alertmanager/types" ) @@ -130,11 +131,11 @@ func TestGetSilencesHandler(t *testing.T) { } } -func createModelMatcher(name string, value string, isRegex bool) *open_api_models.Matcher { - return &open_api_models.Matcher{ - Name: &name, - Value: &value, - IsRegex: &isRegex, +func createSilenceMatcher(name string, pattern string, matcherType silencepb.Matcher_Type) *silencepb.Matcher { + return &silencepb.Matcher{ + Name: name, + Pattern: pattern, + Type: matcherType, } } @@ -143,50 +144,50 @@ func createLabelMatcher(name string, value string, matchType labels.MatchType) * return matcher } -func TestGettableSilenceMatchesFilterLabels(t *testing.T) { +func TestCheckSilenceMatchesFilterLabels(t *testing.T) { type test struct { - silenceMatchers []*open_api_models.Matcher + silenceMatchers []*silencepb.Matcher filterMatchers []*labels.Matcher expected bool } tests := []test{ { - []*open_api_models.Matcher{createModelMatcher("label", "value", false)}, + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_EQUAL)}, []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchEqual)}, true, }, { - []*open_api_models.Matcher{createModelMatcher("label", "value", false)}, + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_EQUAL)}, []*labels.Matcher{createLabelMatcher("label", "novalue", labels.MatchEqual)}, false, }, { - []*open_api_models.Matcher{createModelMatcher("label", "(foo|bar)", true)}, + []*silencepb.Matcher{createSilenceMatcher("label", "(foo|bar)", silencepb.Matcher_REGEXP)}, []*labels.Matcher{createLabelMatcher("label", "(foo|bar)", labels.MatchRegexp)}, true, }, { - []*open_api_models.Matcher{createModelMatcher("label", "foo", true)}, + []*silencepb.Matcher{createSilenceMatcher("label", "foo", silencepb.Matcher_REGEXP)}, []*labels.Matcher{createLabelMatcher("label", "(foo|bar)", labels.MatchRegexp)}, false, }, { - []*open_api_models.Matcher{createModelMatcher("label", "value", false)}, + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_EQUAL)}, []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchRegexp)}, false, }, { - []*open_api_models.Matcher{createModelMatcher("label", "value", true)}, + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_REGEXP)}, []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchEqual)}, false, }, { - []*open_api_models.Matcher{ - createModelMatcher("label", "(foo|bar)", true), - createModelMatcher("label", "value", false), + []*silencepb.Matcher{ + createSilenceMatcher("label", "(foo|bar)", silencepb.Matcher_REGEXP), + createSilenceMatcher("label", "value", silencepb.Matcher_EQUAL), }, []*labels.Matcher{createLabelMatcher("label", "(foo|bar)", labels.MatchRegexp)}, true, @@ -194,12 +195,10 @@ func TestGettableSilenceMatchesFilterLabels(t *testing.T) { } for _, test := range tests { - silence := open_api_models.GettableSilence{ - Silence: open_api_models.Silence{ - Matchers: test.silenceMatchers, - }, + silence := silencepb.Silence{ + Matchers: test.silenceMatchers, } - actual := gettableSilenceMatchesFilterLabels(silence, test.filterMatchers) + actual := checkSilenceMatchesFilterLabels(&silence, test.filterMatchers) if test.expected != actual { t.Fatal("unexpected match result between silence and filter. expected:", test.expected, ", actual:", actual) } From b5ddc5d638787220976078127f824647c0019553 Mon Sep 17 00:00:00 2001 From: Koki Kato Date: Tue, 9 Feb 2021 23:01:21 +0900 Subject: [PATCH 4/4] Add test cases for silences with negative matchers Signed-off-by: Koki Kato --- api/v2/api_test.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/api/v2/api_test.go b/api/v2/api_test.go index e58ee020be..7654687ce5 100644 --- a/api/v2/api_test.go +++ b/api/v2/api_test.go @@ -183,7 +183,36 @@ func TestCheckSilenceMatchesFilterLabels(t *testing.T) { []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchEqual)}, false, }, - + { + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_NOT_EQUAL)}, + []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchNotEqual)}, + true, + }, + { + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_NOT_REGEXP)}, + []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchNotRegexp)}, + true, + }, + { + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_EQUAL)}, + []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchNotEqual)}, + false, + }, + { + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_REGEXP)}, + []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchNotRegexp)}, + false, + }, + { + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_NOT_EQUAL)}, + []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchNotRegexp)}, + false, + }, + { + []*silencepb.Matcher{createSilenceMatcher("label", "value", silencepb.Matcher_NOT_REGEXP)}, + []*labels.Matcher{createLabelMatcher("label", "value", labels.MatchNotEqual)}, + false, + }, { []*silencepb.Matcher{ createSilenceMatcher("label", "(foo|bar)", silencepb.Matcher_REGEXP),