From caf18887ea7e78ddbc3c428226f3092d61a41452 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Tue, 18 May 2021 12:44:16 +0100 Subject: [PATCH] feat(backend): support isEqual silence field in ACLs --- CHANGELOG.md | 5 +- cmd/karma/acl.go | 31 +++- cmd/karma/acl_test.go | 111 +++++++++++++- cmd/karma/proxy_test.go | 136 ++++++++++++++++-- .../tests/testscript/001_acl_example.txt | 16 ++- docs/ACLs.md | 95 +++++++++--- internal/config/acl.go | 6 +- internal/mapper/v017/api.go | 8 ++ 8 files changed, 366 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cb96500a..78f0a264b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,10 @@ ### Added - Added support for alertmanager `v0.22.0` - [negative matchers](https://github.com/prometheus/alertmanager/pull/2434). + [negative matchers](https://github.com/prometheus/alertmanager/pull/2434) + when creating/editing silences and in [ACL rules](/docs/ACLs.md). +- Silence ACL rules no longer default `isRegex` to be `false` for filters + and matchers, see [ACL rules](/docs/ACLs.md) for updated docs. ## v0.85 diff --git a/cmd/karma/acl.go b/cmd/karma/acl.go index e6e2a63a9..3aaeb7cef 100644 --- a/cmd/karma/acl.go +++ b/cmd/karma/acl.go @@ -26,7 +26,8 @@ type silenceFilter struct { NameRegex *regexp.Regexp Value string ValueRegex *regexp.Regexp - IsRegex bool + IsRegex *bool + IsEqual *bool } func (sf *silenceFilter) isMatch(silence *models.Silence) bool { @@ -45,7 +46,17 @@ func (sf *silenceFilter) isMatch(silence *models.Silence) bool { valueMatch = true } - if nameMatch && valueMatch && sf.IsRegex == m.IsRegex { + isRegexMatch := true + if sf.IsRegex != nil && *sf.IsRegex != m.IsRegex { + isRegexMatch = false + } + + isEqualMatch := true + if sf.IsEqual != nil && *sf.IsEqual != m.IsEqual { + isEqualMatch = false + } + + if nameMatch && valueMatch && isRegexMatch && isEqualMatch { return true } } @@ -58,7 +69,8 @@ type silenceMatcher struct { NameRegex *regexp.Regexp Value string ValueRegex *regexp.Regexp - IsRegex bool + IsRegex *bool + IsEqual *bool } func (sm *silenceMatcher) isMatch(m models.SilenceMatcher) bool { @@ -74,7 +86,10 @@ func (sm *silenceMatcher) isMatch(m models.SilenceMatcher) bool { if sm.Value != "" && sm.Value != m.Value { return false } - if sm.IsRegex != m.IsRegex { + if sm.IsRegex != nil && *sm.IsRegex != m.IsRegex { + return false + } + if sm.IsEqual != nil && *sm.IsEqual != m.IsEqual { return false } return true @@ -116,9 +131,11 @@ func (acl *silenceACL) isAllowed(amName string, silence *models.Silence, usernam } } - filterMatch := len(acl.Scope.Filters) == 0 + filterMatch := true for _, aclFilter := range acl.Scope.Filters { - filterMatch = aclFilter.isMatch(silence) + if m := aclFilter.isMatch(silence); !m { + filterMatch = m + } } if groupMatch && amMatch && filterMatch { @@ -207,6 +224,7 @@ func newSilenceACLFromConfig(cfg config.SilenceACLRule) (*silenceACL, error) { Name: filter.Name, Value: filter.Value, IsRegex: filter.IsRegex, + IsEqual: filter.IsEqual, } if filter.NameRegex != "" { @@ -248,6 +266,7 @@ func newSilenceACLFromConfig(cfg config.SilenceACLRule) (*silenceACL, error) { Name: matcherConfig.Name, Value: matcherConfig.Value, IsRegex: matcherConfig.IsRegex, + IsEqual: matcherConfig.IsEqual, } if matcherConfig.NameRegex != "" { diff --git a/cmd/karma/acl_test.go b/cmd/karma/acl_test.go index 611b8e406..103f882da 100644 --- a/cmd/karma/acl_test.go +++ b/cmd/karma/acl_test.go @@ -7,6 +7,16 @@ import ( "github.com/prymitive/karma/internal/models" ) +func truePtr() *bool { + b := true + return &b +} + +func falsePtr() *bool { + b := false + return &b +} + func TestAclSilenceMatcher(t *testing.T) { type testCaseT struct { requiredMatcher silenceMatcher @@ -52,7 +62,7 @@ func TestAclSilenceMatcher(t *testing.T) { requiredMatcher: silenceMatcher{ Name: "foo", Value: "bar", - IsRegex: true, + IsRegex: truePtr(), }, silenceMatcher: models.SilenceMatcher{ Name: "foo", @@ -64,7 +74,7 @@ func TestAclSilenceMatcher(t *testing.T) { requiredMatcher: silenceMatcher{ Name: "foo", Value: "bar", - IsRegex: false, + IsRegex: falsePtr(), }, silenceMatcher: models.SilenceMatcher{ Name: "foo", @@ -73,6 +83,45 @@ func TestAclSilenceMatcher(t *testing.T) { }, isMatch: false, }, + { + requiredMatcher: silenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: falsePtr(), + }, + silenceMatcher: models.SilenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: false, + }, + isMatch: true, + }, + { + requiredMatcher: silenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: truePtr(), + }, + silenceMatcher: models.SilenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: false, + }, + isMatch: false, + }, + { + requiredMatcher: silenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: truePtr(), + }, + silenceMatcher: models.SilenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: true, + }, + isMatch: true, + }, { requiredMatcher: silenceMatcher{ NameRegex: regexp.MustCompile("^.+$"), @@ -121,7 +170,7 @@ func TestAclSilenceMatcher(t *testing.T) { requiredMatcher: silenceMatcher{ NameRegex: regexp.MustCompile("^.+$"), ValueRegex: regexp.MustCompile("^.+$"), - IsRegex: true, + IsRegex: truePtr(), }, silenceMatcher: models.SilenceMatcher{ Name: "foo", @@ -130,6 +179,62 @@ func TestAclSilenceMatcher(t *testing.T) { }, isMatch: false, }, + { + requiredMatcher: silenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: truePtr(), + IsEqual: falsePtr(), + }, + silenceMatcher: models.SilenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: true, + IsEqual: false, + }, + isMatch: true, + }, + { + requiredMatcher: silenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: truePtr(), + IsEqual: truePtr(), + }, + silenceMatcher: models.SilenceMatcher{ + Name: "foo", + Value: "bar", + IsRegex: true, + IsEqual: true, + }, + isMatch: true, + }, + { + requiredMatcher: silenceMatcher{ + Name: "foo", + Value: "bar", + IsEqual: falsePtr(), + }, + silenceMatcher: models.SilenceMatcher{ + Name: "foo", + Value: "bar", + IsEqual: true, + }, + isMatch: false, + }, + { + requiredMatcher: silenceMatcher{ + Name: "foo", + Value: "bar", + IsEqual: truePtr(), + }, + silenceMatcher: models.SilenceMatcher{ + Name: "foo", + Value: "bar", + IsEqual: false, + }, + isMatch: false, + }, } for _, testCase := range testCases { diff --git a/cmd/karma/proxy_test.go b/cmd/karma/proxy_test.go index bf4a394d0..ef238f4e5 100644 --- a/cmd/karma/proxy_test.go +++ b/cmd/karma/proxy_test.go @@ -590,7 +590,7 @@ func TestProxySilenceACL(t *testing.T) { "endsAt": "2000-02-01T00:02:03.000Z", "matchers": [ { "isRegex": false, "name": "alertname", "value": "Fake Alert" }, -{ "isRegex": true, "name": "foo", "value": "(bar|baz)" } +{ "isRegex": true, "isEqual": true, "name": "foo", "value": "(bar|baz)" } ]}` proxyTests := []proxyTest{ @@ -852,7 +852,7 @@ func TestProxySilenceACL(t *testing.T) { { Name: "foo", Value: "(bar|baz)", - IsRegex: true, + IsRegex: truePtr(), }, }, }, @@ -870,7 +870,11 @@ func TestProxySilenceACL(t *testing.T) { Reason: "block all regex silences", Scope: silenceACLScope{ Filters: []silenceFilter{ - {NameRegex: regexp.MustCompile(".*"), ValueRegex: regexp.MustCompile(".*"), IsRegex: true}, + { + NameRegex: regexp.MustCompile(".*"), + ValueRegex: regexp.MustCompile(".*"), + IsRegex: truePtr(), + }, }, Groups: []string{}, Alertmanagers: []string{}, @@ -894,7 +898,11 @@ func TestProxySilenceACL(t *testing.T) { Reason: "block all regex silences", Scope: silenceACLScope{ Filters: []silenceFilter{ - {NameRegex: regexp.MustCompile(".*"), ValueRegex: regexp.MustCompile(".*"), IsRegex: true}, + { + NameRegex: regexp.MustCompile(".*"), + ValueRegex: regexp.MustCompile(".*"), + IsRegex: truePtr(), + }, }, Groups: []string{"admins"}, Alertmanagers: []string{}, @@ -906,7 +914,11 @@ func TestProxySilenceACL(t *testing.T) { Reason: "block all regex silences", Scope: silenceACLScope{ Filters: []silenceFilter{ - {NameRegex: regexp.MustCompile(".*"), ValueRegex: regexp.MustCompile(".*"), IsRegex: true}, + { + NameRegex: regexp.MustCompile(".*"), + ValueRegex: regexp.MustCompile(".*"), + IsRegex: truePtr(), + }, }, Groups: []string{}, Alertmanagers: []string{}, @@ -954,7 +966,11 @@ func TestProxySilenceACL(t *testing.T) { Reason: "block all regex silences", Scope: silenceACLScope{ Filters: []silenceFilter{ - {NameRegex: regexp.MustCompile(".*"), ValueRegex: regexp.MustCompile(".*"), IsRegex: true}, + { + NameRegex: regexp.MustCompile(".*"), + ValueRegex: regexp.MustCompile(".*"), + IsRegex: truePtr(), + }, }, Groups: []string{"admins"}, Alertmanagers: []string{}, @@ -966,7 +982,11 @@ func TestProxySilenceACL(t *testing.T) { Reason: "block all regex silences", Scope: silenceACLScope{ Filters: []silenceFilter{ - {NameRegex: regexp.MustCompile(".*"), ValueRegex: regexp.MustCompile(".*"), IsRegex: true}, + { + NameRegex: regexp.MustCompile(".*"), + ValueRegex: regexp.MustCompile(".*"), + IsRegex: truePtr(), + }, }, Groups: []string{}, Alertmanagers: []string{}, @@ -1085,7 +1105,7 @@ func TestProxySilenceACL(t *testing.T) { { Name: "foo", ValueRegex: regexp.MustCompile("^.+$"), - IsRegex: true, + IsRegex: truePtr(), }, }, }, @@ -1115,7 +1135,7 @@ func TestProxySilenceACL(t *testing.T) { { Name: "alertname", ValueRegex: regexp.MustCompile("^Fake Alert$"), - IsRegex: true, + IsRegex: truePtr(), }, }, }, @@ -1125,6 +1145,98 @@ func TestProxySilenceACL(t *testing.T) { frontednRequestBody: defaultBody, responseCode: 400, }, + { + name: "block negative matchers - block", + authGroups: map[string][]string{ + "admins": {"bob"}, + "users": {"alice"}, + }, + silenceACLs: []*silenceACL{ + { + Action: "block", + Reason: "block negative matchers", + Scope: silenceACLScope{ + Filters: []silenceFilter{ + { + NameRegex: regexp.MustCompile("^.+$"), + ValueRegex: regexp.MustCompile("^.+$"), + IsEqual: falsePtr(), + }, + }, + Groups: []string{}, + Alertmanagers: []string{}, + }, + }, + }, + requestUsername: "uncle", + frontednRequestBody: `{ + "comment": "comment", + "createdBy": "alice", + "startsAt": "2000-02-01T00:00:00.000Z", + "endsAt": "2000-02-01T00:02:03.000Z", + "matchers": [ + { "isRegex": false, "name": "alertname", "value": "Fake Alert" }, + { "isRegex": true, "isEqual": false, "name": "foo", "value": "(bar|baz)" } + ]}`, + responseCode: 400, + }, + { + name: "block negative matchers - pass", + authGroups: map[string][]string{ + "admins": {"bob"}, + "users": {"alice"}, + }, + silenceACLs: []*silenceACL{ + { + Action: "block", + Reason: "block negative matchers", + Scope: silenceACLScope{ + Filters: []silenceFilter{ + { + NameRegex: regexp.MustCompile("^.+$"), + ValueRegex: regexp.MustCompile("^.+$"), + IsEqual: falsePtr(), + }, + }, + Groups: []string{}, + Alertmanagers: []string{}, + }, + }, + }, + requestUsername: "uncle", + frontednRequestBody: defaultBody, + responseCode: 200, + }, + { + name: "require positive matcher", + authGroups: map[string][]string{ + "admins": {"bob"}, + "users": {"alice"}, + }, + silenceACLs: []*silenceACL{ + { + Action: "requireMatcher", + Reason: "require positive matcher", + Scope: silenceACLScope{ + Filters: []silenceFilter{}, + Groups: []string{}, + Alertmanagers: []string{}, + }, + Matchers: aclMatchers{ + Required: []silenceMatcher{ + { + Name: "alertname", + Value: "Fake Alert", + IsEqual: truePtr(), + }, + }, + }, + }, + }, + requestUsername: "uncle", + frontednRequestBody: defaultBody, + responseCode: 200, + }, { name: "invalid silence JSON", silenceACLs: []*silenceACL{ @@ -1133,7 +1245,11 @@ func TestProxySilenceACL(t *testing.T) { Reason: "block all regex silences", Scope: silenceACLScope{ Filters: []silenceFilter{ - {NameRegex: regexp.MustCompile(".*"), ValueRegex: regexp.MustCompile(".*"), IsRegex: true}, + { + NameRegex: regexp.MustCompile(".*"), + ValueRegex: regexp.MustCompile(".*"), + IsRegex: truePtr(), + }, }, Groups: []string{}, Alertmanagers: []string{}, diff --git a/cmd/karma/tests/testscript/001_acl_example.txt b/cmd/karma/tests/testscript/001_acl_example.txt index 977e56366..85291bbe1 100644 --- a/cmd/karma/tests/testscript/001_acl_example.txt +++ b/cmd/karma/tests/testscript/001_acl_example.txt @@ -8,7 +8,7 @@ level=info msg="Reading configuration file" path=karma.yaml level=info msg="Version: dev" level=info msg="Configured Alertmanager source" name=default proxy=false readonly=false uri=https://127.0.0.1:9093 level=info msg="Reading silence ACL config file" path=acl.yaml -level=info msg="Parsed ACL rules" rules=4 +level=info msg="Parsed ACL rules" rules=6 level=info msg="Configuration is valid" -- karma.yaml -- authentication: @@ -69,3 +69,17 @@ rules: required: - name_re: .+ value_re: .+ + - action: block + reason: block negative matchers + scope: + filters: + - name_re: .+ + value_re: .+ + isEqual: false + - action: requireMatcher + reason: require positive matcher + matchers: + required: + - name_re: .+ + value_re: .+ + isEqual: true diff --git a/docs/ACLs.md b/docs/ACLs.md index 956c35c6a..efcc2f490 100644 --- a/docs/ACLs.md +++ b/docs/ACLs.md @@ -16,17 +16,20 @@ Example Alertmanager silence: { "name": "alertname", "value": "Test Alert", - "isRegex": false + "isRegex": false, + "isEqual": true }, { "name": "cluster", "value": "prod", - "isRegex": false + "isRegex": false, + "isEqual": true }, { "name": "instance", "value": "server1", - "isRegex": false + "isRegex": false, + "isEqual": true } ], "startsAt": "2020-03-09T20:11:00.000Z", @@ -79,12 +82,14 @@ Silence example using regex: { "name": "alertname", "value": "Test Alert", - "isRegex": false + "isRegex": false, + "isEqual": true }, { "name": "cluster", "value": "staging|prod", - "isRegex": true + "isRegex": true, + "isEqual": true } ], "startsAt": "2020-03-09T20:11:00.000Z", @@ -110,12 +115,14 @@ so a silence like the one below should be blocked: { "name": "alertname", "value": "Test Alert", - "isRegex": false + "isRegex": false, + "isEqual": true }, { "name": "cluster", "value": "prod", - "isRegex": false + "isRegex": false, + "isEqual": true } ], "startsAt": "2020-03-09T20:11:00.000Z", @@ -131,7 +138,8 @@ But if we would create an ACL rule that simply blocks silences with matcher: { "name": "cluster", "value": "prod", - "isRegex": false + "isRegex": false, + "isEqual": true } ``` @@ -141,7 +149,8 @@ then any user could bypass that with a regex matcher like: { "name": "cluster", "value": "pro[d]", - "isRegex": true + "isRegex": true, + "isEqual": true } ``` @@ -188,6 +197,7 @@ matchers: - `scope:filters` - list of matcher filters evaluated when checking if this ACL should be applied to given silence. Those filters can be used to enforce ACL rules only to some silences and are compared against silence matchers. + All filters must be matching for given silence for ACL rule to be applied. Syntax: ```YAML @@ -196,13 +206,23 @@ matchers: value: string value_re: regex isRegex: bool + isEqual: bool ``` - Every rule must have `name` or `name_re` AND `value` or `value_re`, default - value for `isRegex` is `false`. - Filter works by comparing `name` and `name_re` with silence matcher `name`, - `value` and `value_re` with silence matcher `value` and `isRegex` on the - filter with `isRegex` on silence matcher. See examples below. + Every rule must have `name` or `name_re` AND `value` or `value_re`. + + Filter works by comparing: + + - `name` and `name_re` with silence matcher `name`. + - `value` and `value_re` with silence matcher `value`. + - `isRegex` on the filter with `isRegex` on silence matcher, if `isRegex` is + not set on a filter then that filter will match silences with both `true` + and `false` value on silence `isRegex`. + - `isEqual` on the filter with `isEqual` on silence matcher, if `isEqual` is + not set on a filter then that filter will match silences with both `true` + and `false` value on a silence `isEqual`. + + See examples below. All regexes will be automatically anchored. - `matchers:required` - list of additional matchers that must be part of the @@ -217,6 +237,7 @@ matchers: value: string value_re: regex isRegex: bool + isEqual: bool ``` Fields: @@ -224,20 +245,38 @@ matchers: - `name` - name to match, silence will be required to have a matcher with this exact name. - `name_re` - name regex to match against, silence will be required to have a - matcher with `name` field that matches this regex + matcher with `name` field that matches this regex. - `value` - value to match, silence will be required to have a matcher with - this exact value + this exact value. - `value_re` - value regex to match against, silence will be required to have - a matcher with `value` field that matches this regex + a matcher with `value` field that matches this regex. + - `isRegex` - value of silence matcher `isRegex`, if not set on a required + matcher then any value of `isRegex` on a silence will be allowed. + - `isEqual` - value of silence matcher `isEqual`, if not set on a required + matcher then any value of `isEqual` on a silence will be allowed. A single entry cannot have both `name` & `name_re` or `value` & `value_re` set at the same time. ## Examples +### Block all silences + +This rule will match all silence and block it. + +```YAML +rules: + - action: block + reason: silences are blocked + scope: + filters: + - name_re: .+ + value_re: .+ +``` + ### Block silences using regex matchers -This rule will match all silences with any matcher using regexes +This rule will match all silence with any matcher using regexes (`isRegex: true` on the matcher) and block it. ```YAML @@ -251,7 +290,22 @@ rules: isRegex: true ``` -### Allow group to create any silence +### Block negative matchers on silences + +This rule will match all silence with `isEqual: false` and block it. + +```YAML +rules: + - action: block + reason: silences are blocked + scope: + filters: + - name_re: .+ + value_re: .+ + isEqual: false +``` + +### Allow admin group to create any silence ```YAML rules: @@ -291,6 +345,7 @@ rules: filters: - name: cluster value: prod + isEqual: true ``` ### Require postgresAdmins group to always specify db=postgres in silences @@ -309,6 +364,7 @@ rules: required: - name: db value: postgres + isEqual: true ``` ### Require devTeam group to specify instance=server1-3 @@ -327,6 +383,7 @@ rules: required: - name: instance value_re: server[1-3] + isEqual: true ``` ### Require everyone to always specify `team` matcher in silences diff --git a/internal/config/acl.go b/internal/config/acl.go index badacb274..fa1e1a0cc 100644 --- a/internal/config/acl.go +++ b/internal/config/acl.go @@ -12,7 +12,8 @@ type SilenceMatcher struct { NameRegex string `yaml:"name_re"` Value string `yaml:"value"` ValueRegex string `yaml:"value_re"` - IsRegex bool `yaml:"isRegex"` + IsRegex *bool `yaml:"isRegex"` + IsEqual *bool `yaml:"isEqual"` } type SilenceACLMatchersConfig struct { @@ -24,7 +25,8 @@ type SilenceFilters struct { NameRegex string `yaml:"name_re,omitempty"` Value string `yaml:"value,omitempty"` ValueRegex string `yaml:"value_re,omitempty"` - IsRegex bool `yaml:"isRegex"` + IsRegex *bool `yaml:"isRegex,omitempty"` + IsEqual *bool `yaml:"isEqual,omitempty"` } type SilenceACLRuleScope struct { diff --git a/internal/mapper/v017/api.go b/internal/mapper/v017/api.go index 09eec3e45..590155298 100644 --- a/internal/mapper/v017/api.go +++ b/internal/mapper/v017/api.go @@ -155,11 +155,19 @@ func unmarshal(body []byte) (*models.Silence, error) { CreatedBy: *s.CreatedBy, Comment: *s.Comment, } + + var isEqual bool for _, m := range s.Matchers { + if m.IsEqual != nil { + isEqual = *m.IsEqual + } else { + isEqual = true + } sm := models.SilenceMatcher{ Name: *m.Name, Value: *m.Value, IsRegex: *m.IsRegex, + IsEqual: isEqual, } us.Matchers = append(us.Matchers, sm) }