HeyPuter
Add requirements for multi-platform docker build to the CI
This PR address the problem of multi-platform docker image build.
I would suggest adding a separate docker CI to test the build process without publishing to GitHub registry. And then we can add trigger on PRs.
I removed my change earlier because I broke the github action, so this has created a conflict - you can simply overwrite my change with what's in this PR
neat, apparently it lets me resolve the conflict myself
Great, let's also wait until I can see why it takes so much to build it. I know that the action on my fork will fail at the end (because of image push step) but it is taking too long for the actual build
https://github.com/MohamedElashri/puter/actions/runs/8561767017/job/23463688673
Looks like it got stuck. Is it expected that it got stuck where it did, or does that indicate a problem?
Looks like it got stuck. Is it expected that it got stuck where it did, or does that indicate a problem?
No, and actually my expectation was wrong. This action does not rely on any credentials. It uses GitHub API to publish to the repo packages. So now it pushed the image I build to fork repo packages. So it will proceed to the end (barring any unexpected issues like this one)
I removed linux/arm/v7 and linux/arm/v6 temporarily to see if the action will work, and it did. I will try to dig in and see why does this happen. But for now we have arm64 build without problems. And we can try it here
Since you added make, g++ and other "build" tools, the best practice in this case is to use a multi-stage build process (https://docs.docker.com/build/building/multi-stage/) such that those packages aren't there in the final build image.
Leaving tools like make, g++ and so on in a container image seriously increase the threat profile of the container (let alone the amount of dependancies, and thus potential CVEs to deal with) as it gives a potential attacker access to build tools right from the container's context, which gets them one step closer to breaking out of the container boundaries.
While I'm not an owner of the Puter repo, if this was one of my own repo, I'd rework this such that build dependancies are only present during the build stages of the container image (FROM xxx AS yyy followed by one or more COPY --from=yyy /bin/foo as required).
@KernelDeimos It seems that the problem is that it get stuck with linux/arm/v7 and linux/arm/v6 so probably we don't need to add them for now. They are usually Chromebooks, old rps and phones anyway. Even if we fix them, adding all of these will consume a lot of build time and will incur some cost. Now, both amd64 and arm64 works. Would you accept that?
@shuguet That is a great suggestion, I have done that, and now we have a smaller and potentially more safe image (in the sense that it doesn't add unnecessary tools)
If we can agree on that, then I will move the PR for review.
You're welcome.
LGTM as is, I'll let @KernelDeimos give you his feedback. Thank you for your work!
Now, both amd64 and arm64 works. Would you accept that?
Yes, that sounds good to me. For other architectures I think it's reasonable to consider this out-of-scope and expect that deployment for these architectures includes building an appropriate image.