Add file granularity and CODEOWNERS support (#81)

* wip Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * add project and subproject rules; groups are skipped by default if project is subproject Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * parse codeowners Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * support directories in codeowners Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * handle wildcards Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * use project exploration and applyonsubprojects Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * add also subproject without entry in codeowner Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * do not fail when codeowners is not there Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * remove useless rules Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * cleanup Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * subprojects pattern Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * rename subproject to filepath Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * file block in the config Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * validation Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * fixed tests, removed debug lines, changed some lines from info to debug Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * lint Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * use codeowners lib, add mapping Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * fix tests Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * linter Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * lint Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * remove hardcoded mapping Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * revert config example Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * linter Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * fix test Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * tidy Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * codeowners mapping as top level config Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * linter Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * add one more validation, update readme Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * file granularity example Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * linter and readme Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * kebab case Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> * rename owner map Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> --------- Signed-off-by: Stefano Celentano <[email protected]> Signed-off-by: Stefano Celentano <[email protected]> Co-authored-by: Mathieu Corbin <[email protected]> Co-authored-by: Stefano Celentano <[email protected]>
qonto · Aug 20, 2024 · 1c4c709 · 1c4c709
1 parent bb32cb3
commit 1c4c709
Show file tree

Hide file tree

Showing 20 changed files with 344 additions and 39 deletions.
diff --git 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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/pkg/codeowners/codeowners.go 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
+}