Skip to content
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

chore: Improve Dockerfile #197

Closed
wants to merge 1 commit into from

Conversation

reneleonhardt
Copy link

@reneleonhardt reneleonhardt commented Apr 28, 2024

Image builds faster and smaller, fixed some Dockerfile warnings and 166 security findings 😅
Please double-check if everything is still working as expected (I removed docker-compose) and release a new image.

Consider rebuilding the image automatically with something like https://github.com/aquasecurity/trivy-action and migrating to alpine, should be smaller and have less dependencies (docker also installs engine, cli and buildx).
https://pkgs.alpinelinux.org/packages?name=docker*&branch=v3.19&repo=community&arch=x86_64

@elgohr
Copy link
Owner

elgohr commented May 14, 2024

Thank you for the PR!
Will review in near future

@elgohr
Copy link
Owner

elgohr commented May 15, 2024

I'm thinking about declining the PR.

The Dockerfile is used in context of testing the action within CI/CD. The image will never be in production, but be gone after testing.

There're a few docker optimizations still in place, from a time where this action shipped the image to production. This isn't the case anymore, so the optimizations could also be removed in future.

As the image isn't shipped, the size of the image isn't relevant at all.

Security findings and speed would be an improvement for the release process.

Nevertheless the changes add more complexity to the build process of the image.

I have to check the speed improvement this would add.

Where did you get the security findings that you had been writing about?

@reneleonhardt
Copy link
Author

reneleonhardt commented May 15, 2024

It's used in release.yml, 168 findings today, too many packages installed and too rarely updated (Docker shows 8 months old jammy-20230816).

docker pull vuln ghcr.io/elgohr/publish-docker-github-action/publish-docker-github-action
trivy image --scanners vuln ghcr.io/elgohr/publish-docker-github-action/publish-docker-github-action
publish-docker-github-action jammy-20230816

@elgohr
Copy link
Owner

elgohr commented May 17, 2024

Alright I totally got that for production related images. For testing images these findings seem odd.

@elgohr
Copy link
Owner

elgohr commented May 24, 2024

I measured it multiple times - the image builds at my machine in around 1:15 min.
Main tends to be a little quicker (1:10 - 1:15 min). Ironically the PR seems to be slower (1:15-1:20 min).
So this is no reason.

In addition the screenshot of the vulnerabilities shows an image that was created 8 month ago (see top of screenshot).
In this way an 8 month old jammy is no surprise.

I'm really happy that you tried to improve the action!
Nevertheless I'm going to reject the PR as I'm not seeing the desired improvement - sorry!
Next time please start with an issue, so we don't come to the point to throw away time/code again.

@elgohr elgohr closed this May 24, 2024
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