diff --git a/README.md b/README.md index 9243759..0b53c95 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,23 @@ Checks supports 2 different operators to evaluate `rules`: `and` and `or`. Finally, we configure `groups`: groups contains checks to execute on a project when a condition is true. In this example, the group named `golang` will only be executed on projects where `golang-project` is valid (so, when the `go.mod` file exists). Thanks to the `when` condition, we can specify which check should be executed on which project. +Normally groups and checks within it are executed at project level. It is possible to configure a group to be executed at file level. In order to so, you have to set the `apply-to-files` to true under the `files` block as show below: + +```yaml + - name: paseto-group + files: + apply-to-files: true + files-pattern: "controller.rb" + checks: + - paseto-migration-check + when: + - ruby-controller + - ruby-projects +``` + +When `apply-to-files` is set to true all the files of a project are evaluated. But if you want to evaluate only some files, you can do so by setting `apply-to-files` to true and `files-pattern` to the pattern of the files you want to evaluate. The configuration example above will execute the `paseto-migration-check` check on all the files matching the `files-pattern` pattern. Therefore there will be one check result per file. +Only for certain rules makes sense to apply them at file level. Check the `doc/rules` for more information. For assigning ownership to a file, we support [CODEOWNERS](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners) file parsing. If the project has a CODEOWNERS file, the ownership will be extracted and added to the file labels. If it's not the case, the owner will be extracted as usual according to the provider. + If you navigate in a Golang project (for example, the Standards Insights repository itself) and run `standards-insights run --config config.yaml`, you will get: ``` diff --git a/cmd/server.go b/cmd/server.go index bde235a..223b98a 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -64,7 +64,7 @@ func serverCmd(configPath, logLevel, logFormat *string) *cobra.Command { } daemon, err := daemon.New(checker, providers, projectMetrics, logger, (time.Duration(config.Interval) * time.Second), git, registry) exit(err) - daemon.Start() + daemon.Start(*configPath) go func() { for sig := range signals { diff --git a/config.example.yaml b/config.example.yaml index 4a140ff..ce89d8a 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -29,8 +29,6 @@ providers: url: https://github.com/qonto/standards-insights.git branch: "main" path: "/tmp/projects/standards-insights" - labels: - team: sre # The ArgoCD provider will retrieve projects from ArgoCD and use the Spec.Source.RepoURL url to fetch the # projects. ArgoCD applications labels will automatically be added as labels to the project. argocd: @@ -71,8 +69,22 @@ groups: - go-main when: - golang-projects + - name: files-granularity-example + files: + apply-to-files: true + checks: + - files-granularity-example + when: + - golang-projects checks: + - name: files-granularity-example + labels: + category: upgrade + severity: critical + owner: backend + rules: + - files-granularity-example - name: go-version-latest labels: category: upgrade @@ -116,7 +128,11 @@ rules: recursive: true pattern: "NewGrepRule" match: true - + - name: files-granularity-example + grep: + - recursive: false + pattern: "package" + match: true - name: allowed-projects project: - names: @@ -137,7 +153,7 @@ rules: match: true - name: excluding-all-labels project: - - names: + - labels: team: backend language: golang match: false \ No newline at end of file diff --git a/config/config.go b/config/config.go index f4bb9b1..9922cf7 100644 --- a/config/config.go +++ b/config/config.go @@ -8,14 +8,15 @@ import ( ) type Config struct { - HTTP HTTPConfig `validate:"omitempty"` - Providers ProvidersConfig `validate:"omitempty"` - Groups []Group `validate:"dive"` - Checks []Check `validate:"dive"` - Rules []Rule `validate:"dive"` - Labels []string `validate:"dive"` - Interval int - Git GitConfig + HTTP HTTPConfig `validate:"omitempty"` + Providers ProvidersConfig `validate:"omitempty"` + Groups []Group `validate:"dive"` + Checks []Check `validate:"dive"` + Rules []Rule `validate:"dive"` + Labels []string `validate:"dive"` + Interval int + Git GitConfig + CodeownerOverride map[string]string `yaml:"codeowner-override"` } type ProvidersConfig struct { @@ -55,7 +56,7 @@ type FileRule struct { } type GrepRule struct { - Path string `validate:"required"` + Path string Pattern string `validate:"required"` Include string ExtendedRegexp bool `yaml:"extended-regexp"` @@ -83,11 +84,17 @@ func (c Check) IsAND() bool { } type Group struct { - Name string `validate:"required"` - Checks []string `validate:"required,min=1"` + Name string `validate:"required"` + Files FilesConfig `yaml:"files"` + Checks []string `validate:"required,min=1"` When []string } +type FilesConfig struct { + ApplyToFiles bool `yaml:"apply-to-files"` + FilesPattern string `yaml:"files-pattern"` +} + func New(path string) (*Config, []byte, error) { var config Config diff --git a/config/validate.go b/config/validate.go index ba917fc..ca3117c 100644 --- a/config/validate.go +++ b/config/validate.go @@ -34,7 +34,60 @@ func validate(config Config) error { } } } + if group.Files.ApplyToFiles { + for _, groupCheck := range group.Checks { + check, ok := findCheckByName(config.Checks, groupCheck) + if !ok { + continue + } + for _, checkRule := range check.Rules { + rule, ok := findRuleByName(config.Rules, checkRule) + if !ok { + continue + } + for _, grepRule := range rule.Grep { + if grepRule.Path != "" { + return fmt.Errorf("grep rule %s in check %s has path defined but this is not allowed when the group applies to files", rule.Name, check.Name) + } + } + if rule.Files != nil { + return fmt.Errorf("file rule %s in check %s is not allowed when the group applies to files", rule.Name, check.Name) + } + } + } + + for _, groupWhen := range group.When { + rule, ok := findRuleByName(config.Rules, groupWhen) + if !ok { + return fmt.Errorf("when clause rule %s not defined in group %s", groupWhen, group.Name) + } + for _, grepRule := range rule.Grep { + if grepRule.Path != "" { + return fmt.Errorf("grep rule %s in when clause rule %s has path defined but this is not allowed when the group applies to files", rule.Name, groupWhen) + } + } + } + } } validate := validator.New() return validate.Struct(config) } + +// Helper functions to find check and rule by name +func findCheckByName(checks []Check, name string) (Check, bool) { + for _, check := range checks { + if check.Name == name { + return check, true + } + } + return Check{}, false +} + +func findRuleByName(rules []Rule, name string) (Rule, bool) { + for _, rule := range rules { + if rule.Name == name { + return rule, true + } + } + return Rule{}, false +} diff --git a/doc/rules/files.md b/doc/rules/files.md index 9836e5b..03377b5 100644 --- a/doc/rules/files.md +++ b/doc/rules/files.md @@ -14,7 +14,7 @@ rules: Available options are: -- `path`: the file path to check (mandatory). +- `path`: the file path to check (mandatory). Notice: since the path is mandatory, it does not make sense to use this rule within a check used in a group that is applied to files. - `exists`: should the file exists or not. - `contains`: a regular expression that will be checked against the file content. The rule will be successful if the regular expression find a match. - `not-contains`: a regular expression that will be checked against the file content. The rule will be successful if the regular expression doesn't find a match. diff --git a/doc/rules/grep.md b/doc/rules/grep.md index 582ecc7..0432cd2 100644 --- a/doc/rules/grep.md +++ b/doc/rules/grep.md @@ -17,7 +17,7 @@ rules: Available options are: -- `path`: the file or directory to check (mandatory) +- `path`: the file or directory to check. The path is not mandatory when the rule is used within a check of a group that is applied to files. In this case, the rule will be applied to all the project files that match the pattern. - `pattern`: the string pattern to search for (mandatory) - `recursive`: recursively search subdirectories listed (grep `-r` option) - `match`: if set to true, the module will be successful if the pattern is found. Default to false, where the module will be successful if the pattern is **not** found diff --git a/go.mod b/go.mod index 7138add..5af5237 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/go-chi/chi/v5 v5.1.0 github.com/go-git/go-git/v5 v5.12.0 github.com/go-playground/validator/v10 v10.22.0 + github.com/hairyhenderson/go-codeowners v0.5.0 github.com/prometheus/client_golang v1.19.1 github.com/spf13/cobra v1.8.1 github.com/stretchr/testify v1.9.0 diff --git a/go.sum b/go.sum index 94cda91..6ed1bf2 100644 --- a/go.sum +++ b/go.sum @@ -59,6 +59,8 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= +github.com/hairyhenderson/go-codeowners v0.5.0 h1:dpQB+hVHiRc2VVvc2BHxkuM+tmu9Qej/as3apqUbsWc= +github.com/hairyhenderson/go-codeowners v0.5.0/go.mod h1:R3uW1OQXEj2Gu6/OvZ7bt6hr0qdkLvUWPiqNaWnexpo= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k= diff --git a/internal/metrics/project.go b/internal/metrics/project.go index 04a9dcd..02f9480 100644 --- a/internal/metrics/project.go +++ b/internal/metrics/project.go @@ -12,7 +12,7 @@ type Project struct { } func New(registry *prometheus.Registry, extraLabels []string) (*Project, error) { - labels := append([]string{"name", "project"}, extraLabels...) + labels := append([]string{"name", "project", "filepath"}, extraLabels...) checksGauge := prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "check_result_success", @@ -34,7 +34,7 @@ func (p *Project) Load(results []aggregates.ProjectResult) { for _, project := range results { projectLabels := project.Labels for _, result := range project.CheckResults { - labels := prometheus.Labels{"name": result.Name, "project": project.Name} + labels := prometheus.Labels{"name": result.Name, "project": project.Name, "filepath": project.FilePath} for _, label := range p.extraLabels { labels[label] = "" value, ok := result.Labels[label] diff --git a/pkg/checker/aggregates/types.go b/pkg/checker/aggregates/types.go index 5344fd8..a86dcb0 100644 --- a/pkg/checker/aggregates/types.go +++ b/pkg/checker/aggregates/types.go @@ -11,6 +11,7 @@ type CheckResult struct { type ProjectResult struct { Name string + FilePath string CheckResults []CheckResult Labels map[string]string } diff --git a/pkg/checker/checker.go b/pkg/checker/checker.go index bc7841d..d6990a6 100644 --- a/pkg/checker/checker.go +++ b/pkg/checker/checker.go @@ -3,6 +3,7 @@ package checker import ( "context" "fmt" + "regexp" "log/slog" @@ -37,8 +38,8 @@ func NewChecker(logger *slog.Logger, ruler Ruler, checks []config.Check, groups } func (c *Checker) Run(ctx context.Context, projects []project.Project) []aggregates.ProjectResult { - projectResults := make([]aggregates.ProjectResult, len(projects)) - for i, project := range projects { + projectResults := make([]aggregates.ProjectResult, 0) + for _, project := range projects { c.logger.Info("checking project " + project.Name) projectResult := aggregates.ProjectResult{ Labels: project.Labels, @@ -52,8 +53,41 @@ func (c *Checker) Run(ctx context.Context, projects []project.Project) []aggrega } checkResults := c.executeGroup(ctx, group, project) projectResult.CheckResults = append(projectResult.CheckResults, checkResults...) + + if group.Files.ApplyToFiles { + // use group pattern to filter sub projects + filteredSubProjects := filterSubProjects(project.SubProjects, group.Files.FilesPattern) + // print group name, project name and apply on sub projects + c.logger.Debug(fmt.Sprintf("applying group %s for project %s and sub projects", group.Name, project.Name)) + for _, subProject := range filteredSubProjects { + if c.shouldSkipGroup(ctx, group, subProject) { + c.logger.Debug(fmt.Sprintf("skipping group %s for file %s", group.Name, subProject.FilePath)) + continue + } + subProjectResult := aggregates.ProjectResult{ + Labels: subProject.Labels, + Name: subProject.Name, + FilePath: subProject.FilePath, + CheckResults: []aggregates.CheckResult{}, + } + subProjectCheckResults := c.executeGroup(ctx, group, subProject) + subProjectResult.CheckResults = append(subProjectResult.CheckResults, subProjectCheckResults...) + projectResults = append(projectResults, subProjectResult) + } + } } - projectResults[i] = projectResult + projectResults = append(projectResults, projectResult) } return projectResults } + +func filterSubProjects(projects []project.Project, pattern string) []project.Project { + var filteredProjects []project.Project + re := regexp.MustCompile(pattern) + for _, proj := range projects { + if re.MatchString(proj.FilePath) { + filteredProjects = append(filteredProjects, proj) + } + } + return filteredProjects +} diff --git a/pkg/codeowners/.gitlab/CODEOWNERS b/pkg/codeowners/.gitlab/CODEOWNERS new file mode 100644 index 0000000..9bc2d7c --- /dev/null +++ b/pkg/codeowners/.gitlab/CODEOWNERS @@ -0,0 +1,8 @@ +### team-cft-bookkeeping +[CFT-Bookkeeping] @team-cft-bookkeeping +app/announcers/attachment_announcer.rb @team-cft-bookkeeping + +[cft-sepa] @team-cft-sepa +app/controllers/**/external_transfers_controller.rb @team-cft-sepa +app/controllers/*/mandates_controller.rb @team-cft-sepa +app/services/transfers/ @team-cft-sepa \ No newline at end of file diff --git a/pkg/codeowners/codeowners.go b/pkg/codeowners/codeowners.go new file mode 100644 index 0000000..d22fd58 --- /dev/null +++ b/pkg/codeowners/codeowners.go @@ -0,0 +1,56 @@ +package codeowners + +import ( + "os" + + "github.com/hairyhenderson/go-codeowners" + "github.com/qonto/standards-insights/config" +) + +type Codeowners struct { + codeowners *codeowners.Codeowners + configPath string +} + +func NewCodeowners(path string, configPath string) (*Codeowners, error) { + // open filesystem rooted at current working directory + fsys := os.DirFS(path) + + c, err := codeowners.FromFileWithFS(fsys, "CODEOWNERS") + if err != nil { + return nil, err + } + + return &Codeowners{ + codeowners: c, + configPath: configPath, + }, nil +} + +func (c *Codeowners) GetOwners(path string) (string, bool) { + if c == nil || c.codeowners == nil { + return "", false + } + + owners := c.codeowners.Owners(path) + if len(owners) == 0 { + return "", false + } + + // Attempt to load the configuration + config, _, err := config.New(c.configPath) + var ownerMap map[string]string + if err != nil { + // If config cannot be found, use an empty ownerMap + ownerMap = map[string]string{} + } else { + ownerMap = config.CodeownerOverride + } + + owner := owners[0] + if mappedOwner, found := ownerMap[owner]; found { + return mappedOwner, true + } + + return owner, true +} diff --git a/pkg/codeowners/codeowners_test.go b/pkg/codeowners/codeowners_test.go new file mode 100644 index 0000000..8d8615f --- /dev/null +++ b/pkg/codeowners/codeowners_test.go @@ -0,0 +1,41 @@ +package codeowners + +import ( + "os" + "testing" +) + +func TestCodeowners_FromFile(t *testing.T) { + // Ensure the CODEOWNERS file exists + if _, err := os.Stat(".gitlab/CODEOWNERS"); os.IsNotExist(err) { + t.Fatalf("CODEOWNERS file does not exist") + } + + co, err := NewCodeowners(".", "") + if err != nil { + t.Fatalf("Error creating codeowners: %v", err) + } + + tests := []struct { + path string + expected string + }{ + {"app/announcers/attachment_announcer.rb", "@team-cft-bookkeeping"}, + {"app/controllers/v1/external_transfers_controller.rb", "@team-cft-sepa"}, + {"app/controllers/somepath/v1/external_transfers_controller.rb", "@team-cft-sepa"}, + {"app/controllers/anotherpath/mandates_controller.rb", "@team-cft-sepa"}, + {"app/services/transfers/somefile.rb", "@team-cft-sepa"}, + } + + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + owner, exists := co.GetOwners(tt.path) + if !exists { + t.Errorf("GetOwners() for path %s, expected owner %s, but got none", tt.path, tt.expected) + } + if owner != tt.expected { + t.Errorf("GetOwners() for path %s, expected owner %s, but got %s", tt.path, tt.expected, owner) + } + }) + } +} diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 9240265..9d1cded 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -5,11 +5,13 @@ import ( "fmt" "log/slog" "os" + "path/filepath" "time" "github.com/prometheus/client_golang/prometheus" "github.com/qonto/standards-insights/internal/providers/aggregates" checkeraggregates "github.com/qonto/standards-insights/pkg/checker/aggregates" + "github.com/qonto/standards-insights/pkg/codeowners" "github.com/qonto/standards-insights/pkg/project" ) @@ -91,9 +93,10 @@ func New(checker Checker, }, nil } -func (d *Daemon) tick() { +func (d *Daemon) tick(configPath string) { d.logger.Info("checking projects") projects := []project.Project{} + subProjects := []project.Project{} for _, provider := range d.providers { providerName := provider.Name() ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) @@ -105,16 +108,27 @@ func (d *Daemon) tick() { continue } d.providerRequestsCounter.WithLabelValues(providerName, "success").Inc() - for _, project := range providerProjects { - err = d.cloneOrPull(project) + for _, proj := range providerProjects { + err = d.cloneOrPull(proj) if err != nil { - d.logger.Error(fmt.Sprintf("fail to retrieve project %s: %s", project.Name, err.Error())) + d.logger.Error(fmt.Sprintf("fail to retrieve project %s: %s", proj.Name, err.Error())) d.gitRequestsCounter.WithLabelValues("failure").Inc() continue } d.gitRequestsCounter.WithLabelValues("success").Inc() + + codeowners, err := codeowners.NewCodeowners(proj.Path, configPath) + if err != nil { + d.logger.Warn(fmt.Sprintf("Failed to parse CODEOWNERS for project %s: %s", proj.Name, err.Error())) + } + // Create subprojects based on expanded paths + err = d.exploreProjectFiles(proj.Path, codeowners, proj, &subProjects) + proj.SubProjects = subProjects + if err != nil { + d.logger.Warn(fmt.Sprintf("Failed to explore project files for %s: %s", proj.Name, err.Error())) + } + projects = append(projects, proj) } - projects = append(projects, providerProjects...) } results := d.checker.Run(context.Background(), projects) @@ -122,18 +136,70 @@ func (d *Daemon) tick() { d.logger.Info("projects checked") } -func (d *Daemon) Start() { +func (d *Daemon) exploreProjectFiles(projectPath string, codeowners *codeowners.Codeowners, proj project.Project, projects *[]project.Project) error { + // Use filepath.Walk to explore all files in the project directory + return filepath.Walk(projectPath, func(filePath string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.IsDir() { + relPath, err := filepath.Rel(projectPath, filePath) + if err != nil { + return err + } + // Assign team owner based on CODEOWNERS + team, exists := codeowners.GetOwners(relPath) + if exists { + labels := make(map[string]string) + for k, v := range proj.Labels { + labels[k] = v + } + labels["team"] = team + + subProject := project.Project{ + Name: proj.Name, + URL: proj.URL, + Branch: proj.Branch, + Path: proj.Path, + FilePath: relPath, + Labels: labels, + } + *projects = append(*projects, subProject) + } else { + // Add the path with the same team as the project team + labels := make(map[string]string) + for k, v := range proj.Labels { + labels[k] = v + } + labels["team"] = proj.Labels["team"] // Assuming team is stored in proj.Labels + + subProject := project.Project{ + Name: proj.Name, + URL: proj.URL, + Branch: proj.Branch, + Path: proj.Path, + FilePath: relPath, + Labels: labels, + } + *projects = append(*projects, subProject) + } + } + return nil + }) +} + +func (d *Daemon) Start(configPath string) { d.logger.Info("starting daemon") ticker := time.NewTicker(d.interval) d.ticker = ticker go func() { - d.tick() + d.tick(configPath) for { select { case <-d.done: return case <-ticker.C: - d.tick() + d.tick(configPath) } } }() diff --git a/pkg/project/project.go b/pkg/project/project.go index 0fb5b5c..4dde3d8 100644 --- a/pkg/project/project.go +++ b/pkg/project/project.go @@ -1,9 +1,11 @@ package project type Project struct { - Name string - URL string - Branch string - Path string - Labels map[string]string + Name string + URL string + Branch string + Path string + FilePath string // a path to a file within the project + SubProjects []Project // a list of subprojects within the project + Labels map[string]string } diff --git a/pkg/ruler/builder.go b/pkg/ruler/builder.go index 1e0bf4e..0f759c0 100644 --- a/pkg/ruler/builder.go +++ b/pkg/ruler/builder.go @@ -36,6 +36,7 @@ func newRule(config config.Rule) *rule { value := *config.Simple modules = append(modules, rules.NewSimpleRule(value)) } + result.Modules = modules return result } diff --git a/pkg/ruler/ruler.go b/pkg/ruler/ruler.go index 9757257..bf1d67e 100644 --- a/pkg/ruler/ruler.go +++ b/pkg/ruler/ruler.go @@ -48,7 +48,7 @@ func (r *Ruler) Execute(ctx context.Context, ruleName string, project project.Pr } if len(errorMessages) > 0 { for _, errorMessage := range errorMessages { - r.logger.Error(fmt.Sprintf("error on rule %s for project %s: %s", ruleName, project.Name, errorMessage)) + r.logger.Debug(fmt.Sprintf("error on rule %s for project %s file %s: %s", ruleName, project.Name, project.FilePath, errorMessage)) } return aggregates.RuleResult{ Success: false, diff --git a/pkg/ruler/rules/grep.go b/pkg/ruler/rules/grep.go index 04daa7c..a51200a 100644 --- a/pkg/ruler/rules/grep.go +++ b/pkg/ruler/rules/grep.go @@ -38,7 +38,7 @@ func NewGrepRule(config config.GrepRule) *GrepRule { } func (rule *GrepRule) Do(ctx context.Context, project project.Project) error { - path := filepath.Join(project.Path, rule.Path) + path := filepath.Join(project.Path, project.FilePath, rule.Path) _, err := os.Stat(path) isNotExist := os.IsNotExist(err) if isNotExist && rule.SkipNotFound { @@ -75,7 +75,7 @@ func (rule *GrepRule) Do(ctx context.Context, project project.Project) error { return nil } if exitCode == 1 && rule.Match { - return fmt.Errorf("no match for pattern %s on path %s", rule.Pattern, rule.Path) + return fmt.Errorf("no match for pattern %s on path %s", rule.Pattern, filepath.Join(project.FilePath, rule.Path)) } return fmt.Errorf("failed to execute grep command (error code %d), stderr=%s, error=%w", exitErr.ExitCode(), stdErrBuffer.String(), err) }