Merge pull request cloudflare#230 from cloudflare/owner

Fix rule/owner line reporting

cmd/pint/lint.go

@@ -79,29 +79,22 @@ func actionLint(c *cli.Context) error {
 }
 
 func verifyOwners(entries []discovery.Entry) (reports []reporter.Report) {
-	done := map[string]struct{}{}
 	for _, entry := range entries {
-		if _, ok := done[entry.Path]; ok {
+		if entry.Owner != "" {
 			continue
 		}
-
-		if entry.Owner == "" {
-			reports = append(reports, reporter.Report{
-				Path:          entry.Path,
-				ModifiedLines: []int{1},
-				Rule:          entry.Rule,
-				Problem: checks.Problem{
-					Lines:    []int{1},
-					Reporter: discovery.RuleOwnerComment,
-					Text: fmt.Sprintf(`%s comments are required in all files, please add a "# pint %s $owner" somewhere in this file and/or "# pint %s $owner" on top of each rule`,
-						discovery.RuleOwnerComment, discovery.FileOwnerComment, discovery.RuleOwnerComment),
-					Severity: checks.Bug,
-				},
-			})
-		}
-
-		done[entry.Path] = struct{}{}
+		reports = append(reports, reporter.Report{
+			Path:          entry.Path,
+			ModifiedLines: entry.ModifiedLines,
+			Rule:          entry.Rule,
+			Problem: checks.Problem{
+				Lines:    entry.Rule.Lines(),
+				Reporter: discovery.RuleOwnerComment,
+				Text: fmt.Sprintf(`%s comments are required in all files, please add a "# pint %s $owner" somewhere in this file and/or "# pint %s $owner" on top of each rule`,
+					discovery.RuleOwnerComment, discovery.FileOwnerComment, discovery.RuleOwnerComment),
+				Severity: checks.Bug,
+			},
+		})
 	}
-
 	return
 }

cmd/pint/tests/0066_lint_owner.txt

@@ -4,16 +4,22 @@ cmp stderr stderr.txt
 
 -- stderr.txt --
 level=info msg="Loading configuration file" path=.pint.hcl
-level=info msg="File parsed" path=rules/1.yml rules=2
+level=info msg="File parsed" path=rules/1.yml rules=3
 level=info msg="File parsed" path=rules/2.yml rules=1
 level=info msg="File parsed" path=rules/3.yml rules=1
-rules/1.yml:1: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
-groups:
+rules/1.yml:4-5: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
+  - alert: No Owner
+    expr: up > 0
 
-rules/3.yml:1: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
+rules/1.yml:9-10: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
+  - alert: No Owner
+    expr: up > 0
+
+rules/3.yml:1-2: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
 - alert: No Owner
+  expr: up{job="foo"} == 0
 
-level=info msg="Problems found" Bug=2
+level=info msg="Problems found" Bug=3
 level=fatal msg="Fatal error" error="problems found"
 -- rules/1.yml --
 groups:
@@ -24,6 +30,8 @@ groups:
   # pint rule/owner bob
   - alert: Owner Set
     expr: up == 0
+  - alert: No Owner
+    expr: up > 0
 
 -- rules/2.yml --
 groups:

cmd/pint/tests/0071_ci_owner.txt

@@ -26,12 +26,17 @@ cmp stderr stderr.txt
 exec sh -c 'cat server.pid | xargs kill'
 
 -- stderr.txt --
-rules.yml:1: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
-groups:
+rules.yml:4-5: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
+    - alert: rule1
+      expr: sum(foo) by(job)
 
 rules.yml:5: alert query doesn't have any condition, it will always fire if the metric exists (alerts/comparison)
       expr: sum(foo) by(job)
 
+rules.yml:6-7: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
+    - alert: rule2
+      expr: sum(foo) by(job) > 0
+
 level=fatal msg="Fatal error" error="problems found"
 -- src/v1.yml --
 - alert: rule1

docs/changelog.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## v0.17.7
+
+### Fixed
+
+- Fix problem line reporting for `rule/owner` check.
+- Add missing `rule/owner` documentation page.
+
 ## v0.17.6
 
 ### Fixed

docs/checks/rule/owner.md

@@ -0,0 +1,43 @@
+---
+layout: default
+parent: Checks
+grand_parent: Documentation
+---
+
+# rule/owner
+
+This check can be used to enforce rule ownership comments used by `pint watch`
+command when exporting metrics about problems detected in rules.
+
+If you see this check reports it means that `--require-owner` flag is enabled
+for pint and a rule file is missing required ownership comment.
+
+To set a rule owner add a `# pint file/owner $owner` comment in a file, to set
+an owner for all rules in that file. You can also set an owner per rule, by adding
+`# pint rule/owner $owner` comment around given rule.
+
+Example:
+
+```yaml
+# pint file/owner bob
+
+- alert: ...
+  expr: ...
+
+# pint rule/owner alice
+- alert: ...
+  expr: ...
+```
+
+## Configuration
+
+This check doesn't have any configuration options.
+
+## How to enable it
+
+This check is enabled only if you pass `--require-owner` flag to `pint lint`
+or `pint ci` commands.
+
+## How to disable it
+
+Remove `--require-owner` flag from pint CLI arguments.