diff --git a/README.md b/README.md index dc9a6ec..c5122f9 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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 | -|-----|:--------|:------------| -| REPOSITORY_PATH * | | Path to your repository on your local machine. | -| GITHUB_ACCESS_TOKEN| | 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. | -| GITHUB_BASE_URL| 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. | -| GITHUB_UPLOAD_URL | https://uploads.github.com/ | GitHub upload URL for uploading files.

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. | -| CHECKS| - | List of checks to be executed. By default, all checks are executed. Possible values: `files`,`owners`,`duppatterns`,`syntax`. | -| EXPERIMENTAL_CHECKS | - | The comma-separated list of experimental checks that should be executed. By default, all experimental checks are turned off. Possible values: `notowned`.| -| CHECK_FAILURE_LEVEL | `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`. | -| OWNER_CHECKER_REPOSITORY *| | 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. | -| NOT_OWNED_CHECKER_SKIP_PATTERNS| - | 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` | - - * - Required +This will put `codeowners-validator` in `$(go env GOPATH)/bin`. ## Checks @@ -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 | +|-----|:--------|:------------| +| REPOSITORY_PATH * | | Path to your repository on your local machine. | +| GITHUB_ACCESS_TOKEN| | 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. | +| GITHUB_BASE_URL| 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. | +| GITHUB_UPLOAD_URL | https://uploads.github.com/ | GitHub upload URL for uploading files.

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. | +| CHECKS| - | List of checks to be executed. By default, all checks are executed. Possible values: `files`,`owners`,`duppatterns`,`syntax`. | +| EXPERIMENTAL_CHECKS | - | The comma-separated list of experimental checks that should be executed. By default, all experimental checks are turned off. Possible values: `notowned`.| +| CHECK_FAILURE_LEVEL | `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`. | +| OWNER_CHECKER_REPOSITORY *| | 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. | +| OWNER_CHECKER_IGNORED_OWNERS | `@ghost`| The comma-separated list of owners that should not be validated. Example: `"@owner1,@owner2,@org/team1,example@email.com"`. | +| NOT_OWNED_CHECKER_SKIP_PATTERNS| - | 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` | + + * - Required + #### Exit status codes Application exits with different status codes which allow you to easily distinguish between error categories. diff --git a/action.yml b/action.yml index b9c51fc..d64f889 100644 --- a/action.yml +++ b/action.yml @@ -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' diff --git a/internal/check/duplicated_pattern_test.go b/internal/check/duplicated_pattern_test.go index 223f5c7..b88252c 100644 --- a/internal/check/duplicated_pattern_test.go +++ b/internal/check/duplicated_pattern_test.go @@ -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, }, } @@ -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) diff --git a/internal/check/file_exists_test.go b/internal/check/file_exists_test.go index 71aab3c..ea0dd07 100644 --- a/internal/check/file_exists_test.go +++ b/internal/check/file_exists_test.go @@ -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) @@ -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) diff --git a/internal/check/helpers_test.go b/internal/check/helpers_test.go index 57e8852..087be84 100644 --- a/internal/check/helpers_test.go +++ b/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 @@ -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), } } diff --git a/internal/check/package_test.go b/internal/check/package_test.go index bb3a927..730ca96 100644 --- a/internal/check/package_test.go +++ b/internal/check/package_test.go @@ -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)) diff --git a/internal/check/valid_owner.go b/internal/check/valid_owner.go index b8509f6..de9d2e8 100644 --- a/internal/check/valid_owner.go +++ b/internal/check/valid_owner.go @@ -3,6 +3,7 @@ package check import ( "context" "net/http" + "net/mail" "strings" "github.com/mszostok/codeowners-validator/internal/ctxutil" @@ -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 @@ -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 @@ -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 } @@ -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 } @@ -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: @@ -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() diff --git a/internal/check/valid_owner_check.go b/internal/check/valid_owner_check.go deleted file mode 100644 index d3b38fa..0000000 --- a/internal/check/valid_owner_check.go +++ /dev/null @@ -1,26 +0,0 @@ -package check - -import ( - "net/mail" - "strings" -) - -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 IsGithubGhostUser(s string) bool { - return s == "@ghost" -} diff --git a/internal/check/valid_owner_check_test.go b/internal/check/valid_owner_check_test.go deleted file mode 100644 index 670ed1b..0000000 --- a/internal/check/valid_owner_check_test.go +++ /dev/null @@ -1,52 +0,0 @@ -package check_test - -import ( - "testing" - - "github.com/mszostok/codeowners-validator/internal/check" - - "github.com/stretchr/testify/assert" -) - -func TestValidOwnerChecker(t *testing.T) { - tests := map[string]struct { - user string - isValid bool - }{ - "Invalid Email": { - user: `asda.comm`, - isValid: false, - }, - "Valid Email": { - user: `gmail@gmail.com`, - isValid: true, - }, - "Invalid Team": { - user: `@org/`, - isValid: false, - }, - "Valid Team": { - user: `@org/user`, - isValid: true, - }, - "Invalid User": { - user: `user`, - isValid: false, - }, - "Valid User": { - user: `@user`, - isValid: true, - }, - } - for tn, tc := range tests { - t.Run(tn, func(t *testing.T) { - // when - result := isValidUser(tc.user) - assert.Equal(t, tc.isValid, result) - }) - } -} - -func isValidUser(user string) bool { - return check.IsEmailAddress(user) || check.IsGithubUser(user) || check.IsGithubTeam(user) -} diff --git a/internal/check/valid_owner_test.go b/internal/check/valid_owner_test.go new file mode 100644 index 0000000..17ec8e6 --- /dev/null +++ b/internal/check/valid_owner_test.go @@ -0,0 +1,99 @@ +package check + +import ( + "context" + "testing" + + "github.com/mszostok/codeowners-validator/internal/ptr" + "github.com/stretchr/testify/require" + + "github.com/stretchr/testify/assert" +) + +func TestValidOwnerChecker(t *testing.T) { + tests := map[string]struct { + owner string + isValid bool + }{ + "Invalid Email": { + owner: `asda.comm`, + isValid: false, + }, + "Valid Email": { + owner: `gmail@gmail.com`, + isValid: true, + }, + "Invalid Team": { + owner: `@org/`, + isValid: false, + }, + "Valid Team": { + owner: `@org/user`, + isValid: true, + }, + "Invalid User": { + owner: `user`, + isValid: false, + }, + "Valid User": { + owner: `@user`, + isValid: true, + }, + } + for tn, tc := range tests { + t.Run(tn, func(t *testing.T) { + // when + result := isValidOwner(tc.owner) + assert.Equal(t, tc.isValid, result) + }) + } +} + +func TestValidOwnerCheckerIgnoredOwner(t *testing.T) { + t.Run("Should ignore owner", func(t *testing.T) { + // given + ownerCheck, err := NewValidOwner(ValidOwnerConfig{ + Repository: "org/repo", + IgnoredOwners: []string{"@owner1"}, + }, nil) + require.NoError(t, err) + + givenCodeowners := `* @owner1` + + // when + out, err := ownerCheck.Check(context.Background(), LoadInput(givenCodeowners)) + + // then + require.NoError(t, err) + assert.Empty(t, out.Issues) + }) + + t.Run("Should ignore user only and check the remaining owners", func(t *testing.T) { + // given + ownerCheck, err := NewValidOwner(ValidOwnerConfig{ + Repository: "org/repo", + IgnoredOwners: []string{"@owner1"}, + }, nil) + require.NoError(t, err) + + givenCodeowners := `* @owner1 badOwner` + + expIssue := Issue{ + Severity: Error, + LineNo: ptr.Uint64Ptr(1), + Message: `Not valid owner definition "badOwner"`, + } + + // when + out, err := ownerCheck.Check(context.Background(), LoadInput(givenCodeowners)) + + // then + require.NoError(t, err) + require.Len(t, out.Issues, 1) + require.EqualValues(t, expIssue, out.Issues[0]) + }) +} + +func isValidOwner(owner string) bool { + return isEmailAddress(owner) || isGithubUser(owner) || isGithubTeam(owner) +} diff --git a/internal/check/valid_syntax_test.go b/internal/check/valid_syntax_test.go index 051b71c..f1a1019 100644 --- a/internal/check/valid_syntax_test.go +++ b/internal/check/valid_syntax_test.go @@ -78,7 +78,7 @@ func TestValidSyntaxChecker(t *testing.T) { t.Run(tn, func(t *testing.T) { // when out, err := check.NewValidSyntax(). - Check(context.Background(), loadInput(tc.codeowners)) + Check(context.Background(), check.LoadInput(tc.codeowners)) // then require.NoError(t, err)