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 a flag to only allow teams as owners #127

Merged
merged 1 commit into from
Apr 6, 2022
Merged

Conversation

seveas
Copy link
Contributor

@seveas seveas commented Mar 8, 2022

Hi Mateusz!

I'd like to replace some horrible home-grown checkers with codeowners-validator. There's only one feature missing: in the repository I want to use this for, we require that all owners are teams.

This PR implements an optional addition to the owner checker to enforce this.

@mszostok mszostok self-assigned this Mar 11, 2022
Copy link
Owner

@mszostok mszostok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the pull request, the new option LGTM 👍 Please only add missing entry to the action.yaml and we can merge it and release new version.

@@ -186,6 +190,10 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr
}
}

if !isGitHubTeam(name) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I could be moved before the v. orgTeams initialization.

@@ -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,example@email.com"`. |
| <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 |
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
| <tt>OWNER_CHEKER_OWNERS_MUST_BE_TEAMS</tt> | `false` | Specifies whether only teams are allowed as owners of files |
| <tt>OWNER_CHEKER_OWNERS_MUST_BE_TEAMS</tt> | `false` | Specifies whether only teams are allowed as owners of files. |

Additionally, please add:

  owner_cheker_owners_must_be_teams:
    description:  "Specifies whether only teams are allowed as owners of files."
    default: "false"
    required: false

to action.yml

Copy link

@electriquo electriquo Apr 7, 2022

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

Copy link
Owner

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, thank you @mszostok :)

@mszostok
Copy link
Owner

mszostok commented Apr 6, 2022

As there is no activity under this PR, I will merge it as it is and create a follow-up PR that will address missing parts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants