mszostok / codeowners-validator

The GitHub CODEOWNERS file validator
Apache License 2.0
219 stars 47 forks source link

Migrate the validation for checking if team has a proper perms from REST to GraphQL #21

Open mszostok opened 4 years ago

mszostok commented 4 years ago

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:

njegosrailic commented 4 years ago

It's not working properly. I'll take a look soon because I have introduced this issue.

mszostok commented 4 years ago

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

mszostok commented 3 years ago

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/

mszostok commented 3 years ago

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