Added slim Chromium-only Playwright runner image for E2E CI#29015
Added slim Chromium-only Playwright runner image for E2E CI#29015acburdine wants to merge 1 commit into
Conversation
WalkthroughChangesThis PR adds a Chromium-only Playwright E2E runner Docker image and a GitHub Actions workflow to resolve the Playwright version, build the image, and publish versioned and Sequence Diagram(s)sequenceDiagram
participant Workflow as e2e-runner-image.yml
participant PnpmWorkspace as pnpm-workspace.yaml
participant Buildx
participant GHCR
Workflow->>PnpmWorkspace: read pinned `@playwright/test` version
Workflow->>Workflow: resolve fallback version from e2e/package.json if needed
Workflow->>Buildx: build image with NODE_VERSION and PLAYWRIGHT_VERSION
Workflow->>GHCR: login with GITHUB_TOKEN
Workflow->>GHCR: push v<PLAYWRIGHT_VERSION> and latest tags
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 10m 38s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 34s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 3s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 3m 2s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 17s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
nx run @tryghost/comments-ui:test:acceptance |
✅ Succeeded | 48s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 14s | View ↗ |
Additional runs (7) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-01 16:17:53 UTC
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/e2e-runner-image.yml (1)
43-54: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFragile version extraction from
pnpm-workspace.yaml.
grep -E "'@playwright/test':" | sed -E "s/.*:[[:space:]]*//"breaks silently on quoting/formatting variants (e.g. double quotes, inline comments, multiple catalogs) and only the fully-empty case is guarded. A malformed-but-nonempty extraction would push an incorrectly tagged image to GHCR.Prefer a YAML-aware extraction (e.g.
yq '.catalog."@playwright/test"' pnpm-workspace.yaml) for robustness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/e2e-runner-image.yml around lines 43 - 54, The Playwright version lookup in the “Resolve Playwright version” step is too brittle because the `grep`/`sed` pipeline in `version` depends on exact YAML formatting. Replace that extraction with a YAML-aware read from `pnpm-workspace.yaml` (for example via `yq`) so `PLAYWRIGHT_VERSION` is parsed reliably, and keep the existing empty-value guard plus the `GITHUB_OUTPUT` write using the `playwright_version` output.e2e/Dockerfile.runner (1)
22-43: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider non-root user, but verify docker.sock access first.
Trivy flags the missing
USERdirective. Since this image mounts/var/run/docker.sockat runtime (perrun-playwright-container.sh), switching to a non-root user would likely require matching the socket's host GID, which adds real complexity for uncertain benefit in an ephemeral CI container. Worth a deliberate decision rather than a silent pass.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/Dockerfile.runner` around lines 22 - 43, The Dockerfile.runner image currently runs as root and Trivy flags the missing USER directive; decide explicitly whether to keep root for Docker socket access or switch to a non-root user. If you keep root, add a clear comment near the Dockerfile’s final runtime setup explaining that `/var/run/docker.sock` access via the mounted socket is the reason; if you switch, update the Dockerfile and the container startup flow to create/use a matching user and handle the socket group GID correctly. Reference the Dockerfile’s runtime stages and the Docker socket mount behavior used by the Playwright runner.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e-runner-image.yml:
- Around line 40-41: Update the Checkout step in the e2e runner workflow to
disable credential persistence by setting persist-credentials to false on
actions/checkout. This job only needs read access, so adjust the existing
Checkout configuration in the workflow to avoid leaving the token-backed git
credential available after checkout.
In `@e2e/scripts/load-playwright-container-env.sh`:
- Around line 29-45: The fallback path in ensure_playwright_image ignores
whether docker pull succeeds for PLAYWRIGHT_FALLBACK_IMAGE, so the function can
return success with an invalid image. Update ensure_playwright_image to check
the fallback pull result, and only assign PLAYWRIGHT_IMAGE, export it, and
return 0 when the fallback image is actually pulled successfully; otherwise emit
an error and return 1.
---
Nitpick comments:
In @.github/workflows/e2e-runner-image.yml:
- Around line 43-54: The Playwright version lookup in the “Resolve Playwright
version” step is too brittle because the `grep`/`sed` pipeline in `version`
depends on exact YAML formatting. Replace that extraction with a YAML-aware read
from `pnpm-workspace.yaml` (for example via `yq`) so `PLAYWRIGHT_VERSION` is
parsed reliably, and keep the existing empty-value guard plus the
`GITHUB_OUTPUT` write using the `playwright_version` output.
In `@e2e/Dockerfile.runner`:
- Around line 22-43: The Dockerfile.runner image currently runs as root and
Trivy flags the missing USER directive; decide explicitly whether to keep root
for Docker socket access or switch to a non-root user. If you keep root, add a
clear comment near the Dockerfile’s final runtime setup explaining that
`/var/run/docker.sock` access via the mounted socket is the reason; if you
switch, update the Dockerfile and the container startup flow to create/use a
matching user and handle the socket group GID correctly. Reference the
Dockerfile’s runtime stages and the Docker socket mount behavior used by the
Playwright runner.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 866af497-74c9-4bfc-b4c8-c56e087e68bb
📒 Files selected for processing (5)
.github/workflows/e2e-runner-image.ymle2e/Dockerfile.runnere2e/scripts/load-playwright-container-env.she2e/scripts/prepare-ci-e2e-build-mode.she2e/scripts/run-playwright-container.sh
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29015 +/- ##
==========================================
+ Coverage 74.34% 74.37% +0.02%
==========================================
Files 1565 1565
Lines 135738 135738
Branches 16496 16498 +2
==========================================
+ Hits 100919 100958 +39
+ Misses 33823 33784 -39
Partials 996 996
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
acburdine
force-pushed
the
e2e-chromium-runner-image
branch
from
efda51e to
2f47367
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/e2e-runner-image.yml (2)
32-38: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider adding
timeout-minutesto the job.No timeout is set, so a hung build/push (e.g., stalled
apt-get/Chromium download during Buildx build) could occupy the runner for up to the default 360-minute ceiling.Proposed fix
publish: name: Build and push E2E runner to GHCR runs-on: ubuntu-latest + timeout-minutes: 30 if: github.repository == 'TryGhost/Ghost' && github.ref == 'refs/heads/main'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/e2e-runner-image.yml around lines 32 - 38, The publish job in the e2e runner workflow has no explicit timeout, so add a timeout-minutes limit to the publish job alongside the existing name, runs-on, if, and concurrency settings. Keep the change scoped to the publish job in e2e-runner-image.yml so a stalled Buildx build or push cannot run until the default maximum.
12-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueManual dispatch is effectively restricted to
mainonly.The job-level
if: ... && github.ref == 'refs/heads/main'meansworkflow_dispatchonly proceeds when triggered againstmain(the default branch selection). If someone dispatches this manually from a feature branch to test aDockerfile.runnerchange before merging, the job silently no-ops. Consider whether the guard should be scoped differently forworkflow_dispatch(e.g., only gatepush/scheduleonmain, and allow manual runs from any branch sincepackages:writecombined with the repository check already limits it to same-repo actors).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/e2e-runner-image.yml around lines 12 - 38, The job-level guard in publish is too broad because it blocks workflow_dispatch unless the ref is main. Update the conditional on publish so manual dispatches can run from any branch, while still keeping main-only protection for push and schedule; use the existing github.repository check plus the job name publish to locate the rule and adjust the branch/ref gating accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/e2e-runner-image.yml:
- Around line 32-38: The publish job in the e2e runner workflow has no explicit
timeout, so add a timeout-minutes limit to the publish job alongside the
existing name, runs-on, if, and concurrency settings. Keep the change scoped to
the publish job in e2e-runner-image.yml so a stalled Buildx build or push cannot
run until the default maximum.
- Around line 12-38: The job-level guard in publish is too broad because it
blocks workflow_dispatch unless the ref is main. Update the conditional on
publish so manual dispatches can run from any branch, while still keeping
main-only protection for push and schedule; use the existing github.repository
check plus the job name publish to locate the rule and adjust the branch/ref
gating accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 955afee5-223e-4be2-99df-77ab9f642c47
📒 Files selected for processing (5)
.github/workflows/e2e-runner-image.ymle2e/Dockerfile.runnere2e/scripts/load-playwright-container-env.she2e/scripts/prepare-ci-e2e-build-mode.she2e/scripts/run-playwright-container.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- e2e/scripts/run-playwright-container.sh
- e2e/scripts/prepare-ci-e2e-build-mode.sh
- e2e/Dockerfile.runner
- e2e/scripts/load-playwright-container-env.sh
acburdine
force-pushed
the
e2e-chromium-runner-image
branch
from
2f47367 to
304a44e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
e2e/Dockerfile.runner (2)
22-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider pinning the base image by digest.
node:${NODE_VERSION}-slimis tag-pinned only; pinning by digest would improve reproducibility/supply-chain integrity for a published, cached CI image.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/Dockerfile.runner` around lines 22 - 23, The Docker base image in the runner setup is only tag-pinned via node:${NODE_VERSION}-slim, so update the FROM in the runner Dockerfile to use a digest-pinned image reference for stronger reproducibility and supply-chain integrity. Keep the existing NODE_VERSION ARG, but replace the mutable tag reference with the specific digest form so the built CI image is deterministic.
34-43: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueRoot user flagged by Trivy — acceptable given docker.sock mount, but worth confirming intent.
Static analysis flags the missing
USERdirective (DS-0002). Given the comment that this suite mounts/var/run/docker.sockfor dockerode access, adding a non-rootUSERwould likely hit the classic permission-denied issue unless the container's GID is matched to the host's docker group atdocker runtime — not something this Dockerfile alone can guarantee. For an ephemeral CI-only runner image this may be an acceptable tradeoff, but worth explicitly confirming this was a deliberate decision rather than an oversight.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/Dockerfile.runner` around lines 34 - 43, The Trivy finding is about the runner image still defaulting to root because there is no USER directive in Dockerfile.runner. Review the intent around the e2e runner setup and either explicitly keep root as a documented, deliberate choice for the dockerode/docker.sock workflow, or switch to a non-root USER in the image if permissions can be preserved. If you keep root, add a clear comment near the RUN block or image setup explaining why root is required for this container’s runtime behavior so the decision is explicit.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/Dockerfile.runner`:
- Around line 22-23: The Docker base image in the runner setup is only
tag-pinned via node:${NODE_VERSION}-slim, so update the FROM in the runner
Dockerfile to use a digest-pinned image reference for stronger reproducibility
and supply-chain integrity. Keep the existing NODE_VERSION ARG, but replace the
mutable tag reference with the specific digest form so the built CI image is
deterministic.
- Around line 34-43: The Trivy finding is about the runner image still
defaulting to root because there is no USER directive in Dockerfile.runner.
Review the intent around the e2e runner setup and either explicitly keep root as
a documented, deliberate choice for the dockerode/docker.sock workflow, or
switch to a non-root USER in the image if permissions can be preserved. If you
keep root, add a clear comment near the RUN block or image setup explaining why
root is required for this container’s runtime behavior so the decision is
explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90cb04a9-c3e1-426b-917d-e8cc5c08b20f
📒 Files selected for processing (5)
.github/workflows/e2e-runner-image.ymle2e/Dockerfile.runnere2e/scripts/load-playwright-container-env.she2e/scripts/prepare-ci-e2e-build-mode.she2e/scripts/run-playwright-container.sh
✅ Files skipped from review due to trivial changes (2)
- e2e/scripts/run-playwright-container.sh
- e2e/scripts/prepare-ci-e2e-build-mode.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e/scripts/load-playwright-container-env.sh
- .github/workflows/e2e-runner-image.yml
no ref - the E2E suite runs exclusively on Chromium (e2e/playwright.config.mjs has no Firefox/WebKit projects), but every E2E shard pulls the upstream mcr.microsoft.com/playwright image which ships all three browser engines - publishes ghcr.io/tryghost/ghost-e2e-runner:v<version> from a dedicated, rarely-triggered workflow instead of ci.yml: the image only changes on a Playwright version or Dockerfile bump and shares no layers with the Ghost build, so there is no buildx-cache reason to co-locate it with job_build_artifacts - Chromium-only + glibc Debian-slim base cuts the compressed pull from ~880MB (MCR) to ~500MB per shard (~4.5GB less per CI run), served from in-network GHCR - kept on glibc (not Alpine/musl): Playwright browser builds are glibc-only, and Alpine would force a system Chromium and break the Chromium<->Playwright version lock - consumers fall back to the upstream Playwright image when the matching runner tag is unavailable (version-bump lag, forks without GHCR, offline), so CI stays green and only loses the speedup until the runner workflow publishes the tag
acburdine
force-pushed
the
e2e-chromium-runner-image
branch
2 times, most recently
from
9fc4894 to
c27a75b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
e2e/Dockerfile.runner (2)
22-43: 🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoffNo non-root
USERset (Trivy DS-0002).Container runs as root throughout. Since this image mounts
/var/run/docker.sockfor dockerode, dropping to non-root would require matching the socket's group GID at runtime, which likely varies per CI runner and isn't controllable from this Dockerfile alone. Worth evaluating whether a non-root user with a runtime-configurable GID (e.g., via entrypointusermod/groupmodagainst the mounted socket) is feasible for this CI-only image, but the tradeoff may not be worth it if runner GIDs are inconsistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/Dockerfile.runner` around lines 22 - 43, The Dockerfile.runner image currently runs entirely as root, which triggers the non-root USER finding. Update the e2e runner setup to introduce a non-root user in this image and switch to it before execution, using a runtime-adjustable approach if needed for dockerode access to /var/run/docker.sock (for example, align the user/group GID in the runner entrypoint rather than baking in a fixed one). Keep the change localized to Dockerfile.runner and the runner startup path so the Playwright install and Chromium setup still work while the container defaults to a non-root USER.Source: Linters/SAST tools
29-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
DEBIAN_FRONTEND=noninteractivepersists into the final image viaENV.Using
ENVinstead ofARGfor this build-only setting means it leaks into the runtime environment of every consumer of this image, silently suppressing interactive prompts for anyone whodocker execs in and runsapt-get/dpkg-reconfiguremanually.♻️ Proposed fix
-ENV PLAYWRIGHT_BROWSERS_PATH=/ms-playwright \ - DEBIAN_FRONTEND=noninteractive +ARG DEBIAN_FRONTEND=noninteractive +ENV PLAYWRIGHT_BROWSERS_PATH=/ms-playwright🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/Dockerfile.runner` around lines 29 - 30, The Dockerfile.runner currently sets DEBIAN_FRONTEND with ENV, which makes a build-only setting persist into the final image. Change the Dockerfile to use ARG for DEBIAN_FRONTEND during the build step instead of ENV, while keeping PLAYWRIGHT_BROWSERS_PATH as needed, so the runtime environment does not inherit the noninteractive setting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/Dockerfile.runner`:
- Around line 22-43: The Dockerfile.runner image currently runs entirely as
root, which triggers the non-root USER finding. Update the e2e runner setup to
introduce a non-root user in this image and switch to it before execution, using
a runtime-adjustable approach if needed for dockerode access to
/var/run/docker.sock (for example, align the user/group GID in the runner
entrypoint rather than baking in a fixed one). Keep the change localized to
Dockerfile.runner and the runner startup path so the Playwright install and
Chromium setup still work while the container defaults to a non-root USER.
- Around line 29-30: The Dockerfile.runner currently sets DEBIAN_FRONTEND with
ENV, which makes a build-only setting persist into the final image. Change the
Dockerfile to use ARG for DEBIAN_FRONTEND during the build step instead of ENV,
while keeping PLAYWRIGHT_BROWSERS_PATH as needed, so the runtime environment
does not inherit the noninteractive setting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 40b721e6-2dae-43eb-a96a-a8197e37aba2
📒 Files selected for processing (5)
.github/workflows/e2e-runner-image.ymle2e/Dockerfile.runnere2e/scripts/load-playwright-container-env.she2e/scripts/prepare-ci-e2e-build-mode.she2e/scripts/run-playwright-container.sh
✅ Files skipped from review due to trivial changes (1)
- e2e/scripts/run-playwright-container.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- e2e/scripts/prepare-ci-e2e-build-mode.sh
- e2e/scripts/load-playwright-container-env.sh
- .github/workflows/e2e-runner-image.yml
1 participant
Summary
PR 1 of 2 in an effort to speed up the E2E lane in CI by consolidating how test-runtime images are acquired. This one tackles the largest per-shard pull: the Playwright runner image.
The E2E suite runs exclusively on Chromium (
e2e/playwright.config.mjshas onlymain/analyticsprojects,browserName: 'chromium', no Firefox/WebKit), yet every one of the 12 E2E shards pullsmcr.microsoft.com/playwright:v<ver>-noble, which ships all three browser engines.This adds a slim, Chromium-only runner image published to GHCR and points the E2E scripts at it, with a transparent fallback to the upstream image.
What's here
e2e/Dockerfile.runner—node:22.18.0-slim(glibc Debian) +playwright install --with-deps chromium. Chromium-only,PLAYWRIGHT_BROWSERS_PATH=/ms-playwright. No Docker CLI (the suite drives Docker via dockerode over the mounted socket);node_modulesis bind-mounted at runtime..github/workflows/e2e-runner-image.yml— dedicated publish workflow (mirrorspublish-tb-cli.yml), decoupled fromci.yml. Triggers onworkflow_dispatch, a weeklyschedule, and pushes tomaintouching the Dockerfile / this workflow /pnpm-workspace.yaml(the Playwright version pin). Publishesghcr.io/tryghost/ghost-e2e-runner:v<version>+:latest.load-playwright-container-env.shdefaults to the GHCR runner image and adds anensure_playwright_imagehelper that pulls it and falls back to the upstream Playwright image when the tag is unavailable (version-bump lag, forks without GHCR, offline).prepare-ci-e2e-build-mode.shandrun-playwright-container.shcall the helper and converge on whichever image is local.Why a separate workflow (not a step in
job_build_artifacts)The runner image only changes on a Playwright version or Dockerfile bump — far rarer than
ci.ymlruns — and it shares no layers with the Ghostfull/ghost-e2eimage, so co-locating it buys no buildx-cache warmth. A dedicated, rarely-triggered workflow keeps its build cadence independent, and the fallback means the shards need noneeds:edge to it.Size impact (compressed pull, amd64)
mcr…/playwright:v1.60.0-noble(3 browsers)ghost-e2e-runner(Chromium-only)≈ 380 MB less per shard × 12 shards ≈ ~4.5 GB less per CI run, moved from MCR to in-network GHCR.
Why glibc, not Alpine
Playwright's browser builds are glibc-only and unsupported on musl; Alpine would force using a system Chromium and break the Chromium↔Playwright version lock. The size win comes from dropping unused engines, not from the base OS.
Verification (local, arm64)
@playwright/test1.60.0 launched the baked Chromium (engine 148) and rendered a page — version lock holds.shellcheckclean on the new script code.Follow-ups / notes
tb-cli), soghost-e2e-runnermust be made public after the first publish for the fast path. Until then CI stays green via the MCR fallback, just without the speedup.🤖 Generated with Claude Code