ci: optional Docker Hub login to raise pull rate limits#160
Conversation
Changed Files
|
There was a problem hiding this comment.
Code Review
This pull request adds optional Docker Hub authentication to the setup-docker and setup composite actions to help avoid rate limits. Reviewers noted that the workflow files needed to actually provide these credentials are missing from the PR, and suggested extending the implementation to the release workflow for consistency.
Greptile SummaryThis PR adds optional Docker Hub authentication to CI to raise the anonymous pull rate limit for
Confidence Score: 4/5Safe to merge once the The credential-passing chain is straightforward and the conditional guard correctly skips the login step when either input is absent. The one area to double-check is the
Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
.github/workflows/regular.yaml:45
**`environment: "ci"` may block runs on non-main branches or fork PRs**
The PR description calls out the required-reviewer protection rule, but GitHub environments also support two other protection types that could silently break CI: *deployment branch/tag rules* (if configured to allow only `main`, every PR branch would be blocked) and *wait timers* (all runs pause for the configured duration). Since `regular.yaml` is the primary PR-validation workflow and `build-and-test` now always requires the `ci` environment, any of those protection types would stall or block every PR run — including those from external forks where access to the environment is already restricted. Worth documenting or verifying that the `ci` environment has no branch/tag rules and no wait timer, not just no required reviewers.
Reviews (1): Last reviewed commit: "ci: gate environment: ci on upstream rep..." | Re-trigger Greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Adds a docker/login-action step in setup-docker for Docker Hub, gated on `dockerhub-username`/`dockerhub-token` inputs being non-empty so it no-ops on forks (where the `ci` environment isn't accessible and secrets are scrubbed). Threads the credentials through the `setup` composite action and wires them in `regular.yaml`'s build-and-test job from `vars.DOCKERHUB_USERNAME` / `secrets.DOCKERHUB_TOKEN` via the `ci` environment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switches the build-and-test job's environment binding to a conditional expression — `ci` on the upstream repo, empty string (no environment) elsewhere. Prevents forks from auto-materializing a stray empty `ci` environment in their settings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the regular.yaml wiring on release.yaml's build-addon and build-packages jobs. Same fork-friendly environment guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Forks auto-create the empty environment on first run and the conditional step in setup-docker still skips; the repo-name guard was cosmetic only and brittle to renames. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Makes the docker.io pull (currently mitmproxy) visible as its own step so failures surface clearly instead of being buried in compose up output. Uses --ignore-buildable so the locally built dev image is skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
b7372dd to
14211ff
Compare
Summary
docker/login-actionstep in.github/actions/setup-docker/action.yaml, right after the existing GHCR login.dockerhub-username/dockerhub-tokeninputs being non-empty, so it cleanly skips on forks (where thecienvironment isn't accessible and secrets are scrubbed)..github/actions/setup/action.yaml..github/workflows/regular.yaml'sbuild-and-testjob, bindsenvironment: "ci"and passesvars.DOCKERHUB_USERNAME/secrets.DOCKERHUB_TOKENinto setup.Why
Anonymous docker.io pulls are aggressively rate-limited on shared CI IPs. Authenticating raises the ceiling and reduces transient build failures when pulling base images.
Notes
cienvironment must have no required-reviewer protection rule, otherwisebuild-and-testwill stall waiting for approval on every run.regular.yamlis wired up here;release.yamlalso callssetupbut is left unchanged. If Docker Hub auth is wanted there too, the same two-line addition to itsbuild-addon/build-packagesjobs would extend coverage.Test plan
docker.io.cienv access): confirm the step is skipped with no failure.🤖 Generated with Claude Code