diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ee842cd1..0a6c19928 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,9 +54,13 @@ jobs: # the managed one. `github.event.pull_request` is null outside a pull # request, so a push to `main` and a manual dispatch both take the # trusted arm rather than the fallback. + # Dependabot pull requests run on the hosted pool too (free on a public + # repository). The author is tested, not the actor, which changes when + # someone else pushes to the branch. runs-on: >- - ${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest' - || 'ubicloud-standard-8' }} + ${{ (github.event.pull_request.head.repo.fork + || github.event.pull_request.user.login == 'dependabot[bot]') + && 'ubuntu-latest' || 'ubicloud-standard-8' }} # 90, which is this job's own figure and no longer the coverage jobs': # both of those went to 120 in #486 to clear the 5,400 s cargo watchdog, # and this job has no watchdog to clear. A cold sccache pays about 1.4 s @@ -671,8 +675,9 @@ jobs: if: github.actor != 'dependabot[bot]' && github.event_name != 'push' # The same fork fallback as `build`; see the comment there. runs-on: >- - ${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest' - || 'ubicloud-standard-8' }} + ${{ (github.event.pull_request.head.repo.fork + || github.event.pull_request.user.login == 'dependabot[bot]') + && 'ubuntu-latest' || 'ubicloud-standard-8' }} # Above the 5,400 s cargo watchdog below plus the work either side of # the coverage step, which reached 59 s here and 522 s on the trunk # lane. At 90 minutes the ceiling equalled the watchdog exactly, so a diff --git a/docs/developers-guide.md b/docs/developers-guide.md index e30ed6026..46041c6cf 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -243,17 +243,24 @@ A pull request from a fork cannot obtain an Ubicloud runner, so a job that named one unconditionally would have no runner at all and would fail for a reason with nothing to do with the change under review. The two jobs in `ci.yml` therefore key their label on the event, falling back to a hosted -runner for a fork and keeping the managed one for everything else: +runner for a fork and for a pull request Dependabot opened, and keeping the +managed one for everything else: ```yaml runs-on: >- - ${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest' - || 'ubicloud-standard-8' }} + ${{ (github.event.pull_request.head.repo.fork + || github.event.pull_request.user.login == 'dependabot[bot]') + && 'ubuntu-latest' || 'ubicloud-standard-8' }} ``` -`github.event.pull_request` is null outside a pull request, so a push and a -manual dispatch both take the trusted arm. GitHub-hosted minutes are free on a -public repository, so the fallback costs only its own wall clock. +The Dependabot test reads the pull request's author, not `github.actor`, +because the actor is whoever triggered the run, so a person pushing to a +Dependabot branch would flip the runner. Dependabot opens many pull requests, +and GitHub-hosted minutes are free on a public repository, so the fallback +costs only its own wall clock: a Dependabot pull request builds cold on the +hosted pool. `github.event.pull_request` is absent outside a pull request, so +its properties are empty strings (falsy) and a push and a manual dispatch both +take the trusted arm. `coverage-main.yml` is deliberately outside the rule. It fires on a push to `main`, which a fork cannot perform, so a fallback there would be a branch that diff --git a/tests/workflow_contracts/runner_labels_test.py b/tests/workflow_contracts/runner_labels_test.py index 173389c27..2264b449c 100644 --- a/tests/workflow_contracts/runner_labels_test.py +++ b/tests/workflow_contracts/runner_labels_test.py @@ -23,8 +23,9 @@ [ pytest.param("ubuntu-latest", {"ubuntu-latest"}, id="label"), pytest.param( - "${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest'" - " || 'ubicloud-standard-8' }}", + "${{ (github.event.pull_request.head.repo.fork" + " || github.event.pull_request.user.login == 'dependabot[bot]')" + " && 'ubuntu-latest' || 'ubicloud-standard-8' }}", {"ubuntu-latest", "ubicloud-standard-8"}, id="expression", ), diff --git a/tests/workflow_contracts/runner_placement_test.py b/tests/workflow_contracts/runner_placement_test.py index 25e7f804e..9dcbd905d 100644 --- a/tests/workflow_contracts/runner_placement_test.py +++ b/tests/workflow_contracts/runner_placement_test.py @@ -35,6 +35,17 @@ #: The expression field that is true only for a pull request from a fork. FORK_FIELD: typ.Final[str] = "github.event.pull_request.head.repo.fork" +#: The pull request's author, not `github.actor`: the actor is whoever +#: triggered the run, so a person pushing to a Dependabot branch would flip the +#: runner. Hosted minutes cost nothing on a public repository, so Dependabot's +#: pull requests take the hosted arm too. +DEPENDABOT_FIELD: typ.Final[str] = ( + "github.event.pull_request.user.login == 'dependabot[bot]'" +) + +#: The exact condition under which a pull request takes the hosted arm. +HOSTED_CONDITION: typ.Final[str] = f"({FORK_FIELD} || {DEPENDABOT_FIELD})" + #: The runner a fork falls back to. GitHub-hosted minutes are free on a #: public repository, so the fallback costs nothing beyond its wall clock. FORK_RUNNER: typ.Final[str] = "ubuntu-latest" @@ -47,7 +58,7 @@ #: would pass on an expression that named it and then ignored it, and one #: that searched for `ubuntu-latest` would pass with the two arms swapped. _FALLBACK = re.compile( - r"^\$\{\{\s*(?P[A-Za-z0-9_.]+)\s*" + r"^\$\{\{\s*(?P[A-Za-z0-9_.]+|\(.+?\))\s*" r"&&\s*'(?P[^']*)'\s*" r"\|\|\s*'(?P[^']*)'\s*\}\}$" ) @@ -185,9 +196,9 @@ def test_a_paid_lane_a_fork_can_reach_falls_back_to_a_hosted_runner( f"fork's pull request cannot obtain one, so the label must be the " f"event-keyed expression" ) - assert parsed["condition"] == FORK_FIELD, ( + assert parsed["condition"] == HOSTED_CONDITION, ( f"{filename} job {job_id!r} keys its runner on " - f"{parsed['condition']!r}; only {FORK_FIELD} distinguishes a fork's " + f"{parsed['condition']!r}; only {HOSTED_CONDITION} distinguishes a fork's " f"pull request from every other event" ) assert parsed["fork"] == FORK_RUNNER, (