-
Notifications
You must be signed in to change notification settings - Fork 39
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
ci: fix artifact upload issue on release build #2389
Conversation
Warning Rate limit exceeded@lklimek has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 55 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes in this pull request focus on updating the workflow configuration for building and publishing Docker images within the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/release-docker-image.yml (1)
Line range hint
23-24
: Consider adding documentation for the artifact naming scheme.The
DIGEST_NAME
environment variable now includes platform-specific information in the artifact names. Consider adding comments to document this naming scheme and its importance for multi-platform builds.env: + # Format: digests-{org}-{image}-{tag}-{profile}-{sha}-{platform} + # Platform suffix is added during artifact upload to prevent conflicts in multi-platform builds DIGEST_NAME: digests-${{ inputs.image_org }}-${{ inputs.image_name }}-${{ inputs.tag }}-${{ inputs.cargo_profile }}-${{ github.sha }} DIGEST_DIR_PATH: /tmp/digests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release-docker-image.yml
(2 hunks)
🔇 Additional comments (2)
.github/workflows/release-docker-image.yml (2)
85-85
: Excellent fix for the artifact conflict issue!
Adding the platform to the artifact name is a good solution that prevents the 409 Conflict error by ensuring unique names for artifacts from different platforms.
96-100
: Verify the artifact pattern matching scope.
The download configuration looks good and properly complements the upload changes. However, let's verify that the pattern ${{ env.DIGEST_NAME }}*
only matches the intended artifacts.
✅ Verification successful
This will help us verify if there are any potential naming conflicts with artifacts in other workflows and understand how DIGEST_NAME is defined to ensure its uniqueness.
Pattern matching is safe and specific to this workflow
The artifact pattern is well-scoped and safe. The DIGEST_NAME
environment variable includes multiple unique components (org, image name, tag, cargo profile, and commit SHA) making it highly specific to this workflow's execution. No other workflows in the repository use similar naming patterns for their artifacts, eliminating the risk of unintended matches.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential naming conflicts with other workflow artifacts
# Search for other workflow files that might create artifacts with similar names
echo "Checking for other workflows that create artifacts with similar naming patterns..."
rg -l "actions/(upload|download)-artifact" .github/workflows/
Length of output: 459
Script:
#!/bin/bash
# Check artifact naming patterns in other workflows
echo "Checking artifact names in other workflows..."
rg -A 3 -B 1 "actions/(upload|download)-artifact.*with:" .github/workflows/ | grep -E "name:|pattern:"
# Check DIGEST_NAME definition
echo -e "\nChecking DIGEST_NAME environment variable definition..."
rg "DIGEST_NAME" .github/workflows/release-docker-image.yml
Length of output: 633
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/release-docker-image.yml (2)
Line range hint
137-144
: Add error handling for manifest creationWhile the manifest creation logic is correct, consider adding error handling to make the workflow more robust.
Consider this improvement:
- name: Create manifest list and push working-directory: ${{ env.DIGEST_DIR_PATH }} run: | ls -lah + if [ -z "$(ls)" ]; then + echo "Error: No digest files found in ${{ env.DIGEST_DIR_PATH }}" + exit 1 + fi docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ $(printf '${{ inputs.image_org }}/${{ inputs.image_name }}@sha256:%s ' *) || { + echo "Error: Failed to create manifest list" + exit 1 + }🧰 Tools
🪛 actionlint (1.7.4)
85-85: parser did not reach end of input after parsing the expression. 5 remaining token(s) in the input: "(", "STRING", ",", "STRING", ")"
(expression)
Line range hint
1-144
: Overall implementation looks solidThe changes effectively solve the artifact upload conflict issue by:
- Making artifact names unique per platform and profile
- Properly handling multi-platform builds using download-artifact@v4's new features
- Maintaining robust manifest creation with proper tag handling
The implementation follows GitHub Actions best practices and should prevent the "409 Conflict" errors mentioned in the PR description.
For future maintenance:
- Consider documenting the artifact naming convention in the workflow comments
- Monitor GitHub Actions version updates for potential breaking changes in artifact handling
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release-docker-image.yml
(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-docker-image.yml
85-85: parser did not reach end of input after parsing the expression. 5 remaining token(s) in the input: "(", "STRING", ",", "STRING", ")"
(expression)
🔇 Additional comments (2)
.github/workflows/release-docker-image.yml (2)
85-87
: LGTM: Improved artifact naming strategy
The updated artifact naming strategy that includes the platform identifier helps prevent naming conflicts when uploading artifacts from multiple platforms. This directly addresses the 409 Conflict error mentioned in the PR description.
🧰 Tools
🪛 actionlint (1.7.4)
85-85: parser did not reach end of input after parsing the expression. 5 remaining token(s) in the input: "(", "STRING", ",", "STRING", ")"
(expression)
96-100
: LGTM: Proper artifact download configuration
The configuration properly handles multi-platform builds by:
- Using download-artifact@v4
- Using pattern matching to download all platform-specific digests
- Merging multiple artifacts into one directory
Let's verify the artifact pattern matches all platform variants:
✅ Verification successful
Let me verify the actual pattern used in the workflow file to ensure it matches what we tested.
Pattern correctly matches all platform-specific digest artifacts
The workflow correctly handles multi-platform artifacts:
- The
DIGEST_NAME
env var forms the base name:digests-{org}-{name}-{tag}-{profile}-{sha}
- The upload uses this pattern with platform suffix:
{DIGEST_NAME}-{platform}
- The download pattern
{DIGEST_NAME}*
will match all platform variants - The
merge-multiple: true
option ensures all matched artifacts are combined
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the artifact pattern matches all platform variants
# Expected: Pattern should match both amd64 and arm64 platform digests
# Test the pattern against example artifact names
echo "digests-dashpay-drive-v0.1.0-release-abc123-linux-amd64" | grep -E "digests-.*"
echo "digests-dashpay-drive-v0.1.0-release-abc123-linux-arm64" | grep -E "digests-.*"
Length of output: 283
Script:
#!/bin/bash
# Get the actual pattern used in the workflow file
cat .github/workflows/release-docker-image.yml | grep -A3 "download-artifact@v4"
# Also check if there are any env var definitions for DIGEST_NAME
cat .github/workflows/release-docker-image.yml | grep "DIGEST_NAME"
Length of output: 549
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.
👍
Issue being fixed or feature implemented
Update to upload-artifact v4 caused the following issue:
What was done?
Fixed the issue by updating the workflow
How Has This Been Tested?
GHA https://github.com/dashpay/platform/actions/runs/12315436050/job/34373645414
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
cargo_profile
input to enhance workflow configuration.Bug Fixes
Chores