acme-dns
acme-dns copied to clipboard
small Dockerfile improvements
This PR improves the following points in the Dockerfile:
- build exact git tag instead of the current master
- update of the current golang build environment from 1.9.2 to 1.11.4
- use scratch instead of an alpine image
- run container as user 1000 (instead of root)
- change listening ports from system ports 53, 80, 443 to 5353, 8080, 8443
WARNING - this PR contains breaking changes in the Dockerfile The container that is started from the image that is built from this Dockerfile is running as user 1000 instead of root (0). As a non root user cannot (per default) listen on system ports (<1024), I had to change them to ones that are higher than 1024. In this case 5353, 8080. Updates from older versions should not be a problem, as long as the user 1000 or the group 0 has read permissions on the config file, write permissions on the sqlite db file and the listening ports are higher than 1024.
Coverage remained the same at 93.831% when pulling 6ada9dd5b92747148d3d196641f3cd66ef3be880 on schue30:master into e1f1d6af34f82b3b75021883a02f72db9d9d41b5 on joohoi:master.
Thanks for the PR! This looks like a good update, I'm just slightly hesitant to merge it straight away because of the backwards breaking changes. I'll test it throughly in the near future to be able to figure out a good way to inform the current users about the changes they need to make.
FYI, this PR should also include changes to docker-compose.yml (at a minimum I believe the port mappings need to be updated).
Also, this fixes #79
FYI, this PR should also include changes to
docker-compose.yml(at a minimum I believe the port mappings need to be updated).
Yes, for sure. I will add the change for the docker-compose.yml.
Ok, this PR looks good. I had to pick the Go base image line from this PR for the work done in #147 so there's a merge conflict for that line, I'm sorry.
In addition to the merge conflict resolution, I think we should update the README to point to the new port configuration in: https://github.com/joohoi/acme-dns#using-docker
Ping @schue30 are you interested to continue working on this PR or can someone else continue from here on? I would love to hear your opinion especially about the volume change, I'm not a Docker expert by any means myself.
Pong @joohoi
Thanks for all the work you've put into this @schue30 and @tcely ! Looks robust. I'm still wondering how to notify the current Docker users of acme-dns about the breaking changes. What's the usual process?
Regarding communication, normally I'd suggest a major version bump as a way of denoting that the changes could break stuff. In this case though it doesn't seem like there are any changes to acme-dns itself, just to the docker container. So I guess you could just bump the docker container version, but then it'd be out of sync with the acme-dns version. Hmm...
Thanks for all the work you've put into this @schue30 and @tcely ! Looks robust. I'm still wondering how to notify the current Docker users of
acme-dnsabout the breaking changes. What's the usual process?
I'm not sure we can do any better than a note at the top of the README.md file.
I see that breaking changes stop this PR from being merged. There are ways to accomplish nearly the same without breaking changes.
- File permissions problem: Start container as root with a new entrypoint script. The new entrypoint script ensures correct file permissions on every startup and then drops root with e.g. gosu. This is common practice in several Docker images.
- Port problem: use old ports (80, etc) and add container capability NET_BIND_SERVICE so non-root processes can open lower ports in this container. (As explained in Docker security documentation)
// edit: fixed broken link.
Hey folks, i've just implemented my self-hosted acme-dns instance into production using the modified version of the Dockerfile: https://github.com/joohoi/acme-dns/commit/43c4f1bef8a695ffa7686730dbff9bfa44fd9189
I don't want to create a PR yet as i see you are trying to solve similar docker image improvements here. My proposal is to use a multi stage docker build using the scratch as a final image. This has following features:
- resulting image contains only the
acme-dnsbinary - much more secure and decreased attack surface by using the scratch image
- backward compatible with the old image
This solves all the previous implementation discussions that should be rather solved by outside of the project:
- USER/uid discussion - you don't need to solve this for scratch images
- EXPOSE/VOLUME - those are exposed/mapped overriden at the container runtime and are implementation-specific. In the Dockerfile they have practically only informational purpose.
- config shouldn't be included in the docker image as the default one is not possible to use for the real implementation either
I think this can significantly your whole workflow.
Root in scratch image is fine for my level of paranoia.
Would make things much more simple.