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

feat(docker): add dockerfile best practices #163

Merged
merged 3 commits into from
Feb 12, 2025

Conversation

0xlildoudou
Copy link
Contributor

Hi,

I have add Dockerfile best practices (https://docs.docker.com/build/building/best-practices/)

Check with Hadolint:

$ docker run --rm -i hadolint/hadolint hadolint "$@" - < Watcher/Dockerfile 
-:25 DL3059 info: Multiple consecutive `RUN` instructions. Consider consolidation.
-:28 DL3059 info: Multiple consecutive `RUN` instructions. Consider consolidation.

https://github.com/hadolint/hadolint

@ygalnezri
Copy link
Collaborator

Hi @0xlildoudou,

Thank you for your contribution! We are happy to accept the proposed modifications to the Watcher/Dockerfile as they align well with the recommended practices. However, we would like to keep the current version of the docker-compose.yml file as it is, specifically with the following line that specifies the image:

image: felix83000/watcher:latest

We believe this configuration works best for our current setup. Once you update the pull request with the modifications to the Dockerfile while keeping the docker-compose.yml unchanged, we will be happy to accept it.

Kind regards,
Ygal

@0xlildoudou
Copy link
Contributor Author

Hi,

I have change the image on docker-compose.yml.

Personally, I'd rather build the image in a deployment than retrieve an image from dockerhub that I don't know how it was built.

@ygalnezri ygalnezri merged commit 48d893c into thalesgroup-cert:master Feb 12, 2025
1 check passed
@0xlildoudou 0xlildoudou deleted the dockerfile branch February 12, 2025 10:32
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