-
Notifications
You must be signed in to change notification settings - Fork 32
Produce and publish multi-platform (linux/amd64 and linux/arm64) container images #167
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
base: trunk
Are you sure you want to change the base?
Produce and publish multi-platform (linux/amd64 and linux/arm64) container images #167
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
The builds in my repo are currently failing due to Docker Rate limits. I will rerun them in a few hours and make sure they go green. The CI/Build Checks within this repo will continue to fail until the additional variables/secrets are added. |
Hey @GingerGeek! Thanks for working on this! I won't have time this week to review this, but hoping to get around to it some time next week. But I just wanted to acknowledge that I'd seen the pull request and it is on my radar. |
5a601ae
to
1ea582f
Compare
Thanks, I had a bit of time a few days ago so went through and pinned the GitHub action version as that seemed to be the convention :) |
@desrosj Hey, just wondering if you've had a chance to look at this? |
@GingerGeek Thanks a lot for working on this! As stated in this comment on the issue, I'm somewhat interested in having arm64 versions of our images. I might have a vague grasp of the problem, but I'm far from being a Docker expert. Could you maybe give me a quick breakdown of the individual jobs introduced in the workflow template? Some of them seem to be specifically related to creating digests and manifests -- something that we don't seem to have done before, and that I also didn't come across in the multiplatform docs. I'm guessing that some of this is in order to allow us to build a given image for amd64 and arm64 in separate workflow jobs, and then "couple" them together before uploading to the Docker registry. Is that correct? If so, would it simplify the workflow if we used one job to build for both platforms? |
@ockham yeah, it's a performance thing really. It uses arm64 GitHub Action runner nodes to build the arm64 image and amd64 GitHub Action runner nodes for the amd64 container build. These digests then need to be bundled as you suggested. You can build via emulation but it's slower. This method is vaguely documented here https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners It was also a mention here #127 (comment) that there was a want to adopt official docker github actions etc |
Right, thank you for the explanation! I just came across this example which seems to follow the same strategy (aside from using emulation).
Gotcha. Fast native build sounds better than slow emulation, so no contest here 👍
Which IIUC is what this PR is doing, correct? I'll try the images locally (as you suggested). I'm not sure if and when I can properly review this PR, since it's clearly a bit out of my wheelhouse 😅 |
I've now tested this with Rosetta disabled, with the following changes made to diff --git a/docker-compose.yml b/docker-compose.yml
index 48f3abc607..07196f0a51 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -32,7 +32,7 @@ services:
# The PHP container.
##
php:
- image: wordpressdevelop/php:${LOCAL_PHP-latest}
+ image: gingergeek/wpdev-docker-images-php:${LOCAL_PHP-latest}
networks:
- wpdevnet
@@ -95,7 +95,7 @@ services:
# The WP CLI container.
##
cli:
- image: wordpressdevelop/cli:${LOCAL_PHP-latest}
+ image: gingergeek/wpdev-docker-images-cli:${LOCAL_PHP-latest}
networks:
- wpdevnet And it works like a charm! 🎉 |
Yes!
No worries, it will need a couple tweaks to the GitHub settings by project maintainers anyway so the CI works for the new setup anyway |
I've been discussing how we can safely allow pull requests to build and publish containers to the GHCR for testing purposes with @johnbillion. The first step for considering this is #168. Test containers shouldn't have to come directly from branches on the repository and would make testing pull requests open to anyone and not dependent on a maintainer recreating the PR. After we get that ironed out we should be able to properly test this PR! |
labels: ${{ steps.meta.outputs.labels }} | ||
tags: ${{ vars.DOCKERHUB_PHP_IMAGE }} | ||
outputs: type=image,push-by-digest=true,name-canonical=true,push=true | ||
context: "{{defaultContext}}:images/${{ env.PHP_VERSION }}/php" |
Check warning
Code scanning / zizmor
env.PHP_VERSION may expand into attacker-controllable code Warning
run: | | ||
docker image tag "$PACKAGE_REGISTRY/php:$PHP_VERSION-fpm$PR_TAG" "$PACKAGE_REGISTRY/php:latest$PR_TAG" | ||
docker images | ||
docker push "$PACKAGE_REGISTRY/php:latest$PR_TAG" | ||
mkdir -p ${{ runner.temp }}/digests | ||
digest="${{ steps.build.outputs.digest }}" | ||
touch "${{ runner.temp }}/digests/${digest#sha256:}" |
Check notice
Code scanning / zizmor
steps.build.outputs.digest may expand into attacker-controllable code Note
run: | | ||
docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ | ||
$(printf '${{ vars.DOCKERHUB_PHP_IMAGE }}@sha256:%s ' *) |
Check notice
Code scanning / zizmor
vars.DOCKERHUB_PHP_IMAGE may expand into attacker-controllable code Note
run: | | ||
docker buildx imagetools inspect ${{ vars.DOCKERHUB_PHP_IMAGE }}:${{ steps.meta.outputs.version }} |
Check notice
Code scanning / zizmor
vars.DOCKERHUB_PHP_IMAGE may expand into attacker-controllable code Note
run: | | ||
docker buildx imagetools inspect ${{ vars.DOCKERHUB_PHP_IMAGE }}:${{ steps.meta.outputs.version }} |
Check notice
Code scanning / zizmor
steps.meta.outputs.version may expand into attacker-controllable code Note
run: | | ||
docker buildx imagetools inspect ${{ vars.GHCR_PHP_IMAGE }}:${{ steps.meta.outputs.version }} |
Check notice
Code scanning / zizmor
vars.GHCR_PHP_IMAGE may expand into attacker-controllable code Note
run: | | ||
docker buildx imagetools inspect ${{ vars.GHCR_PHP_IMAGE }}:${{ steps.meta.outputs.version }} |
Check notice
Code scanning / zizmor
steps.meta.outputs.version may expand into attacker-controllable code Note
labels: ${{ steps.meta.outputs.labels }} | ||
tags: ${{ vars.GHCR_CLI_IMAGE }} | ||
outputs: type=image,push-by-digest=true,name-canonical=true,push=true | ||
context: "{{defaultContext}}:images/${{ env.PHP_VERSION }}/cli" |
Check warning
Code scanning / zizmor
env.PHP_VERSION may expand into attacker-controllable code Warning
run: | | ||
docker build \ | ||
--build-arg PACKAGE_REGISTRY="$PACKAGE_REGISTRY" \ | ||
--build-arg PR_TAG="$PR_TAG" \ | ||
-t "$PACKAGE_REGISTRY/cli:$PHP_VERSION-fpm$PR_TAG" \ | ||
"images/$PHP_VERSION/cli" | ||
mkdir -p ${{ runner.temp }}/digests | ||
digest="${{ steps.build.outputs.digest }}" | ||
touch "${{ runner.temp }}/digests/${digest#sha256:}" |
Check notice
Code scanning / zizmor
steps.build.outputs.digest may expand into attacker-controllable code Note
run: | | ||
docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ | ||
$(printf '${{ vars.GHCR_CLI_IMAGE }}@sha256:%s ' *) |
Check notice
Code scanning / zizmor
vars.GHCR_CLI_IMAGE may expand into attacker-controllable code Note
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.
octoscan found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Bumps the github-actions group with 3 updates: [actions/checkout](https://github.com/actions/checkout), [shivammathur/setup-php](https://github.com/shivammathur/setup-php) and [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv). Updates `actions/checkout` from 4.1.7 to 4.2.2 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4.1.7...11bd719) Updates `shivammathur/setup-php` from 2.31.1 to 2.34.1 - [Release notes](https://github.com/shivammathur/setup-php/releases) - [Commits](shivammathur/setup-php@c541c15...0f7f1d0) Updates `astral-sh/setup-uv` from 6.1.0 to 6.3.0 - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@f0ec1fc...445689e) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 4.2.2 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: shivammathur/setup-php dependency-version: 2.34.1 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: astral-sh/setup-uv dependency-version: 6.3.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions ... Signed-off-by: dependabot[bot] <support@github.com>
…to multiarch-container-images
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.
Sorry for all the noise with the new linting annotations. We can address those out as we go.
I've added a few points to the PR. @GingerGeek I think that the testing you are doing in your repository is showing the same issue here where the branch must exist on the repo itself and not within a fork.
I could open a PR from a branch on the repo for testing this if we'd like. Alternatively, we can solve that issue separately before adjusting this PR accordingly.
echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV | ||
- name: Docker meta | ||
id: meta | ||
uses: docker/metadata-action@902fa8ec7d6ecbf8d84d538b9b233a880e428804 # 5.7.0 |
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.
I have configured the two GHCR related vars on the repository, but I've been unable to see them being passed to this step.
I couldn't find this anywhere in the GHA documentation, but ChatGPT tells me that "If the workflow was triggered by a pull request from a fork, vars (like secrets) won’t be passed unless the pull_request_target event is used and trusted permissions are configured."
If this is the case, using variables is unfortunately not an option unless things are reworked a bit. An example of this may be how changes to built files are committed back to a PR branch in WordPress/wordpress-develop.
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.
I have read into this some more, it is not possible for a pull request from a forked repository to be given write access to packages
Forks can modify the GitHub Action files, and even with hardening granting write access to packages is broad.
Perhaps for PRs we should generate an image that can be imported as a tar etc?
|
||
registry: ghcr.io | ||
username: ${{ github.actor }} | ||
password: ${{ secrets.GITHUB_TOKEN }} |
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.
At first it was not obvious why this would actually work. How would wileEcoyote
be able to authenticate using GITHUB_TOKEN
if they do not belong to the organization. But after researching, it appears that the username
is not used for authentication and it's handled by the token itself.
We should include an inline comment here briefly explaining that for clarity. Same below for the CLI images.
This PR introduces building
arm64
container images.Both
linux/amd64
andlinux/arm64
are then combined into a multi-platform imageAs part of the PR I also slightly changed the required variables that need to be configured in the repo (sorry)
You can see/test the result of the images here for Docker Hub:
and here for the GHCR (NB tags have an extra - as these are designed for PR flows):
https://github.com/GingerGeek?tab=packages&repo_name=wpdev-docker-images
GHCR pushes happen with the provided workflow permissions, although this may break the PR flow, I am unsure.
Secrets
DOCKERHUB_TOKEN
- PAT/Token for Docker Hub authenticationVariables
DOCKERHUB_USERNAME
- Username for Docker Hub authenticationDOCKERHUB_PHP_IMAGE
- Destination for PHP image in Docker Hub (i.e.wordpressdevelop/php
)DOCKERHUB_CLI_IMAGE
- Destination for CLI image in Docker Hub (i.e.wordpressdevelop/cli
)GHCR_PHP_IMAGE
- Destination for PHP image in GitHub Container Registry (i.e.ghcr.io/wordpress/wpdev-docker-images/php
)GHCR_CLI_IMAGE
- Destination for CLI image in GitHub Container Registry (i.e.ghcr.io/wordpress/wpdev-docker-images/cli
)Solves #127