diff --git a/.github/pint/pint.hcl b/.github/pint/pint.hcl index bee0f6a6..3506f5a9 100644 --- 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 index dd579af8..dfa7819f 100644 --- 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 index 32a1d961..06f3e14a 100644 --- 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 index 37ca59cc..19aae9d7 100644 --- 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 index 41fe90a8..c32f26dd 100644 --- 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 index d131ff65..7c172844 100644 --- 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 index 3677d003..1827196a 100644 --- 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 index 57aedf48..aadc9d04 100644 --- 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 index e290a69d..6b5256a1 100644 --- 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 index 013672ff..9a6652db 100644 --- 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 index 14239204..a6c96f9b 100644 --- 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 index 5d9b48a6..4d41292b 100644 --- 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 index b219250a..1a26319c 100644 --- 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 index c9873d0f..ec962482 100644 --- 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 new file mode 100644 index 00000000..cf1546d3 --- /dev/null +++ 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 new file mode 100644 index 00000000..a759ebd0 --- /dev/null +++ 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 new file mode 100644 index 00000000..701fda72 --- /dev/null +++ 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 index 0ba7f5c1..cfd58a55 100644 --- 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 index 2d7f7244..7094093c 100644 --- 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 index 5c9cde2d..2e9cb580 100644 --- 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 index e151c838..2a2ea333 100644 --- 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 index a3e4779f..346b2eeb 100644 --- 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 index acf8f26e..a620401e 100644 --- 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 index e245c28a..2bab6424 100644 --- a/internal/config/ci_test.go +++ b/internal/config/ci_test.go @@ -15,20 +15,6 @@ func TestCISettings(t *testing.T) { } testCases := []testCaseT{ - { - conf: CI{ - MaxCommits: 1, - BaseBranch: "main", - Include: []string{"foo/.+"}, - }, - }, - { - conf: CI{ - MaxCommits: 1, - BaseBranch: "main", - Exclude: []string{"foo/.+"}, - }, - }, { conf: CI{ MaxCommits: -5, @@ -41,20 +27,6 @@ func TestCISettings(t *testing.T) { }, err: errors.New("maxCommits cannot be <= 0"), }, - { - conf: CI{ - MaxCommits: 20, - Include: []string{".+++"}, - }, - err: errors.New("error parsing regexp: invalid nested repetition operator: `++`"), - }, - { - conf: CI{ - MaxCommits: 20, - Exclude: []string{".+++"}, - }, - err: errors.New("error parsing regexp: invalid nested repetition operator: `++`"), - }, } for _, tc := range testCases { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ab6b278b..cb39a4b1 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1971,7 +1971,7 @@ func TestConfigErrors(t *testing.T) { err: "maxCommits cannot be <= 0", }, { - config: `ci {include = [".+", ".+++"]}`, + config: `parser {include = [".+", ".+++"]}`, err: "error parsing regexp: invalid nested repetition operator: `++`", }, { diff --git a/internal/config/owners.go b/internal/config/owners.go index accba30e..fafa090d 100644 --- a/internal/config/owners.go +++ b/internal/config/owners.go @@ -23,8 +23,6 @@ func (o Owners) CompileAllowed() (r []*regexp.Regexp) { r = append(r, regexp.MustCompile(".*")) return r } - for _, pattern := range o.Allowed { - r = append(r, regexp.MustCompile("^"+pattern+"$")) - } + r = append(r, MustCompileRegexes(o.Allowed...)...) return r } diff --git a/internal/config/parser.go b/internal/config/parser.go index 5d190055..d8ce70e6 100644 --- a/internal/config/parser.go +++ b/internal/config/parser.go @@ -6,6 +6,8 @@ import ( type Parser struct { Relaxed []string `hcl:"relaxed,optional" json:"relaxed,omitempty"` + Include []string `hcl:"include,optional" json:"include,omitempty"` + Exclude []string `hcl:"exclude,optional" json:"exclude,omitempty"` } func (p Parser) validate() error { @@ -15,12 +17,16 @@ func (p Parser) validate() error { return err } } - return nil -} + for _, path := range p.Include { + if _, err := regexp.Compile(path); err != nil { + return err + } + } -func (p Parser) CompileRelaxed() (r []*regexp.Regexp) { - for _, pattern := range p.Relaxed { - r = append(r, regexp.MustCompile("^"+pattern+"$")) + for _, path := range p.Exclude { + if _, err := regexp.Compile(path); err != nil { + return err + } } - return r + return nil } diff --git a/internal/config/parser_test.go b/internal/config/parser_test.go index 4425e08b..b3cce899 100644 --- a/internal/config/parser_test.go +++ b/internal/config/parser_test.go @@ -29,6 +29,18 @@ func TestParserSettings(t *testing.T) { }, err: errors.New("error parsing regexp: invalid nested repetition operator: `++`"), }, + { + conf: Parser{ + Include: []string{"(.+++)"}, + }, + err: errors.New("error parsing regexp: invalid nested repetition operator: `++`"), + }, + { + conf: Parser{ + Exclude: []string{"(.+++)"}, + }, + err: errors.New("error parsing regexp: invalid nested repetition operator: `++`"), + }, } for _, tc := range testCases { diff --git a/internal/config/rule.go b/internal/config/rule.go index 7a29a0b0..c46c9c55 100644 --- a/internal/config/rule.go +++ b/internal/config/rule.go @@ -353,3 +353,10 @@ func isEnabled(enabledChecks, disabledChecks []string, rule parser.Rule, name st func strictRegex(s string) *regexp.Regexp { return regexp.MustCompile("^" + s + "$") } + +func MustCompileRegexes(l ...string) (r []*regexp.Regexp) { + for _, pattern := range l { + r = append(r, strictRegex(pattern)) + } + return r +} diff --git a/internal/git/filter.go b/internal/git/filter.go index 414a9a50..9dad8cb6 100644 --- a/internal/git/filter.go +++ b/internal/git/filter.go @@ -1,6 +1,9 @@ package git -import "regexp" +import ( + "log/slog" + "regexp" +) func NewPathFilter(include, exclude, relaxed []*regexp.Regexp) PathFilter { return PathFilter{ @@ -23,17 +26,19 @@ func (pf PathFilter) IsPathAllowed(path string) bool { for _, pattern := range pf.exclude { if pattern.MatchString(path) { + slog.Debug("File path is in the exclude list", slog.String("path", path), slog.Any("exclude", pf.exclude)) return false } } for _, pattern := range pf.include { if pattern.MatchString(path) { + slog.Debug("File path is in the include list", slog.String("path", path), slog.Any("include", pf.include)) return true } } - return false + return len(pf.include) == 0 } func (pf PathFilter) IsRelaxed(path string) bool {