codeowners-validator
codeowners-validator copied to clipboard
Migrate the validation for checking if team has a proper perms from REST to GraphQL
Description
On this PR https://github.com/kyma-project/kyma/pull/6270 functionality that was added for checking perms is probably not working properly with GitHub teams.
AC:
- checking
write
perms works properly both for GitHub users and teams
It's not working properly. I'll take a look soon because I have introduced this issue.
the problem is that in current approach we are checking team permission entry which is out-dated
It no longer identifies thepermission a team has on its repos, but only specifies the default permission a repo is initially added with.
source: https://github.com/google/go-github/blob/6e0f6ebdef7d6db18d0eb92bb6f7aa9c0c7d4101/github/teams.go#L146-L151
what we need to do is to list the team repos and then check the permission:
func teamHasPermissions(team *github.Team, repoName string) (bool, error) {
repos, _, err := ghClient.Teams.ListTeamRepos(ctx, team.GetID(), nil)
if err != nil {
return false, err
}
for _, r := range repos {
if r.GetName() == repoName {
var (
perm = r.GetPermissions()
hasAdmin = containsPerm(perm, "admin")
hasPush = containsPerm(perm, "push")
)
return hasAdmin || hasPush, nil
}
}
return false, nil
}
func containsPerm(perms map[string]bool, name string) bool {
perm, _ := perms[name]
return perm == true
}
other option is to use Review a user's permission level but it works currently only with users
Probably the best option is to use the GraphQL query to remove the overfetching problme, example query:
{
organization(login: "gh-codeowners") {
teams(first: 2) {
pageInfo {
hasNextPage
}
nodes {
slug
repositories(query: "codeowners-sample") {
nodes {
name
}
edges {
permission
}
}
}
}
}
}
can be used in https://developer.github.com/v4/explorer/
This issue was solved by #62. I do not close it because we can recheck if it is worth to change the implemented logic from REST to GraphQL.
More context: https://github.com/mszostok/codeowners-validator/pull/62#discussion_r561273525