codeowners-validator icon indicating copy to clipboard operation
codeowners-validator copied to clipboard

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

Open mszostok opened this issue 5 years ago • 4 comments

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

mszostok avatar Nov 19 '19 07:11 mszostok

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

njegosrailic avatar Jan 09 '20 14:01 njegosrailic

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 avatar Mar 15 '20 00:03 mszostok

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 avatar Oct 24 '20 19:10 mszostok

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

mszostok avatar Jan 21 '21 16:01 mszostok