Skip to content

Commit

Permalink
Add configurability for the list of ignored owners (#55)
Browse files Browse the repository at this point in the history
  • Loading branch information
Maja Kurcius committed Oct 31, 2020
1 parent 97fb795 commit 4f46df1
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 118 deletions.
41 changes: 21 additions & 20 deletions README.md
Expand Up @@ -62,7 +62,7 @@ Check the [Configuration](#configuration) section for more info on how to enable

## Installation

It's highly recommended to install a fixed version of ` codeowners-validator`. Releases are available on the [releases page](https://github.com/mszostok/codeowners-validator/releases).
It's highly recommended to install a fixed version of `codeowners-validator`. Releases are available on the [releases page](https://github.com/mszostok/codeowners-validator/releases).

#### From Release

Expand Down Expand Up @@ -90,25 +90,7 @@ You can install `codeowners-validator` with `env GO111MODULE=on go get -u github

> NOTE: please use the latest Go to do this, ideally Go 1.15 or greater.
This will put `codeowners-validator` in `$(go env GOPATH)/bin`

## Configuration

Use the following environment variables to configure the application:

| Name | Default | Description |
|-----|:--------|:------------|
| <tt>REPOSITORY_PATH</tt> <b>*</b> | | Path to your repository on your local machine. |
| <tt>GITHUB_ACCESS_TOKEN</tt>| | GitHub access token. Instruction for creating a token can be found [here](./docs/gh-token.md). If not provided, the owners validating functionality may not work properly. For example, you may reach the API calls quota or, if you are setting GitHub Enterprise base URL, an unauthorized error may occur. |
| <tt>GITHUB_BASE_URL</tt>| https://api.github.com/ | 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. |
| <tt>GITHUB_UPLOAD_URL</tt> | https://uploads.github.com/ | GitHub upload URL for uploading files. <br> <br>It is taken into account only when `GITHUB_BASE_URL` is also set. If only `GITHUB_BASE_URL` is provided, this parameter defaults to the `GITHUB_BASE_URL` value. |
| <tt>CHECKS</tt>| - | List of checks to be executed. By default, all checks are executed. Possible values: `files`,`owners`,`duppatterns`,`syntax`. |
| <tt>EXPERIMENTAL_CHECKS</tt> | - | The comma-separated list of experimental checks that should be executed. By default, all experimental checks are turned off. Possible values: `notowned`.|
| <tt>CHECK_FAILURE_LEVEL</tt> | `warning` | Defines the level on which the application should treat check issues as failures. Defaults to `warning`, which treats both errors and warnings as failures, and exits with error code 3. Possible values are `error` and `warning`. |
| <tt>OWNER_CHECKER_REPOSITORY</tt> <b>*</b>| | The owner and repository name separated by slash. For example, gh-codeowners/codeowners-samples. Used to check if GitHub owner is in the given organization. |
| <tt>NOT_OWNED_CHECKER_SKIP_PATTERNS</tt>| - | The comma-separated list of patterns that should be ignored by `not-owned-checker`. For example, you can specify `*` and as a result, the `*` pattern from the **CODEOWNERS** file will be ignored and files owned by this pattern will be reported as unowned unless a later specific pattern will match that path. It's useful because often we have default owners entry at the begging of the CODOEWNERS file, e.g. `* @global-owner1 @global-owner2` |

<b>*</b> - Required
This will put `codeowners-validator` in `$(go env GOPATH)/bin`.

## Checks

Expand All @@ -131,6 +113,25 @@ To enable experimental check set `EXPERIMENTAL_CHECKS=notowned` environment vari

Check the [Configuration](#configuration) section for more info on how to enable and configure given checks.

## Configuration

Use the following environment variables to configure the application:

| Name | Default | Description |
|-----|:--------|:------------|
| <tt>REPOSITORY_PATH</tt> <b>*</b> | | Path to your repository on your local machine. |
| <tt>GITHUB_ACCESS_TOKEN</tt>| | GitHub access token. Instruction for creating a token can be found [here](./docs/gh-token.md). If not provided, the owners validating functionality may not work properly. For example, you may reach the API calls quota or, if you are setting GitHub Enterprise base URL, an unauthorized error may occur. |
| <tt>GITHUB_BASE_URL</tt>| https://api.github.com/ | 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. |
| <tt>GITHUB_UPLOAD_URL</tt> | https://uploads.github.com/ | GitHub upload URL for uploading files. <br> <br>It is taken into account only when `GITHUB_BASE_URL` is also set. If only `GITHUB_BASE_URL` is provided, this parameter defaults to the `GITHUB_BASE_URL` value. |
| <tt>CHECKS</tt>| - | List of checks to be executed. By default, all checks are executed. Possible values: `files`,`owners`,`duppatterns`,`syntax`. |
| <tt>EXPERIMENTAL_CHECKS</tt> | - | The comma-separated list of experimental checks that should be executed. By default, all experimental checks are turned off. Possible values: `notowned`.|
| <tt>CHECK_FAILURE_LEVEL</tt> | `warning` | Defines the level on which the application should treat check issues as failures. Defaults to `warning`, which treats both errors and warnings as failures, and exits with error code 3. Possible values are `error` and `warning`. |
| <tt>OWNER_CHECKER_REPOSITORY</tt> <b>*</b>| | The owner and repository name separated by slash. For example, gh-codeowners/codeowners-samples. Used to check if GitHub owner is in the given organization. |
| <tt>OWNER_CHECKER_IGNORED_OWNERS</tt> | `@ghost`| The comma-separated list of owners that should not be validated. Example: `"@owner1,@owner2,@org/team1,example@email.com"`. |
| <tt>NOT_OWNED_CHECKER_SKIP_PATTERNS</tt>| - | The comma-separated list of patterns that should be ignored by `not-owned-checker`. For example, you can specify `*` and as a result, the `*` pattern from the **CODEOWNERS** file will be ignored and files owned by this pattern will be reported as unowned unless a later specific pattern will match that path. It's useful because often we have default owners entry at the begging of the CODOEWNERS file, e.g. `* @global-owner1 @global-owner2` |

<b>*</b> - Required

#### Exit status codes

Application exits with different status codes which allow you to easily distinguish between error categories.
Expand Down
4 changes: 4 additions & 0 deletions action.yml
Expand Up @@ -43,6 +43,10 @@ inputs:
required: false
default: "${{ github.repository }}"

owner_checker_ignored_owners:
description: "The comma-separated list of owners that should not be validated. Example: @owner1,@owner2,@org/team1,example@email.com."
required: false

runs:
using: 'docker'
image: 'docker://mszostok/codeowners-validator:v0.5.0'
Expand Down
4 changes: 2 additions & 2 deletions internal/check/duplicated_pattern_test.go
Expand Up @@ -43,7 +43,7 @@ func TestDuplicatedPattern(t *testing.T) {
},
},
"Should not report any issues with correct CODEOWNERS file": {
codeownersInput: validCODEOWNERS,
codeownersInput: check.FixtureValidCODEOWNERS,
expectedIssues: nil,
},
}
Expand All @@ -54,7 +54,7 @@ func TestDuplicatedPattern(t *testing.T) {
sut := check.NewDuplicatedPattern()

// when
out, err := sut.Check(context.TODO(), loadInput(tc.codeownersInput))
out, err := sut.Check(context.TODO(), check.LoadInput(tc.codeownersInput))

// then
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions internal/check/file_exists_test.go
Expand Up @@ -169,7 +169,7 @@ func TestFileExists(t *testing.T) {
defer cancel()

// when
in := loadInput(tc.codeownersInput)
in := check.LoadInput(tc.codeownersInput)
in.RepoDir = tmp
out, err := fchecker.Check(ctx, in)

Expand All @@ -191,7 +191,7 @@ func TestFileExistCheckFileSystemFailure(t *testing.T) {
err = os.MkdirAll(filepath.Join(tmpdir, "foo"), 0222)
require.NoError(t, err)

in := loadInput("* @pico")
in := check.LoadInput("* @pico")
in.RepoDir = tmpdir

ctx, cancel := context.WithTimeout(context.Background(), 30*time.Millisecond)
Expand Down
9 changes: 4 additions & 5 deletions internal/check/helpers_test.go
@@ -1,13 +1,12 @@
package check_test
package check

import (
"strings"

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

var validCODEOWNERS = `
var FixtureValidCODEOWNERS = `
# These owners will be the default owners for everything
* @global-owner1 @global-owner2
Expand All @@ -21,10 +20,10 @@ var validCODEOWNERS = `
/script m.t@g.com
`

func loadInput(in string) check.Input {
func LoadInput(in string) Input {
r := strings.NewReader(in)

return check.Input{
return Input{
CodeownersEntries: codeowners.ParseCodeowners(r),
}
}
2 changes: 1 addition & 1 deletion internal/check/package_test.go
Expand Up @@ -33,7 +33,7 @@ func TestRespectingCanceledContext(t *testing.T) {
cancel()

// when
out, err := sut.Check(ctx, loadInput(validCODEOWNERS))
out, err := sut.Check(ctx, check.LoadInput(check.FixtureValidCODEOWNERS))

// then
assert.True(t, errors.Is(err, context.Canceled))
Expand Down
50 changes: 41 additions & 9 deletions internal/check/valid_owner.go
Expand Up @@ -3,6 +3,7 @@ package check
import (
"context"
"net/http"
"net/mail"
"strings"

"github.com/mszostok/codeowners-validator/internal/ctxutil"
Expand All @@ -13,6 +14,11 @@ import (

type ValidOwnerConfig struct {
Repository string
// IgnoredOwners contains a list of owners that should not be validated.
// Defaults to @ghost.
// More info about the @ghost user: https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-your-github-user-account/deleting-your-user-account
// Tip on how @ghost can be used: https://github.community/t5/How-to-use-Git-and-GitHub/CODEOWNERS-file-with-a-NOT-file-type-condition/m-p/31013/highlight/true#M8523
IgnoredOwners []string `envconfig:"default=@ghost"`
}

// ValidOwner validates each owner
Expand All @@ -22,6 +28,7 @@ type ValidOwner struct {
orgName string
orgTeams []*github.Team
orgRepoName string
ignOwners map[string]struct{}
}

// NewValidOwner returns new instance of the ValidOwner
Expand All @@ -31,10 +38,16 @@ func NewValidOwner(cfg ValidOwnerConfig, ghClient *github.Client) (*ValidOwner,
return nil, errors.Errorf("Wrong repository name. Expected pattern 'owner/repository', got '%s'", cfg.Repository)
}

ignOwners := map[string]struct{}{}
for _, n := range cfg.IgnoredOwners {
ignOwners[n] = struct{}{}
}

return &ValidOwner{
ghClient: ghClient,
orgName: split[0],
orgRepoName: split[1],
ignOwners: ignOwners,
}, nil
}

Expand All @@ -61,6 +74,10 @@ func (v *ValidOwner) Check(ctx context.Context, in Input) (Output, error) {
return Output{}, ctx.Err()
}

if v.isIgnoredOwner(ownerName) {
continue
}

if _, alreadyChecked := checkedOwners[ownerName]; alreadyChecked {
continue
}
Expand All @@ -79,13 +96,34 @@ func (v *ValidOwner) Check(ctx context.Context, in Input) (Output, error) {
return bldr.Output(), nil
}

func isEmailAddress(s string) bool {
_, err := mail.ParseAddress(s)
return err == nil
}

func isGithubTeam(s string) bool {
hasPrefix := strings.HasPrefix(s, "@")
containsSlash := strings.Contains(s, "/")
split := strings.SplitN(s, "/", 3) // 3 is enough to confirm that is invalid + will not overflow the buffer
return hasPrefix && containsSlash && len(split) == 2 && len(split[1]) > 0
}

func isGithubUser(s string) bool {
return !strings.Contains(s, "/") && strings.HasPrefix(s, "@")
}

func (v *ValidOwner) isIgnoredOwner(name string) bool {
_, found := v.ignOwners[name]
return found
}

func (v *ValidOwner) selectValidateFn(name string) func(context.Context, string) *validateError {
switch {
case IsGithubUser(name):
case isGithubUser(name):
return v.validateGithubUser
case IsGithubTeam(name):
case isGithubTeam(name):
return v.validateTeam
case IsEmailAddress(name):
case isEmailAddress(name):
// TODO(mszostok): try to check if e-mail really exists
return func(context.Context, string) *validateError { return nil }
default:
Expand Down Expand Up @@ -161,12 +199,6 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr
}

func (v *ValidOwner) validateGithubUser(ctx context.Context, name string) *validateError {
// Ignore @ghost user
// https://github.community/t5/How-to-use-Git-and-GitHub/CODEOWNERS-file-with-a-NOT-file-type-condition/m-p/31013/highlight/true#M8523
if IsGithubGhostUser(name) {
return nil
}

if v.orgMembers == nil { //TODO(mszostok): lazy init, make it more robust.
if err := v.initOrgListMembers(ctx); err != nil {
return newValidateError("Cannot initialize organization member list: %v", err).AsPermanent()
Expand Down
26 changes: 0 additions & 26 deletions internal/check/valid_owner_check.go

This file was deleted.

52 changes: 0 additions & 52 deletions internal/check/valid_owner_check_test.go

This file was deleted.

0 comments on commit 4f46df1

Please sign in to comment.