Github Permissions & Teams cleanup

Hello,

Over the years we’ve granted many people commit access to various repos that are used by the project. As time goes by, some people have moved on to other pursuits and are no longer actively involved in the project. Additionally, we’ve created some teams so we can give access to various repos, some of which are also outdated.
The last time we did any sort of clean-up as far as my search-fu could find was 4 years ago.
While we try to enforce 2-factor authentication for committers, having many people with commit access also increases the potential attack surface in case any committer’s account gets compromised.

Therefor, I propose that we agree on a certain period of inactivity (say, 6 months?) after which direct commit access will be removed from users and they will be removed from any groups that grant commit access.
In the case any such contributor ever decide they want to return to the project, they will be granted the same level of access they had previously without having to go through the formal nomination process again.

Once we finish this cleanup, we would likely find many teams that have no users left. Then we can decide if the specific team should be dropped (e.g. a team that was used for managing a deprecated plugin), or if we have a gap and we need to find new people who can maintain certain parts of the project.
For some repos we may also see they have no committers left, and will need to take similar action - call for maintainers or consider deprecation.

What are other people thinking?

9 Likes

I’ve also been thinking about a cleanup and I think this is needed. Some things I wonder about.

Do we first want to make a post what we’re going to change and give people a certain period to respond? A large discourse post may not be best since some memberships may be private. Perhaps an email with everyone in BCC? In that email we can also thank them for their contributions over the years.

Do we want to clean up Redmine while we’re at it? They’re likely the same people.

I’d be in favor of keeping teams at first. We can then evaluate which teams we still need. If they are both needed and empty, that’s a clear sign that there is a gap.

1 Like

Happy to hear this being discussed. As one of the mentioned people who has moved on, I’m 100% happy to have all my various permissions revoked :slight_smile:

1 Like

In this case, I’d be in favor of permissions revoking ASAP. We should definitely communicate this via blog post, twitter, perhaps community demo, but we shouldn’t be waiting too long. We just need to make sure we also communicate clearly, if “archived” maintainers want to become active again, they just need to reach out. With that, I don’t think we need to wait for more than a week or two.

6 months of inactivity sounds good to me, is there a way to automate some report for that? Or how we’d maintain this?

Taking a first stab at generating this list, I started with pulling out all people with commit access across our two GH organizations (and pardon my ugly code):

require 'octokit'
require 'set'

ORGS = %w[theforeman katello].freeze

client = Octokit::Client.new(access_token: '[redacted]')
client.auto_paginate = true
committers = Set.new

ORGS.each do |org|
  client.repos(org).map(&:full_name).each do |repo|
    puts "Processing repo #{repo}"
    committers |= client.collaborators(repo).select { |u| u[:permissions][:push] || u[:permissions][:admin] }.map(&:login)
  end
end

which gave me 108 committers.
To narrow down the list that needs to be checked, I generated a list of active users using the GH user_event API (which is unfortunately limited to the last 90 days, and max 100 events afaict):

active = Set.new
committers.each do |committer|
  active.add(committer) if client.user_events(committer).any?{|e| e.repo.name.start_with?('theforeman/') || e.repo.name.start_with?('katello/')}
end

Which gave me 50 people who have interacted with one of our repos recently.

That leaves us with 58 potentially inactive committers to go over and check for their activity manually, unless someone has an idea of how to do that programmatically.

1 Like

Looks like repo name is case sensitive and Katello uses a capital K. Fixing the check to:

active = Set.new
committers.each do |committer|
  active.add(committer) if client.user_events(committer).any?{|e| e.repo.name.start_with?('theforeman/') || e.repo.name.start_with?('Katello/')}
end

now gives 52 active committers (looks like only 2 Katello committers had no activity on any foreman repo recently!). 56 more to go.

As an initial pass I looked at the list for people who are no longer involved with the project as far as I know, leading to the following 21 users who I’ll remove access for right away. If any of them wish to restore their commit access, please contact me or any of the other maintainers:

akofink
apuntamb
bastilian
bronhaim
cfouant
daviddavis
dLobatog
dmitri-d
gnurag
GregSutcliffe
iNecas
kgaikwad
mbacovsky
sean797
shiramax
shlomizadok
sideangleside
stbenjam
terezanovotna
thomasmckay
tstrachota

Additionally, I’ve revoked access from a couple of bot accounts - hound-ci and centos-ci.
We are now down to 85 committers, 33 of whom haven’t been recently active.

4 Likes

I’ve cleaned up 11 teams that were used to manage archived repos in the foreman organization, and we are now down to 79 committers. I’ll continue gradually cleaning up this list, as most of the remaining people require individual action to check their history and find the latest contribution.

The cleanup has been completed. We are now down to 55 committers across the two orgs. There are many teams that have no members currently, meaning repos with no maintainers.
While going over the various permissions and repos, I also noticed that there are many repos which have not had any commits in over a year - meaning they are likely unmaintained. It may be a good idea to follow up with going over these repos and consider archiving them if maintainers cannot be found.

1 Like