Skip to content

Commit

Permalink
Add proper exit codes. Recognize: cfg error, closing due to SIGINT or…
Browse files Browse the repository at this point in the history
… SIGTERM, check error
  • Loading branch information
mszostok committed Mar 24, 2019
1 parent ac640c6 commit 3d45ad2
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 27 deletions.
13 changes: 12 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,27 @@ The Codeowners Validator project validates the GitHub [CODEOWNERS](https://help.
Use the following environment variables to configure the application:

| Name | Required | Default | Description |
|-----|---------|--------|------------|
|-----|:---------:|:--------:|:------------|
| **REPOSITORY_PATH** | Yes | - | The repository path to your repository on your local machine. |
| **GITHUB_ACCESS_TOKEN** | No | - | The GitHub access token. Instruction for creating token can be found [here](https://help.github.com/articles/creating-a-personal-access-token-for-the-command-line/#creating-a-token). If not provided then validating owners functionality could not work properly, e.g. you can reach the API calls quota or if you are setting GitHub Enterprise base URL then an unauthorized error can occur. |
| **GITHUB_BASE_URL** | No | https://api.github.com/ | The GitHub base URL for API requests. Defaults to the public GitHub API, but can be set to a domain endpoint to use with GitHub Enterprise. |
| **GITHUB_UPLOAD_URL** | No | https://uploads.github.com/ | The GitHub upload URL for uploading files. <br> <br>It is taken into account only when the `GITHUB_BASE_URL` is also set. If only the `GITHUB_BASE_URL` is provided then this parameter defaults to the `GITHUB_BASE_URL` value. |
| **CHECK_FAILURE_LEVEL** | No | `warning` | Defines the level on which application should treat check issue as a failure. Defaults to `warning`, which treats both `error` and `warning` as failure and exits with error code 2. Possible values are: `error` and `warning`. |
| **VALID_OWNER_CHECKER_ORGANIZATION_NAME** | Yes | - | The organization name where the repository is created. Used to check if GitHub owner is in the given organization. |


![usage](./docs/assets/usage.png)

### Exit status codes

Application exits with different statuses codes which allows you to easily distinguish error category.

| Code | Description |
|:-----:|:------------|
| **1** | The application startup failed due to wrong configuration or internal error. |
| **2** | The application was closed because the OS sends termination signal (SIGINT or SIGTERM). |
| **3** | The CODEOWNERS validation failed - executed checks found some issues. |

## Roadmap

_Sorted with priority. First - most important._
Expand Down
17 changes: 17 additions & 0 deletions internal/check/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package check

import (
"context"
"fmt"
"strings"

"github.com/mszostok/codeowners-validator/pkg/codeowners"
)
Expand Down Expand Up @@ -75,3 +77,18 @@ func (s SeverityType) String() string {
return ""
}
}

// Unmarshal provides custom parsing of severity type.
// Implements envconfig.Unmarshal interface.
func (s *SeverityType) Unmarshal(in string) error {
switch strings.ToLower(in) {
case "error", "err":
*s = Error
case "warning", "warn":
*s = Warning
default:
return fmt.Errorf("not a valid severity type: %q", in)
}

return nil
}
2 changes: 1 addition & 1 deletion internal/printer/tty.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (TTYPrinter) severityPrintfFunc(severity check.SeverityType) func(format st
return p.PrintfFunc()
}

func (TTYPrinter) PrintSummary(allCheck uint64, failedChecks uint64) {
func (TTYPrinter) PrintSummary(allCheck, failedChecks int) {
failures := "no"
if failedChecks > 0 {
failures = fmt.Sprintf("%d", failedChecks)
Expand Down
79 changes: 58 additions & 21 deletions internal/runner/runner_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,68 +6,105 @@ import (
"time"

"github.com/mszostok/codeowners-validator/internal/check"
"github.com/mszostok/codeowners-validator/internal/printer"
"github.com/mszostok/codeowners-validator/pkg/codeowners"

"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

const (
// MaxUint defines the max unsigned int value.
MaxUint = ^uint(0)
// MaxInt defines the max signed int value.
MaxInt = int(MaxUint >> 1)
)

// Printer prints the checks results
type Printer interface {
PrintCheckResult(checkName string, duration time.Duration, checkOut check.Output)
PrintSummary(allCheck uint64, failedChecks uint64)
PrintSummary(allCheck int, failedChecks int)
}

// CheckRunner runs all registered checks in parallel.
// Needs to be initialized via NewCheckRunner func.
type CheckRunner struct {
log logrus.FieldLogger
checks []check.Checker
repoPath string
c []codeowners.Entry
printer Printer
log logrus.FieldLogger
codeowners []codeowners.Entry
repoPath string
treatedAsFailure check.SeverityType
checks []check.Checker
printer Printer
allFoundIssues map[check.SeverityType]uint32
notPassedChecksCnt int
}

// NewCheckRunner is a constructor for CheckRunner
func NewCheckRunner(log logrus.FieldLogger, printer Printer, c []codeowners.Entry, repoPath string, checks ...check.Checker) *CheckRunner {
func NewCheckRunner(log logrus.FieldLogger, co []codeowners.Entry, repoPath string, treatedAsFailure check.SeverityType, checks ...check.Checker) *CheckRunner {
return &CheckRunner{
log: log.WithField("service", "check:runner"),
checks: checks,
c: c,
printer: printer,
repoPath: repoPath,
log: log.WithField("service", "check:runner"),
repoPath: repoPath,
treatedAsFailure: treatedAsFailure,
codeowners: co,
checks: checks,

printer: &printer.TTYPrinter{},
allFoundIssues: map[check.SeverityType]uint32{},
}
}

// Run executes given test in a loop with given throttle
func (r *CheckRunner) Run(ctx context.Context) {
// timeout per check?

wg := sync.WaitGroup{}

// TODO(mszostok): timeout per check?
wg.Add(len(r.checks))
failedCnt := uint64(0)
for _, c := range r.checks {
go func(c check.Checker) {
defer wg.Done()
startTime := time.Now()
out, err := c.Check(ctx, check.Input{
CodeownerEntries: r.c,
CodeownerEntries: r.codeowners,
RepoDir: r.repoPath,
})
if err != nil {
// TODO(mszostok): add err handling (logging it internally is not enough)
r.log.Errorf(errors.Wrapf(err, "while executing checker %s", c.Name()).Error())
return
}

if len(out.Issues) != 0 {
failedCnt++
}
r.collectMetrics(out)

// TODO(mszostok): concurrency support (lock per print)
r.printer.PrintCheckResult(c.Name(), time.Since(startTime), out)
}(c)
}

wg.Wait()

r.printer.PrintSummary(uint64(len(r.checks)), failedCnt)
r.printer.PrintSummary(len(r.checks), r.notPassedChecksCnt)
}

func (r *CheckRunner) ShouldExitWithCheckFailure() bool {
higherOccurredIssue := check.SeverityType(MaxInt)
for key := range r.allFoundIssues {
if higherOccurredIssue > key {
higherOccurredIssue = key
}
}

if higherOccurredIssue <= r.treatedAsFailure {
return true
}

return false
}

func (r *CheckRunner) collectMetrics(checkOut check.Output) {
for _, i := range checkOut.Issues {
r.allFoundIssues[i.Severity] += 1
}

if len(checkOut.Issues) > 0 {
r.notPassedChecksCnt++
}
}
14 changes: 11 additions & 3 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"syscall"

"github.com/mszostok/codeowners-validator/internal/check"
"github.com/mszostok/codeowners-validator/internal/printer"
"github.com/mszostok/codeowners-validator/internal/runner"
"github.com/mszostok/codeowners-validator/pkg/codeowners"
"github.com/mszostok/codeowners-validator/pkg/url"
Expand All @@ -28,6 +27,7 @@ type Config struct {
BaseURL string `envconfig:"optional"`
UploadURL string `envconfig:"optional"`
}
CheckFailureLevel check.SeverityType `envconfig:"default=warning"`
ValidOwnerChecker check.ValidOwnerCheckerConfig
}

Expand Down Expand Up @@ -57,7 +57,7 @@ func main() {
codeownersEntries, err := codeowners.NewFromPath(cfg.RepositoryPath)
fatalOnError(err)

// gather checks
// aggregates checks
checks := []check.Checker{
check.NewFileExist(),
check.NewValidOwner(cfg.ValidOwnerChecker, ghClient),
Expand All @@ -67,8 +67,16 @@ func main() {
absRepoPath, err := filepath.Abs(cfg.RepositoryPath)
fatalOnError(err)

checkRunner := runner.NewCheckRunner(log, &printer.TTYPrinter{}, codeownersEntries, absRepoPath, checks...)
checkRunner := runner.NewCheckRunner(log, codeownersEntries, absRepoPath, cfg.CheckFailureLevel, checks...)
checkRunner.Run(ctx)

if ctx.Err() != nil {
log.Error("Application was interrupted by operating system")
os.Exit(2)
}
if checkRunner.ShouldExitWithCheckFailure() {
os.Exit(3)
}
}

func newGithubClient(cfg Config, httpClient *http.Client) (ghClient *github.Client, err error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/codeowners/owners.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func parseCodeowners(r io.Reader) []Entry {
continue
}

if strings.HasPrefix(fields[0], "#"){ // comment
if strings.HasPrefix(fields[0], "#") { // comment
continue
}

Expand Down

0 comments on commit 3d45ad2

Please sign in to comment.