-
Notifications
You must be signed in to change notification settings - Fork 278
[no-ci] pr-metadata-check: restrict cuda_bindings changes to NVIDIA organization members #1871
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
Changes from 1 commit
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 |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| name: "CI: Enforce assignee/label/milestone on PRs" | ||
| name: "CI: Enforce PR metadata and cuda_bindings policy" | ||
|
|
||
| on: | ||
| pull_request_target: | ||
|
|
@@ -19,18 +19,32 @@ on: | |
|
|
||
| jobs: | ||
| check-metadata: | ||
| name: PR has assignee, labels, and milestone | ||
| name: PR has required metadata and valid cuda_bindings author | ||
| if: github.repository_owner == 'NVIDIA' | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
| steps: | ||
| - name: Check for assignee, labels, and milestone | ||
| - name: Check PR metadata and cuda_bindings policy | ||
| env: | ||
| # PR metadata inputs | ||
| ASSIGNEES: ${{ toJson(github.event.pull_request.assignees) }} | ||
| AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association || 'NONE' }} | ||
| LABELS: ${{ toJson(github.event.pull_request.labels) }} | ||
| MILESTONE: ${{ github.event.pull_request.milestone && github.event.pull_request.milestone.title || '' }} | ||
| PR_AUTHOR: ${{ github.event.pull_request.user.login }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| PR_URL: ${{ github.event.pull_request.html_url }} | ||
|
|
||
| # Gating booleans | ||
| IS_BOT: ${{ github.actor == 'dependabot[bot]' || github.actor == 'pre-commit-ci[bot]' || github.actor == 'copy-pr-bot[bot]' }} | ||
| IS_DRAFT: ${{ github.event.pull_request.draft }} | ||
|
|
||
| # API request context/auth | ||
| GITHUB_API_URL: ${{ github.api_url }} | ||
| GH_TOKEN: ${{ github.token }} | ||
| REPO: ${{ github.repository }} | ||
| run: | | ||
| if [ "$IS_BOT" = "true" ] || [ "$IS_DRAFT" = "true" ]; then | ||
| echo "Skipping check for bot or draft PR." | ||
|
|
@@ -103,10 +117,45 @@ jobs: | |
| ERRORS="${ERRORS}- **Blocked label detected**: label \`$label\` prevents merging. Remove it when the PR is ready.\n" | ||
| done <<<"$BLOCKED_LABELS" | ||
|
|
||
| # Only NVIDIA organization members may change code under cuda_bindings. | ||
| if [ "$AUTHOR_ASSOCIATION" != "MEMBER" ] && [ "$AUTHOR_ASSOCIATION" != "OWNER" ]; then | ||
|
Member
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. Q: I assume checking |
||
| page=1 | ||
| TOUCHES_CUDA_BINDINGS=false | ||
| while true; do | ||
| FILES_JSON=$(curl --silent --show-error --fail \ | ||
|
Contributor
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. Why are you using curl here instead of You can also have it run that jq script using You can then avoid looping over API invocations and burning up gh rate limits as well as all the overwrought bash that hardcodes the page size and checks for pages with fewer files than that number.
Contributor
Author
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. Done: commit a52b862
Yes. I assumed it'd know such things. Probably next month :-) |
||
| -H "Authorization: Bearer $GH_TOKEN" \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| -H "X-GitHub-Api-Version: 2022-11-28" \ | ||
| "$GITHUB_API_URL/repos/$REPO/pulls/$PR_NUMBER/files?per_page=100&page=$page") | ||
|
|
||
| if jq -e ' | ||
| .[] | ||
| | select( | ||
| (.filename | startswith("cuda_bindings/")) | ||
| or ((.previous_filename // "") | startswith("cuda_bindings/")) | ||
| ) | ||
| ' <<<"$FILES_JSON" >/dev/null; then | ||
| TOUCHES_CUDA_BINDINGS=true | ||
| break | ||
| fi | ||
|
|
||
| FILE_COUNT=$(jq 'length' <<<"$FILES_JSON") | ||
| if [ "$FILE_COUNT" -lt 100 ]; then | ||
| break | ||
| fi | ||
|
|
||
| page=$((page + 1)) | ||
| done | ||
|
|
||
| if [ "$TOUCHES_CUDA_BINDINGS" = "true" ]; then | ||
| ERRORS="${ERRORS}- **cuda_bindings policy**: See \`cuda_bindings/LICENSE\`. Only NVIDIA organization members may modify files under \`cuda_bindings/\` (PR author \`$PR_AUTHOR\` has association \`$AUTHOR_ASSOCIATION\`).\n" | ||
| fi | ||
| fi | ||
|
|
||
| if [ -n "$ERRORS" ]; then | ||
| echo "::error::This PR is missing required metadata. See the job summary for details." | ||
| echo "::error::This PR failed the required metadata/policy checks. See the job summary for details." | ||
| { | ||
| echo "## PR Metadata Check Failed" | ||
| echo "## PR Requirements Check Failed" | ||
| echo "" | ||
| printf '%b' "$ERRORS" | ||
| echo "" | ||
|
|
@@ -118,9 +167,10 @@ jobs: | |
| ASSIGNEE_LIST=$(echo "$ASSIGNEES" | jq -r '.[].login' | paste -sd ', ' -) | ||
| LABEL_LIST=$(echo "$LABELS" | jq -r '.[].name' | paste -sd ', ' -) | ||
| { | ||
| echo "## PR Metadata Check Passed" | ||
| echo "## PR Requirements Check Passed" | ||
| echo "" | ||
| echo "- **Assignees**: $ASSIGNEE_LIST" | ||
| echo "- **Labels**: $LABEL_LIST" | ||
| echo "- **Milestone**: $MILESTONE" | ||
| echo "- **Author association**: $AUTHOR_ASSOCIATION" | ||
| } >> "$GITHUB_STEP_SUMMARY" | ||
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.
As discussed in this team meeting, it would be awesome if the org check was a separate line item to make it obvious when it fails.