feat(release): release process using PRs (no push to main allowed)#143
feat(release): release process using PRs (no push to main allowed)#143mdelapenya merged 11 commits intodocker:mainfrom
Conversation
![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: 2e6f703d91
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR changes the project’s release automation to a PR-based, two-phase flow: Phase 1 prepares a release PR (no direct pushes to main), and Phase 2 auto-tags on merge to main when */version.go changes.
Changes:
- Adds a unified
ReleaseGitHub Actions workflow supporting Phase 1 (prepare PR) and Phase 2 (auto-tag on merge), and removes the prior “release all” / “release single module” workflows. - Introduces
.github/scripts/prepare-release-pr.sh(Phase 1) and.github/scripts/tag-release.sh(Phase 2), and updatesrelease.shto no longer push/tags/proxy. - Updates Make targets and
RELEASING.mdto document and support the new process.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
commons-test.mk |
Adds a module-level tag-release target for manual Phase 2 tagging. |
Makefile |
Adds a repo-level tag-release target to tag all modules. |
RELEASING.md |
Rewrites docs around the new two-phase PR-based release process. |
.github/workflows/release.yml |
New unified workflow implementing Phase 1 (dispatch) and Phase 2 (push to main with */version.go). |
.github/workflows/release-modules.yml |
Removed legacy “Release All Modules” workflow. |
.github/workflows/release-module.yml |
Removed legacy “Release Single Module” workflow. |
.github/scripts/tag-release.sh |
New Phase 2 script to create tags from version.go and trigger Go proxy. |
.github/scripts/prepare-release-pr.sh |
New Phase 1 script to bump versions, push a branch, and open a PR. |
.github/scripts/release.sh |
Updates behavior/docs to stop pushing/tags/proxy as part of release finalization. |
.github/scripts/common.sh |
Improves origin remote validation to handle token-auth HTTPS URLs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
.github/scripts/common.sh:106
validate_git_remotelogs the rawactual_originvalue in both the success and error paths, but in CI this URL often embeds GitHub credentials (for examplehttps://x-access-token:<token>@github.com/...), so those secrets will be written to logs. An attacker with access to build logs could extract these tokens and gain unauthorized access to the repository. Mask credentials before logging (or log the normalized origin that strips credentials) instead of printingactual_origindirectly.
normalized_origin=$(echo "$actual_origin" | sed -E 's|https://[^@]+@|https://|' | sed 's|\.git$||')
local expected_normalized="https://github.com/docker/go-sdk"
local expected_ssh="git@github.com:docker/go-sdk"
if [[ "$normalized_origin" != "$expected_normalized" ]] && \
[[ "$normalized_origin" != "$expected_ssh" ]]; then
echo "❌ Error: Git remote 'origin' points to the wrong repository"
echo " Expected: ${EXPECTED_ORIGIN_SSH}"
echo " (or ${EXPECTED_ORIGIN_HTTPS})"
echo " Actual: ${actual_origin}"
echo ""
echo "To fix this, update your origin remote:"
echo " git remote set-url origin ${EXPECTED_ORIGIN_SSH}"
exit 1
fi
echo "✅ Git remote validation passed: origin → ${actual_origin}"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
.github/scripts/common.sh:106
- The
validate_git_remotefunction logs the fulloriginURL (actual_origin) in both error and success messages, which may include embedded credentials such ashttps://x-access-token:<token>@github.com/.... In CI or local setups where the remote URL contains PATs or other secrets, this behavior leaks those credentials into logs that can be accessed or retained. Avoid logging raw remote URLs by using the normalized, credential-stripped value for messages or explicitly masking credentials before printing.
echo "❌ Error: Git remote 'origin' points to the wrong repository"
echo " Expected: ${EXPECTED_ORIGIN_SSH}"
echo " (or ${EXPECTED_ORIGIN_HTTPS})"
echo " Actual: ${actual_origin}"
echo ""
echo "To fix this, update your origin remote:"
echo " git remote set-url origin ${EXPECTED_ORIGIN_SSH}"
exit 1
fi
echo "✅ Git remote validation passed: origin → ${actual_origin}"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
.github/scripts/common.sh:106
- The
validate_git_remotefunction logs the raworiginURL viaecho " Actual: ${actual_origin}"andecho "✅ Git remote validation passed: origin → ${actual_origin}", which may include embedded credentials (for example, HTTPS remotes of the formhttps://user:token@github.com/...). In CI or shared environments this will expose tokens or passwords in logs, which can then be exfiltrated by anyone with log access. To avoid leaking credentials, avoid logging the full remote URL or sanitizeactual_originbefore printing (for example, stripping any userinfo component).
echo "❌ Error: Git remote 'origin' points to the wrong repository"
echo " Expected: ${EXPECTED_ORIGIN_SSH}"
echo " (or ${EXPECTED_ORIGIN_HTTPS})"
echo " Actual: ${actual_origin}"
echo ""
echo "To fix this, update your origin remote:"
echo " git remote set-url origin ${EXPECTED_ORIGIN_SSH}"
exit 1
fi
echo "✅ Git remote validation passed: origin → ${actual_origin}"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
.github/scripts/common.sh:106
- The
validate_git_remotefunction logs the fulloriginURL viaactual_origin, both in the error path (Actual: ${actual_origin}) and on success (Git remote validation passed: origin → ${actual_origin}). In CI or local setups where the remote URL embeds credentials (e.g.,https://x-access-token:<token>@github.com/...), this will leak those secrets into logs and terminals. To avoid exposing tokens or passwords, normalize or mask the remote before logging (for example, strip any credentials and only log the host/repository), or stop logging the raw remote URL entirely.
echo "❌ Error: Git remote 'origin' points to the wrong repository"
echo " Expected: ${EXPECTED_ORIGIN_SSH}"
echo " (or ${EXPECTED_ORIGIN_HTTPS})"
echo " Actual: ${actual_origin}"
echo ""
echo "To fix this, update your origin remote:"
echo " git remote set-url origin ${EXPECTED_ORIGIN_SSH}"
exit 1
fi
echo "✅ Git remote validation passed: origin → ${actual_origin}"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Check if tag already exists on remote (the authoritative source) | ||
| # Use fixed-string match (-F) to avoid regex issues with dots in version numbers | ||
| if git ls-remote --tags origin 2>/dev/null | grep -Fq "refs/tags/${TAG}"; then | ||
| echo "⏭️ Skipping ${m}: tag ${TAG} already exists on remote" | ||
| tags_skipped=$((tags_skipped + 1)) | ||
| continue | ||
| fi |
There was a problem hiding this comment.
Remote-tag existence check can produce false positives because it searches for the substring refs/tags/${TAG}. For example, checking for client/v1.0.0 would match an existing client/v1.0.0-alpha001 tag and incorrectly skip tag creation. Use an exact ref lookup (e.g., git ls-remote --tags --refs origin "refs/tags/${TAG}") or ensure the grep matches the full ref name (end-of-line) to avoid prefix matches.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # - Squash merge: PR title is the subject line | ||
| # - Regular merge: PR title appears in the body | ||
| # - Rebase merge: original commit message is preserved | ||
| if echo "$COMMIT_MESSAGES" | grep -qE '^chore\((release|[a-z0-9_-]+)\): bump (module )?versions?[[:space:]]*$'; then |
There was a problem hiding this comment.
The release-commit detection regex is too strict for common merge strategies. With GitHub squash merges, the default commit subject is often " (#123)", which would not match this pattern and would cause Phase 2 tagging to be skipped even for valid release PRs. Consider relaxing the check (e.g., allow an optional " (#)" suffix and/or match the pattern as a substring within a line) so tagging reliably triggers on merged release PRs.
| # - Squash merge: PR title is the subject line | |
| # - Regular merge: PR title appears in the body | |
| # - Rebase merge: original commit message is preserved | |
| if echo "$COMMIT_MESSAGES" | grep -qE '^chore\((release|[a-z0-9_-]+)\): bump (module )?versions?[[:space:]]*$'; then | |
| # - Squash merge: PR title is the subject line (often with " (#<PR>)" suffix) | |
| # - Regular merge: PR title appears in the body | |
| # - Rebase merge: original commit message is preserved | |
| if echo "$COMMIT_MESSAGES" | grep -qE '^chore\((release|[a-z0-9_-]+)\): bump (module )?versions?( \(#[0-9]+\))?[[:space:]]*$'; then |
| echo " Tags created: ${tags_created}" | ||
| echo " Tags skipped (already exist): ${tags_skipped}" | ||
|
|
||
| if [[ ${tags_created} -gt 0 ]]; then | ||
| echo "" | ||
| echo "✅ Tags created and pushed successfully!" | ||
| echo "Tags on HEAD:" | ||
| git --no-pager tag --list --points-at HEAD | ||
| elif [[ ${tags_skipped} -gt 0 ]]; then | ||
| echo "" | ||
| echo "✅ All tags already exist — nothing to do (idempotent)." | ||
| else | ||
| echo "" | ||
| echo "⚠️ No modules were processed." |
There was a problem hiding this comment.
In DRY_RUN=true mode, this script still increments tags_created and later prints "Tags created and pushed successfully!" even though execute_or_echo does not actually create/push tags. This makes dry-run output misleading. Consider either not incrementing tags_created in dry run, or using separate counters/messages for "would create" vs "created", and avoid reporting a successful push when nothing was executed.
| echo " Tags created: ${tags_created}" | |
| echo " Tags skipped (already exist): ${tags_skipped}" | |
| if [[ ${tags_created} -gt 0 ]]; then | |
| echo "" | |
| echo "✅ Tags created and pushed successfully!" | |
| echo "Tags on HEAD:" | |
| git --no-pager tag --list --points-at HEAD | |
| elif [[ ${tags_skipped} -gt 0 ]]; then | |
| echo "" | |
| echo "✅ All tags already exist — nothing to do (idempotent)." | |
| else | |
| echo "" | |
| echo "⚠️ No modules were processed." | |
| if [[ "${DRY_RUN:-false}" == "true" ]]; then | |
| echo " Tags that would be created: ${tags_created}" | |
| echo " Tags skipped (already exist): ${tags_skipped}" | |
| if [[ ${tags_created} -gt 0 ]]; then | |
| echo "" | |
| echo "ℹ️ DRY RUN: No tags were actually created or pushed." | |
| elif [[ ${tags_skipped} -gt 0 ]]; then | |
| echo "" | |
| echo "ℹ️ DRY RUN: All tags already exist — no changes would be made." | |
| else | |
| echo "" | |
| echo "ℹ️ DRY RUN: No modules were processed." | |
| fi | |
| else | |
| echo " Tags created: ${tags_created}" | |
| echo " Tags skipped (already exist): ${tags_skipped}" | |
| if [[ ${tags_created} -gt 0 ]]; then | |
| echo "" | |
| echo "✅ Tags created and pushed successfully!" | |
| echo "Tags on HEAD:" | |
| git --no-pager tag --list --points-at HEAD | |
| elif [[ ${tags_skipped} -gt 0 ]]; then | |
| echo "" | |
| echo "✅ All tags already exist — nothing to do (idempotent)." | |
| else | |
| echo "" | |
| echo "⚠️ No modules were processed." | |
| fi |
| MODULE=$(echo "${1:-}" | tr '[:upper:]' '[:lower:]') | ||
| BUMP_TYPE="${BUMP_TYPE:-prerelease}" | ||
| TIMESTAMP="$(date +%Y%m%d%H%M%S)" | ||
|
|
||
| # Determine branch name and commit title | ||
| if [[ -n "${MODULE}" ]]; then | ||
| BRANCH_NAME="release/bump-${MODULE}-${TIMESTAMP}" | ||
| COMMIT_TITLE="chore(${MODULE}): bump version" | ||
| else | ||
| BRANCH_NAME="release/bump-versions-${TIMESTAMP}" | ||
| COMMIT_TITLE="chore(release): bump module versions" | ||
| fi |
There was a problem hiding this comment.
MODULE is lowercased and then used to build the branch name and to run pre-release, but there is no validation that the module exists in go.work (or that <module>/version.go exists). When run locally, a typo will fail later with less actionable errors and may leave you on a newly created release branch. Consider validating the module name against get_modules (and/or checking for the version.go file) early and failing fast with a clear error message, similar to the workflow’s validation.
What does this PR do?
Updates the release process to use a two-step release process: automatic creation of a pull request, and then automatic detection of the bump commits to trigger the tag (and release) creation.
Why is it important?
Satisfy this policy: pushes to main must happen through a pull request
Related issues