Skip to content

Commit

Permalink
Various improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
infinisil committed Oct 3, 2024
1 parent 237cad3 commit aaf1d6b
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 111 deletions.
23 changes: 19 additions & 4 deletions .github/OWNERS
Original file line number Diff line number Diff line change
@@ -1,4 +1,19 @@
# Like CODEOWNERS, but manually implemented with .github/workflows/codeowners.yml
# Allows requesting reviews from people without write access.
# And prevents mass pings when the base branch is wrong.
# CODEOWNERS is intended to be migrated to this file soon.
#
# Currently unused! Use CODEOWNERS for now, see workflows/codeowners.yml
#
####################
#
# This file is used to describe who owns what in this repository.
# Users/teams will get review requests for PRs that change their files.
#
# This file does not replace `meta.maintainers`
# but is instead used for other things than derivations and modules,
# like documentation, package sets, and other assets.
#
# This file uses the same syntax as the natively supported CODEOWNERS file,
# see https://help.github.com/articles/about-codeowners/ for documentation.
# However it comes with some notable differences:
# - There is no need for user/team listed here to have write access.
# - No reviews will be requested for PRs that target the wrong base branch.
#
# Processing of this file is implemented in workflows/codeowners.yml
61 changes: 36 additions & 25 deletions .github/workflows/codeowners.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
name: Codeowners

on:
pull_request_target:
types: [opened, ready_for_review, synchronize, reopened, edited]

env:
OWNERS_FILE: .github/OWNERS

# This workflow depends on a GitHub App with the following permissions:
# - Repository > Administration: read-only
# - Organization > Members: read-only
Expand All @@ -15,27 +8,42 @@ env:
# the OWNER_APP_ID repository variable needs to be set
# the OWNER_APP_PRIVATE_KEY repository secret needs to be set

on:
pull_request_target:
types: [opened, ready_for_review, synchronize, reopened, edited]

env:
# TODO: Once confirmed that this works by seeing that the action would request
# reviews from the same people (or refuse for wrong base branches),
# move all entries from CODEOWNERS to OWNERS and change this value here
# OWNERS_FILE: .github/OWNERS
OWNERS_FILE: .github/CODEOWNERS

jobs:
check-owners:
# Check that code owners is valid
check:
name: Check
runs-on: ubuntu-latest
steps:
- uses: cachix/install-nix-action@ba0dd844c9180cbf77aa72a116d6fbc515d0e87b # v27
- uses: cachix/install-nix-action@9f70348d77d0422624097c4b7a75563948901306 # v29

- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
# Important: Because we use pull_request_target, this checks out the base branch of the PR, not the PR itself.
# We later build and run code from the base branch with access to secrets,
# so it's important this is not the PRs code.
- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
with:
path: base

- name: Build codeowners validator
run: nix-build base/ci -A codeownersValidator

- uses: actions/create-github-app-token@v1
- uses: actions/create-github-app-token@5d869da34e18e7287c1daad50e0b8ea0f506ce69 # v1.11.0
id: app-token
with:
app-id: ${{ vars.OWNER_APP_ID }}
private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }}

- uses: actions/checkout@v4
- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
with:
ref: refs/pull/${{ github.event.number }}/merge
path: pr
Expand All @@ -50,26 +58,29 @@ jobs:
# Set this to "notowned,avoid-shadowing" to check that all files are owned by somebody
EXPERIMENTAL_CHECKS: "avoid-shadowing"

# Request reviews from code owners
request:
name: Request
runs-on: ubuntu-latest
# Don't trigger on draft PRs
if: ${{ ! github.event.pull_request.draft }}
steps:
- uses: cachix/install-nix-action@ba0dd844c9180cbf77aa72a116d6fbc515d0e87b # v27
- uses: cachix/install-nix-action@9f70348d77d0422624097c4b7a75563948901306 # v29

- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
# Important: Because we use pull_request_target, this checks out the base branch of the PR, not the PR head.
# This is intentional, because we need to request the review of owners as declared in the base branch.
- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0

- name: Build review request package
run: nix-build ci -A requestReviews
- uses: actions/create-github-app-token@5d869da34e18e7287c1daad50e0b8ea0f506ce69 # v1.11.0
id: app-token
with:
app-id: ${{ vars.OWNER_APP_ID }}
private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }}

- uses: actions/create-github-app-token@v1
id: app-token
with:
app-id: ${{ vars.OWNER_APP_ID }}
private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }}
- name: Build review request package
run: nix-build ci -A requestReviews

- name: Request reviews
run: ./result/bin/request-reviews.sh ${{ github.repository }} ${{ github.event.number }} "$OWNERS_FILE"
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
- name: Request reviews
run: result/bin/request-reviews.sh ${{ github.repository }} ${{ github.event.number }} "$OWNERS_FILE"
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
7 changes: 6 additions & 1 deletion ci/codeowners-validator/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,13 @@ buildGoModule {
url = "https://github.com/mszostok/codeowners-validator/compare/f3651e3810802a37bd965e6a9a7210728179d076...840eeb88b4da92bda3e13c838f67f6540b9e8529.patch";
hash = "sha256-t3Dtt8SP9nbO3gBrM0nRE7+G6N/ZIaczDyVHYAG/6mU=";
})
./owners-file-name.patch
# Undoes part of the above PR: We don't want to require write access
# to the repository, that's only needed for GitHub's native CODEOWNERS.
# Furthermore, it removes an unneccessary check from the code
# that breaks tokens generated for GitHub Apps.
./permissions.patch
# Allows setting a custom CODEOWNERS path using the OWNERS_FILE env var
./owners-file-name.patch
];
postPatch = "rm -r docs/investigation";
vendorHash = "sha256-R+pW3xcfpkTRqfS2ETVOwG8PZr0iH5ewroiF7u8hcYI=";
Expand Down
64 changes: 34 additions & 30 deletions ci/request-reviews/get-reviewers.sh
Original file line number Diff line number Diff line change
@@ -1,50 +1,50 @@
#!/usr/bin/env bash

# This script gets the list of codeowning users and teams based on a codeowners file
# from a base commit and all files that have been changed since then.
# The result is suitable as input to the GitHub REST API call to request reviewers for a PR.
# This can be used to simulate the automatic codeowner review requests
# Get the code owners of the files changed by a PR,
# suitable to be consumed by the API endpoint to request reviews:
# https://docs.github.com/en/rest/pulls/review-requests?apiVersion=2022-11-28#request-reviewers-for-a-pull-request

set -euo pipefail

tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' exit
log() {
echo "$@" >&2
}

if (( "$#" < 3 )); then
echo "Usage: $0 LOCAL_REPO BASE_REF HEAD_REF OWNERS_FILE" >&2
if (( "$#" < 5 )); then
log "Usage: $0 GIT_REPO BASE_REF HEAD_REF OWNERS_FILE PR_AUTHOR"
exit 1
fi
localRepo=$1

gitRepo=$1
baseRef=$2
headRef=$3
ownersFile=$4
prAuthor=$5

readarray -d '' -t touchedFiles < \
<(
# The names of all files, null-delimited, starting from HEAD, stopping before the base
git -C "$localRepo" diff --name-only -z --merge-base "$baseRef" "$headRef" |
# Remove duplicates
sort -z --unique
)
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' exit

#echo "These files were touched: ${touchedFiles[*]}" >&2
readarray -t touchedFiles < \
<(git -C "$gitRepo" diff --name-only --merge-base "$baseRef" "$headRef")

log "This PR touches ${#touchedFiles[@]} files"

# Get the owners file from the base, because we don't want to allow PRs to
# remove code owners to avoid pinging them
git -C "$localRepo" show "$baseRef":"$ownersFile" > "$tmp"/codeowners
git -C "$gitRepo" show "$baseRef":"$ownersFile" > "$tmp"/codeowners

# Associative array, where the key is the team/user, while the value is "1"
# This makes it very easy to get deduplication
# Associative arrays with the team/user as the key for easy deduplication
declare -A teams users

for file in "${touchedFiles[@]}"; do
read -r file owners <<< "$(codeowners --file "$tmp"/codeowners "$file")"
result=$(codeowners --file "$tmp"/codeowners "$file")

read -r file owners <<< "$result"
if [[ "$owners" == "(unowned)" ]]; then
#echo "File $file doesn't have an owner" >&2
log "File $file is unowned"
continue
fi
#echo "Owner of $file is $owners" >&2
log "File $file is owned by $owners"

# Split up multiple owners, separated by arbitrary amounts of spaces
IFS=" " read -r -a entries <<< "$owners"
Expand All @@ -53,27 +53,31 @@ for file in "${touchedFiles[@]}"; do
# GitHub technically also supports Emails as code owners,
# but we can't easily support that, so let's not
if [[ ! "$entry" =~ @(.*) ]]; then
echo -e "\e[33mCodeowner \"$entry\" for file $file is not valid: Must start with \"@\"\e[0m" >&2
warn -e "\e[33mCodeowner \"$entry\" for file $file is not valid: Must start with \"@\"\e[0m" >&2
# Don't fail, because the PR for which this script runs can't fix it,
# it has to be fixed in the base branch
continue
fi
# The first regex match is everything after the @
entry=${BASH_REMATCH[1]}
if [[ "$entry" =~ .*/(.*) ]]; then
# Only teams have a /
teams[${BASH_REMATCH[1]}]=1
# Teams look like $org/$team, where we only need $team for the API
# call to request reviews from teams
teams[${BASH_REMATCH[1]}]=
else
# Everything else is a user
# But cannot request a review from the author
if [[ "$entry" != "$prAuthor" ]]; then
users[$entry]=1
fi
users[$entry]=
fi
done

done

# Cannot request a review from the author
if [[ -v users[$prAuthor] ]]; then
log "One or more files are owned by the PR author, ignoring"
unset 'users[$prAuthor]'
fi

# Turn it into a JSON for the GitHub API call to request PR reviewers
jq -n \
--arg users "${!users[*]}" \
Expand Down
55 changes: 41 additions & 14 deletions ci/request-reviews/request-reviews.sh
Original file line number Diff line number Diff line change
@@ -1,49 +1,76 @@
#!/usr/bin/env bash

# Requests reviews for a PR after verifying that the base branch is correct

set -euo pipefail
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' exit
SCRIPT_DIR=$(dirname "$0")

log() {
echo "$@" >&2
}

if (( $# < 3 )); then
log "Usage: $0 GITHUB_REPO PR_NUMBER OWNERS_FILE"
exit 1
fi
baseRepo=$1
prNumber=$2
ownersFile=$3

log "Fetching PR info"
prInfo=$(gh api \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
"/repos/$baseRepo/pulls/$prNumber")

baseBranch=$(jq -r .base.ref <<< "$prInfo")
log "Base branch: $baseBranch"
prRepo=$(jq -r .head.repo.full_name <<< "$prInfo")
log "PR repo: $prRepo"
prBranch=$(jq -r .head.ref <<< "$prInfo")
log "PR branch: $prBranch"
prAuthor=$(jq -r .user.login <<< "$prInfo")
log "PR author: $prAuthor"

headRef=refs/remotes/fork/pr
extraArgs=()
if pwdRepo=$(git rev-parse --show-toplevel 2>/dev/null); then
# Speedup for local runs
extraArgs+=(--reference-if-able "$pwdRepo")
fi

log "Fetching Nixpkgs commit history"
git clone --bare --filter=tree:0 --no-tags --origin upstream "${extraArgs[@]}" https://github.com/"$baseRepo".git "$tmp"/nixpkgs.git

git clone --bare --filter=tree:0 --no-tags --origin upstream https://github.com/"$baseRepo".git "$tmp"/nixpkgs.git
log "Fetching the PR commit history"
# Fetch the PR
git -C "$tmp/nixpkgs.git" remote add fork https://github.com/"$prRepo".git
# Make sure we only fetch the commit history, nothing else
git -C "$tmp/nixpkgs.git" config remote.fork.promisor true
git -C "$tmp/nixpkgs.git" config remote.fork.partialclonefilter tree:0

# This should not conflict with any refs in Nixpkgs
headRef=refs/remotes/fork/pr
# Only fetch into a remote ref, because the local ref namespace is used by Nixpkgs, don't want any conflicts
git -C "$tmp/nixpkgs.git" fetch --no-tags fork "$prBranch":"$headRef"


log "Checking correctness of the base branch"
"$SCRIPT_DIR"/verify-base-branch.sh "$tmp/nixpkgs.git" "$headRef" "$baseRepo" "$baseBranch" "$prRepo" "$prBranch"

reviewersJSON=$("$SCRIPT_DIR"/get-reviewers.sh "$tmp/nixpkgs.git" "$baseBranch" "$headRef" "$ownersFile" "$prAuthor")
log "Getting code owners to request reviews from"
"$SCRIPT_DIR"/get-reviewers.sh "$tmp/nixpkgs.git" "$baseBranch" "$headRef" "$ownersFile" "$prAuthor" > "$tmp/reviewers.json"

echo "$reviewersJSON"
log "Requesting reviews from: $(<"$tmp/reviewers.json")"

if ! response=$(curl -LsS --fail-with-body \
-X POST \
-H "Accept: application/vnd.github+json" \
-H "Authorization: Bearer $(gh auth token)" \
-H "X-GitHub-Api-Version: 2022-11-28" \
https://api.github.com/repos/"$baseRepo"/pulls/"$prNumber"/requested_reviewers \
-d "$reviewersJSON"); then
echo "Failed to request reviews: $response"
exit 1
if ! response=$(gh api \
--method POST \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
"/repos/$baseRepo/pulls/$prNumber/requested_reviewers" \
--input "$tmp/reviewers.json"); then
log "Failed to request reviews: $response"
exit 1
fi

log "Successfully requested reviews"
Loading

0 comments on commit aaf1d6b

Please sign in to comment.