Skip to content
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
13 changes: 9 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 13 additions & 6 deletions docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions tests/workflow_contracts/runner_labels_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
Expand Down
17 changes: 14 additions & 3 deletions tests/workflow_contracts/runner_placement_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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<condition>[A-Za-z0-9_.]+)\s*"
r"^\$\{\{\s*(?P<condition>[A-Za-z0-9_.]+|\(.+?\))\s*"
r"&&\s*'(?P<fork>[^']*)'\s*"
r"\|\|\s*'(?P<trusted>[^']*)'\s*\}\}$"
)
Expand Down Expand Up @@ -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, (
Expand Down
Loading