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

[prow] Add support for github teams in OWNERS files #10065

Open
jtnord opened this Issue Nov 7, 2018 · 8 comments

Comments

6 participants
@jtnord
Copy link

jtnord commented Nov 7, 2018

as per title, if a team has a large amount of repositories it is cumbersome to update all the OWNERS files when the membership of the team changes.

https://github.com/kubernetes/test-infra/blame/master/prow/plugins/approve/approvers/README.md#L36 points out erroneously that teams are not audited - but that is not the case as documented

So it would be great if prow added support for teams in the owners file.

@cjwagner

This comment has been minimized.

Copy link
Member

cjwagner commented Nov 8, 2018

How would we would distinguish OWNERS_ALIASES from GitHub teams?
Using GitHub teams instead of OWNERS_ALIASES would dramatically increase API token usage for all plugins that use OWNERS files unless the github cache is deployed (which is something we cannot assume) so I'd be hesitant to accept this change unless we can demonstrate that that is not a concern.
Even if this is feasible I won't have time to work on this Q4 so I'm unassigning myself.
/unassign

@jtnord

This comment has been minimized.

Copy link
Author

jtnord commented Nov 8, 2018

how would we would distinguish OWNERS_ALIASES from GitHub teams

I may be wrong but today aliased and users have the same form in the OWNERS file ( - string)

approvers:
  - alice
  - bob     # this is a comment
reviewers:
  - alice
  - carol   # this is another comment
  - sig-foo # this is an alias

There is no difference here between sig-foo and alice for example.

A team in github is referenced in the form org/team so it can be distinguished by virtue of having a slash. Now I do not find a definitive spec on what is or is not allowed in an alias so it could possibly contain arbitrary characters including the slash but then I would turn the table and ask how you distinguish today between users and aliases? Isn't that the exact same problem that is already solved?
either the string is an alias or it is not. As users can not contain a / it logically follows that if the entry is not an alias and contains a slash it should be treated as a team.

The API token usage is a valid concern but given this would be opt in (no one is making anyone use teams, and you can tell up front if the thing smells like a team before even calling the API) it should be possible to implement with no impact on anyone not using teams in an OWNERS.

@BenTheElder

This comment has been minimized.

Copy link
Member

BenTheElder commented Nov 8, 2018

it should be possible to implement with no impact on anyone not using teams in an OWNERS.

Only if the feature were disable-able. If one repo uses this, the token usage affects everyone on that Prow deployment.

I also dislike this a little bit, because other than hook, prow nominally supports other git hosts (mainly gerrit atm) and teams is a github thing, whereas owners and aliases are quite portable.

Teams have some major UX downsides for contributors too, non-org members can't @ mention them last I checked, and more importantly cannot view who is in the team at all. ALIASES are public and we tend to prefer checking in config.

One thought: A different way of doing this is to have a mechanism to keep aliases synchronized with teams. Perhaps something for peribolos @fejta

@nikhita

This comment has been minimized.

Copy link
Member

nikhita commented Dec 26, 2018

/sig contributor-experience
/kind feature
/area prow/peribolos

@spiffxp

This comment has been minimized.

Copy link
Member

spiffxp commented Jan 17, 2019

Really interested in using this going forward. I think it would help simplify the fact that we have too many OWNERS files in too many places that end up going stale.

My evil plan is something like:

  • add support github teams to approvers plugin, dereference/expand the team when it comes to choosing who to assign (aka do not consider teams valid assignees or reviewers, only their members)
  • manage those teams from a single repo (kubernetes/org) and make that a very self-service process
  • link to the files that drive those teams in approve bot comments
  • suggest to the community that we prefer the use of teams rather than named individuals in owners files
@spiffxp

This comment has been minimized.

Copy link
Member

spiffxp commented Jan 17, 2019

teams are not audited - but that is not the case as documented

This is only viewable to admins of a given GitHub org, we prefer public transparency. Further, there is no way to know the reasons people are added or removed based off of the audit log. PRs help with that.

@BenTheElder

This comment has been minimized.

Copy link
Member

BenTheElder commented Jan 17, 2019

add support github teams to approvers plugin, dereference/expand the team when it comes to choosing who to assign (aka do not consider teams valid assignees or reviewers, only their members)

manage those teams from a single repo (kubernetes/org) and make that a very self-service process

FWIW, you do not need github teams or the github API to do this, instead you can let the approvers package be aware of this central teams config ...

@spiffxp

This comment has been minimized.

Copy link
Member

spiffxp commented Apr 17, 2019

/lifecycle frozen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.