-
Notifications
You must be signed in to change notification settings - Fork 248
ci: release process with breaking changes #2998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
|
Claude finished @auricom's task in 2m 46s —— View job Comprehensive Code ReviewI've completed a thorough review of the release process changes in this PR. The changes introduce a well-structured release workflow with AI-generated release notes and comprehensive documentation. Below are my findings organized by category. 🔒 Security Issues (High Priority)1. Unpinned Third-Party ActionsSeverity: High | Files: The workflow uses unpinned third-party actions with tag references instead of commit SHAs: uses: anthropics/claude-code-action@v1 # Line 168
uses: softprops/action-gh-release@v2 # Line 179Risk: Tag-based references are mutable and can be modified by action maintainers (or attackers with compromised accounts), potentially introducing malicious code into your CI/CD pipeline. Recommendation: Pin actions to specific commit SHAs with comments indicating the version: uses: anthropics/claude-code-action@a1b2c3d4 # v1
uses: softprops/action-gh-release@e5f6g7h8 # v2Note: 2. Secrets ManagementStatus: ✅ Good | File: The workflow properly uses GitHub Secrets for the Claude API token: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}Verification needed: Ensure this secret is:
🏗️ Workflow Architecture & Error Handling3. Robust Tag ValidationStatus: ✅ Excellent | File: The semver validation regex is comprehensive and handles:
The error messages are clear and actionable. Well done! 4. Missing Error Handling in Shell ScriptsSeverity: Medium | File: The "Get list of Docker images" step lacks error handling and validation: for app_dir in apps/*/; do
if [ -f "${app_dir}Dockerfile" ]; then
APP_NAME=$(basename "$app_dir")
IMAGE_NAME="ev-node-${APP_NAME}"
IMAGES="${IMAGES}ghcr.io/${{ github.repository_owner }}/${IMAGE_NAME}:${{ steps.extract-version.outputs.tag }}\n"
fi
doneIssues:
Recommendation: # Add error handling
if [ ! -d "apps" ]; then
echo "::error::apps/ directory not found"
exit 1
fi
IMAGE_COUNT=0
for app_dir in apps/*/; do
if [ -f "${app_dir}Dockerfile" ]; then
APP_NAME=$(basename "$app_dir")
IMAGE_NAME="ev-node-${APP_NAME}"
IMAGES="${IMAGES}ghcr.io/${{ github.repository_owner }}/${IMAGE_NAME}:${{ steps.extract-version.outputs.tag }}\n"
IMAGE_COUNT=$((IMAGE_COUNT + 1))
echo "::notice::Found app: $APP_NAME"
fi
done
if [ $IMAGE_COUNT -eq 0 ]; then
echo "::warning::No Docker images found in apps/"
fi
echo "::notice::Total Docker images found: $IMAGE_COUNT"5. Insufficient Previous Tag FallbackSeverity: Low | File: The previous tag detection falls back to initial commit, which could generate very large changelogs for first releases: if [ -z "$PREV_TAG" ]; then
echo "::notice::No previous tag found, using initial commit"
PREV_TAG=$(git rev-list --max-parents=0 HEAD)
fiRecommendation: Consider adding a flag or input to specify the changelog starting point for first releases, or limit the changelog to the last N commits. 📚 Documentation Quality6. Excellent Documentation StructureStatus: ✅ Outstanding | File: The updated
This is a model documentation file! 7. Missing Critical Context in Quick StepsSeverity: Medium | File: The GitHub Release quick steps say "Ensure CHANGELOG.md is updated" but don't specify what that means: # 1. Ensure CHANGELOG.md is updated for the versionIssues:
Recommendation: Add explicit details: # 1. Update CHANGELOG.md before triggering the workflow:
# - Move changes from [Unreleased] to a new version section: ## v1.2.3
# - Include sections: Added, Changed, Fixed, Removed, Security (as applicable)
# - Mark breaking changes with **BREAKING:** prefix
# - Commit and push to main branch8. Unclear Workflow Dependency OrderingSeverity: Medium | File: The "How It Works" section lists steps but doesn't clarify critical timing requirements: Missing information:
Recommendation: Add a prerequisites subsection before "How It Works": ### Prerequisites
Before running the workflow, you must:
1. **Update CHANGELOG.md** on the main branch with the version you're releasing
2. **Ensure the version section exists** - the workflow reads from CHANGELOG.md
3. **Have all Docker images built** - the workflow lists images that should be available9. Inconsistent TerminologySeverity: Low | File: The document uses inconsistent terminology:
Recommendation: Standardize on:
⚡ Performance & Efficiency10. Inefficient Changelog Prompt ConstructionSeverity: Low | File: The changelog prompt is constructed using a heredoc, then modified with cat > claude-prompt.txt << 'EOF_PROMPT'
...
EOF_PROMPT
sed -i "s/\$VERSION/${VERSION}/g" claude-prompt.txt
sed -i "s/\$TAG/${TAG}/g" claude-prompt.txt
echo "${{ steps.get-images.outputs.images }}" | while IFS= read -r image; do
echo "- $image" >> claude-prompt.txt
done
cat >> claude-prompt.txt << 'EOF_PROMPT'
...
EOF_PROMPTRecommendation: Use variable substitution directly in bash (though the current approach is more readable for complex prompts, so this is a minor optimization): cat > claude-prompt.txt << EOF
Read the file CHANGELOG.md and find the section for version ${VERSION} (tag ${TAG}).
...
EOFHowever, the current approach is acceptable because it keeps the prompt template readable and avoids escaping issues. No action needed unless performance becomes a concern. 🧪 Testing & Reliability11. No Validation of AI-Generated OutputSeverity: Medium | File: The workflow uses AI to generate release notes but doesn't validate the output before creating the release: - name: Generate release notes with Claude
id: claude-release
uses: anthropics/claude-code-action@v1
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
prompt: ${{ steps.changelog-prompt.outputs.prompt }}Issues:
Recommendation: Add a validation step: - name: Validate Claude output
run: |
OUTPUT='${{ steps.claude-release.outputs.structured_output }}'
if [ -z "$OUTPUT" ]; then
echo "::error::Claude did not return any output"
exit 1
fi
RELEASE_NOTES=$(echo "$OUTPUT" | jq -r '.release_notes // empty')
RELEASE_DATE=$(echo "$OUTPUT" | jq -r '.release_date // empty')
if [ -z "$RELEASE_NOTES" ]; then
echo "::error::release_notes field is missing or empty"
exit 1
fi
if [ -z "$RELEASE_DATE" ]; then
echo "::error::release_date field is missing or empty"
exit 1
fi
# Validate date format (YYYY-MM-DD)
if ! echo "$RELEASE_DATE" | grep -qE '^[0-9]{4}-[0-9]{2}-[0-9]{2}$'; then
echo "::error::Invalid date format: $RELEASE_DATE (expected YYYY-MM-DD)"
exit 1
fi
echo "::notice::Validation passed"12. Limited Allowed Tools for ClaudeStatus: ✅ Good Practice | File: The workflow restricts Claude to only reading CHANGELOG.md: allowed_tools: "Read(CHANGELOG.md)"This is excellent security practice - principle of least privilege! The AI can't modify files or read sensitive information. 13. Missing Testing for Multi-Platform Docker BuildsSeverity: Low | File: The Docker workflow builds for multiple platforms but doesn't test them: platforms: linux/amd64,linux/arm64Recommendation: Consider adding smoke tests for each platform in a separate job that runs after the build. However, this may be overkill depending on your testing strategy elsewhere. 🎯 Go Module Release Process14. Excellent Dependency Graph VisualizationStatus: ✅ Outstanding | File: The ASCII dependency graph is clear and makes the release order obvious. Great visual aid! 15. Missing go.mod File VerificationSeverity: Medium | File: The Go module release process mentions removing
Recommendation: Add a verification section: #### Before Starting: Verification Checklist
Run this command to find any `replace` directives:
\`\`\`bash
find . -name "go.mod" -exec grep -l "^replace" {} \;
\`\`\`
If any are found, remove them and commit the changes:
\`\`\`bash
# Review and remove replace directives manually
# They prevent consumers from getting the correct transitive dependencies
vim ./path/to/go.mod
git add . && git commit -m "Remove replace directives for release"
git push origin main
\`\`\`16. Unclear Go Proxy Propagation TimeSeverity: Low | File: The document mentions "5-10 minutes" in one place and "5-30 minutes" in another, and "up to 30 minutes" in the troubleshooting section. This inconsistency could cause confusion. Recommendation: Use consistent timing guidance:
🔐 Permissions & Access Control17. Appropriate Workflow PermissionsStatus: ✅ Excellent | File: The workflow uses least-privilege permissions: permissions:
contents: write # Needed to create releases
id-token: write # Needed for OIDC tokenThe 18. No Pull Request Triggers (By Design)Status: ✅ Good | File: The GitHub Release workflow only triggers on 📝 Additional Observations19. Breaking Changes DocumentationStatus: ✅ Strong | File: The release notes template includes clear sections for breaking changes with actionable steps. The CHANGELOG.md examples I reviewed show this pattern is already being followed (e.g., forced inclusion changes in v1.0.0-rc.1). 20. No Rollback Instructions for GitHub ReleasesSeverity: Low | File: The rollback section mentions that Go module tags should never be deleted, but doesn't provide instructions for rolling back or deleting GitHub Releases. Recommendation: Add: 5. **GitHub Release Rollback**:
- Draft releases can be deleted from the GitHub UI with no impact
- Published releases can be deleted but this may confuse users
- Better approach: Create a new patch release (e.g., v1.2.4) with fixes
- If you must delete a published release, also delete the associated tag✅ Summary & Priority ActionsHigh Priority (Address Before Merging)
Medium Priority (Address Soon)
Low Priority (Nice to Have)
🎉 What's Working Well
This is a significant improvement to the release process. Once the high-priority security and error-handling issues are addressed, this will be production-ready. | Branch: |
Summary of ChangesHello @auricom, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly overhauls the project's release documentation by introducing a structured and comprehensive release process. It defines clear version support policies, details both automated and manual steps for creating and publishing releases, and establishes robust guidelines for changelog management and external communication. The changes aim to standardize release procedures, enhance transparency, and provide a clearer understanding of release content and upgrade compatibility for users and contributors. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a comprehensive and well-documented release process, including changelog management and automated workflows. The changes are primarily in RELEASE.md and provide detailed steps for both Docker image and Go module releases. My review focuses on improving the clarity of the new process. I've suggested clarifying the manual steps required before tagging a release and detailing the lifecycle of the temporary release branches created by the automation. Overall, this is a great step towards standardizing the release workflow.
RELEASE.md
Outdated
| 1. 📝 Review the release branch `release/<tag-name>` | ||
| 2. 📝 Edit and refine the generated changelog if needed | ||
| 3. 📝 Add **recommended upgrade priority** (Critical/High/Medium/Low) | ||
| 4. 📝 Add **general description** of the release | ||
| 5. 📝 Ensure **tested upgrade paths** are documented (1-2 lines in changelog) | ||
| 6. ✅ Merge the release branch or update the GitHub Release directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The process for refining the changelog could be clarified. The 'Quick Steps' (lines 254-258) show editing and pushing to the release/<tag-name> branch. However, it's not clear how these committed changes are then reflected in the final GitHub Release. Does the developer need to manually copy the refined changelog from the branch to the GitHub Release draft?
Additionally, step 6 here says 'Merge the release branch...'. It would be helpful to specify the merge target for this branch (e.g., a version branch like v0.3), or confirm if it should be discarded after the release is published (as suggested on line 666). Clarifying the lifecycle and purpose of the release/<tag-name> branch would prevent confusion.
RELEASE.md
Outdated
|
|
||
| ### Changelog Workflow | ||
|
|
||
| The release workflow automatically generates changelogs from `./CHANGELOG.md`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To improve clarity, it would be helpful to explicitly state that CHANGELOG.md needs to be manually updated before the automated workflow is triggered by a tag push. This involves moving changes from [Unreleased] to a new version section. While this is mentioned in other parts of the document (e.g., line 246), adding it here would make the Changelog Workflow section self-contained and easier to follow.
| The release workflow automatically generates changelogs from `./CHANGELOG.md`: | |
| The release workflow starts with a manual update to `./CHANGELOG.md`, after which automation takes over: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2998 +/- ##
==========================================
- Coverage 56.24% 56.23% -0.01%
==========================================
Files 118 118
Lines 12075 12075
==========================================
- Hits 6791 6790 -1
- Misses 4537 4539 +2
+ Partials 747 746 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
f91abaa to
26cad2f
Compare
|
|
||
| - name: Generate release notes with Claude | ||
| id: claude-release | ||
| uses: anthropics/claude-code-action@v1 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
Uses Step: claude-release
|
|
||
| - name: Create Draft GitHub Release | ||
| id: create-release | ||
| uses: softprops/action-gh-release@v2 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
Overview