Add permissions check to valid_owner

[highb]

Description

Changes proposed in this pull request:

• Changes the initial check for the validity/existence of a team to use the Teams List Teams API endpoint
• Adds a permissions check that uses the Teams IsTeamRepoBySlug endpoint, to check that the owner has write permissions on the repo

I haven't contributed to many Go Projects, so hopefully I'm following good conventions.

Related issue(s)

Fixes #40

[codecov[bot]]

Codecov Report

Merging #62 (d03ee34) into master (cde24ed) will decrease coverage by 3.13%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master      #62      +/-   ##
==========================================
- Coverage   50.13%   47.00%   -3.14%     
==========================================
  Files          12       12              
  Lines         375      400      +25     
==========================================
  Hits          188      188              
- Misses        183      208      +25     
  Partials        4        4              

Impacted Files	Coverage Δ
internal/check/valid_owner.go	24.46% <0.00%> (-5.37%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update cde24ed...d03ee34. Read the comment docs.

[highb]

I'm not surprised to see that I should add more unit tests. I'll work on that when I have a moment.

[jspiro]

This is just what we need, thanks for fixing this!

[jspiro]

I don't see an easy way to increase coverage in this file. All of the active check code is uncovered as it hits GitHub APIs – I can't tell if they're mocked nor how, but when I gave it a trivial try by calling Check with @org/team, the github API segfaulted (!).

This might explain the file being 50% coverage target. @mszostok what's the path forward here? This is a critical fix for teams using parent team hierarchies, and the code looks good to me!

[mszostok]

Hi @jspiro today I will take a look. Basically, implemented functionality is covered by the integration tests: https://github.com/mszostok/codeowners-validator/blob/master/docs/development.md#integration-tests but the test coverage is not included in the report (see: https://github.com/mszostok/codeowners-validator/issues). Unfortunately, integration tests cannot be executed on PRs from external contributors because of the problem with tokens. I will run them manually.

Btw. I see that the job failed on code quality check, see: https://travis-ci.com/github/mszostok/codeowners-validator/jobs/471190231 - it is easy to fix :)

[highb]

@mszostok Thanks for taking a look! If there's anything I can do to help out, please ask.

[highb]

@mszostok FYI @jspiro is my rockstar colleague who I granted collaborator access on my fork to. 😄 It looks like he's essentially addressed everything and I'm 👍 on the changes.

[jspiro]

@mszostok As you indicated, Travis is hitting API limits, presumably due to unauthenticated requests having a limit of 60 per hour per IP. I suppose this is a good reason for GQL ;-)

        +    [err] line 8: Cannot initialize organization member list: GET https://api.github.com/orgs/gh-codeowners/members?per_page=100: 403 API rate limit exceeded for 207.254.16.35. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.) [rate reset in 35m37s]
253

Have you considered porting this to GitHub Actions? I suspect that would paper over this issue, at least.

[jspiro]

If this is acceptable and gets merged, please remember to tag it :-)

[mszostok]

Have you considered porting this to GitHub Actions? I suspect that would paper over this issue, at least.

Yes, this is on my TODO list, as for all other project I already use the GitHub Actions. Maybe during this weekend I will have some time to do it and add also release pipeline cause right now it is pain in the ass as it is manual 😄

[highb]

:shipit: Looking forward to rolling this out on all our repos! Thanks for putting the project out there!