diff --git a/README.md b/README.md index 891a1ef..52b8356 100644 --- a/README.md +++ b/README.md @@ -96,12 +96,12 @@ This will put `codeowners-validator` in `$(go env GOPATH)/bin`. The following checks are enabled by default: -| Name | Description | -|------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| duppatterns | **[Duplicated Pattern Checker]**

Reports if CODEOWNERS file contain duplicated lines with the same file pattern. | -| files | **[File Exist Checker]**

Reports if CODEOWNERS file contain lines with the file pattern that do not exist in a given repository. | -| owners | **[Valid Owner Checker]**

Reports if CODEOWNERS file contain invalid owners definition. Allowed owner syntax: `@username`, `@org/team-name` or `user@example.com`
_source: https://help.github.com/articles/about-code-owners/#codeowners-syntax_.

**Checks:**
    1. Check if the owner's definition is valid (is either a GitHub user name, an organization team name or an email address).

    2. Check if a GitHub owner has a GitHub account

    3. Check if a GitHub owner is in a given organization

    4. Check if an organization team exists | -| syntax | **[Valid Syntax Checker]**

Reports if CODEOWNERS file contain invalid syntax definition. It is imported as:
    "If any line in your CODEOWNERS file contains invalid syntax, the file will not be detected
    and will not be used to request reviews. Invalid syntax includes inline comments
    and user or team names that do not exist on GitHub."

_source: https://help.github.com/articles/about-code-owners/#codeowners-syntax_. | +| Name | Description | +|-------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| duppatterns | **[Duplicated Pattern Checker]**

Reports if CODEOWNERS file contain duplicated lines with the same file pattern. | +| files | **[File Exist Checker]**

Reports if CODEOWNERS file contain lines with the file pattern that do not exist in a given repository. | +| owners | **[Valid Owner Checker]**

Reports if CODEOWNERS file contain invalid owners definition. Allowed owner syntax: `@username`, `@org/team-name` or `user@example.com`
_source: https://help.github.com/articles/about-code-owners/#codeowners-syntax_.

**Checks:**
    1. Check if the owner's definition is valid (is either a GitHub user name, an organization team name or an email address).

    2. Check if a GitHub owner has a GitHub account

    3. Check if a GitHub owner is in a given organization

    4. Check if an organization team exists | +| syntax | **[Valid Syntax Checker]**

Reports if CODEOWNERS file contain invalid syntax definition. It is imported as:
    "If any line in your CODEOWNERS file contains invalid syntax, the file will not be detected
    and will not be used to request reviews. Invalid syntax includes inline comments
    and user or team names that do not exist on GitHub."

_source: https://help.github.com/articles/about-code-owners/#codeowners-syntax_. | The experimental checks are disabled by default: @@ -117,20 +117,20 @@ Check the [Configuration](#configuration) section for more info on how to enable 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"`. | -| OWNER_CHECKER_ALLOW_UNOWNED_PATTERNS | `true` | Specifies whether CODEOWNERS may have unowned files. For example:

`/infra/oncall-rotator/ @sre-team`
`/infra/oncall-rotator/oncall-config.yml`

The `/infra/oncall-rotator/oncall-config.yml` file is not owned by anyone. | -| OWNER_CHEKER_OWNERS_MUST_BE_TEAMS | `false` | Specifies whether only teams are allowed as owners of files | -| 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` | +| 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"`. | +| OWNER_CHECKER_ALLOW_UNOWNED_PATTERNS | `true` | Specifies whether CODEOWNERS may have unowned files. For example:

`/infra/oncall-rotator/ @sre-team`
`/infra/oncall-rotator/oncall-config.yml`

The `/infra/oncall-rotator/oncall-config.yml` file is not owned by anyone. | +| OWNER_CHEKER_OWNERS_MUST_BE_TEAMS | `false` | Specifies whether only teams are allowed as owners of files. | +| 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 diff --git a/action.yml b/action.yml index 74a9c29..da4ec50 100644 --- a/action.yml +++ b/action.yml @@ -52,6 +52,11 @@ inputs: default: "true" required: false + owner_checker_owners_must_be_teams: + description: "Specifies whether only teams are allowed as owners of files." + default: "false" + required: false + runs: using: 'docker' image: 'docker://ghcr.io/mszostok/codeowners-validator:v0.7.1' diff --git a/docs/gh-action.md b/docs/gh-action.md index 2bbd7b5..beae009 100644 --- a/docs/gh-action.md +++ b/docs/gh-action.md @@ -70,6 +70,9 @@ jobs: # Specifies whether CODEOWNERS may have unowned files. For example, `/infra/oncall-rotator/oncall-config.yml` doesn't have owner and this is not reported. owner_checker_allow_unowned_patterns: "true" + + #Specifies whether only teams are allowed as owners of files. + owner_checker_owners_must_be_teams: "false" ``` The best is to run this as a cron job and not only if you applying changes to CODEOWNERS file itself, e.g. the CODEOWNERS file can be invalidate when you removing someone from the organization. diff --git a/internal/check/valid_owner.go b/internal/check/valid_owner.go index 6fb98c8..7c33ac9 100644 --- a/internal/check/valid_owner.go +++ b/internal/check/valid_owner.go @@ -137,7 +137,14 @@ func (v *ValidOwner) isIgnoredOwner(name string) bool { func (v *ValidOwner) selectValidateFn(name string) func(context.Context, string) *validateError { switch { - case v.ownersMustBeTeams || isGitHubTeam(name): + case v.ownersMustBeTeams: + return func(ctx context.Context, s string) *validateError { + if !isGitHubTeam(name) { + return newValidateError("Only team owners allowed and %q is not a team", name) + } + return v.validateTeam(ctx, s) + } + case isGitHubTeam(name): return v.validateTeam case isGitHubUser(name): return v.validateGitHubUser @@ -190,10 +197,6 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr } } - if !isGitHubTeam(name) { - return newValidateError("%s is not a team", name) - } - // called after validation it's safe to work on `parts` slice parts := strings.SplitN(name, "/", 2) org := parts[0] diff --git a/internal/check/valid_owner_test.go b/internal/check/valid_owner_test.go index 8297d88..a3a7a7d 100644 --- a/internal/check/valid_owner_test.go +++ b/internal/check/valid_owner_test.go @@ -6,9 +6,10 @@ import ( "github.com/mszostok/codeowners-validator/internal/check" - "github.com/mszostok/codeowners-validator/internal/ptr" "github.com/stretchr/testify/require" + "github.com/mszostok/codeowners-validator/internal/ptr" + "github.com/stretchr/testify/assert" ) @@ -118,3 +119,43 @@ func TestValidOwnerCheckerIgnoredOwner(t *testing.T) { } }) } + +func TestValidOwnerCheckerOwnersMustBeTeams(t *testing.T) { + tests := map[string]struct { + codeowners string + issue *check.Issue + allowUnownedPatterns bool + }{ + "Bad owner definition": { + codeowners: `* @owner1`, + issue: &check.Issue{ + Severity: check.Error, + LineNo: ptr.Uint64Ptr(1), + Message: `Only team owners allowed and "@owner1" is not a team`, + }, + }, + "No owners but allow empty": { + codeowners: `*`, + issue: nil, + allowUnownedPatterns: true, + }, + } + for tn, tc := range tests { + t.Run(tn, func(t *testing.T) { + // given + ownerCheck, err := check.NewValidOwner(check.ValidOwnerConfig{ + Repository: "org/repo", + AllowUnownedPatterns: tc.allowUnownedPatterns, + OwnersMustBeTeams: true, + }, nil) + require.NoError(t, err) + + // when + out, err := ownerCheck.Check(context.Background(), LoadInput(tc.codeowners)) + + // then + require.NoError(t, err) + assertIssue(t, tc.issue, out.Issues) + }) + } +}