Merge pull request #1059 from cloudflare/issue-631

prymitive · web-flow · commit c095cdaf3175 · 2024-08-08T17:31:44.000+01:00
Move include/exclude from ci to parser
diff --git a/.github/pint/pint.hcl b/.github/pint/pint.hcl
@@ -1,3 +1,3 @@
-ci {
+parser {
   include    = [".github/pint/rules/.*"]
 }
diff --git a/cmd/pint/ci.go b/cmd/pint/ci.go
@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"log/slog"
 	"os"
-	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -64,16 +63,6 @@ func actionCI(c *cli.Context) error {
 		return err
 	}
 
-	includeRe := []*regexp.Regexp{}
-	for _, pattern := range meta.cfg.CI.Include {
-		includeRe = append(includeRe, regexp.MustCompile("^"+pattern+"$"))
-	}
-
-	excludeRe := []*regexp.Regexp{}
-	for _, pattern := range meta.cfg.CI.Exclude {
-		excludeRe = append(excludeRe, regexp.MustCompile("^"+pattern+"$"))
-	}
-
 	meta.cfg.CI = detectCI(meta.cfg.CI)
 	baseBranch := meta.cfg.CI.BaseBranch
 	if c.String(baseBranchFlag) != "" {
@@ -92,7 +81,11 @@ func actionCI(c *cli.Context) error {
 	slog.Info("Finding all rules to check on current git branch", slog.String("base", baseBranch))
 
 	var entries []discovery.Entry
-	filter := git.NewPathFilter(includeRe, excludeRe, meta.cfg.Parser.CompileRelaxed())
+	filter := git.NewPathFilter(
+		config.MustCompileRegexes(meta.cfg.Parser.Include...),
+		config.MustCompileRegexes(meta.cfg.Parser.Exclude...),
+		config.MustCompileRegexes(meta.cfg.Parser.Relaxed...),
+	)
 
 	entries, err = discovery.NewGlobFinder([]string{"*"}, filter).Find()
 	if err != nil {
diff --git a/cmd/pint/lint.go b/cmd/pint/lint.go
@@ -63,7 +63,11 @@ func actionLint(c *cli.Context) error {
 	}
 
 	slog.Info("Finding all rules to check", slog.Any("paths", paths))
-	finder := discovery.NewGlobFinder(paths, git.NewPathFilter(nil, nil, meta.cfg.Parser.CompileRelaxed()))
+	finder := discovery.NewGlobFinder(paths, git.NewPathFilter(
+		config.MustCompileRegexes(meta.cfg.Parser.Include...),
+		config.MustCompileRegexes(meta.cfg.Parser.Exclude...),
+		config.MustCompileRegexes(meta.cfg.Parser.Relaxed...),
+	))
 	entries, err := finder.Find()
 	if err != nil {
 		return err
diff --git a/cmd/pint/tests/0065_ci_include.txt b/cmd/pint/tests/0065_ci_include.txt
@@ -37,8 +37,8 @@ stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=b.yml'
 -- src/.pint.hcl --
 ci {
   baseBranch = "main"
-  include = ["xxx"]
 }
 parser {
+  include = ["xxx"]
   relaxed = [".*"]
 }
diff --git a/cmd/pint/tests/0071_ci_owner.txt b/cmd/pint/tests/0071_ci_owner.txt
@@ -56,9 +56,11 @@ groups:
 
 -- src/.pint.hcl --
 ci {
-  include    = [".+.yml"]
   baseBranch = "main"
 }
+parser {
+  include    = [".+.yml"]
+}
 repository {
   bitbucket {
     uri        = "http://127.0.0.1:6071"
diff --git a/cmd/pint/tests/0076_ci_group_errors.txt b/cmd/pint/tests/0076_ci_group_errors.txt
@@ -75,6 +75,8 @@ groups:
 -- src/.pint.hcl --
 ci {
   baseBranch = "main"
+}
+parser {
   include    = [".+.yml"]
 }
 repository {
diff --git a/cmd/pint/tests/0098_rule_file_symlink_gh.txt b/cmd/pint/tests/0098_rule_file_symlink_gh.txt
@@ -53,7 +53,7 @@ ci {
 repository {
   github {
     baseuri   = "http://127.0.0.1:6098"
-	uploaduri = "http://127.0.0.1:6098"
+	  uploaduri = "http://127.0.0.1:6098"
     owner     = "cloudflare"
     repo      = "pint"
   }
diff --git a/cmd/pint/tests/0118_ci_dir_move.txt b/cmd/pint/tests/0118_ci_dir_move.txt
@@ -46,7 +46,7 @@ ci {
 repository {
   github {
     baseuri   = "http://127.0.0.1:6118"
-	uploaduri = "http://127.0.0.1:6118"
+	  uploaduri = "http://127.0.0.1:6118"
     owner     = "cloudflare"
     repo      = "pint"
   }
diff --git a/cmd/pint/tests/0123_ci_owner_allowed.txt b/cmd/pint/tests/0123_ci_owner_allowed.txt
@@ -65,6 +65,8 @@ owners {
 }
 ci {
   baseBranch = "main"
+}
+parser {
   include    = [".+.yml"]
 }
 repository {
diff --git a/cmd/pint/tests/0139_ci_exclude.txt b/cmd/pint/tests/0139_ci_exclude.txt
@@ -37,8 +37,8 @@ stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=b.yml'
 -- src/.pint.hcl --
 ci {
   baseBranch = "main"
-  exclude = [".*.yml"]
 }
 parser {
+  exclude = [".*.yml"]
   relaxed = [".*"]
 }
diff --git a/cmd/pint/tests/0140_ci_include_exclude.txt b/cmd/pint/tests/0140_ci_include_exclude.txt
@@ -37,9 +37,9 @@ stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=b.yml'
 -- src/.pint.hcl --
 ci {
   baseBranch = "main"
-  include = [".*.yml"]
-  exclude = [".*.yml"]
 }
 parser {
+  include = [".*.yml"]
+  exclude = [".*.yml"]
   relaxed = [".*"]
 }
diff --git a/cmd/pint/tests/0160_ci_comment_edit.txt b/cmd/pint/tests/0160_ci_comment_edit.txt
@@ -103,6 +103,8 @@ groups:
 -- src/.pint.hcl --
 ci {
   baseBranch = "main"
+}
+parser {
   include    = [".+.yml"]
 }
 prometheus "prom" {
diff --git a/cmd/pint/tests/0161_ci_move_files.txt b/cmd/pint/tests/0161_ci_move_files.txt
@@ -78,6 +78,8 @@ groups:
 -- src/.pint.hcl --
 ci {
   baseBranch = "main"
+}
+parser {
   include    = [".+.yml"]
 }
 prometheus "prom1" {
diff --git a/cmd/pint/tests/0162_ci_deleted_dependency.txt b/cmd/pint/tests/0162_ci_deleted_dependency.txt
@@ -47,6 +47,8 @@ groups:
 -- src/.pint.hcl --
 ci {
   baseBranch = "main"
+}
+parser {
   include    = [".+.yml"]
 }
 prometheus "prom" {
diff --git a/cmd/pint/tests/0178_parser_include.txt b/cmd/pint/tests/0178_parser_include.txt
@@ -0,0 +1,24 @@
+pint.ok -l debug --no-color lint rules
+! stdout .
+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"]
+level=DEBUG msg="File path is in the include list" path=rules/0001.yml include=["^rules/0001.yml$"]
+level=DEBUG msg="File parsed" path=rules/0001.yml 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.yml record=ok lines=1-2
+level=DEBUG msg="Configured checks for rule" enabled=["promql/syntax","alerts/for","alerts/comparison","alerts/template","promql/fragile","promql/regexp"] path=rules/0001.yml rule=ok
+-- rules/0001.yml --
+- record: ok
+  expr: sum(foo) without(job)
+-- rules/0002.yml --
+- record: failing
+  expr: sum()
+-- .pint.hcl --
+parser {
+  relaxed = [".*"]
+  include = ["rules/0001.yml"]
+}
diff --git a/cmd/pint/tests/0179_parser_exclude.txt b/cmd/pint/tests/0179_parser_exclude.txt
@@ -0,0 +1,24 @@
+pint.ok -l debug --no-color lint rules
+! stdout .
+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"]
+level=DEBUG msg="File parsed" path=rules/0001.yml rules=1
+level=DEBUG msg="File path is in the exclude list" path=rules/0002.yml exclude=["^rules/0002.yml$"]
+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.yml record=ok lines=1-2
+level=DEBUG msg="Configured checks for rule" enabled=["promql/syntax","alerts/for","alerts/comparison","alerts/template","promql/fragile","promql/regexp"] path=rules/0001.yml rule=ok
+-- rules/0001.yml --
+- record: ok
+  expr: sum(foo) without(job)
+-- rules/0002.yml --
+- record: failing
+  expr: sum()
+-- .pint.hcl --
+parser {
+  relaxed = [".*"]
+  exclude = ["rules/0002.yml"]
+}
diff --git a/cmd/pint/tests/0180_parser_exclude_md.txt b/cmd/pint/tests/0180_parser_exclude_md.txt
@@ -0,0 +1,24 @@
+pint.ok -l debug --no-color lint rules
+! stdout .
+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"]
+level=DEBUG msg="File parsed" path=rules/0001.yml rules=1
+level=DEBUG msg="File path is in the exclude list" path=rules/README.md exclude=["^.*.md$"]
+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.yml record=ok lines=1-2
+level=DEBUG msg="Configured checks for rule" enabled=["promql/syntax","alerts/for","alerts/comparison","alerts/template","promql/fragile","promql/regexp"] path=rules/0001.yml rule=ok
+-- rules/0001.yml --
+- record: ok
+  expr: sum(foo) without(job)
+-- rules/README.md --
+This is a readme file
+`foo`
+-- .pint.hcl --
+parser {
+  relaxed = [".*"]
+  exclude = [".*.md"]
+}
diff --git a/cmd/pint/watch.go b/cmd/pint/watch.go
@@ -308,7 +308,11 @@ func (c *problemCollector) scan(ctx context.Context, workers int, isOffline bool
 	}
 
 	slog.Info("Finding all rules to check", slog.Any("paths", paths))
-	entries, err := discovery.NewGlobFinder(paths, git.NewPathFilter(nil, nil, c.cfg.Parser.CompileRelaxed())).Find()
+	entries, err := discovery.NewGlobFinder(paths, git.NewPathFilter(
+		config.MustCompileRegexes(c.cfg.Parser.Include...),
+		config.MustCompileRegexes(c.cfg.Parser.Exclude...),
+		config.MustCompileRegexes(c.cfg.Parser.Relaxed...),
+	)).Find()
 	if err != nil {
 		return err
 	}
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -5,6 +5,8 @@
 ### Added
 
 - Added `ignoreLabelsValue` to [promql/series](checks/promql/series.md) check settings.
+- Added `include` & `exclude` options were moved from the `ci` to the `parser` configuration block.
+  They now apply to all pint commands, not just `pint ci` - #631.
 
 ## v0.63.0
 
diff --git a/docs/configuration.md b/docs/configuration.md
@@ -64,11 +64,18 @@ Syntax:
 
 ```js
 parser {
+  include = [ "(.*)", ... ]
+  exclude = [ "(.*)", ... ]
   relaxed = [ "(.*)", ... ]
 }
 ```
 
-- `relaxed` - by default, pint will now parse all files in strict mode, where
+- `include` - list of file patterns to check when running checks. Only files
+  matching those regexp rules will be checked, other modified files will be ignored.
+- `exclude` - list of file patterns to ignore when running checks.
+  This option takes precedence over `include`, so if a file path matches both
+  `include` & `exclude` patterns, it will be excluded.
+- `relaxed` - by default, pint will parse all files in strict mode, where
   all rule files must have the exact syntax Prometheus expects:
 
   ```yaml
@@ -116,18 +123,11 @@ Syntax:
 
 ```js
 ci {
-  include    = [ "(.*)", ... ]
-  exclude    = [ "(.*)", ... ]
   maxCommits = 20
   baseBranch = "master"
 }
 ```
 
-- `include` - list of file patterns to check when running checks. Only files
-  matching those regexp rules will be checked, other modified files will be ignored.
-- `exclude` - list of file patterns to ignore when running checks.
-  This option takes precedence over `include`, so if a file path matches both
-  `include` & `exclude` patterns, it will be excluded.
 - `maxCommits` - by default, pint will try to find all commits on the current branch,
   this requires full git history to be present, if we have a shallow clone this
   might fail to find only current branch commits and give us a huge list.
diff --git a/docs/examples/ci.hcl b/docs/examples/ci.hcl
@@ -1,13 +1,15 @@
 # This is an example config to be used when running pint as a CI job
 # to validate pull requests.
 
-ci {
+parser {
   # Check all files inside rules/alerting and rules/recording dirs.
   include    = ["rules/(alerting|recording)/.+"]
 
   # Ignore all *.md and *.txt files.
   exclude    = [".+.md", ".+.txt"]
+}
 
+ci {
   # Don't run pint if there are more than 50 commits on current branch.
   maxCommits = 50
 
diff --git a/docs/index.md b/docs/index.md
@@ -107,15 +107,6 @@ jobs:
 To customise pint checks create a `.pint.hcl` file in the root of your repository.
 See [Configuration](configuration.md) for a description of all options.
 
-If your repository contains other files, not only Prometheus rules, then tell pint
-to only check selected paths when running checks on a pull request:
-
-```js
-ci {
-  include    = [ "rules/dev/.*.yml", "rules/prod/.*" ]
-}
-```
-
 When pint runs checks after a push to a branch (for example after a merge), then
 it will pass `workdir` option to `pint lint`, which means that all files inside
 `rules` directory will be checked.
@@ -317,6 +308,15 @@ See [parser](configuration.md#parser) documentation for more details.
 while "strict" parser mode will fail if it reads a file that wouldn't load
 cleanly as Prometheus config file.
 
+If your repository contains other files, not only Prometheus rules, then tell pint
+to only check selected paths when running pint:
+
+```js
+parser {
+  include    = [ "rules/dev/.*.yml", "rules/prod/.*" ]
+}
+```
+
 ## Control comments
 
 There is a number of comments you can add to your rule files in order to change
diff --git a/internal/config/ci.go b/internal/config/ci.go
@@ -2,33 +2,16 @@ package config
 
 import (
 	"errors"
-	"regexp"
 )
 
 type CI struct {
-	BaseBranch string   `hcl:"baseBranch,optional" json:"baseBranch,omitempty"`
-	Include    []string `hcl:"include,optional" json:"include,omitempty"`
-	Exclude    []string `hcl:"exclude,optional" json:"exclude,omitempty"`
-	MaxCommits int      `hcl:"maxCommits,optional" json:"maxCommits,omitempty"`
+	BaseBranch string `hcl:"baseBranch,optional" json:"baseBranch,omitempty"`
+	MaxCommits int    `hcl:"maxCommits,optional" json:"maxCommits,omitempty"`
 }
 
 func (ci CI) validate() error {
 	if ci.MaxCommits <= 0 {
 		return errors.New("maxCommits cannot be <= 0")
 	}
-
-	for _, pattern := range ci.Include {
-		_, err := regexp.Compile(pattern)
-		if err != nil {
-			return err
-		}
-	}
-
-	for _, pattern := range ci.Exclude {
-		_, err := regexp.Compile(pattern)
-		if err != nil {
-			return err
-		}
-	}
 	return nil
 }
diff --git a/internal/config/ci_test.go b/internal/config/ci_test.go
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
diff --git a/internal/config/owners.go b/internal/config/owners.go
diff --git a/internal/config/parser.go b/internal/config/parser.go
diff --git a/internal/config/parser_test.go b/internal/config/parser_test.go
diff --git a/internal/config/rule.go b/internal/config/rule.go
diff --git a/internal/git/filter.go b/internal/git/filter.go

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,3 @@`
`1`		`-ci {`
	`1`	`+parser {`
`2`	`2`	`include = [".github/pint/rules/.*"]`
`3`	`3`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,8 +37,8 @@ stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=b.yml'`
`37`	`37`	`-- src/.pint.hcl --`
`38`	`38`	`ci {`
`39`	`39`	`baseBranch = "main"`
`40`		`- include = ["xxx"]`
`41`	`40`	`}`
`42`	`41`	`parser {`
	`42`	`+ include = ["xxx"]`
`43`	`43`	`relaxed = [".*"]`
`44`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -75,6 +75,8 @@ groups:`
`75`	`75`	`-- src/.pint.hcl --`
`76`	`76`	`ci {`
`77`	`77`	`baseBranch = "main"`
	`78`	`+}`
	`79`	`+parser {`
`78`	`80`	`include = [".+.yml"]`
`79`	`81`	`}`
`80`	`82`	`repository {`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ ci {`
`53`	`53`	`repository {`
`54`	`54`	`github {`
`55`	`55`	`baseuri = "http://127.0.0.1:6098"`
`56`		`- uploaduri = "http://127.0.0.1:6098"`
	`56`	`+ uploaduri = "http://127.0.0.1:6098"`
`57`	`57`	`owner = "cloudflare"`
`58`	`58`	`repo = "pint"`
`59`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ ci {`
`46`	`46`	`repository {`
`47`	`47`	`github {`
`48`	`48`	`baseuri = "http://127.0.0.1:6118"`
`49`		`- uploaduri = "http://127.0.0.1:6118"`
	`49`	`+ uploaduri = "http://127.0.0.1:6118"`
`50`	`50`	`owner = "cloudflare"`
`51`	`51`	`repo = "pint"`
`52`	`52`	`}`
Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,8 @@ owners {`
`65`	`65`	`}`
`66`	`66`	`ci {`
`67`	`67`	`baseBranch = "main"`
	`68`	`+}`
	`69`	`+parser {`
`68`	`70`	`include = [".+.yml"]`
`69`	`71`	`}`
`70`	`72`	`repository {`