From e173f231567e160def56076ba092aae93edf2d9d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 11 Jul 2024 19:20:04 +0200 Subject: [PATCH 1/3] Remove broken unprivileged owners code --- scripts/review-body.sh | 9 --------- 1 file changed, 9 deletions(-) diff --git a/scripts/review-body.sh b/scripts/review-body.sh index 0073227..fa4623b 100755 --- a/scripts/review-body.sh +++ b/scripts/review-body.sh @@ -5,8 +5,6 @@ set -euo pipefail # This script outputs the contents of the regular review issue, see ./github/workflows/review.yml -SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) - if (( $# != 2 )); then echo "Usage: $0 PATH OWNER/REPO" exit 1 @@ -75,10 +73,3 @@ listDir() { } listDir "" "" - -echo "" - -# Check that all code owners have write permissions -# `|| true` because this script fails when there are code owners without permissions, -# which is useful to fail PRs, but not here -bash "$SCRIPT_DIR"/unprivileged-owners.sh "$root" "$repo" || true From b49970ffe79a21176f74286d1856939171df78b6 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 11 Jul 2024 19:20:22 +0200 Subject: [PATCH 2/3] Only list doc dir in monthly reviews --- CONTRIBUTING.md | 5 +++-- scripts/review-body.sh | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 364b035..e3f56da 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,12 +8,13 @@ Thus, all organisational changes tracked in this repository should be proposed w and the changes should only to be implemented when the PR is merged. This may be done manually (e.g. by the person merging the PR) or automatically (e.g. using CD). -All files should have at least somebody in charge of keeping it up-to-date, which should be described with an entry in [CODEOWNERS](./.github/CODEOWNERS). Those people will be requested for a review and be given write access to the repository, see also [permissions of this repository](./doc/org-repo.md). +All files should have at least somebody in charge of keeping it up-to-date, which should be described with an entry in [CODEOWNERS](./.github/CODEOWNERS). +Those people will be requested for a review and be given write access to the repository, see also [permissions of this repository](./doc/org-repo.md). ## Regular manual reviews Unavoidibly it can also happen for reality to deviate from the documentation without a PR. -To mitigate this, all people with [code owner entries](./.github/CODEOWNERS) must regularly review their files. +To mitigate this, all people with [code owner entries](./.github/CODEOWNERS) for files in the `doc` directory must regularly review their files. This is done by [automatically opening an issue every month](./.github/workflows/review.yml) to ping all code owners. This serves as an initial fallback, but more automatic approaches could be implemented in the future, e.g. by scraping and comparing the state. diff --git a/scripts/review-body.sh b/scripts/review-body.sh index fa4623b..ae05d89 100755 --- a/scripts/review-body.sh +++ b/scripts/review-body.sh @@ -72,4 +72,4 @@ listDir() { done } -listDir "" "" +listDir "" "doc" From 0d473e1105234ff9f13b26ab6f5266d2b9cc3a6d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 11 Jul 2024 21:09:23 +0200 Subject: [PATCH 3/3] Resolve teams to users for reviews The monthly reviews mention teams, but that doesn't give a notification to the team, see also https://github.com/orgs/community/discussions/39049. This commit fixes that by expanding the team to the users that are part of it. --- .github/workflows/ci.yml | 1 + .github/workflows/review.yml | 11 +++++++-- scripts/review-body.sh | 44 ++++++++++++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1ed1a9..f43a7ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,6 +45,7 @@ jobs: env: # See https://github.com/mszostok/codeowners-validator/blob/main/docs/gh-auth.md#public-repositories # And https://github.com/mszostok/codeowners-validator/pull/222#issuecomment-2079521185 + # The same App is also used in ./review.yml GITHUB_APP_ID: ${{ secrets.APP_ID }} GITHUB_APP_INSTALLATION_ID: ${{ secrets.APP_INSTALLATION_ID }} GITHUB_APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }} diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml index bc70633..c9c8a43 100644 --- a/.github/workflows/review.yml +++ b/.github/workflows/review.yml @@ -17,11 +17,18 @@ jobs: with: path: repo + - uses: actions/create-github-app-token@v1 + id: app-token + with: + # This GitHub App needs Organization/Members/read-only access + # to figure out who's part of the team + app-id: ${{ secrets.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + - name: Generate issue body run: repo/scripts/review-body.sh repo ${{ github.repository }} > body env: - # This token has read-only admin access to see who has write access to this repo - GH_TOKEN: "${{ secrets.OWNERS_VALIDATOR_GITHUB_SECRET }}" + GH_TOKEN: ${{ steps.app-token.outputs.token }} - run: | gh api \ diff --git a/scripts/review-body.sh b/scripts/review-body.sh index ae05d89..506c0e3 100755 --- a/scripts/review-body.sh +++ b/scripts/review-body.sh @@ -1,5 +1,5 @@ #!/usr/bin/env nix-shell -#!nix-shell -i bash --pure --keep GH_TOKEN -I nixpkgs=channel:nixpkgs-unstable -p codeowners github-cli gitMinimal +#!nix-shell -i bash --pure --keep GH_TOKEN -I nixpkgs=channel:nixpkgs-unstable -p jq codeowners github-cli gitMinimal set -euo pipefail @@ -36,9 +36,49 @@ These are the [current code owners](https://github.com/$repo/tree/$rev/.github/C declare -A codeowners +cacheDir=${XDG_CACHE_HOME:-$HOME/.cache}/org-review-body + +# Resolves a team from e.g. "@NixOS/org" to "@NixOS/foo (@bar @baz)", caching the result +resolve_team() { + local org=$1 + local id=$2 + local resolved + local cacheFile="$cacheDir/$org/$id" + if [[ -f "$cacheFile" ]]; then + cat "$cacheFile" + else + mkdir -p "$(dirname "$cacheFile")" + # For a GitHub App, this needs Organization/Members/read-only access + resolved=$(gh api \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + /orgs/"$org"/teams/"$id"/members | + jq -r 'map(.login | "@\(.)") | join(" ")') + echo "@$org/$id ($resolved)" | tee "$cacheFile" + fi +} + +# Resolves all owners, e.g. from "@foo @NixOS/bar" to "@foo @NixOS/bar (@baz @qux)" +resolve_owners() { + local team=$1 + local -a entries + local result=() + IFS=" " read -r -a entries <<< "$team" + + for entry in "${entries[@]}"; do + if [[ "$entry" =~ @(.*)/(.*) ]]; then + result+=("$(resolve_team "${BASH_REMATCH[1]}" "${BASH_REMATCH[2]}")") + else + result+=("$entry") + fi + done + echo "${result[*]}" +} + while read -r file owners; do if [[ "$owners" != "(unowned)" ]]; then - codeowners[$file]=$owners + resolved=$(resolve_owners "$owners") + codeowners[$file]=$resolved fi done < <(cd "$root"; codeowners)