-
Notifications
You must be signed in to change notification settings - Fork 51
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 a flag to only allow teams as owners #127
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -129,6 +129,7 @@ Use the following environment variables to configure the application: | |
| <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,[email protected]"`. | | ||
| <tt>OWNER_CHECKER_ALLOW_UNOWNED_PATTERNS</tt> | `true` | Specifies whether CODEOWNERS may have unowned files. For example: <br> <br> `/infra/oncall-rotator/ @sre-team` <br> `/infra/oncall-rotator/oncall-config.yml` <br> <br> The `/infra/oncall-rotator/oncall-config.yml` file is not owned by anyone. | | ||
| <tt>OWNER_CHEKER_OWNERS_MUST_BE_TEAMS</tt> | `false` | Specifies whether only teams are allowed as owners of files | | ||
| <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 file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,8 @@ type ValidOwnerConfig struct { | |
// | ||
// The `/infra/oncall-rotator/oncall-config.yml` this file is not owned by anyone. | ||
AllowUnownedPatterns bool `envconfig:"default=true"` | ||
// OwnersMustBeTeams specifies whether owners must be teams in the same org as the repository | ||
OwnersMustBeTeams bool `envconfig:"default=false"` | ||
} | ||
|
||
// ValidOwner validates each owner | ||
|
@@ -37,6 +39,7 @@ type ValidOwner struct { | |
orgRepoName string | ||
ignOwners map[string]struct{} | ||
allowUnownedPatterns bool | ||
ownersMustBeTeams bool | ||
} | ||
|
||
// NewValidOwner returns new instance of the ValidOwner | ||
|
@@ -57,6 +60,7 @@ func NewValidOwner(cfg ValidOwnerConfig, ghClient *github.Client) (*ValidOwner, | |
orgRepoName: split[1], | ||
ignOwners: ignOwners, | ||
allowUnownedPatterns: cfg.AllowUnownedPatterns, | ||
ownersMustBeTeams: cfg.OwnersMustBeTeams, | ||
}, nil | ||
} | ||
|
||
|
@@ -133,10 +137,10 @@ func (v *ValidOwner) isIgnoredOwner(name string) bool { | |
|
||
func (v *ValidOwner) selectValidateFn(name string) func(context.Context, string) *validateError { | ||
switch { | ||
case v.ownersMustBeTeams || isGitHubTeam(name): | ||
return v.validateTeam | ||
case isGitHubUser(name): | ||
return v.validateGitHubUser | ||
case isGitHubTeam(name): | ||
return v.validateTeam | ||
case isEmailAddress(name): | ||
// TODO(mszostok): try to check if e-mail really exists | ||
return func(context.Context, string) *validateError { return nil } | ||
|
@@ -186,6 +190,10 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr | |
} | ||
} | ||
|
||
if !isGitHubTeam(name) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I could be moved before the |
||
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] | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
Additionally, please add:
to action.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sadly, this is not documented in the readme https://github.com/mszostok/codeowners-validator#checks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for letting me know, I will add it there too. For now, it was documented in configuration and in the release notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, thank you @mszostok :)