Skip to content

Commit

Permalink
Add option to run integration tests from PRs from forks (#167)
Browse files Browse the repository at this point in the history
With our [first community
PR](#161) (馃帀 ), there was
an issue that the integration tests for the PR did not run since they
could not access the API token for the test user from the repo secrets.

This changes the check workflows so that the tests run on the
`pull_request_target` event, not on the `pull_request` event, which
allows passing the secrets to the workflow even in case of forks. To
prevent leaking the secrets, running the workflows on a PR from a fork
requires approving the workflow run by a maintainer via an environment
protection rule. I've put @vdusek, @janbuchar, @jirimoravcik and me as
maintainers of that environment, so that we get notified in case of a
fork PR and can approve the workflow runs.

The `pull_request_target` event runs in the context of the PR base
branch, not the head branch, for security reasons, so to run the tests
against the PR code, we have to check it out explicitly instead of
checking out the default ref.
  • Loading branch information
fnesveda committed Jan 9, 2024
1 parent 530403a commit 27b752a
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 2 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/check_version_availability.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ jobs:
if: (!startsWith(github.event.pull_request.title, 'docs:'))

steps:
# We need to check out the head commit in case of PRs,
# and the default ref otherwise (during release).
- name: Checkout repository
uses: actions/checkout@v4
with:
ref: "${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || '' }}"

- name: Set up Python
uses: actions/setup-python@v4
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/integration_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@ jobs:
max-parallel: 1 # no concurrency on this level, to not overshoot the test user limits

steps:
# We need to check out the head commit in case of PRs,
# and the default ref otherwise (during release).
- name: Checkout repository
uses: actions/checkout@v4
with:
ref: "${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || '' }}"

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/lint_and_type_checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ jobs:
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]

steps:
# We need to check out the head commit in case of PRs,
# and the default ref otherwise (during release).
- name: Checkout repository
uses: actions/checkout@v4
with:
ref: "${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || '' }}"

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
Expand Down
27 changes: 26 additions & 1 deletion .github/workflows/run_checks.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: Code quality checks

on:
pull_request:
pull_request_target:

jobs:
check_version_availability:
Expand All @@ -17,8 +17,33 @@ jobs:
needs: [lint_and_type_checks]
uses: ./.github/workflows/unit_tests.yaml

# If the PR comes from the main repo, run integration tests directly
integration_tests:
if: github.event.pull_request.base.repo.owner == 'apify'
name: Run integration tests
needs: [lint_and_type_checks, unit_tests]
uses: ./.github/workflows/integration_tests.yaml
secrets: inherit

# If the PR comes from a fork,
# we need to approve running the workflow first before allowing it to run,
# so that we can check for any unauthorized access to our secrets.
# We need two workflow jobs for that,
# because jobs calling reusable workflows can't use an environment.
# The first job is a dummy job that just asks for approval to use the `fork-worklows` environment.
integration_tests_fork_approve:
if: github.event.pull_request.base.repo.owner != 'apify'
name: Approve integration tests from fork
needs: [lint_and_type_checks, unit_tests]
environment: fork-pr-integration-tests
runs-on: ubuntu-latest
steps:
- name: Dummy step
run: true

# The second job is the actual integration tests job.
integration_tests_fork:
name: Run integration tests from fork
needs: [integration_tests_fork_approve]
uses: ./.github/workflows/integration_tests.yaml
secrets: inherit
4 changes: 4 additions & 0 deletions .github/workflows/unit_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ jobs:
runs-on: ${{ matrix.os }}

steps:
# We need to check out the head commit in case of PRs,
# and the default ref otherwise (during release).
- name: Checkout repository
uses: actions/checkout@v4
with:
ref: "${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || '' }}"

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
Expand Down
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

## [1.5.1](../../releases/tag/v1.5.1) - Unreleased

...
### Internal changes

- Allowed running integration tests from PRs from forks, after maintainer approval

## [1.5.0](../../releases/tag/v1.5.0) - 2024-01-03

Expand Down

0 comments on commit 27b752a

Please sign in to comment.