feat(template): harden the optional Docker Hub publish channel - #51
Merged
Conversation
Harden the optional Docker Hub publish channel in the generated release-please workflow: - Fail the docker-publish job immediately with an actionable error when exactly one of DOCKERHUB_USERNAME/DOCKERHUB_TOKEN is set, instead of letting docker/login-action fail cryptically after QEMU/Buildx setup. The token is surfaced into env only as a presence flag, never its value. - Emit a ::notice:: annotation when neither secret is set, so the GHCR-only behavior is visible in the run summary rather than only a skipped step. - Fix the Docker Hub image namespace: it was built from github.repository, which silently assumes the Docker Hub username equals the GitHub owner and fails the push when it does not. Build it from DOCKERHUB_USERNAME instead. - Document the secret pair as an optional step 6 in the generated CONTRIBUTING.md repository-setup section (rendered when include_web).
hasansezertasan
added
the
no-issue
Contributor
Reviewer's GuideTemplate changes harden the optional Docker Hub publishing path in the generated release workflow, adding explicit preflight gating and clearer behavior, fixing the Docker Hub namespace construction, and documenting the all-or-nothing secret configuration in CONTRIBUTING.md. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
ifcondition for failing on half-configured Docker Hub credentials ((env.DOCKERHUB_USERNAME != '') != (env.DOCKERHUB_TOKEN_SET == 'true')) is correct but non-obvious; consider rewriting it as a clearer XOR or splitting into two explicit checks for readability and easier maintenance. - Because
DOCKERHUB_TOKEN_SETis derived fromsecrets.DOCKERHUB_TOKEN != '', it might be worth explicitly handling the case where the secret is missing vs. present-but-empty to avoid future confusion about how an empty token string is treated.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `if` condition for failing on half-configured Docker Hub credentials (`(env.DOCKERHUB_USERNAME != '') != (env.DOCKERHUB_TOKEN_SET == 'true')`) is correct but non-obvious; consider rewriting it as a clearer XOR or splitting into two explicit checks for readability and easier maintenance.
- Because `DOCKERHUB_TOKEN_SET` is derived from `secrets.DOCKERHUB_TOKEN != ''`, it might be worth explicitly handling the case where the secret is missing vs. present-but-empty to avoid future confusion about how an empty token string is treated.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a4d4fc1b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The README's Docker Hub section predated the workflow hardening: it linked to the retired hub.docker.com/settings/security token page, did not name the Read & Write scope, and still described username-only configuration as sufficient. Rewrite it as a step-by-step walkthrough matching the PyPI trusted-publishing section, covering the all-or-nothing secret pair and the namespace following the Docker Hub account.
Address review: the half-configured-secrets check ran inside
docker-publish, but pypi-publish fans out from build concurrently, so
the irreversible PyPI upload could complete before the check blocked
finalize-release, stranding a partial release. Hoist the check into a
dedicated docker-publish-preflight job (permissions: {}) that both
pypi-publish and docker-publish depend on, so an inconsistent secret
pair blocks every publish channel before anything is uploaded.
hasansezertasan
changed the title
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardens the optional Docker Hub publish channel in the generated
release-please.yml(rendered wheninclude_webis enabled) and brings the documentation on both sides of the template up to date.Workflow
docker-publish-preflightjob (permissions: {}) validates theDOCKERHUB_USERNAME/DOCKERHUB_TOKENsecret pair up front. Bothpypi-publishanddocker-publishdepend on it, so a half-configured pair (exactly one secret set) fails fast with an actionable::error::before the first irreversible upload — previously the failure surfaced as a cryptic registry auth error deep insidedocker-publish, and worse,pypi-publishcould complete concurrently, stranding a partial release. Recovery: fix the secrets → Re-run failed jobs.envonly as aDOCKERHUB_TOKEN_SETpresence flag, never as its value.::notice::annotation records that the run publishes to GHCR only, instead of leaving only a grey skipped step.docker.io/${{ github.repository }}, which assumes the Docker Hub username equals the GitHub owner; the push is denied whenever they differ. The image name is now built from the secret:docker.io/<DOCKERHUB_USERNAME>/<repo-name>.Behavior matrix: both secrets → GHCR + Docker Hub; neither → GHCR with a notice; exactly one → loud failure, nothing published anywhere.
Documentation
CONTRIBUTING.md: new optional step 6 in the one-time repository-setup section documenting the all-or-nothing secret pair, withgh secret setcommands and the required token scope.README.md: the Docker Hub section was stale — it linked to the retiredhub.docker.com/settings/securitytoken page and predated the fail-fast behavior. Rewritten as a step-by-step walkthrough matching the PyPI trusted-publishing section (current token URL, Read & Write least-privilege scope, expiry, both secrets, resulting image path).Test plan
include_web=true—actionlintpasses;pypi-publishanddocker-publishboth gate ondocker-publish-preflight.example-input.ymldefaults (web disabled) —actionlintpasses; zero Docker leftovers in workflow, README, or CONTRIBUTINGprek runhooks pass on the changed files (pre-existingMD060errors indocs/adr/*.mdare unrelated — introduced by the markdownlint-cli2 v0.22.1 bump onmain)