Revert "fix: Make CI works for external contributors" #509
Conversation
This reverts commit 836026f.
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| dockerfile_path: './Dockerfile' | ||
| build_args: TASKBROKER_GIT_REVISION=${{ github.sha }} | ||
| ghcr: ${{ github.event.pull_request.head.repo.fork == false }} | ||
| ghcr: true |
There was a problem hiding this comment.
Bug: Hardcoding ghcr: true causes 403 Forbidden errors when pushing images from fork pull requests due to insufficient GITHUB_TOKEN permissions.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The change to ghcr: true at .github/workflows/image.yml:31 forces the action to attempt pushing images to GHCR for all pull requests. For pull requests originating from forks, the GITHUB_TOKEN lacks the necessary permissions to push to the upstream organization's GHCR registry, resulting in a 403 Forbidden error. This prevents the CI workflow from completing successfully for external contributions.
💡 Suggested Fix
Reintroduce the conditional ghcr: ${{ github.event.pull_request.head.repo.fork == false }} to prevent pushing to GHCR for fork pull requests. Restore the artifact saving/loading mechanism for PRs.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/image.yml#L31
Potential issue: The change to `ghcr: true` at `.github/workflows/image.yml:31` forces
the action to attempt pushing images to GHCR for all pull requests. For pull requests
originating from forks, the `GITHUB_TOKEN` lacks the necessary permissions to push to
the upstream organization's GHCR registry, resulting in a 403 Forbidden error. This
prevents the CI workflow from completing successfully for external contributions.
Did we get this right? 👍 / 👎 to inform future reviews.
| assemble-taskbroker-image: | ||
| runs-on: ubuntu-latest | ||
| needs: [build] | ||
| if: ${{ (github.ref_name == 'main' || startsWith(github.ref_name, 'releases/')) && github.event_name != 'pull_request' }} |
There was a problem hiding this comment.
Bug: E2E job skips for all pull requests because its dependency assemble-taskbroker-image is skipped, and no if: always() is present.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The assemble-taskbroker-image job at .github/workflows/image.yml:41 is configured with an if condition that causes it to be skipped for all pull requests. The self-hosted-end-to-end job has a needs: [assemble-taskbroker-image] dependency but lacks an if: always() condition or status check. Consequently, when assemble-taskbroker-image is skipped, the self-hosted-end-to-end job is also skipped by default, preventing e2e tests from running for any pull request.
💡 Suggested Fix
Modify the self-hosted-end-to-end job to handle skipped dependencies, possibly by adding if: always() or reintroducing the artifact mechanism for PRs.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/image.yml#L41
Potential issue: The `assemble-taskbroker-image` job at `.github/workflows/image.yml:41`
is configured with an `if` condition that causes it to be skipped for all pull requests.
The `self-hosted-end-to-end` job has a `needs: [assemble-taskbroker-image]` dependency
but lacks an `if: always()` condition or status check. Consequently, when
`assemble-taskbroker-image` is skipped, the `self-hosted-end-to-end` job is also skipped
by default, preventing e2e tests from running for any pull request.
Did we get this right? 👍 / 👎 to inform future reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #509 +/- ##
=======================================
Coverage 88.83% 88.83%
=======================================
Files 20 20
Lines 5867 5867
=======================================
Hits 5212 5212
Misses 655 655 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Reverts #503
If this doesn't fix my issues I will unrevert.