Skip to content

Make CI workflow fork-friendly (skip GHCR login and Codecov on forks)#5661

Merged
tdonohue merged 4 commits into
DSpace:mainfrom
qultoltd:QREPO-406-CI-error-in-forked-projects
May 14, 2026
Merged

Make CI workflow fork-friendly (skip GHCR login and Codecov on forks)#5661
tdonohue merged 4 commits into
DSpace:mainfrom
qultoltd:QREPO-406-CI-error-in-forked-projects

Conversation

@kanasznagyzoltan
Copy link
Copy Markdown
Contributor

References

  • Related Slack thread: discussion with @tdonohue about forks failing at the Docker login / GHCR pull stage in CI.
  • No related REST API PR required.

Description

Make the Build GitHub Actions workflow fork-friendly: forks can now run the full CI (including e2e tests) without access to upstream-only secrets (GITHUB_TOKEN scoped to dspace/* packages, CODECOV_TOKEN).

Instructions for Reviewers

Today, .github/workflows/build.yml always sets DOCKER_REGISTRY=ghcr.io, always runs docker/login-action, and always runs the codecov job. On forks this leads to two hard failures:

  1. Start DSpace REST Backend via Docker fails with unauthorized when pulling ghcr.io/dspace/dspace-solr, ghcr.io/dspace/dspace, ghcr.io/dspace/dspace-postgres-loadsql — a fork's GITHUB_TOKEN cannot read packages from the dspace org.
  2. codecov job fails with Token required because branch is protected because forks do not have the CODECOV_TOKEN secret.

This PR scopes those upstream-only operations to the upstream repository only, while preserving current behavior for dspace/dspace-angular.

List of changes in this PR

  • DOCKER_REGISTRY is now conditional: ghcr.io on dspace/dspace-angular (unchanged behavior, avoids Docker Hub rate limits), docker.io on forks (where the same dspace/dspace-solr, dspace/dspace, dspace/dspace-postgres-loadsql images are publicly pullable without authentication).
  • Login to ${{ env.DOCKER_REGISTRY }} step is guarded by if: github.repository == 'dspace/dspace-angular'. On forks the login is skipped because the fallback registry (docker.io) does not require authentication for these images, and forks cannot authenticate against ghcr.io/dspace/* with their own GITHUB_TOKEN anyway.
  • codecov job is guarded by if: github.repository == 'dspace/dspace-angular'. Forks do not have CODECOV_TOKEN and have no Codecov project to upload to, so running the job on forks produces only noise.

No changes to test logic, dependencies, build steps, or upstream behavior.

How to test / review

On the upstream repo (no behavior change expected):

  • CI should run exactly as before: DOCKER_REGISTRY=ghcr.io, GHCR login executes, codecov job runs and uploads coverage. The three new if: conditions all evaluate to true because github.repository == 'dspace/dspace-angular'.

On a fork (the fix):

  • DOCKER_REGISTRY resolves to docker.io.
  • The GHCR login step is skipped (visible as a skipped step in the job summary).
  • docker compose -f ./docker/docker-compose-ci.yml up -d successfully pulls docker.io/dspace/dspace-solr:latest, docker.io/dspace/dspace:latest-test, docker.io/dspace/dspace-postgres-loadsql:latest anonymously.
  • E2E tests, SSR verification, and all other steps run as normal.
  • The codecov job is skipped on forks (visible as a skipped job).

Verified against a fork (qultoltd/dspace-angular): both the Node 20.x and Node 22.x matrix jobs now go fully green end-to-end, including the e2e/Cypress and SSR verification steps, where they previously failed at the Docker pull step.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using npm run lint
  • My PR doesn't introduce circular dependencies (verified via npm run check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR aligns with Accessibility guidelines if it makes changes to the user interface.
  • My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations.
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

@kanasznagyzoltan
Copy link
Copy Markdown
Contributor Author

This was the CI result in our fork: https://github.com/qultoltd/dspace-angular/actions/runs/25857222186

@kanasznagyzoltan
Copy link
Copy Markdown
Contributor Author

kanasznagyzoltan commented May 14, 2026

I haven't changed any source code and test related code, but 1 test is failed:

SUMMARY:
✔ 5879 tests completed
ℹ 2 tests skipped
✖ 1 test failed

FAILED TESTS:
  SubmissionService
    redirectToEditItem
      ✖ should redirect to Item page
        Chrome Headless 148.0.0.0 (Linux x86_64)
      Error: <spyOn> : setStaleByHrefSubstring has already been spied upon
      Usage: spyOn(<object>, <methodName>)
      Error: <spyOn> : setStaleByHrefSubstring has already been spied upon
      Usage: spyOn(<object>, <methodName>)
          at <Jasmine>
          at UserContext.apply (src/app/submission/submission.service.spec.ts:1002:7)
          at UserContext.apply (node_modules/zone.js/fesm2015/zone-testing.js:1715:26)
          at _ZoneDelegate.invoke (node_modules/zone.js/fesm2015/zone.js:398:28)
          at ProxyZoneSpec.onInvoke (node_modules/zone.js/fesm2015/zone-testing.js:2132:39)
          at _ZoneDelegate.invoke (node_modules/zone.js/fesm2015/zone.js:397:34)
          at ZoneImpl.run (node_modules/zone.js/fesm2015/zone.js:113:43)
          at runInTestZone (node_modules/zone.js/fesm2015/zone-testing.js:216:38)

TOTAL: 1 FAILED, 5879 SUCCESS

The problem is here:

  • src/app/submission/submission.service.spec.ts#L1002
    For the mock that already creates the spy:
  • src/app/core/testing/request.service.mock.ts#L19
    spyOn(requestServce as any, 'setStaleByHrefSubstring').and.returnValue(cold('a', { a: true })); -> spyOn an already-spied method maybe a fix could be: (requestServce.setStaleByHrefSubstring as jasmine.Spy).and.returnValue(cold('a', { a: true }));

@kanasznagyzoltan
Copy link
Copy Markdown
Contributor Author

I triggered a CI rerun using an empty commit, now the 20.x tests are succeeding.

@tdonohue tdonohue self-requested a review May 14, 2026 13:41
@tdonohue tdonohue added code task 1 APPROVAL pull request only requires a single approval to merge testing framework Related specifically to Unit or Integration (e2e) Tests labels May 14, 2026
@tdonohue tdonohue added this to the 10.0 milestone May 14, 2026
@tdonohue tdonohue added port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release labels May 14, 2026
Copy link
Copy Markdown
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Thanks @kanasznagyzoltan ! This all looks good to me. Merging immediately & porting to all active maintenance branches.

@github-project-automation github-project-automation Bot moved this to 👍 Reviewer Approved in DSpace 10.0 Release May 14, 2026
@tdonohue tdonohue merged commit 7d563aa into DSpace:main May 14, 2026
16 checks passed
@github-project-automation github-project-automation Bot moved this from 👍 Reviewer Approved to ✅ Done in DSpace 10.0 Release May 14, 2026
@dspace-bot
Copy link
Copy Markdown
Contributor

Backport failed for dspace-7_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-7_x
git worktree add -d .worktree/backport-5661-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-5661-to-dspace-7_x
git switch --create backport-5661-to-dspace-7_x
git cherry-pick -x c055a095349180ab921f9ad53e5bfe7a70208605 d86c1d11b19fabdc49a14b88624cc616fc43921f 61a9cacfb70e87d51539d329f9e1b0290e1cbc88 b9fe33c06415d1e0f957b83f2d4c10f1e8c07066

@dspace-bot
Copy link
Copy Markdown
Contributor

Backport failed for dspace-8_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-8_x
git worktree add -d .worktree/backport-5661-to-dspace-8_x origin/dspace-8_x
cd .worktree/backport-5661-to-dspace-8_x
git switch --create backport-5661-to-dspace-8_x
git cherry-pick -x c055a095349180ab921f9ad53e5bfe7a70208605 d86c1d11b19fabdc49a14b88624cc616fc43921f 61a9cacfb70e87d51539d329f9e1b0290e1cbc88 b9fe33c06415d1e0f957b83f2d4c10f1e8c07066

@dspace-bot
Copy link
Copy Markdown
Contributor

Backport failed for dspace-9_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-9_x
git worktree add -d .worktree/backport-5661-to-dspace-9_x origin/dspace-9_x
cd .worktree/backport-5661-to-dspace-9_x
git switch --create backport-5661-to-dspace-9_x
git cherry-pick -x c055a095349180ab921f9ad53e5bfe7a70208605 d86c1d11b19fabdc49a14b88624cc616fc43921f 61a9cacfb70e87d51539d329f9e1b0290e1cbc88 b9fe33c06415d1e0f957b83f2d4c10f1e8c07066

@tdonohue
Copy link
Copy Markdown
Member

Manual port to 9.x: #5662

@tdonohue tdonohue removed port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release labels May 14, 2026
@tdonohue
Copy link
Copy Markdown
Member

Manual ported also to 8.x and 7.x. (Realized that the automated ports don't work as well with these GitHub action scripts)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge code task testing framework Related specifically to Unit or Integration (e2e) Tests

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants