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

WIP - Reworking the grant_revoke_gem_authority script #47

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,9 @@ and change roles/functions/laptops.
## Rake Tasks

There exist rake tasks that can be used to propogate some of these templates.

## Scripts

* [./script/grant_revoke_gem_authority.rb](./script/grant_revoke_gem_authority.rb)
is a convenience script to synchronize authorship of Samvera
rubygems.
39 changes: 39 additions & 0 deletions data/definitive-rubygems-authors-list.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# This file has the definitive and declarative list of Samvera ruby
# gems owners. Each line must either be either an email address
# of the associated Rubygems profile.
Comment on lines +2 to +3
Copy link
Member

Choose a reason for hiding this comment

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

I think there may be a typo in this second sentence. How about:

Suggested change
# gems owners. Each line must either be either an email address
# of the associated Rubygems profile.
# gems owners. Each line must either be a rubygems.org email address
# or username.

[email protected]
[email protected]
[email protected]
[email protected]
bess
bmquinn
[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Recommend removing this address as @carolyncole no longer works at Penn State. If she still works w/ Samvera-y things, she can supply her new rubygems.org username/email address, I reckon. (Hi, Carolyn! Wishing you well!)

[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

hey @cbeer, do you want keys to the samvera castle or nah

cjcolvar
[email protected]
[email protected]
[email protected]
[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Not familiar with this one. Maybe someone else can vouch.

[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this one, and revoke his access to our gems.

elrayle
[email protected]
[email protected]
[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Are you doing Samvera-y things, @jenlindner? (And, hi!)

[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

I see that [email protected] (DCE's @little9) isn't in this list, but perhaps he should be.

[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

hey @jkeck, do you want keys to the samvera castle or nah

Copy link
Member

@jkeck jkeck Jun 15, 2020

Choose a reason for hiding this comment

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

I find it useful to be able to contribute to some gems in samvera land that we depend on from time to time (although that seems to be fewer and farther between). I don't particularly mind, but if you all are trying to clean things up, feel free to 🔪 me.

Copy link
Member

Choose a reason for hiding this comment

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

Nope, I think it's fine for you to retain access. Thought it prudent to ask first! Thanks, Jessie.

[email protected]
[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

hey @jrochkind , do you want keys to the samvera castle or nah

Choose a reason for hiding this comment

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

Thanks! Same answer as jkeck! I find it useful to contribute to some gems in samvera we depend on from time to time, but if you are trying to clean things up feel free to get rid of me! (from rubygem owners; I guess from samvera github commit auth is another thing?)

Copy link
Member

Choose a reason for hiding this comment

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

@jrochkind Same response from me, then: Nope, I think it's fine for you to retain access. Thought it prudent to ask first! Thanks, J-Rock. :)

[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

I suspect Stroop's ruby gem days are behind him.

[email protected]
[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Does Kyle still do Samvera things, @hortongn @scherztc ?

[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

hey @ndushay , do you want keys to the samvera castle or nah

[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Joe no longer works in the Samvera community, so you can remove this address.

[email protected]
[email protected]
[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Josh G. no longer works in the Samvera community, so you can remove this address.

[email protected]
[email protected]
168 changes: 72 additions & 96 deletions script/grant_revoke_gem_authority.rb
Original file line number Diff line number Diff line change
@@ -1,166 +1,142 @@
# frozen_string_literal: true

# The purpose of this script is to manage the authors of the rubygems
# managed by the Samvera community.
#
# 1. Get a personal access token from GitHub (https://github.com/settings/tokens)
# with the following scopes enabled:
# * public_repo
# * read:org
# * user:email
# 2. Set an ENV variable named 'GITHUB_SAMVERA_TOKEN' containing your token
# 3. Then run this script:
# $ ruby ./script/grant_revoke_gem_authority.rb
# $ bundle exec ruby ./script/grant_revoke_gem_authority.rb
#
# By default the above script will iterate over samvera and
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious about what is driving this. Why not do -deprecated automatically as well? If rate-limiting concerns, ok, no problem.

# samvera-labs repositories. If you set the WITH_DEPRECATED
# environment variable, then the script will include
# samvera-deprecated organization.
#
# To also revoke ownership from users whose email addresses are not in the list:
# $ WITH_REVOKE=true ruby ./script/grant_revoke_gem_authority.rb
#
# $ WITH_REVOKE=true bundle exec ruby ./script/grant_revoke_gem_authority.rb
#
# To print more information on what the script is doing:
#
# $ VERBOSE=true ruby ./script/grant_revoke_gem_authority.rb

require 'github_api'
require 'open3'

AUTHORIZATION_TOKEN = ENV['GITHUB_SAMVERA_TOKEN'] || raise("GitHub authorization token was not found in the GITHUB_SAMVERA_TOKEN environment variable")
ORGANIZATION_NAMES = ['samvera', 'samvera-labs', 'samvera-deprecated']
# Some GitHub user instances do not have an email address defined,
# so start with the prior list of addresses (registered with Rubygems.org)
KNOWN_COMMITTER_EMAIL_ADDRESSES = {
'aaron-collier' => '[email protected]',
'awead' => "[email protected]",
'atz' => '[email protected]',
'barmintor' => "[email protected]",
'bess' => "[email protected]",
'cam156' => "[email protected]",
'cbeer' => "[email protected]",
'cjcolvar' => "[email protected]",
'coblej' => "[email protected]",
'DanCoughlin' => "[email protected]",
'darrenleeweber' => '[email protected]',
'dchandekstark' => "[email protected]",
'dheles' => '[email protected]',
'dunn' => '[email protected]',
'elrayle' => '[email protected]',
'escowles' => '[email protected]',
'grosscol' => '[email protected]',
'hackmastera' => '[email protected]',
'hortongn' => '[email protected]',
'jeremyf' => "[email protected]",
'jenlindner' => '[email protected]',
'jkeck' => "[email protected]",
'jpstroop' => "[email protected]",
'jrgriffiniii' => '[email protected]',
'jrochkind' => '[email protected]',
'jcoyne' => "[email protected]",
'lawhorkl' => '[email protected]',
'mjgiarlo' => "[email protected]",
'mark-dce' => "[email protected]",
'mbklein' => "[email protected]",
'mkorcy' => "[email protected]",
'ndushay' => "[email protected]",
'tpendragon' => "[email protected]",
'carrickr' => '[email protected]',
'no_reply' => '[email protected]',
'revgum' => '[email protected]',
'rmkadel' => '[email protected]',
'randalldfloyd' => '[email protected]'
}
# Some GitHub repositories are named differently from their gems

if ENV.fetch("WITH_DEPRECATED", false)
ORGANIZATION_NAMES = ['samvera', 'samvera-labs', 'samvera-deprecated']
else
ORGANIZATION_NAMES = ['samvera', 'samvera-labs']
end

# Some GitHub repositories are named differently from their gems. We
# could read each repositories .gemspec to determine its
# name. However, this is a far quicker short-cut.
KNOWN_MISMATCHED_GEM_NAMES = {
'active_fedora' => 'active-fedora',
'fcrepo-admin' => 'fcrepo_admin',
'questioning_authority' => 'qa'
}

# GitHub repositories with matching gems that aren't from Samvera
FALSE_POSITIVES = [
'hypatia',
'rdf-vocab',
'lerna',
'hydrangea',
'valkyrie'
'hydrangea'
]

# Gems that do not have their own GitHub repositories
HANGERS_ON = [
'hydra-core',
'hydra-access-controls',
'sufia-models',
'curation_concerns-models'
]
# Email addresses that are known not to be registered at rubygems.org
SKIP_EMAILS = [
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]'
]

VERBOSE = ENV.fetch('VERBOSE', false)

puts "(Hang in there! This script takes a couple minutes to run.)"
# See https://guides.rubygems.org/rubygems-org-api/#rate-limits I'm
# opting for the slower 0.2 so as to not come close to the rate limit.
GEM_SLEEP_INTERVAL = 0.2

github = Github.new(oauth_token: AUTHORIZATION_TOKEN, auto_pagination: true)
$stdout.puts "Hang in there! This script takes a couple minutes to run."

# Get the ID of the GitHub-provided "Owners" team from the samvera org
# We don't currently grant gem ownership to the folks in the other two orgs
# (This just preserves the prior behavior.)
owner_team_id = github.orgs.teams.list(org: 'samvera').select { |team| team.name == 'Admins' }.first.id
owners = github.orgs.teams.list_members(owner_team_id)
# Start with the prior (known to work) list of email addresses
committer_map = KNOWN_COMMITTER_EMAIL_ADDRESSES.dup
owners.each do |owner|
user = github.users.get(user: owner.login)
# Move along if the user doesn't have an email addy or if there's already an entry in the map
next if !user.respond_to?(:email) || user.email.nil? || user.email.empty? || !committer_map[user.login].nil?
committer_map[user.login] = user.email
end
committer_emails = committer_map.values.sort.uniq
github = Github.new(oauth_token: AUTHORIZATION_TOKEN, auto_pagination: true)

# Keep track of things
@errors = []
@bogus_gem_names = []
@gem_names = HANGERS_ON

def exists?(name)
system("gem owner #{name} > /dev/null 2>&1")
end

def replace_known_mismatch(name)
KNOWN_MISMATCHED_GEM_NAMES.fetch(name, name)
end
@definitive_committers = File.read(
File.expand_path("../../data/definitive-rubygems-authors-list.txt", __FILE__)
).split("\n").map { |line| line.sub(/ *#.*$/, "") }.select { |line| !line.empty? }
Comment on lines +80 to +81
Copy link
Member

Choose a reason for hiding this comment

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

I don't feel strongly about this, but a different way to deal with this is to store the list of gem authors as a simple yaml file, and then the YAML parser will deal with the empty lines and comments for us. Something like:

an advantage of this approach is we can validate the YAML in CI (which I see this project doesn't have... but it could?).


ORGANIZATION_NAMES.each do |org_name|
github.repos.list(org: org_name).each do |repo|
name = replace_known_mismatch(repo.name)
if exists?(name)
# Replace known mismatched names
name = KNOWN_MISMATCHED_GEM_NAMES.fetch(repo.name, repo.name)

# Let's move on to any gem that we know will be a false positive.
next if FALSE_POSITIVES.include?(name)

# TODO: We are twice calling `gem owner` for information. We could
# rework the logic to only call it once.
sleep(GEM_SLEEP_INTERVAL)
if system("gem owner #{name} > /dev/null 2>&1")
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, what is the purpose of this call? To flag failures for stashing in @bogus_gem_names?

@gem_names << name
else
@bogus_gem_names << repo.full_name
end
end
end

def gem_owner_with_error_check(gemname, params)
command = "gem owner #{gemname} #{params}"
puts "running: #{command}" if VERBOSE
OWNERSHIP_ACTIONS = {
Copy link
Member

Choose a reason for hiding this comment

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

I have a slight preference for declaring constants together near the top of a file but I'm not sure this file already respects this preference, so 🤷‍♂️

add: '-a',
remove: '-r'
}

# @param gemname [String]
# @param profile [String] either an email associated with Rubygems or the
# Rubygems.org profile name
# @param action [Symbol] either :add or :remove (see OWNERSHIP_ACTIONS)
def modify_gem_ownership(gemname:, profile:, action:)
switch = OWNERSHIP_ACTIONS.fetch(action)
sleep(GEM_SLEEP_INTERVAL)
command = "gem owner #{gemname} #{switch} #{profile}"
$stdout.puts "running: #{command}" if VERBOSE
Open3.popen3(command) do |stdin, stdout, stderr, wait_thr|
@errors << "#{gemname} #{params}: #{stdout.read.chomp}" unless wait_thr.value.success?
@errors << "#{command}: #{stdout.read.chomp}" unless wait_thr.value.success?
end
end

@gem_names.reject { |gemname| FALSE_POSITIVES.include?(gemname) }.sort.each do |gemname|

@gem_names.sort.each do |gemname|
$stdout.puts "Updating #{gemname}..."
Copy link
Member

Choose a reason for hiding this comment

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

👍


sleep(GEM_SLEEP_INTERVAL)
current_committers = `gem owner #{gemname} | grep -e ^-`.split("\n")
current_committers.collect! { |cc| cc.sub(/^.\s+/,'')}

committers_to_add = @definitive_committers - current_committers
committers_to_add.each do |profile|
modify_gem_ownership(gemname: gemname, profile: profile, action: :add)
end

if ENV.fetch('WITH_REVOKE', false)
committers_to_remove = current_committers - committer_emails
remove_params = committers_to_remove.map { |email| "-r #{email}" }.join(' ')
gem_owner_with_error_check(gemname, remove_params)
committers_to_remove.each do |profile|
modify_gem_ownership(gemname: gemname, profile: profile, action: :remove)
end
end

committers_to_add = committer_emails - current_committers - SKIP_EMAILS
add_params = committers_to_add.map { |email| "-a #{email}" }.join(' ')
gem_owner_with_error_check(gemname, add_params)
end

if @bogus_gem_names.any?
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it's worth spitting this out. If not, can also remove an extra gem owner call above, possibly? It seemed a good idea at some point, buuuuuut...

Expand Down