-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
add Github Access Structure Overview section #5503
base: master
Are you sure you want to change the base?
Conversation
Contributing.md
Outdated
|
||
* Technical Committee – admin permissions across all Express orgs ([expressjs](https://github.com/expressjs), [jshttp](https://github.com/jshttp), [pillarjs](https://github.com/pillarjs)). Highest level of access, owner permissions on repos and orgs. TC members will be added to a github team in each org, and permissions will be granted to the team. | ||
|
||
* Project Captain – admin permissions on a specific repo. Permissions similar to TC, but scoped to a specific repo not an entire org. Permissions must be applied manually to a Project Captain user as a result. Creating a github team for this is optional, as the permissions are not applied at the team level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
admin permissions on a specific repo. Permissions similar to TC, but scoped to a specific repo not an entire org.
This feels redundant, maybe just delete the second sentence?
Permissions must be applied manually to a Project Captain user as a result
Is that true? I thought we could manage that with a team where the team was added as admin on the repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I missed this comment.
For
Permissions must be applied manually to a Project Captain user as a result
Is that true? I thought we could manage that with a team where the team was added as admin on the repo.
I feel like Im spliting hairs here, but you are correct that we can do that. I see benefits to the approach, however, I also see it as creating extra teams to manage and becoming confusing/hard to parse.
Let's say we have 8 different repo captains in a given org and together they captain 12 repos (assuming that of the 60ish repos across all orgs, most repos will not have a captain, but that some people are captains to multiple repos).
That makes 12 unique Github teams. One for each repo, as I understand the suggestion. That's a lot of teams, and will grow as we add more captains. Benefits I see are it's simple enough to onboard someone to the permissions, create the team if it doesn't exist and add admin rights on the given repo to the teama, invite the captain. Offboarding means removing them from the team. The above can be accomplished via individually applied permissions as well, without creating 12 teams of 1. That's basically my whole argument, fewer teams to manage, and if we expect mostly 1 captain per repo the team approach is no less adhoc than individually applied permissions to a user account.
We could also have a situation where a captain steps down without one stepping up. We either delete the team outright, or let it coast as something which can be @ mentioned but has 0 members to see the ping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to bikeshed on this. I will be happy with whatever approach is picked. I brought my own opinions after thinking through the above. I'd prefer to do everything via teams, but I don't think teams map well to the repo captain concept when it comes to applying permissions due to noise/overhead created by teams of 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need some more work on this. I left a few initial questions but also I think we need to define what the team names are and likely what specific permissions they have. This should both be a doc for folks to understand what it means but also for us on how explicitly to manage it so we are all on the same page on how to do it.
Contributing.md
Outdated
|
||
* Project Captain – admin permissions on a specific repo. Permissions similar to TC, but scoped to a specific repo not an entire org. Permissions must be applied manually to a Project Captain user as a result. Creating a github team for this is optional, as the permissions are not applied at the team level. | ||
|
||
* Committer – commit bit on a given repo or repos. Permissions must be applied manually. Github team here is optional, but recommended. All members of the org who have a commit bit for one or more repos can be members. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Permissions must be applied manually
Not sure what this means. All of this will be manually managed 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Manually here means going into settings in github, selecting a given member from the org and setting their bespoke permissions (write on expressjs/body-parser, expressjs/multer, for example). Removing their perms requires the same manual work.
Contrast that to permission management with teams. TC/owners get admin on everything in the org. You can give those specific perms to a team, and then just add the user to the team. To revoke perms, remove them from the team.
That works well for TC and Triage, who have a homogenous set of perms, but not well for folks w/ ad hoc write access like Committer or Project captain. Which forces you to manually manage perms for individual accounts in the Github UI. It's extra work but I don't think there's a better way.
Here's github docs about these two different approaches which they call "Individually managing access" and "Managing Team Access". These are both very light docs that link to the same place, but they are two distinct access management strategies.
Co-authored-by: Wes Todd <[email protected]>
I left the team names out because naming is hard! I agree we should define names and specific perms Github has built in roles that are close to what we want, but we should evaluate if we need deviations. I err on the side of running with the built in perms where possible and adjusting when necessary. an aside...re: evaluating deviations from the built in roles: e.g. Triagers can review/approve/reject PRs, but the reviews don't count towards merging, as approval by user w/ write permissions is required. It's a nit, but PRs that don't get merged despite being approved by N folks w/o write looks worse than a PR not merging bc it hasn't been approved at all. Taking away approval perms from Triagers is not a priority, but it is an example of a perm we could deviate on and a weak justification. Honestly, this is likely moot bc you can't create custom roles without having Github EnterpriseHere is a summary of the groups I outlined in the PR and what permissions I am suggesting:
Glossary for Above
|
Here's a first pass at adding a section about planned Org/Repo access levels.
This doesn't spell out what has changed or will change about who has access. You can see some of the shaking out of dust in this issue expressjs/discussions#132
We should document that somewhere, but I wanted to open a PR to land what I've seen proposed.
I went with my own definitions and opinions here, and specifically am curious if I got the role for Committer correct re: access to a single repo vs all repos in an org.