Skip to content

Conversation

@markphelps
Copy link
Contributor

@markphelps markphelps commented Jan 14, 2026

Summary

  • Fixes integration tests failing for forked PRs due to missing Docker Hub credentials
  • Makes Docker Hub login conditional to skip for forked PRs while keeping it for main repo
  • Unblocks public contributions while maintaining rate limit protection for main repo PRs

Background

GitHub secrets (DOCKERHUB_USERNAME and DOCKERHUB_TOKEN) are not accessible to workflows triggered by forked pull requests for security reasons. This causes the Docker login step to fail with "Username and password required", blocking CI for external contributors.

The Docker Hub login was added in #2443 to avoid rate limit (429) errors when pulling base images like python:* and nvidia/cuda:* during builds. While authenticated pulls have higher rate limits, anonymous pulls should be sufficient for most forked PRs.

Changes

Added conditional logic to both Docker Hub login steps in CI:

if: github.event.pull_request.head.repo.full_name == github.repository || github.event_name != 'pull_request'

This ensures:

  • ✅ Forked PRs skip login and use anonymous Docker Hub pulls
  • ✅ PRs from branches in the main repo use authenticated pulls
  • ✅ Push events, tags, and manual workflows use authenticated pulls

Trade-offs

Pros:

  • Unblocks external contributions
  • Maintains rate limit protection for main repo
  • Simple, safe implementation
  • Standard practice for open-source projects

Cons:

  • Forked PRs may occasionally hit Docker Hub's anonymous rate limits (~100 pulls per 6 hours per IP)
  • This is an acceptable trade-off given the low likelihood and the importance of enabling public contributions

Testing

This fix will allow PR #2619 (and other forked PRs) to run integration tests successfully.

Forked PRs cannot access GitHub secrets (DOCKERHUB_USERNAME and
DOCKERHUB_TOKEN), causing the Docker login step to fail with 'Username
and password required'. This blocks external contributions.

This change makes the Docker Hub login conditional:
- Skips login for forked PRs (secrets unavailable)
- Runs login for PRs from branches in the main repo
- Runs login for push events, tags, and manual workflows

Docker Hub authentication helps avoid rate limits (429 errors) when
pulling base images like python:* and nvidia/cuda:* during builds.
Forked PRs will use anonymous Docker Hub pulls and may occasionally
hit rate limits, but this is an acceptable trade-off to unblock public
contributions.

Fixes the issue reported in PR #2619 where forked PRs fail at the
Docker login step in CI.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes integration test failures for forked pull requests by making Docker Hub authentication conditional. Secrets are unavailable to forked PRs for security reasons, causing login failures. The solution allows forked PRs to skip authentication and use anonymous Docker Hub pulls while maintaining authenticated access for the main repository.

Changes:

  • Added conditional logic to skip Docker Hub login for forked PRs in both integration test jobs
  • Forked PRs will use anonymous Docker Hub pulls, while main repo PRs and other events retain authenticated access

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

Copy link
Member

@michaeldwan michaeldwan left a comment

Choose a reason for hiding this comment

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

nice!

@markphelps markphelps merged commit 68fd30c into main Jan 14, 2026
38 checks passed
@markphelps markphelps deleted the fix/docker-login-forked-prs branch January 14, 2026 23:07
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