-
Notifications
You must be signed in to change notification settings - Fork 3
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. | ||
[email protected] | ||
[email protected] | ||
[email protected] | ||
[email protected] | ||
bess | ||
bmquinn | ||
[email protected] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not familiar with this one. Maybe someone else can vouch. |
||
[email protected] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you doing Samvera-y things, @jenlindner? (And, hi!) |
||
[email protected] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that |
||
[email protected] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hey @jkeck, do you want keys to the samvera castle or nah There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hey @jrochkind , do you want keys to the samvera castle or nah There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
[email protected] | ||
[email protected] | ||
[email protected] | ||
[email protected] | ||
[email protected] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] |
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: - [email protected]
- [email protected]
# ... 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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
@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 = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}..." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
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 there may be a typo in this second sentence. How about: