Skip to content

Let fork PRs run test/lint by sharing images as artifacts#118

Open
ShanaLMoore wants to merge 2 commits into
mainfrom
fork-pr-share-images-via-artifacts
Open

Let fork PRs run test/lint by sharing images as artifacts#118
ShanaLMoore wants to merge 2 commits into
mainfrom
fork-pr-share-images-via-artifacts

Conversation

@ShanaLMoore

Copy link
Copy Markdown
Contributor

Summary

Makes the reusable build/test/lint workflows fully pass on pull requests opened from a fork, without granting forks any elevated permissions.

Follow-up to #117 (cc @laritakr). Context: samvera-labs/hyku_knapsack#53 / samvera-labs/hyku_knapsack#58.

The problem #117 left open

Fork PRs run with a read-only GITHUB_TOKEN and no secrets, so they can't push images to GHCR. #117 correctly stopped the build step from failing on that denied push. But test.yaml and lint.yaml obtain the built images by pulling them from GHCR (docker compose pull / up). On a fork PR nothing was pushed, so there's nothing to pull:

  • web / worker have a build: context in the app compose file, so compose can rebuild them locally, but
  • solr (and any registry-only service) has only an image: reference, so docker compose up can neither pull nor build it and the test job fails.

So #117 moved the failure from the build step to the test step (as @rob and Max both called out).

The fix: hand images to test/lint as artifacts instead of via the registry

On fork PRs only:

  • build.yaml — instead of pushing, load: true the amd64 image into the local daemon, retag it from :${TAG}-amd64 to the platform-less :${TAG} that the (skipped) multiarch manifest job would have created and that docker-compose references, docker save it, and upload it as a fork-image-<component> artifact. Only the amd64 leg is exported since test/lint run on ubuntu-latest; the arm leg still builds to validate the Dockerfile.
  • test.yaml / lint.yamldownload-artifact + docker load the tarballs before docker compose, so the images are present locally and the registry pull no-ops.

This keeps forks fully unprivileged: no secrets, no write token, no pull_request_target. It just shares the already-built images within the same workflow run (test/lint already needs: build in the callers).

Not changed

Every new step is gated on github.event_name == 'pull_request' && head.repo.full_name != github.repository. Same-repo pushes, same-repo PRs, merges to the default branch, and workflow_dispatch take none of the new paths and push/pull via the registry exactly as before.

Tradeoffs / open questions for review

  • Artifact size/time. docker save of a full Rails app image is large; upload+download+load adds minutes to fork-PR runs and consumes artifact storage (retention-days: 1 to limit it). Acceptable cost for "forks can run CI at all," but worth a sanity check. Callers that build base will also export it unless they scope components.
  • Pairs well with turning on "require approval to run workflows from fork PRs" in repo settings — that gates when a fork run executes (anti-abuse); this PR is what makes the run actually pass.
  • Not yet exercised by a real fork PR against this repo; I validated the YAML and the logic against the hyku_knapsack caller. Happy to wire up a throwaway fork PR to prove it end-to-end before tagging a release.

🤖 Generated with Claude Code

Fork PRs get a read-only GITHUB_TOKEN and no secrets, so they cannot push
images to GHCR. #117 stopped the build step from failing on that push, but
the downstream test and lint jobs pull the built images from the registry,
so on a fork PR they had nothing to pull and still failed.

Instead of pushing on fork PRs, the build job now loads the amd64 image
locally, retags it to the platform-less tag the multiarch manifest would
have produced (what docker-compose references), and uploads it as a
workflow artifact. The test and lint jobs download and `docker load` those
artifacts before `docker compose`, so the images are present locally and
the registry pull no-ops.

All new steps are gated on the fork-PR condition, so same-repo pushes,
same-repo PRs, merges to the default branch, and workflow_dispatch are
unchanged and continue to push/pull via the registry exactly as before.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Enables fork-based pull requests to run test and lint jobs successfully without requiring any registry push permissions by passing built Docker images between jobs via workflow artifacts (fork PRs only).

Changes:

  • build.yaml: On fork PRs, load the amd64 image locally, retag it to the platform-less :${TAG}, docker save it, and upload it as a fork-image-* artifact.
  • test.yaml / lint.yaml: On fork PRs, download fork-image-* artifacts and docker load them before running docker compose, so images are available locally even when GHCR push/pull is unavailable.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/build.yaml Exports fork-PR-built images as artifacts (amd64 only) for downstream jobs to consume.
.github/workflows/test.yaml Downloads and loads fork-PR image artifacts so docker compose can start without pulling from GHCR.
.github/workflows/lint.yaml Downloads and loads fork-PR image artifacts so lint can run using locally available images.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/test.yaml
Comment thread .github/workflows/lint.yaml
Addresses Copilot feedback: the load steps use the bash-specific shopt
builtin, so declare shell: bash rather than rely on the runner default.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants