-
-
Notifications
You must be signed in to change notification settings - Fork 62
feat: support externally managed TLS via tls_external_cert_and_key option #860
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
Changes from all commits
71eca56
fb9c25f
fc3e08d
12d6cbb
5036c36
cf7e9fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,16 +11,15 @@ | |
|
|
||
| from chatmaild.config import read_config | ||
| from pyinfra import facts, host, logger | ||
| from pyinfra.facts import hardware | ||
| from pyinfra.api import FactBase | ||
| from pyinfra.facts import hardware | ||
| from pyinfra.facts.files import Sha256File | ||
| from pyinfra.facts.systemd import SystemdEnabled | ||
| from pyinfra.operations import apt, files, pip, server, systemd | ||
|
|
||
| from cmdeploy.cmdeploy import Out | ||
|
|
||
| from .acmetool import AcmetoolDeployer | ||
| from .selfsigned.deployer import SelfSignedTlsDeployer | ||
| from .basedeploy import ( | ||
| Deployer, | ||
| Deployment, | ||
|
|
@@ -30,11 +29,13 @@ | |
| has_systemd, | ||
| ) | ||
| from .dovecot.deployer import DovecotDeployer | ||
| from .external.deployer import ExternalTlsDeployer | ||
| from .filtermail.deployer import FiltermailDeployer | ||
| from .mtail.deployer import MtailDeployer | ||
| from .nginx.deployer import NginxDeployer | ||
| from .opendkim.deployer import OpendkimDeployer | ||
| from .postfix.deployer import PostfixDeployer | ||
| from .selfsigned.deployer import SelfSignedTlsDeployer | ||
| from .www import build_webpages, find_merge_conflict, get_paths | ||
|
|
||
|
|
||
|
|
@@ -540,6 +541,20 @@ def activate(self): | |
| ) | ||
|
|
||
|
|
||
| def get_tls_deployer(config, mail_domain): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High-level problem with this is that if you have a server with acmetool and want to reconfigure with external certificate, acmetool does not get uninstalled. I think what we need is not selecting one deployer, but run all deployers with some option like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is addressed in the separate #869 (the first commit, the second does some follow up refactoring) all other review comments were fixed in the branch here. |
||
| """Select the appropriate TLS deployer based on config.""" | ||
| tls_domains = [mail_domain, f"mta-sts.{mail_domain}", f"www.{mail_domain}"] | ||
|
|
||
| if config.tls_cert_mode == "acme": | ||
| return AcmetoolDeployer(config.acme_email, tls_domains) | ||
| elif config.tls_cert_mode == "self": | ||
| return SelfSignedTlsDeployer(mail_domain) | ||
| elif config.tls_cert_mode == "external": | ||
| return ExternalTlsDeployer(config.tls_cert_path, config.tls_key_path) | ||
| else: | ||
| raise ValueError(f"Unknown tls_cert_mode: {config.tls_cert_mode}") | ||
|
|
||
|
|
||
| def deploy_chatmail(config_path: Path, disable_mail: bool, website_only: bool) -> None: | ||
| """Deploy a chat-mail instance. | ||
|
|
||
|
|
@@ -608,12 +623,7 @@ def deploy_chatmail(config_path: Path, disable_mail: bool, website_only: bool) - | |
| ) | ||
| exit(1) | ||
|
|
||
| tls_domains = [mail_domain, f"mta-sts.{mail_domain}", f"www.{mail_domain}"] | ||
|
|
||
| if config.tls_cert_mode == "acme": | ||
| tls_deployer = AcmetoolDeployer(config.acme_email, tls_domains) | ||
| else: | ||
| tls_deployer = SelfSignedTlsDeployer(mail_domain) | ||
| tls_deployer = get_tls_deployer(config, mail_domain) | ||
|
|
||
| all_deployers = [ | ||
| ChatmailDeployer(mail_domain), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import io | ||
|
|
||
| from pyinfra import host | ||
| from pyinfra.facts.files import File | ||
| from pyinfra.operations import files, systemd | ||
|
|
||
| from cmdeploy.basedeploy import Deployer, get_resource | ||
|
|
||
|
|
||
| class ExternalTlsDeployer(Deployer): | ||
| """Expects TLS certificates to be managed on the server. | ||
|
|
||
| Validates that the configured certificate and key files | ||
| exist on the remote host. Installs a systemd path unit | ||
| that watches the certificate file and automatically | ||
| restarts/reloads affected services when it changes. | ||
| """ | ||
|
|
||
| def __init__(self, cert_path, key_path): | ||
| self.cert_path = cert_path | ||
| self.key_path = key_path | ||
|
|
||
| def configure(self): | ||
| # Verify cert and key exist on the remote host using pyinfra facts. | ||
| for path in (self.cert_path, self.key_path): | ||
| info = host.get_fact(File, path=path) | ||
| if info is None: | ||
| raise Exception(f"External TLS file not found on server: {path}") | ||
|
|
||
| # Deploy the .path unit (templated with the cert path). | ||
| # pkg=__package__ is required here because the resource files | ||
| # live in cmdeploy.external, not the default cmdeploy package. | ||
| source = get_resource("tls-cert-reload.path.f", pkg=__package__) | ||
|
hpk42 marked this conversation as resolved.
|
||
| content = source.read_text().format(cert_path=self.cert_path).encode() | ||
|
|
||
| path_unit = files.put( | ||
| name="Upload tls-cert-reload.path", | ||
| src=io.BytesIO(content), | ||
| dest="/etc/systemd/system/tls-cert-reload.path", | ||
| user="root", | ||
| group="root", | ||
| mode="644", | ||
| ) | ||
|
|
||
| service_unit = files.put( | ||
| name="Upload tls-cert-reload.service", | ||
| src=get_resource("tls-cert-reload.service", pkg=__package__), | ||
|
hpk42 marked this conversation as resolved.
|
||
| dest="/etc/systemd/system/tls-cert-reload.service", | ||
| user="root", | ||
| group="root", | ||
| mode="644", | ||
| ) | ||
|
|
||
| if path_unit.changed or service_unit.changed: | ||
| self.need_restart = True | ||
|
|
||
| def activate(self): | ||
| systemd.service( | ||
| name="Enable tls-cert-reload path watcher", | ||
| service="tls-cert-reload.path", | ||
| running=True, | ||
| enabled=True, | ||
| restarted=self.need_restart, | ||
| daemon_reload=self.need_restart, | ||
| ) | ||
| # No explicit reload needed here: dovecot/nginx read the cert | ||
| # on startup, and the .path watcher handles live changes. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Watch the TLS certificate file for changes. | ||
| # When the cert is updated (e.g. renewed by an external process), | ||
| # this triggers tls-cert-reload.service to reload the affected services. | ||
| # | ||
| # NOTE: changes to the certificates are not detected if they cross bind-mount boundaries. | ||
| # After cert renewal, you must then trigger the reload explicitly: | ||
| # systemctl start tls-cert-reload.service | ||
| [Unit] | ||
| Description=Watch TLS certificate for changes | ||
|
|
||
| [Path] | ||
| PathChanged={cert_path} | ||
|
|
||
| [Install] | ||
| WantedBy=multi-user.target |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Reload services that cache the TLS certificate. | ||
| # | ||
| # dovecot: caches the cert at startup; reload re-reads SSL certs | ||
| # without dropping existing connections. | ||
| # nginx: caches the cert at startup; reload gracefully picks up | ||
| # the new cert for new connections. | ||
| # postfix: reads the cert fresh on each TLS handshake, | ||
| # does NOT need a reload/restart. | ||
| [Unit] | ||
| Description=Reload TLS services after certificate change | ||
|
|
||
| [Service] | ||
| Type=oneshot | ||
| ExecStart=/bin/systemctl try-reload-or-restart dovecot | ||
| ExecStart=/bin/systemctl try-reload-or-restart nginx |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -395,7 +395,7 @@ class Remote: | |
| def __init__(self, sshdomain): | ||
| self.sshdomain = sshdomain | ||
|
|
||
| def iter_output(self, logcmd=""): | ||
| def iter_output(self, logcmd="", ready=None): | ||
| getjournal = "journalctl -f" if not logcmd else logcmd | ||
| print(self.sshdomain) | ||
| match self.sshdomain: | ||
|
|
@@ -410,10 +410,12 @@ def iter_output(self, logcmd=""): | |
| while 1: | ||
| line = self.popen.stdout.readline() | ||
| res = line.decode().strip().lower() | ||
| if res: | ||
| yield res | ||
| else: | ||
| if not res: | ||
| break | ||
| if ready is not None: | ||
| ready() | ||
| ready = None | ||
| yield res | ||
|
Comment on lines
398
to
+418
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a test fix on the side for a flaky test, took me a while to figure out. There are concurrency issues with getting log lines and triggering the send message. I think this is now reliable. |
||
|
|
||
|
|
||
| @pytest.fixture | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.