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

Add PR triage workflow #17881

Merged
merged 22 commits into from
Dec 10, 2024
Merged
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
48 changes: 48 additions & 0 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,54 @@ using this for very simple tasks such as applying labels or adding comments to P

_We must never run the untrusted PR code in the elevated `pull_request_target` context_

## Our Workflows

### Trunk Build

The [ci.yml](ci.yml) is run when commits are pushed to trunk. This calls into [build.yml](build.yml)
to run our main build. In the trunk build, we do not read from the Gradle cache,
but we do write to it. Also, the test catalog is only updated from trunk builds.

### PR Build

Similar to trunk, this workflow starts in [ci.yml](ci.yml) and calls into [build.yml](build.yml).
Unlike trunk, the PR builds _will_ utilize the Gradle cache.

### PR Triage

In order to get the attention of committers, we have a triage workflow for Pull Requests
opened by non-committers. This workflow consists of three files:

* [pr-update.yml](pr-update.yml) When a PR is created add the `triage` label if the PR
was opened by a non-committer.
* [pr-reviewed-trigger.yml](pr-reviewed-trigger.yml) Runs when any PR is reviewed.
Used as a trigger for the next workflow
* [pr-reviewed.yml](pr-reviewed.yml) Remove the `triage` label after a PR has been reviewed

_The pr-update.yml workflow includes pull_request_target!_

### CI Approved

Due to a combination of GitHub security and ASF's policy, we required explicit
approval of workflows on PRs submitted by non-committers (and non-contributors).
To simply this process, we have a `ci-approved` label which automatically approves
these workflows.

There are two files related to this workflow:

* [pr-labeled.yml](pr-labeled.yml) approves a pending approval for PRs that have
been labeled with `ci-approved`
* [ci-requested.yml](ci-requested.yml) approves future CI requests automatically
if the PR has the `ci-approved` label

_The pr-labeled.yml workflow includes pull_request_target!_

### Stale PRs

This one is straightforward. Using the "actions/stale" GitHub Action, we automatically
label and eventually close PRs which have not had activity for some time. See the
[stale.yml](stale.yml) workflow file for specifics.

## GitHub Actions Quirks

### Composite Actions
Expand Down
42 changes: 42 additions & 0 deletions .github/workflows/pr-reviewed-trigger.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

name: Pull Request Reviewed
Copy link
Member

Choose a reason for hiding this comment

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

We're starting to have quite a few actions. I think it would be good to add the description field to all of them to clearly state their role, how they work, and the potential relationships between each others.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think there is a description field for the workflow files. I've been adding comments here and there to help people navigate the workflow files. I think name is as good as we get unfortunately.

Here's the docs: https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#about-yaml-syntax-for-workflows

And a similar question on SO: https://stackoverflow.com/questions/70941956/description-for-a-workflow-in-github-actions

Copy link
Member

Choose a reason for hiding this comment

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

My bad I was looking at https://docs.github.com/en/actions/sharing-automations/creating-actions/metadata-syntax-for-github-actions#description which is not available for workflows.

So let's use comments to describe the workflows. Maybe it's only me (I'm not super familiar with GitHub Actions) but looking at the files (without reading the PR), it's not immediately obvious what each one does.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just added some docs to the README in the .github/workflows directory. PTAL

Copy link
Member

Choose a reason for hiding this comment

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

Thanks that's very helpful!


on:
pull_request_review:
Copy link
Contributor

Choose a reason for hiding this comment

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

This event brings a interesting race condition:

  1. Approve the Pull Request (PR):

    • This action triggers the "Pull Request Reviewed" workflow.
    • Example Run
  2. Add ci-approve:

    • This action attempts to locate the "pending" run_id.
    • it mistakenly retrieves the run_id of the "Pull Request Reviewed" workflow, resulting in an error.
      Error Example

Perhaps the issue is invalid, as pull requests should not be approved before the CI process completes. 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed #18200 to (hopefully) fix this

types:
- submitted

jobs:
# This job is a workaround for the fact that pull_request_review lacks necessary permissions to modify PRs.
# Also, there is no pull_request_target analog to pull_request_review. The approach taken here is taken from
# https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/.
pr-review-trigger:
name: Reviewed
runs-on: ubuntu-latest
steps:
- name: Env
run: printenv
env:
GITHUB_CONTEXT: ${{ toJson(github) }}
- name: Capture PR Number
run:
echo ${{ github.event.pull_request.number }} >> pr-number.txt
- name: Archive Event
uses: actions/upload-artifact@v4
with:
name: pr-number.txt
path: pr-number.txt
53 changes: 53 additions & 0 deletions .github/workflows/pr-reviewed.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

name: Remove Triage Label

on:
workflow_run:
workflows: [Pull Request Reviewed]
types:
- completed

jobs:
# This job runs with elevated permissions and the ability to modify pull requests. The steps taken here
# should be limited to updating labels and adding comments to PRs. This approach is taken from
# https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/.
remove-triage:
if: ${{ github.event.workflow_run.conclusion == 'success' }}
runs-on: ubuntu-latest
steps:
- name: Env
run: printenv
env:
GITHUB_CONTEXT: ${{ toJson(github) }}
- uses: actions/download-artifact@v4
with:
github-token: ${{ github.token }}
run-id: ${{ github.event.workflow_run.id }}
name: pr-number.txt
- name: Remove label
uses: actions/github-script@v7
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
var fs = require('fs');
var pr_number = Number(fs.readFileSync('./pr-number.txt'));
await github.rest.issues.removeLabel({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pr_number,
name: 'triage'
});
25 changes: 24 additions & 1 deletion .github/workflows/pr-update.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ on:
# * https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/
pull_request_target:
types: [opened, reopened, synchronize]
branches:
- trunk

jobs:
label_PRs:
add-labeler-labels:
name: Labeler
permissions:
contents: read
Expand All @@ -45,3 +47,24 @@ jobs:
PR_NUM: ${{github.event.number}}
run: |
./.github/scripts/label_small.sh

add-triage-label:
if: github.event.action == 'opened' || github.event.action == 'reopened'
name: Add triage label
runs-on: ubuntu-latest
permissions:
pull-requests: write
steps:
- name: Env
run: printenv
env:
GITHUB_CONTEXT: ${{ toJson(github) }}
# If the PR is from a non-committer, add triage label
- if: |
github.event.pull_request.author_association != 'MEMBER' &&
github.event.pull_request.author_association != 'OWNER'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GH_REPO: ${{ github.repository }}
NUMBER: ${{ github.event.pull_request.number }}
run: gh pr edit "$NUMBER" --add-label triage
16 changes: 16 additions & 0 deletions .github/workflows/stale.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,22 @@ permissions:
pull-requests: write

jobs:
needs-attention:
runs-on: ubuntu-latest
steps:
- uses: actions/stale@v9
with:
debug-only: ${{ inputs.dryRun || false }}
operations-per-run: ${{ inputs.operationsPerRun || 500 }}
days-before-stale: 7
days-before-close: -1
ignore-pr-updates: true
only-pr-labels: 'triage'
stale-pr-label: 'needs-attention'
stale-pr-message: |
A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the `triage` label
should be removed to prevent this automation from happening again.
stale:
runs-on: ubuntu-latest
steps:
Expand Down
Loading