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

Reduce org owners/admins #189

Merged
merged 13 commits into from
Feb 16, 2024
Merged

Reduce org owners/admins #189

merged 13 commits into from
Feb 16, 2024

Conversation

BigLep
Copy link
Contributor

@BigLep BigLep commented Feb 12, 2024

Summary

This aligns with the "reduce org owners" step listed in ipfs/ipfs#511.

This is the first step of wider 2024Q1 permissions cleanup.

Why do you need this?

Github org safety. See ipfs/ipfs#511 for more info.

Timeline

Reviewer's Checklist

  • It is clear where the request is coming from (if unsure, ask)
  • All the automated checks passed
  • The YAML changes reflect the summary of the request
  • The Terraform plan posted as a comment reflects the summary of the request

@BigLep BigLep self-assigned this Feb 12, 2024
@BigLep BigLep requested review from a team as code owners February 12, 2024 17:01
@BigLep BigLep requested a review from galargh February 12, 2024 17:01
Copy link
Contributor

github-actions bot commented Feb 12, 2024

Before merge, verify that all the following plans are correct. They will be applied as-is after the merge.

Terraform plans

ipfs

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  + create
  ~ update in-place
  - destroy

Terraform will perform the following actions:

  # github_membership.this["aschmahmann"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:aschmahmann"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["autonome"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:autonome"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["biglep"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:BigLep"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["cwaring"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:cwaring"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["daviddias"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:daviddias"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["hsanjuan"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:hsanjuan"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["jbenet"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:jbenet"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["lidel"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:lidel"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["momack2"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:momack2"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["olizilla"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:olizilla"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["stebalien"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:Stebalien"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_membership.this["whyrusleeping"] will be updated in-place
  ~ resource "github_membership" "this" {
        id       = "ipfs:whyrusleeping"
      ~ role     = "admin" -> "member"
        # (2 unchanged attributes hidden)
    }

  # github_team.this["github-mgmt stewards"] will be updated in-place
  ~ resource "github_team" "this" {
      + create_default_maintainer = false
      ~ description               = "Users that are effectively org admins" -> "Users that are effectively org owners/admins"
        id                        = "6421993"
        name                      = "github-mgmt stewards"
        # (5 unchanged attributes hidden)
    }

  # github_team_membership.this["github-mgmt stewards:achingbrain"] will be destroyed
  # (because key ["github-mgmt stewards:achingbrain"] is not in for_each map)
  - resource "github_team_membership" "this" {
      - etag     = "W/\"4900bd94633af471623fb2f3918330ed02d6f03a0e08984d5a436c713551f051\"" -> null
      - id       = "6421993:achingbrain" -> null
      - role     = "member" -> null
      - team_id  = "6421993" -> null
      - username = "achingbrain" -> null
    }

  # github_team_membership.this["github-mgmt stewards:aschmahmann"] will be updated in-place
  ~ resource "github_team_membership" "this" {
        id       = "6421993:aschmahmann"
      ~ role     = "maintainer" -> "member"
        # (3 unchanged attributes hidden)
    }

  # github_team_membership.this["github-mgmt stewards:biglep"] will be destroyed
  # (because key ["github-mgmt stewards:biglep"] is not in for_each map)
  - resource "github_team_membership" "this" {
      - etag     = "W/\"1eaac4caf12c4b1f421bc332559aee65e32e428ae500be78b3c822f176506c66\"" -> null
      - id       = "6421993:BigLep" -> null
      - role     = "maintainer" -> null
      - team_id  = "6421993" -> null
      - username = "BigLep" -> null
    }

  # github_team_membership.this["github-mgmt stewards:lidel"] will be updated in-place
  ~ resource "github_team_membership" "this" {
        id       = "6421993:lidel"
      ~ role     = "maintainer" -> "member"
        # (3 unchanged attributes hidden)
    }

  # github_team_membership.this["github-mgmt stewards:mishmosh"] will be created
  + resource "github_team_membership" "this" {
      + etag     = (known after apply)
      + id       = (known after apply)
      + role     = "member"
      + team_id  = "6421993"
      + username = "mishmosh"
    }

  # github_team_membership.this["github-mgmt stewards:stebalien"] will be created
  + resource "github_team_membership" "this" {
      + etag     = (known after apply)
      + id       = (known after apply)
      + role     = "member"
      + team_id  = "6421993"
      + username = "stebalien"
    }

Plan: 2 to add, 15 to change, 2 to destroy.

@BigLep
Copy link
Contributor Author

BigLep commented Feb 12, 2024

@cwaring
@daviddias
@hsanjuan
@jbenet
@momack2
@olizilla
@vesahc
@whyrusleeping

I'm @mentioning you to inform you that your ipfs github "org ownership" permissions will be removed as part of a ipfs/ipfs#511 unless you respond back with your use-case for having these broad permissions by 2024-02-14. You will still be a member of the ipfs github org, retain your existing direct github repo permissions or team permissions, and be able to make permission requests through https://github.com/ipfs/github-mgmt.

The current plan is to merge this change on Wednesday, 2024-02-14.

That said, this isn't a one-way door. If we got this wrong or you don't see the notification after the fact, a new PR can be created to fix permissions.

Thanks and let me know if you have any questions or concerns.

@vesahc
Copy link

vesahc commented Feb 12, 2024

Hi @BigLep, thanks for the heads up. I do have a slight preference to keep perms until Fleek migrations are completed due to the amount of repos involved and needing the ability to manage the Fleek app. Would also be more than happy to hand this off though 😅

@BigLep
Copy link
Contributor Author

BigLep commented Feb 13, 2024

Hi @BigLep, thanks for the heads up. I do have a slight preference to keep perms until Fleek migrations are completed due to the amount of repos involved and needing the ability to manage the Fleek app. Would also be more than happy to hand this off though 😅

@vesahc: doh - good point. I don't think anyone wants to put friction in the way of the Fleek stuff. I have kept you back as an org owner.

@momack2
Copy link

momack2 commented Feb 13, 2024

Similar to libp2p - would advocate to keep Juan as an org owner. Would also suggest @mishmosh given her role on IPFS

@mishmosh
Copy link

Would also suggest @mishmosh given her role on IPFS

This would be helpful for me/the project.

@BigLep
Copy link
Contributor Author

BigLep commented Feb 14, 2024

@momack2:

Similar to libp2p - would advocate to keep Juan as an org owner.

I removed Juan because I view having these superpowers as a burden. He hasn't had ipfs github org activity in the last 6 months, and thus I assume he's not getting utility from carrying the burden. In addition, his account would be a prime account for attackers to go after, and this change reduces the blast radius in the unfortunate-I-never-hope-it-happens event that his account does get attacked.

My general mindset: Github org ownership permissions are very powerful, and they enable someone to do things in the UI without review that can have significant consequences. Reducing the ownership set to a small set of people helps ensure:

  1. more actions go through github-mgmt (which has review) and
  2. that the few humans who are doing things outside of github-mgmt are also the humans who live in the github tools more, thus potentially more aware of how to wield them safely. (This is sort of similar to how AWS service teams don't allow Management Console/UI access to any of the production accounts except in "break glass" scenarios where others are in the loop and handholding together.)

Obviously Juan is the founder of the ipfs github org/project, and if he wants to have this permission level without having to "break glass" to get it, I don't expect anyone is going to oppose (beyond what I've done here) :)


@momack2 / @mishmosh:

Would also suggest @mishmosh given her role on IPFS

Having you as an org owner representatiing the IPFS Foundation makes sense. That said, is that a burden (per above) you want to carry? If you have been getting by fine without it, I'd encourage not having that permission level by default. A PR can be made to escalate your permissions if/when needed.


General notes:

  • I'm not the ultimate decider here. (I'm not sure who is.). I'm only putting up some resistance to double-check that this is really needed.
  • If this conversation doesn't resolve before 2/14, I think we're fine to move forward (merge as is) and followup with additions/changes. There's nothing here that can't be undone or amended.

@olizilla
Copy link
Member

your ipfs github "org ownership" permissions are being removed

A decent way to approach this would be to ask those who you deem burdened, to recuse themselves of the org owner role.

@BigLep
Copy link
Contributor Author

BigLep commented Feb 14, 2024

@olizilla

your ipfs github "org ownership" permissions are being removed

A decent way to approach this would be to ask those who you deem burdened, to recuse themselves of the org owner role.

Doh - yes, this wasn't phrased well by me.

I believe I should have phrased this as, "I'm @mentioning you to inform you that your ipfs github "org ownership" permissions will be removed as part of a ipfs/ipfs#511 unless you respond back with your use-case for having these broad permissions by 2024-02-14. You will still be a member of the ipfs github org, retain your existing direct github repo permissions or team permissions, and be able to make permission requests through https://github.com/ipfs/github-mgmt." I have updated my original text in #189 (comment)

The main thing I want to get across is that action will be taken unless someone responds. It should come across as there being an invitation to influence the action. Given this isn't a one-way door, I don't want this phrased as "I'll only do this if you give explicit approval". That will cause this to drag on for weeks or require a lot of chasing. Instead, a change in action (e.g., not removing someone) results because someone raises their hand saying they disagree with the proposal.

I agree my original phrasing didn't make it clear that there is an invitation to influence the plan. It came across too much "as things are decided and there's not much you can do about it". This certainly wasn't the intent - my bad. Thanks for the feedback.

@BigLep
Copy link
Contributor Author

BigLep commented Feb 14, 2024

Given the sub-par messaging by me, I'll leave this open until 2024-02-15 before merging.

github/ipfs.yml Outdated Show resolved Hide resolved
github/ipfs.yml Outdated
Comment on lines 8955 to 8957
# Intentionally don't have any "maintainers" so that additional membership is done through github-mgmt rather than the GitHub UI.
# That said, since most of these people are also "org owners" ("members.admin" above),
# they can still make changes in the UI.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have this loophole? ipfs/ipfs#511 (comment)

@BigLep
Copy link
Contributor Author

BigLep commented Feb 16, 2024

Based on ipfs/ipfs#511 (comment), I made more changes here, reducing the only true org admins to @galargh and @andyschwab-admin (and @vesahc temporarily).

Here is the summary:

Org members who are no longer org-admins and are not in the "github-mgmt stewards" team (thus a true reduction of permissions):

Org members who are no longer org-admins but are still in the "github-mgmt stewards" team (thus can still get to "org owner" mode if needed):

Org members who were on the "github-mgmt stewards" team but have been removed (thus have had a reduction in permissions):

  • @achingbrain 🆕 - I did this since there are already two IP Shipyard representatives)

New additions to the "github-mgmt stewards"

  • @mishmosh 🆕 - I did this given her role as the director of the IPFS Foundation being stood up and operationalized.

🆕 indicates changes as of 2024-02-15

There are comments in the PR explaining more about the decision-making. Feedback welcome. I am shooting to be able to merge this on Friday, 2024-02-16. (I will be away on leave starting 2024-02-17, but others like @galargh and @lidel can certainly engage here if we can't wrap this up by then.)

# 1. Director of the being-stood-up-in-2024 IPFS Foundation
- mishmosh
# Why @stebalien?
# 1. Not involved in the IPFS day-to-day currently, but has a lot of historical knowledge. Provides an informed outside perspective.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to specific individuals but is this a good enough reason?

Group membership here gives perms to create/delete/configure repos and mess with the GH org in general, if people aren't doing these essentially administrative tasks with reasonable frequency, why do they have the perms to do so?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair callout @achingbrain - makes sense.
I was biasing here to make sure we had some more organizational diversity which is why I pulled in the IPFS Foundation.
Folks on this "github-mgmt stewards" team also have access to all security incidents reported through github, which seems appropriate for the IPFS Foundation to have insight on.
Github only allows for one team to be "security manager" though.
Using "github-mgmt stewards" is a shortcut, but in thinking more, it should ideally be its own team with high overlap with "github-mgmt stewards".
Given I'm out of time in this season to push on this further, I'm going to propose we go with what we currently have as its still better than what was there before, and future improvements can be made later.

@BigLep
Copy link
Contributor Author

BigLep commented Feb 16, 2024

I'm merging, given we've had time on this to iterate and fine-tune, 👍's up in #189 (comment), @lidel support in ipfs/ipfs#511 (comment), that we're in a better place than we were before, and knowing further changes can be done in a followup if needed. (A phase 2 to this 2024Q1 permissions cleanup will likely start happening week of 2024-02-19 per ipfs/ipfs#511.)

@BigLep BigLep merged commit 3c85076 into master Feb 16, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants