Skip to content

Commit

Permalink
More advanced label checks
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Dec 14, 2023
1 parent 19b966b commit d11ae9c
Show file tree
Hide file tree
Showing 20 changed files with 832 additions and 241 deletions.
1 change: 1 addition & 0 deletions .github/spellcheck/wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ templated
Thanos
TLS
toc
tokenize
uber
UI
unmarshal
Expand Down
4 changes: 2 additions & 2 deletions cmd/pint/tests/0007_alerts.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ rules/0001.yml:9-10 Warning: `severity` label is required. (rule/label)
9 | - alert: ServiceIsDown
10 | expr: up == 0

rules/0001.yml:14 Warning: `severity` label value must match `^critical|warning|info$`. (rule/label)
rules/0001.yml:14 Warning: `severity` label value `bad` must match `^critical|warning|info$`. (rule/label)
14 | severity: bad

rules/0001.yml:16 Bug: `url` annotation value must match `^https://wiki.example.com/page/(.+).html$`. (alerts/annotation)
rules/0001.yml:16 Bug: `url` annotation value `bad` must match `^https://wiki.example.com/page/(.+).html$`. (alerts/annotation)
16 | url: bad

rules/0002.yml:5 Fatal: Template failed to parse with this error: `undefined variable "$label"`. (alerts/template)
Expand Down
2 changes: 1 addition & 1 deletion cmd/pint/tests/0058_templated_check.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ rules/0001.yml:4-6 Bug: `alert_for` annotation is required. (alerts/annotation)
5 | expr: up == 0
6 | for: 5m

rules/0001.yml:12 Bug: `alert_for` annotation value must match `^{{ $for }}$`. (alerts/annotation)
rules/0001.yml:12 Bug: `alert_for` annotation value `4m` must match `^{{ $for }}$`. (alerts/annotation)
12 | alert_for: 4m

level=INFO msg="Problems found" Bug=2
Expand Down
2 changes: 1 addition & 1 deletion cmd/pint/tests/0137_annotation_regex_key_fail.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ cmp stderr stderr.txt
-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/0001.yml:4 Bug: `annotation_.*` annotation value must match `^bar$`. (alerts/annotation)
rules/0001.yml:4 Bug: `annotation_.*` annotation value `foo` must match `^bar$`. (alerts/annotation)
4 | annotation_foo: foo

level=INFO msg="Problems found" Bug=1
Expand Down
13 changes: 13 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,23 @@

## v0.52.0

### Added

- Both [alerts/annotation](checks/alerts/annotation.md) and [rule/label](checks/rule/label.md)
now support more advance validation of label and annotation values with extra `token` option.
In addition to the `value` regexp matching you can also validate values against a static
list of allowed values using new `values` option.
See both checks documentation for detail.

### Changed

- More reports will now be merged into a single comments when using BitBucket.

### Fixed

- Fixed YAML anchor parsing.
- Fixed regexp matching for label names in [rule/label](checks/rule/label.md).

## v0.51.1

### Fixed
Expand Down
40 changes: 38 additions & 2 deletions docs/checks/alerts/annotation.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ Syntax:
```js
annotation "$pattern" {
severity = "bug|warning|info"
token = "(.*)"
value = "(.*)"
values = ["...", ...]
required = true|false
}
```
Expand All @@ -24,8 +26,16 @@ annotation "$pattern" {
to reference checked rule fields, see [Configuration](../../configuration.md)
for details.
- `severity` - set custom severity for reported issues, defaults to a warning.
- `value` - optional value pattern to enforce, if not set only the.
- `required` - if `true` pint will require every alert to have this annotation set,
- `token` - optional regexp to tokenize annotation value before validating it.
By default the whole annotation value is validated against `value` regexp or
the `values` list. If you want to break the value into sub-strings and
validate each of them independently you can do this by setting `token`
to a regexp that captures a single sub-string.
- `value` - optional value regexp to enforce, if not set only pint will only
check if the annotation exists.
- `values` - optional list of allowed values - this is alternative to using
`value` regexp. Set this to the list of all possible valid annotation values.
- `required` - if `true` pint will require every rule to have this annotation set,
if `false` it will only check values where annotation is set.

## How to enable it
Expand Down Expand Up @@ -76,6 +86,32 @@ rule {
}
```

If you have an annotation that can contain multiple different values as a single string,
for example `components: "db api memcached"`, and you want to ensure only valid values
are included then use `token` and `values`.
By setting `token` to a regexp that matches only a sequence of letters (`[a-zA-Z]+`)
you tell pint to split `"db api memcached"` into `["db", "api", "memcached"]`.
Then it iterates this list and checks each element independently.
This allows you to have validation for multi-value strings.

{% raw %}

```js
rule {
annotation "components" {
required = true
token = "[a-zA-Z]+"
values = [
"prometheus",
"db",
"memcached",
"api",
"storage",
]
}
}
```

{% endraw %}

## How to disable it
Expand Down
48 changes: 43 additions & 5 deletions docs/checks/rule/label.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,28 @@ Syntax:
```js
label "$pattern" {
severity = "bug|warning|info"
value = "..."
token = "(.*)"
value = "(.*)"
values = ["...", ...]
required = true|false
}
```

- `$pattern` - regexp pattern to match label name on, this can be templated
to reference checked rule fields, see [Configuration](../../configuration.md)
for details
- `severity` - set custom severity for reported issues, defaults to a warning
- `value` - optional value pattern to enforce, if not set only the
for details.
- `severity` - set custom severity for reported issues, defaults to a warning.
- `token` - optional regexp to tokenize label value before validating it.
By default the whole label value is validated against `value` regexp or
the `values` list. If you want to break the value into sub-strings and
validate each of them independently you can do this by setting `token`
to a regexp that captures a single sub-string.
- `value` - optional value regexp to enforce, if not set only pint will only
check if the label exists.
- `values` - optional list of allowed values - this is alternative to using
`value` regexp. Set this to the list of all possible valid label values.
- `required` - if `true` pint will require every rule to have this label set,
if `false` it will only check values where label is set
if `false` it will only check values where label is set.

## How to enable it

Expand Down Expand Up @@ -75,6 +85,34 @@ rule {

{% endraw %}

If you have a label that can contain multiple different values as a single string,
for example `components: "db api memcached"`, and you want to ensure only valid values
are included then use `token` and `values`.
By setting `token` to a regexp that matches only a sequence of letters (`[a-zA-Z]+`)
you tell pint to split `"db api memcached"` into `["db", "api", "memcached"]`.
Then it iterates this list and checks each element independently.
This allows you to have validation for multi-value strings.

{% raw %}

```js
rule {
label "components" {
required = true
token = "[a-zA-Z]+"
values = [
"prometheus",
"db",
"memcached",
"api",
"storage",
]
}
}
```

{% endraw %}

## How to disable it

You can disable this check globally by adding this config block:
Expand Down
36 changes: 36 additions & 0 deletions docs/examples/labels.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# This example shows how to enforce labels where the label value itself can
# be a string with one or more sub-strings in it.
# For example when you have a 'components' label that can be any of these:
# components: 'db'
# components: 'api'
# components: 'proxy'
# components: 'db api'
# components: 'db api proxy'
# components: 'proxy api db'
# components: 'proxy db'

rule {
# Only run these checks on alerting rules.
# Ignore recording rules.
match {
kind = "alerting"
}
label "components" {
# Every alerting rule must have this label set.
required = true
# If any alerting rule fails our check pint will report his as a 'Bug'
# severity problem, which will fail (exit with non-zero exit code)
# when running 'pint lint' or 'pint ci'.
# Set it to 'warning' if you don't want to fail pint runs.
severity = "bug"
# Split label value into sub-strings using the 'token' regexp.
# \w is an alias for [0-9A-Za-z_] match.
# Notice that we must escape '\' in HCL config files.
token = "\\w+"
# This is the list of allowed values.
values = [
"db",
"api",
"proxy",
]
}
80 changes: 65 additions & 15 deletions internal/checks/alerts_annotation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,35 @@ package checks
import (
"context"
"fmt"
"strconv"
"strings"

"github.com/cloudflare/pint/internal/discovery"
"github.com/cloudflare/pint/internal/parser"

"golang.org/x/exp/slices"
)

const (
AnnotationCheckName = "alerts/annotation"
)

func NewAnnotationCheck(keyRe, valueRe *TemplatedRegexp, isReguired bool, severity Severity) AnnotationCheck {
return AnnotationCheck{keyRe: keyRe, valueRe: valueRe, isReguired: isReguired, severity: severity}
func NewAnnotationCheck(keyRe, tokenRe, valueRe *TemplatedRegexp, values []string, isReguired bool, severity Severity) AnnotationCheck {
return AnnotationCheck{
keyRe: keyRe,
tokenRe: tokenRe,
valueRe: valueRe,
values: values,
isReguired: isReguired,
severity: severity,
}
}

type AnnotationCheck struct {
keyRe *TemplatedRegexp
tokenRe *TemplatedRegexp
valueRe *TemplatedRegexp
values []string
isReguired bool
severity Severity
}
Expand Down Expand Up @@ -63,24 +76,15 @@ func (c AnnotationCheck) Check(_ context.Context, _ string, rule parser.Rule, _
return problems
}

var foundAnnotation bool
annotations := make([]*parser.YamlKeyValue, 0, len(rule.AlertingRule.Annotations.Items))

for _, annotation := range rule.AlertingRule.Annotations.Items {
if c.keyRe.MustExpand(rule).MatchString(annotation.Key.Value) {
foundAnnotation = true
if c.valueRe != nil && !c.valueRe.MustExpand(rule).MatchString(annotation.Value.Value) {
problems = append(problems, Problem{
Lines: annotation.Value.Position.Lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("`%s` annotation value must match `%s`.", c.keyRe.original, c.valueRe.anchored),
Severity: c.severity,
})
return problems
}
annotations = append(annotations, annotation)
}
}

if !foundAnnotation && c.isReguired {
if len(annotations) == 0 && c.isReguired {
problems = append(problems, Problem{
Lines: rule.AlertingRule.Annotations.Lines(),
Reporter: c.Reporter(),
Expand All @@ -90,5 +94,51 @@ func (c AnnotationCheck) Check(_ context.Context, _ string, rule parser.Rule, _
return problems
}

return nil
for _, ann := range annotations {
if c.tokenRe != nil {
for _, match := range c.tokenRe.MustExpand(rule).FindAllString(ann.Value.Value, -1) {
problems = append(problems, c.checkValue(rule, match, ann.Value.Position.Lines)...)
}
} else {
problems = append(problems, c.checkValue(rule, ann.Value.Value, ann.Value.Position.Lines)...)
}
}

return problems
}

func (c AnnotationCheck) checkValue(rule parser.Rule, value string, lines []int) (problems []Problem) {
if c.valueRe != nil && !c.valueRe.MustExpand(rule).MatchString(value) {
problems = append(problems, Problem{
Lines: lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("`%s` annotation value `%s` must match `%s`.", c.keyRe.original, value, c.valueRe.anchored),
Severity: c.severity,
})
}
if len(c.values) > 0 {
if !slices.Contains(c.values, value) {
var details strings.Builder
details.WriteString("List of allowed values:\n\n")
for i, allowed := range c.values {
details.WriteString("- `")
details.WriteString(allowed)
details.WriteString("`\n")
if i >= 5 && len(c.values) > 8 {
details.WriteString("\nAnd ")
details.WriteString(strconv.Itoa(len(c.values) - i - 1))
details.WriteString(" other value(s).")
break
}
}
problems = append(problems, Problem{
Lines: lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("`%s` annotation value `%s` is not one of valid values.", c.keyRe.original, value),
Details: details.String(),
Severity: c.severity,
})
}
}
return problems
}
Loading

0 comments on commit d11ae9c

Please sign in to comment.