WIP: Remove inactive members, collaborators and teams #65
Conversation
| collaborators: | ||
| admin: | ||
| - vmx |
There was a problem hiding this comment.
Why am I removed? I created that spec and repo. What does it mean if there are no admins? I guess me having push access would still make sense?
| @@ -2707,10 +2639,6 @@ repositories: | |||
| collaborators: | |||
| admin: | |||
| - Stebalien | |||
| push: | |||
| - dvc94ch | |||
There was a problem hiding this comment.
dvc94ch was the original creator, we then moved this repo. In good open source spirit, someone should at least talk with him about it and not just removing push access.
|
@vmx and others: this is in draft and hasn't been reviewed and shouldn't be taken as truth yet. This PR is a starting place so we can start framing the discussion around doing permission cleanup. There is also a corresponding FIL Slack channel #ip-stack-github-permissions-cleanup-2024q1 The current PR is what results from a base script that IPDX has. I'm reviewing to give input on what to adjust. @mentions will go out with who is affected when it's ready to be looked at. Folks are certainly welcome to look at it sooner, but I just want folks to know this isn't being viewed yet as ready for review (hence the reason its in draft). |
| @@ -3105,48 +3030,77 @@ teams: | |||
| Admin: | |||
There was a problem hiding this comment.
I think we should remove the Admin team entirely. If someone needs admin permissions for a certain repo, it can be requested/checked through github-mgmt.
| - wanderer | ||
| - warpfork | ||
| - whizzzkid | ||
| - whyrusleeping | ||
| privacy: closed |
There was a problem hiding this comment.
I can't remember how this maps. Does "closed" mean "visible" or "secret" (per https://docs.github.com/en/organizations/organizing-members-into-teams/changing-team-visibility )? I assume we want visible. We ideally don't want them getting @mentioned but we want it to be visible who the alumni are.
| - wanderer | ||
| - warpfork | ||
| - whizzzkid | ||
| - whyrusleeping | ||
| privacy: closed | ||
| Contributors: |
There was a problem hiding this comment.
I don't see the purpose of this team. I think we should remove it.
| Core: | ||
| description: Core Team |
There was a problem hiding this comment.
| Core: | |
| description: Core Team | |
| General Maintainers: | |
| description: Invested maintainers who can act as a catch-basin in the absence of anyone else. |
I think this name is more clear? I don't think we want to imply that there is a "Core" team who are dedicated to IPLD, since that isn't the case currently.
| @@ -3155,18 +3109,10 @@ teams: | |||
| - rvagg | |||
| - Stebalien | |||
| - vmx | |||
| - warpfork | |||
| member: | |||
| - BigLep | |||
There was a problem hiding this comment.
| - BigLep | |
| - aschmahmann |
Swap Adin for me as an IP Shipyard representative.
| - victorb | ||
| - wanderer | ||
| privacy: closed | ||
| Python Team: |
There was a problem hiding this comment.
I agree it can be removed given the one repo that uses it has dhruvbaldawa
| maintainer: | ||
| - daviddias | ||
| member: | ||
| - dhruvbaldawa | ||
| privacy: closed | ||
| research: |
There was a problem hiding this comment.
Lets remove the team entirely since it is only providing coverage on ipld/research which is archived.
| reviewers: | ||
| members: | ||
| maintainer: | ||
| - rvagg | ||
| - warpfork | ||
| member: | ||
| - BigLep | ||
| - RangerMauve | ||
| - willscott | ||
| privacy: closed |
There was a problem hiding this comment.
Lets remove. It has low repo coverage, and where it is, those folks already have access.
| member: | ||
| - BigLep | ||
| - RangerMauve | ||
| - willscott | ||
| privacy: closed | ||
| Rust Team: |
There was a problem hiding this comment.
| Rust Team: | |
| Rust Maintainers |
To be consistent and make clear there isn't a "Rust Team"?
| maintainer: | ||
| - daviddias | ||
| member: | ||
| - cloutiertyler | ||
| privacy: closed | ||
| w3dt-stewards: |
There was a problem hiding this comment.
Lets delete this team, it's legacy.
Given there is github magmt where folks can explicitly request permissions where needed, I don't see value in blank admin permissions that "w3dt-stewards" has.
| - galargh | ||
| - jbenet | ||
| - marten-seemann | ||
| - rvagg |
There was a problem hiding this comment.
eh? I'd like to hear a justification for the list left in here
There was a problem hiding this comment.
I made this change (quickly).
I assume there is no questioning about the general push to reduce this given the power an "organization admin" has per https://docs.github.com/en/organizations/managing-peoples-access-to-your-organization-with-roles/roles-in-an-organization#permissions-for-organization-roles.
I assume the concern is more on why was @rvagg removed? :)
This was my doing of a proposal to pull one person from a few different organizations knowing additional people could be added necessary through github-mgmt and often times don't need for organizational ownership access. I quickly pulled vmx for PLGO given he had just commented on the PR. It was an oversight not to include your Rod. I personally have no concerns with adding you back in as an owner/admin. I'll put that in as a suggestion now.
(terraform members.admin is a github "org owner")
There was a problem hiding this comment.
I add @rvagg back as admin. I'm also seeing in the process I had fully removed him as a member as well. That certainly wasn't intentional! Anyways, all fixed now.
There was a problem hiding this comment.
I agree with the push to simplify this, and even the ideal of trimming the admin permissions right down to near zero (although zero would imply a great deal of faith in the tooling and it being future proof); I was more wondering if there's context that I missed in that one of the only two people who contribute to, and maintain this org on a regular basis were removed from the list, leaving three who don't (nucleation uncertainty opens up all sorts of interesting interpretations).
|
I put more higher-level comments about this operation in https://github.com/ipld/github-mgmt/pull/65/files#diff-0593c68eb7d39dfb1df1409c0f14feab10ef45843d217501705e315a3f49e788 for easier threaded replies. |
|
Even though there have been additional changes/updates on this PR, this isn't ready for general scrutiny (but please feel free to go ahead if you wish). The current PR is what results from a base script that IPDX has. @mentions will go out with who is affected when it's ready to be looked at. Also note that 2024Q1 permissions cleanup is starting with ipld, but we (@galargh, @BigLep) intend to take the learnings/process to libp2p and ipfs orgs afterwards. |
|
Just as @BigLep said, this PR is not currently in a reviewable state - hence, "Draft" state + WIP in the title. It's only a result of running https://github.com/ipld/github-mgmt/blob/master/scripts/src/actions/remove-inactive-members.ts#L41 against a recently generated Audit log. To reassure you, our intention is definitely not to remove any long-term contributors and maintainers who are doing a tremendous job keeping this org alive 🙇 You can expect many updates before this PR is ready for review. That's also part of the reason why we chose to start process refining with ipld - we expected to see many edge cases here that the automation should be made aware of to provide us with a better base in the future. Additionally, we never want to automate the cleanup process entirely. It will always remain driven and reviewed by humans. Sorry for not stating the above earlier and causing worries. I'll make sure proper disclaimers are included in any future, similar issues. |
There was a problem hiding this comment.
This review includes some notes from the 2024-02-08 verbal conversation between @galargh, @andyschwab, and @BigLep.
Ultimately this PR is going to get closed. Instead some new PRs will get opened over the next week:
- Reducing org owner set. This is the single biggest thing can do to help protect the org. It requires human insight/judgement and can be done independent of github-mgmt improvements.
- "archiving" of inactive members and teams. Archiving can mean moving inactive team members to an Alumni team and it can mean deleting an unused team.
Some additional github issues will be created. I'll link those to the relevant comments and link this PR to a master tracking issue for the general 2024Q1 permissions cleanup that is happening across ipld/libp2p/ipfs.
|
|
||
| That by itself will cut down on some of the confusion, but it will still need a disclaimer (e.g., "Even though your direct repo permissions have been removed, you may still have access through a team. Please check the full diff."). | ||
|
|
||
| ## More intelligent sorting of keys |
There was a problem hiding this comment.
2024-02-08 verbal: agreed to put this on github-mgmt backlog. @BigLep will create the backlog item.
| # Somewhere between general and IPLD-specific | ||
| These are at least things that apply to the 2024Q1 cleanup for ipld, libp2p, and ipfs orgs. Some of them can be generalized. | ||
|
|
||
| ## Archived repos cause clutter |
There was a problem hiding this comment.
2024-02-08 conversation:
- Agreed that it is good to be prescriptive on archiving repos. Archiving removes permissions.
- For the first pass, have a way to declutter. Will do a pass after for handling archived repo permissions/
- Open operational question for first pass: what happens if we want to delete w3dt-stewards team but it's on an archived repo. Github UI is probably smart enough to handle this but we're not sure terraform is.
| In PLv10 / PL Innovation Network era, there is no corrolary for this. | ||
| If broad action is needed, it can be done broadly via github-mgmt. And where it can't be done directly with github-mgmt, github-mgmt can at least be done to give broad permissions to go do the broad action. | ||
|
|
||
| ## Default access of the ipdx team |
There was a problem hiding this comment.
2024-02-08 conversation: if you're an owner, you don't need to be an admin of the repo. You already have god mode. As a result, it's fine to strip out ipdx everywhere since they already have permissions as org owners.
| If I should do something else, please let me know! | ||
|
|
||
| # General github-mgmt feedback/wishes | ||
| ## Documentation I wish was more readily accessible when using github-mgmt |
There was a problem hiding this comment.
2024-02-08 conversation: @BigLep to extract this out to a generic github-mgmt backlog issue. It's not required for this permission cleanup campaign.
| * Terraform “pull” corresponds with Github “read”. | ||
| * Terraform members.admin correspond with Github "org owner" per https://registry.terraform.io/providers/integrations/github/latest/docs/resources/membership | ||
|
|
||
| ## User-oriented view of changes |
There was a problem hiding this comment.
| * Generalizing this, maybe github-mgmt can be configured to do a merge of any specified files, which lets repo organizers decide if/how they want to break up their yaml file in general? | ||
| * Go the IPFS route and actually have an "ipfs-inactive" org. Archived projects can get moved there. This makes things very clear about the maintenance status and also solves the declutter problem. | ||
|
|
||
| ## Strip out access permissions to archived repos |
There was a problem hiding this comment.
2024-02-08 conversation: covered above.
| Someone can always unarchive and add permissions through github-mgmt. | ||
| In addition to this reducing clutter, I this proposed process is good because it gives clear visibility to a significant repo event (e.g., unarchiving). | ||
|
|
||
| ## Remove the w3dt-stewards team |
There was a problem hiding this comment.
2024-02-08 conversation: agreed this should get done.
There was a problem hiding this comment.
@BigLep is this something we will execute soon, a long term plan? This group has been a "maintainer" team in places like https://github.com/ipfs/someguy/settings/access and if we just remove it, it will disrupt day-to-day
There was a problem hiding this comment.
It was clear-cut to do in ipld land.
I know more thought will be needed in ipfs.
At the minimum what we'd do in IPFS is rename the team (e.g., ip-shipyard) and reduce the people who are on that team to those who are actually on shipyard.
Master tracking issue for when we get into ipfs org: ipfs/ipfs#511
| It looks like the ipdx team has admin permissions on most repos. | ||
| Should we maybe reduce this and instead ensure they just have github-mgmt admin access, which still enables them to "break glass" and escalate their access when needed? | ||
|
|
||
| ## Commit/PR order |
There was a problem hiding this comment.
2024-02-08 conversation: agreed that it's good to have a commit/PR that is not cluttered so that when folks are @mentioned that they only see the changes that really pertain to them.
| 6. Other changes that the IPDX scripts picks up based on audit log inactivity and based on additional manual changes. (This is the diff we need people to be scrutinizing.) | ||
|
|
||
| # IPLD specifics | ||
| ## Org admins |
There was a problem hiding this comment.
2024-02-08 conversation: This is the higher-order bit and requires human intervention. It can be done ahead of everything else and already makes things significantly more locked down. @BigLep will take on opening PRs about reducing org owners ("members.admin") for ipfs/libp2p/ipld/etc. later 2024-02-09.
There was a problem hiding this comment.
This has been called out as the key item in the overarching tracking item in ipfs/ipfs#511
There was a problem hiding this comment.
Various backlog items have been created and linked to the relevant comments.
The overarching tracking effort for this cleanup has been created in ipfs/ipfs#511
| If I should do something else, please let me know! | ||
|
|
||
| # General github-mgmt feedback/wishes | ||
| ## Documentation I wish was more readily accessible when using github-mgmt |
|
|
||
| That by itself will cut down on some of the confusion, but it will still need a disclaimer (e.g., "Even though your direct repo permissions have been removed, you may still have access through a team. Please check the full diff."). | ||
|
|
||
| ## More intelligent sorting of keys |
| * Terraform “pull” corresponds with Github “read”. | ||
| * Terraform members.admin correspond with Github "org owner" per https://registry.terraform.io/providers/integrations/github/latest/docs/resources/membership | ||
|
|
||
| ## User-oriented view of changes |
| # Somewhere between general and IPLD-specific | ||
| These are at least things that apply to the 2024Q1 cleanup for ipld, libp2p, and ipfs orgs. Some of them can be generalized. | ||
|
|
||
| ## Archived repos cause clutter |
| * Generalizing this, maybe github-mgmt can be configured to do a merge of any specified files, which lets repo organizers decide if/how they want to break up their yaml file in general? | ||
| * Go the IPFS route and actually have an "ipfs-inactive" org. Archived projects can get moved there. This makes things very clear about the maintenance status and also solves the declutter problem. | ||
|
|
||
| ## Strip out access permissions to archived repos |
| 6. Other changes that the IPDX scripts picks up based on audit log inactivity and based on additional manual changes. (This is the diff we need people to be scrutinizing.) | ||
|
|
||
| # IPLD specifics | ||
| ## Org admins |
There was a problem hiding this comment.
This has been called out as the key item in the overarching tracking item in ipfs/ipfs#511
|
I think we are in a place to close this PR given the relevant info has been factored out into other linked issues and the larger tracking issue at ipfs/ipfs#511 I'll open a PR to reduce "org owners" by end of week (and linked from the tracking issue above). Another PR will happen next week to "archive" inactive users and teams.
|
This is in draft and hasn't been reviewed and shouldn't be taken as truth yet. This PR is a starting place so we can start framing the discussion around doing permission cleanup. There is also a corresponding FIL Slack channel #ip-stack-github-permissions-cleanup-2024q1
The current PR is what results from a base script that IPDX has. @mentions will go out with who is affected when it's ready to be looked at.
Also note that 2024Q1 permissions cleanup is starting with ipld, but we (@galargh, @BigLep) intend to take the learnings/process to libp2p and ipfs orgs afterwards.
Summary
Why do you need this?
What else do we need to know?
DRI: myself
Reviewer's Checklist