Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

Description

Manifest not using right tags by arch -- fix by getting tags as variable.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally and in CI (bun run test)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have updated version numbers as needed (if needed)
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Security Considerations:

  • My changes do not introduce any new security vulnerabilities
  • I have considered the security implications of my changes

@vercel
Copy link

vercel bot commented Jul 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 25, 2025 11:19pm
sim 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 25, 2025 11:19pm

labels: ${{ steps.meta.outputs.labels }}
cache-from: type=gha,scope=build-v2
cache-to: type=gha,mode=max,scope=build-v2
provenance: false
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need these to not make manifest lists

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR fixes a critical bug in the Docker manifest creation process within the GitHub Actions workflow. The core issue was that the manifest creation logic wasn't properly constructing architecture-specific image names, which would cause multi-architecture manifest creation to fail or produce incomplete manifests.

The key changes include:

  1. Variable renaming for clarity: Changed TAGS to MANIFEST_TAGS to better reflect the purpose of the variable
  2. Fixed architecture-specific tag construction: The workflow now properly appends -amd64 and -arm64 suffixes to the base manifest tags when looking for architecture-specific images
  3. Added build configuration flags: Disabled provenance and SBOM generation (provenance: false, sbom: false) to prevent potential interference with manifest creation
  4. Improved error handling: Added detailed logging and changed from warning-based handling to hard failures when architecture images are missing

This fix is crucial for the Docker build pipeline as it ensures that multi-architecture manifests are properly created and pushed. The workflow builds images with architecture suffixes (as defined in the build matrix), but the previous manifest creation logic failed to account for this naming convention. Now users can pull images without specifying architecture-specific tags, and the manifest will properly route to the correct architecture-specific image.

Confidence score: 4/5

• This is a solid bug fix that addresses a fundamental issue in the Docker manifest creation process
• The score reflects that while the logic is sound, CI/CD changes can have subtle deployment implications that may not be immediately apparent
• The .github/workflows/build.yml file needs careful attention to ensure the matrix build strategy aligns with the new manifest creation logic

1 file reviewed, no comments

Edit Code Review Bot Settings | Greptile

@icecrasher321 icecrasher321 merged commit a251122 into staging Jul 25, 2025
2 of 4 checks passed
@icecrasher321 icecrasher321 mentioned this pull request Jul 25, 2025
12 tasks
@waleedlatif1 waleedlatif1 deleted the fix/manifests branch July 27, 2025 01:33
arenadeveloper02 pushed a commit to arenadeveloper02/p2-sim that referenced this pull request Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants