hardware / mailserver

:warning: UNMAINTAINED - Simple and full-featured mail server using Docker

https://store.docker.com/community/images/hardware/mailserver

MIT License

1.29k stars 322 forks source link

Feature: Certificate Watching #366

Closed Matchlighter closed 5 years ago

Matchlighter commented 5 years ago

Description

(Built on top of https://github.com/hardware/mailserver/pull/336)

Adds FS watches on Traefik's acme.json file and on a mounted Let's Encrypt live directory. Automatically reparses the cert files and reloads Postfix and Dovecot.

Watcher is debounced to 3 seconds and compares the old and new privkey.pem before reloading Postfix and Dovecot.

Mount locations for acme.json and live were kept the same. Moved dump.log to <mount>/mail/ssl/acme_dump.log.

Risks

Moved around in-container certificate paths a little. This could affect downstream containers if they rely on those paths.

Type of change

[x] New feature
[x] Refactoring

Status

[x] Ready
[ ] In development
[ ] Hold

How has this been tested ?

[x] In production with Traefik and Let's Encrypt Certs
[x] By touching the acme.json file from outside the container and validating expectations
[x] Ran provided Test Suite

sknight80 commented 5 years ago

As I understand the python code and the bash script, I think this is good. When can we merge?

sknight80 commented 5 years ago

@Matchlighter do we need to wait for the https://github.com/hardware/mailserver/pull/336 PR to be merge before we merge this one?

Matchlighter commented 5 years ago

Yeah, this one is based on #336 - so merging this one would automatically merge #336.

On May 29, 2019, 11:22 AM, at 11:22 AM, Istvan Szabo notifications@github.com wrote:

@Matchlighter do we need to wait for the https://github.com/hardware/mailserver/pull/336 PR to be merge before we merge this one?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/hardware/mailserver/pull/366#issuecomment-497030933

sknight80 commented 5 years ago

Awesome. Then can we merge these PRs?

Matchlighter commented 5 years ago

I'm down. I don't have merge perms though.

sknight80 commented 5 years ago

@Matchlighter looks like I have the power to do it. Should we merge this or we should merge the other PR first?

Matchlighter commented 5 years ago

@sknight80 Semantically, I'd probably recommend merging #336 first. This one does completely include it, but merging the other first will make sure that it has a merge-commit in the Git history.

sknight80 commented 5 years ago

336 merged. :)

sknight80 commented 5 years ago

@Matchlighter, I am going to merge this PR as well.

fasheng commented 4 years ago

Well, I use traefik v2 wildcard acme.json, and it always show [INFO] Live Certificates match and ignore the new certs. docker restart not works, you have to run docker stop then docker start to the service, another workaround is delete /ssl directory in container before updating the acme.json file. This issue really confuse me. Hope helps!