Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configurability for the list of ignored owners #55

Merged
merged 2 commits into from Oct 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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.