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

Peribolos does not support new Github team repo permissions #14325

Closed
coverprice opened this issue Sep 13, 2019 · 10 comments · Fixed by #17569
Closed

Peribolos does not support new Github team repo permissions #14325

coverprice opened this issue Sep 13, 2019 · 10 comments · Fixed by #17569
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@coverprice
Copy link

What happened:
When dumping the Peribolos config skeleton for a Github org that contains a team with the beta Triage or Maintain permissions on a repo, the repo is dumped with permission none, which is invalid.

e.g.

$ peribolos --github-token-path=/token --dump some-github-org --dump-full
    [snip]
    team-foo:
        [snip]
        members:
        - someuser
        privacy: secret
        repos:
          somerepo: none           <------------ team-foo has permission 'Maintain' on somerepo.
          someotherrepo: read
        [snip]

What you expected to happen:
The repo is listed with the correct permission (presumably triage or maintain)

How to reproduce it (as minimally and precisely as possible):

  1. Assign any Github team to a repo, and grant it Triage or Maintain permissions.
  2. Run the Peribolos dump script on the organization (see above for an example)

Please provide links to example occurrences, if any:

Anything else we need to know?:
I don't know whether it's necessary to only fix the dump code, or whether there's also other code needed when Peribolos attempts to apply these permissions.

And also it may be useful to make Peribolos more robust in the event that future team permissions are added by Github, by erroring out if it encounters a permission it does not recognize.

@coverprice coverprice added the kind/bug Categorizes issue or PR as related to a bug. label Sep 13, 2019
@stevekuznetsov
Copy link
Contributor

/cc @fejta @cjwagner

@adshmh
Copy link
Contributor

adshmh commented Oct 4, 2019

This happens because GitHub API calls on ListTeamRepos returns false for all permission levels, i.e. push, pull, admin, when a team has the Maintainer role.

I couldn't find any v3 API calls that returns the beta new roles on a repository, but v4, i.e. graphql endpoint, does seem to report Maintainer or Triage levels

Peribolos does not currently use graphql endpoint, but perhaps it can be updated to do so. This could also mean fewer, ideally a single, calls to get the organisation structure, thus saving tokens.

I'd be happy to submit a patch if this sounds like a good approach.

@stevekuznetsov
Copy link
Contributor

That sounds like it would require a full rewrite of how peribolos works -- I'm not sure that is worth it. @guineveresaenger @nickvanw are there any plans to expose these new fields in v3 API?

@nickvanw
Copy link

nickvanw commented Oct 7, 2019

Hey @stevekuznetsov!

I've passed this feedback on to the PM that owns this part of the product - I agree that it would be great to expose this to the v3 API, and I'll follow up to see if it's on their schedule and, if so, when to expect it.

@stevekuznetsov
Copy link
Contributor

Thanks, @nickvanw! Appreciate your help here :)

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 5, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 4, 2020
@cblecker
Copy link
Member

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Feb 11, 2020
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 11, 2020
@adshmh
Copy link
Contributor

adshmh commented May 11, 2020

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants