Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add locked rule support #1217

Merged
merged 1 commit into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions cmd/pint/tests/0207_locked_rule.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
! exec pint --no-color -l debug lint rules
! stdout .
cmp stderr stderr.txt

-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=DEBUG msg="Adding pint config to the parser exclude list" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
level=DEBUG msg="File parsed" path=rules/0001.yaml rules=1
level=DEBUG msg="Glob finder completed" count=1
level=DEBUG msg="Generated all Prometheus servers" count=0
level=DEBUG msg="Found recording rule" path=rules/0001.yaml record=colo_job:up:byinstance lines=6-7 state=noop
level=DEBUG msg="Configured checks for rule" enabled=["promql/syntax","alerts/for","alerts/comparison","alerts/template","promql/fragile","promql/regexp","promql/aggregate(job:true)"] path=rules/0001.yaml rule=colo_job:up:byinstance
rules/0001.yaml:7 Bug: `job` label is required and should be preserved when aggregating `^.+$` rules, use `by(job, ...)`. (promql/aggregate)
7 | expr: sum(byinstance) by(instance)

level=INFO msg="Problems found" Bug=1
level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher"
-- rules/0001.yaml --
groups:
- name: foo
rules:

# pint disable promql/aggregate(job:true)
- record: colo_job:up:byinstance
expr: sum(byinstance) by(instance)

-- .pint.hcl --
rule {
locked = true
aggregate ".+" {
keep = [ "job" ]
severity = "bug"
}
}
3 changes: 3 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
}
```

- Rules configured in `pint` config can now be locked - when a rule is locked it cannot
be disabled by users by adding a `# pint disable ...` or `# pint snooze ...` comments.

### Fixed

- The console reporter won't color the output if `--no-color` flag is set.
Expand Down
3 changes: 3 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,7 @@ Syntax:

```js
rule {
locked = true|false
match {
path = "(.+)"
state = [ "any|added|modified|renamed|removed|unmodified", ... ]
Expand Down Expand Up @@ -569,6 +570,8 @@ rule {
}
```

- `locked` - if set to `true` this rule will be locked, which means that it cannot be disabled using
`# pint disable ...` or `# pint snooze ...` comments.
- `match:path` - only files matching this pattern will be checked by this rule.
- `match:state` - only match rules based on their state. Default value for `state` depends on the
pint command that is being run, for `pint ci` the default value is `["added", "modified", "renamed"]`,
Expand Down
136 changes: 136 additions & 0 deletions internal/config/__snapshots__/config_test.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3285,3 +3285,139 @@
]
}
---

[TestGetChecksForRule/multiple_checks_and_disable_comment_/_locked_rule - 1]
{
"ci": {
"baseBranch": "master",
"maxCommits": 20
},
"parser": {},
"checks": {
"enabled": [
"alerts/absent",
"alerts/annotation",
"alerts/count",
"alerts/external_labels",
"alerts/for",
"alerts/template",
"labels/conflict",
"promql/aggregate",
"alerts/comparison",
"promql/fragile",
"promql/range_query",
"promql/rate",
"promql/regexp",
"promql/syntax",
"promql/vector_matching",
"query/cost",
"promql/counter",
"promql/series",
"rule/dependency",
"rule/duplicate",
"rule/for",
"rule/name",
"rule/label",
"rule/link",
"rule/reject",
"rule/report"
]
},
"owners": {},
"rules": [
{
"aggregate": [
{
"name": ".+",
"severity": "bug",
"keep": [
"job"
]
}
]
},
{
"aggregate": [
{
"name": ".+",
"comment": "this is rule comment",
"severity": "bug",
"strip": [
"instance",
"rack"
]
}
],
"locked": true
}
]
}
---

[TestGetChecksForRule/multiple_checks_and_snooze_comment_/_locked_rule - 1]
{
"ci": {
"baseBranch": "master",
"maxCommits": 20
},
"parser": {},
"checks": {
"enabled": [
"alerts/absent",
"alerts/annotation",
"alerts/count",
"alerts/external_labels",
"alerts/for",
"alerts/template",
"labels/conflict",
"promql/aggregate",
"alerts/comparison",
"promql/fragile",
"promql/range_query",
"promql/rate",
"promql/regexp",
"promql/syntax",
"promql/vector_matching",
"query/cost",
"promql/counter",
"promql/series",
"rule/dependency",
"rule/duplicate",
"rule/for",
"rule/name",
"rule/label",
"rule/link",
"rule/reject",
"rule/report"
]
},
"owners": {},
"rules": [
{
"aggregate": [
{
"name": ".+",
"severity": "bug",
"keep": [
"job"
]
}
]
},
{
"aggregate": [
{
"name": ".+",
"comment": "this is rule comment",
"severity": "bug",
"strip": [
"instance",
"rack"
]
}
],
"locked": true
}
]
}
---
2 changes: 1 addition & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (cfg *Config) GetChecksForEntry(ctx context.Context, gen *PrometheusGenerat
if !isMatch(ctx, entry, pr.ignore, pr.match) {
continue
}
if pr.isEnabled(ctx, cfg.Checks.Enabled, cfg.Checks.Disabled, enabled, entry, cfg.Rules) {
if pr.isEnabled(ctx, cfg.Checks.Enabled, cfg.Checks.Disabled, enabled, entry, cfg.Rules, pr.locked) {
enabled = append(enabled, pr.check)
}
}
Expand Down
82 changes: 82 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2192,6 +2192,88 @@ rule {
checks.ReportCheckName,
},
},
{
title: "multiple checks and disable comment / locked rule",
config: `
rule {
locked = false
aggregate ".+" {
severity = "bug"
keep = ["job"]
}
}
rule {
locked = true
aggregate ".+" {
comment = "this is rule comment"
severity = "bug"
strip = ["instance", "rack"]
}
}`,
entry: discovery.Entry{
State: discovery.Modified,
Path: discovery.Path{
Name: "rules.yml",
SymlinkTarget: "rules.yml",
},
Rule: newRule(t, `
- record: foo
# pint disable promql/aggregate
expr: sum(foo)
`),
},
checks: []string{
checks.SyntaxCheckName,
checks.AlertForCheckName,
checks.ComparisonCheckName,
checks.TemplateCheckName,
checks.FragileCheckName,
checks.RegexpCheckName,
checks.AggregationCheckName + "(instance:false)",
checks.AggregationCheckName + "(rack:false)",
},
},
{
title: "multiple checks and snooze comment / locked rule",
config: `
rule {
locked = false
aggregate ".+" {
severity = "bug"
keep = ["job"]
}
}
rule {
locked = true
aggregate ".+" {
comment = "this is rule comment"
severity = "bug"
strip = ["instance", "rack"]
}
}`,
entry: discovery.Entry{
State: discovery.Modified,
Path: discovery.Path{
Name: "rules.yml",
SymlinkTarget: "rules.yml",
},
Rule: newRule(t, `
- record: foo
# pint snooze 2099-11-28 promql/aggregate
expr: sum(foo)
`),
},
checks: []string{
checks.SyntaxCheckName,
checks.AlertForCheckName,
checks.ComparisonCheckName,
checks.TemplateCheckName,
checks.FragileCheckName,
checks.RegexpCheckName,
checks.AggregationCheckName + "(instance:false)",
checks.AggregationCheckName + "(rack:false)",
},
},
}

dir := t.TempDir()
Expand Down
Loading
Loading