-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
Refactor Docker files for ARM performance improvements #282
Conversation
Update Dockerfile to attempt to optimise build times,
Update with optimisation attempts
Fix my absolute shit-show of a Docker refactor.
Update the action for Arm
WalkthroughThe changes in this pull request involve significant updates to Docker publishing workflows for ARM architecture and rootless images, alongside modifications to Dockerfiles. The workflows now include refined triggers, environment variable settings, and enhanced build steps utilizing Docker Buildx for improved performance. Both workflows have introduced image attestation for build provenance. The Dockerfiles have been optimized for cross-platform compatibility and caching efficiency, with structural adjustments to enhance clarity and functionality. Changes
Possibly related PRs
Suggested labels
Security Recommendations
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
I didn't mean to use a remote build... Force of habit!
Fixing a force-of-habit mistake 🙁
This is starting to annoy me now
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 (6)
.github/workflows/docker-publish-rootless-arm.yaml (6)
53-56
: Approve Docker Buildx setup with a security recommendation.The update to Docker Buildx action v3.0.0 and the addition of 'install' and 'use' options are good improvements. They ensure the latest stable version is used and Buildx is properly set up for the job.
For enhanced security, consider pinning the action to a specific commit SHA instead of using a version tag. This prevents potential supply chain attacks if the tag is compromised. For example:
uses: docker/setup-buildx-action@f95db51fddba0c2d1ec667646a06c2ce06100226 # v3.0.0You can find the commit SHA for a specific version in the action's release notes or GitHub repository.
62-62
: Approve Docker login action update with a security recommendation.Updating the Docker login action to v3.0.0 is a good practice for maintaining up-to-date and secure dependencies.
Similar to the previous recommendation, consider pinning this action to a specific commit SHA for enhanced security:
uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d # v3.0.0This practice helps prevent potential supply chain attacks by ensuring you're using a specific, verified version of the action.
72-78
: Approve metadata action update and tagging strategy with security recommendations.The update to Docker metadata action v5.0.0 and the enhanced tagging strategy are excellent improvements. The new tagging approach provides better versioning granularity and supports nightly builds, which is beneficial for tracking and potential rollbacks.
Consider the following security recommendations:
Pin the action to a specific commit SHA:
uses: docker/metadata-action@31cebacef4805868f9ce9a0cb03ee36c32df2ac4 # v5.0.0For the nightly builds, consider adding a SHA or build number to the tag to ensure uniqueness and traceability:
type=schedule,pattern=nightly-{{sha}}
Implement a retention policy for nightly builds to manage the accumulation of images over time.
These practices enhance the security and manageability of your Docker images.
86-97
: Approve build and push action update with security recommendations.The update to Docker build-push action v5.0.0 and the additions for multi-platform builds, caching, and build arguments are excellent improvements. These changes enhance build performance, cross-platform compatibility, and image traceability.
Consider the following security recommendations:
Pin the action to a specific commit SHA:
uses: docker/build-push-action@4a13e500e55cf31b7a5d59a38ab2040ab0f42f56 # v5.0.0Implement content trust for image signing and verification:
with: ... trust-server-cert: ${{ secrets.DOCKER_TRUST_SERVER_CERT }}Consider using BuildKit's secret passing feature for sensitive build arguments:
secrets: | "mysecret=${{ secrets.MYSECRET }}"Implement image scanning in your workflow to detect vulnerabilities before pushing:
- name: Scan image uses: anchore/scan-action@v3 with: image: "${{ steps.build-and-push.outputs.imageid }}" fail-build: true severity-cutoff: highThese practices will further enhance the security of your build and push process.
99-108
: Highly approve image attestation addition with further security recommendations.The addition of image attestation is an excellent security improvement. It enhances the provenance and integrity of your built images, which is crucial for maintaining a secure software supply chain.
To further strengthen your security posture, consider the following recommendations:
Pin the attestation action to a specific commit SHA:
uses: actions/attest-build-provenance@2d7a3f2d1f2d0d0e0d0d0d0d0d0d0d0d0d0d0d0d # v1Implement a policy to verify attestations before deployment:
- name: Verify attestation run: | cosign verify-attestation ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@${{ steps.build-and-push.outputs.digest }}Consider implementing additional attestations, such as SLSA provenance:
- uses: slsa-framework/slsa-github-generator@v1 with: base64-subjects: ${{ steps.build-and-push.outputs.digest }} provenance-name: provenance.intoto.jsonlEnsure that your artifact consumers are set up to verify these attestations before using the images.
These practices will further enhance the security and trustworthiness of your build artifacts.
81-81
: Remove trailing space for better code hygiene.There is a trailing space on this line. While this doesn't affect functionality, it's good practice to remove it for code cleanliness.
Please remove the trailing space from this line:
- +This small change will improve code consistency and eliminate potential issues with systems sensitive to trailing spaces.
🧰 Tools
🪛 yamllint
[error] 81-81: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .github/workflows/docker-publish-arm.yaml (2 hunks)
- .github/workflows/docker-publish-rootless-arm.yaml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/docker-publish-arm.yaml
🧰 Additional context used
🪛 yamllint
.github/workflows/docker-publish-rootless-arm.yaml
[error] 81-81: trailing spaces
(trailing-spaces)
@coderabbitai full review |
✅ Actions performedFull review triggered. |
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Bit of a messy PR sorry as I had a bit of a 'mare with the old Docker commands (force of habit of using remote build controls) In testing, I've gotten ARM builds down from an average of ~24minutes to an average of ~14 minutes. I suspect this may get better with more aggressive caching. |
What type of PR is this?
What this PR does / why we need it:
Make adjustments to Docker build processes to better leverage cache and refactor to improve overall performance and reliability.
Which issue(s) this PR fixes:
#281
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores