search

oxsecurity / megalinter

🦙 MegaLinter analyzes 50 languages, 22 formats, 21 tooling formats, excessive copy-pastes, spelling mistakes and security issues in your repository sources with a GitHub Action, other CI tools or locally.
https://megalinter.io
GNU Affero General Public License v3.0
1.81k stars 215 forks source link

Support for ARM64 for docker image #1180

Open munyoudoum opened 2 years ago

munyoudoum commented 2 years ago

Currently only see linux/amd64 on https://hub.docker.com/r/megalinter/megalinter/tags

It would be amazing to have arm64 also

Thanks

nvuillam commented 2 years ago

It's probably possible :) https://medium.com/swlh/using-github-actions-to-build-arm-based-docker-images-413a8d498ee

But will take some time if I have to do it myself because of my little available time these days ^^

We could have "mirror" images with -arm , like megalinter/megalinter:arm-v5 , arm-beta , etc ...

Do you have Github Actions knownledge ? I would glady accept a PR :)

echoix commented 2 years ago

Today I tried to build an arm64 docker image.

The solution to build multiple docker images (same name, but different CPU architectures combined) is to use buildx. Since it is running in a shell script and not in pure GitHub action, then the setup is to do

docker buildx create --use

Then, replacing 

docker build --no-cache --build-arg "BUILD_DATE=${BUILD_DATE}" --build-arg "BUILD_REVISION=${BUILD_REVISION}" --build-arg "BUILD_VERSION=${BUILD_VERSION}" -t "${CONTAINER_URL}:${IMAGE_VERSION}" -f "${DOCKERFILE_PATH}" . 2>&1

with 

docker buildx build --platform linux/amd64,linux/arm64 --no-cache --build-arg "BUILD_DATE=${BUILD_DATE}" --build-arg "BUILD_REVISION=${BUILD_REVISION}" --build-arg "BUILD_VERSION=${BUILD_VERSION}" -t "${CONTAINER_URL}:${IMAGE_VERSION}" -f "${DOCKERFILE_PATH}" . 2>&1

should do the work (replacing all the places with build by buildx build and platforms.

However, there is some prep work to make everything work. Currently, in the Dockerfile some tools are downloaded with an explicit url targeting x86_64 or amd64, etc. There must be a way of defining different urls or download links per tool. Not all tools name arm64 the same way, so no variable in URL works for everyone (some call it aarch64 for example). Also, some tools, like dotenv-linter have problems building for arm64 on Docker because of a long-standing problem of a rust crypto library, Ring. I tried to see if I could fix it myself, but it seems harder than it seems.

I also tried building the Dockerfile locally on a Raspberry Pi 4, but it failed when it came time to pull FROM checkmarx/kics:alpine as kics since my Raspberry Pi 4 was not installed on a 64-bit OS, so it reported armv7l, asking linux/arm/v7, which they don't publish.

So, I'll try to come back with a TODO list of linters to fix the install to get a building image, and one day we could talk about actual failing linters. In the meantime, some prep work to integrate having different URL installers should be done. Most of the Docker tools are ok, especially when they target only alpine, or many arch are available.

nvuillam commented 2 years ago

Thank a lot for your analysis :)

About not compliant linters, we could eventually flag them in descriptors so they are simply not added within arm64 images

Story to be continued... :)

echoix commented 2 years ago

I just hope that to start, at least one flavour would have all linters working... We'll see :)

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

echoix commented 2 years ago

I'd like to keep it as a milestone task, but it'll be over the next two months though... I'm still thinking it out regularly

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

natpicone commented 2 years ago

really needed

echoix commented 2 years ago

@natpicone What linters or flavours do you use in priority? It is not a simple task to have MegaLinter on arm, since we would have to ask for changes in many of the included projets. Pure Python linters should run without any problems, but we still need to figure out how to built multi arch image Dockerfiles.

natpicone commented 2 years ago

@natpicone What linters or flavours do you use in priority? It is not a simple task to have MegaLinter on arm, since we would have to ask for changes in many of the included projets. Pure Python linters should run without any problems, but we still need to figure out how to built multi arch image Dockerfiles.

I'm currently using ruby and javascript ones; running them on my M1 is a pain because sometimes docker x86 emulation crashes or takes a lot of time.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

Kurt-von-Laven commented 2 years ago

For some reason the bot didn’t honor the nostale label, so I removed the stale label manually.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

bdovaz commented 2 years ago

I have started working on it: #1553

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

echoix commented 1 year ago

No stale, the other PR is not finished

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

jokay commented 6 months ago

@nvuillam any progress on this one?

nvuillam commented 6 months ago

@jokay there was a PR which was quite progressing, but other maintainers are following the topic and probably could tell you more :)

Cc @echoix @Kurt-von-Laven @bdovaz

bdovaz commented 6 months ago

Well, the truth is quite scattered...

I started with #2273 but I found several blocks....

@waterfoul followed with #2549 (which I really miss what was trying to be achieved here) but has not given signs of life for some time and it's a pity because it looks like good progress.

And finally @echoix is with #3193 which allows to unlock some problem.

nvuillam commented 6 months ago

The summary seems to be "Someday in a MegaLinter v8" ^^

bdovaz commented 6 months ago

@echoix now that #3193 has been merged what do you see as the next step? I still don't understand what we were trying to solve in #2549 and if it's necessary for my PR #2273 ...

echoix commented 6 months ago

@bdovaz it would allow to run a something like https://github.com/echoix/megalinter/pull/28/files#diff-c40ecec00fa976e23241a7188bb7ca5e905d288775c1286f81dd85d1e07ba2f0R41-R48 (and the changes that I made under that to push to that) without failing from out of size when testing (because it pulled a second container). So, having this service means we can push a locally built image (even if built for multiple platforms), and still have it available in the same run. This solves the error that we weren't able to "load" a multiplatform image to test test it in the same run.

But we will need to fight against the disk space, since at one point, when pushing the built image, will have the space used by docker buildx plus the space used by that local registry that it is copied to. Multi-platform image means that we will have 2x the space used. (4x the size I'm of the image). I can image a way that we can lower the max disk space used to 3x the image size, by building one after the other and cleaning buildx in between, but still. That will be an issue to figure out. But at least it should allow us to advance a bit more.

Maybe we could start by merging #2549 into #2273, (without having everything working yet), apply the changes that use the metadata action, then we could add the local registry and really start building and work around the failures of missing space or builds too long/too big.

bdovaz commented 6 months ago

@echoix I think you misunderstood me, I was referring to #2549 which is the one I don't know what I was trying to solve and it's half-finished and @waterfoul gives no sign.

And I don't know what improves / solves over my PR (#2273).

echoix commented 6 months ago

@echoix I think you misunderstood me, I was referring to #2549 which is the one I don't know what I was trying to solve and it's half-finished and @waterfoul gives no sign.

And I don't know what improves / solves over my PR (#2273).

2549 had some pretty nice optimizations, Dockerfile organization and patterns that would be a shame to loose. It solved multiple problems (that it had) when he was doing it. I know it might be impossible to do a real "merge" because of how fast the repo changes, but the ideas are still clearly there.

mbyio commented 4 days ago

Hi, I just wanted to chime in to note that this issue might be a little more relevant now, due to the release of ARM64 GitHub Actions runners.