From 683cf2d862c47f26b799f0d3f106453f12fc4edd Mon Sep 17 00:00:00 2001 From: Sudip Bhattarai Date: Tue, 28 Apr 2026 23:59:21 +0545 Subject: [PATCH 1/4] Delegate certificate reneawl to certapi, other fixes --- README.md | 3 +- getssl | 79 ++--- nginx_proxy/Host.py | 1 + nginx_proxy/Location.py | 35 ++ nginx_proxy/SSL.py | 314 ------------------ nginx_proxy/certificate_backend.py | 87 +++++ .../ssl_certificate_processor.py | 208 ++++++------ .../pre_processors/virtual_host_processor.py | 37 ++- requirements.txt | 4 +- tests/unit/test_backend_target.py | 49 +++ tests/unit/test_error_resilience.py | 225 ++++++++++--- tests/unit/test_ssl_certapi_batch_domains.py | 183 ++++++++++ tests/unit/test_webserver_events.py | 4 +- 13 files changed, 695 insertions(+), 534 deletions(-) delete mode 100644 nginx_proxy/SSL.py create mode 100644 nginx_proxy/certificate_backend.py create mode 100644 tests/unit/test_ssl_certapi_batch_domains.py diff --git a/README.md b/README.md index 318cdf7..bf4f53d 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,7 @@ Control the default behavior of `nginx-proxy`: | `DOCKER_SWARM` | `ignore` | Treats every container like local by defeault. Set `enable` for Swarm support, `strict` for Swarm-only or`exclude` to not include swarm containers | | `SWARM_DOCKER_HOST` | - | URL of the Swarm manager socket (e.g., `tcp://manager:2375`). | | `CERTAPI_URL` | - | External Certificate API URL. | +| `CERTAPI_BATCH_DOMAINS` | `true` | When using `CERTAPI_URL`, request safe domain batching (`batch_domains=true`) to avoid recursive domain-order errors. | | `CHALLENGE_DIR` | `/etc/nginx/challenges/` | Base directory for acme challenge store, when requesting certificates with acme. `.well-known/acme-challenge` folder lives inside this.| | `CLOUDFLARE_API_KEY_KEY*` | - | Cloudflare api keys to issue DNS certificates.| @@ -227,4 +228,4 @@ We are constantly improving `nginx-proxy` to make it the most robust and versati - **High Availability & Multi-Node Swarm Deployment:** Comprehensive guides and templates for deploying `nginx-proxy` in a multi-node Swarm environment for maximum uptime. - **100% Test Coverage:** Ensuring rock-solid stability and reliability for every release. - **Remote Swarm Cluster Support:** Monitor and route traffic for Swarm clusters running on remote nodes seamlessly. -- **Pangolin Support:** Integration with Pangolin as an alternative reverse-proxy engine. \ No newline at end of file +- **Pangolin Support:** Integration with Pangolin as an alternative reverse-proxy engine. diff --git a/getssl b/getssl index 501da1e..2ac31b9 100755 --- a/getssl +++ b/getssl @@ -1,27 +1,32 @@ #!/usr/bin/env python3 +import argparse import sys import os from nginx import Nginx -from nginx_proxy import SSL +from nginx_proxy.certificate_backend import build_certificate_backend from urllib.parse import urlparse -def print_usage(): - print("Usage: Obtain Let'sEncrypt ssl certificate for a domain or multiple domains") - print() - print(" getssl [hostname2 ...]") - print() - print("Note: This script respects environment variables: CERTAPI_URL, SSL_DIR, CHALLENGE_DIR, etc.") - exit(1) +def parse_args(): + parser = argparse.ArgumentParser( + description="Obtain Let's Encrypt SSL certificate for one or more domains.", + epilog="This script respects environment variables: CERTAPI_URL, SSL_DIR, CHALLENGE_DIR, etc.", + ) + parser.add_argument( + "--force", + action="store_true", + help="Skip certapi/nginx-proxy self-verification before requesting the certificate.", + ) + parser.add_argument("domains", nargs="+", help="Domain(s) to include in the certificate.") + return parser.parse_args() + + +def flatten_2d_array(two_d_array): + return [item for sublist in two_d_array for item in sublist] if __name__ == "__main__": - if len(sys.argv) < 2: - print_usage() - - arg_set = set(sys.argv[1:]) - if any(x in arg_set for x in ['-h', '--help', 'help']): - print_usage() + args = parse_args() # Load configuration from environment variables (consistent with NginxProxyApp) def _strip_end(s: str, char="/") -> str: @@ -42,16 +47,7 @@ if __name__ == "__main__": if port is None: port = 443 if parsed.scheme == "https" else 80 - mock_server_config["certapi"] = { - "url": certapi_url, - "host": parsed.hostname, - "scheme": parsed.scheme, - "port": port - } - - class MockServer: - def __init__(self, config): - self.config = config + mock_server_config["certapi"] = {"url": certapi_url, "host": parsed.hostname, "scheme": parsed.scheme, "port": port} config_path = os.path.join(conf_dir, "conf.d/gen-ssl-direct.conf") # Make sure challenge dir exists for local mode @@ -62,22 +58,31 @@ if __name__ == "__main__": pass # Might fail if readonly, but then Nginx might scream nginx = Nginx.Nginx(config_path, challenge_dir=challenge_dir) - - # Initialize SSL - ssl_manager = SSL.SSL( - ssl_dir, - nginx=nginx, - update_threshold_seconds=cert_renew_threshold_days * 24 * 3600, - server=MockServer(mock_server_config) + + backend_info = build_certificate_backend( + ssl_dir, + nginx, + config=mock_server_config, + renew_threshold_days=max(10, cert_renew_threshold_days), ) - domains = [x for x in sys.argv[1:] if not x.startswith("-")] - if not domains: - print("No domains provided.") - print_usage() - try: - ssl_manager.register_certificate(domains) + result = backend_info.backend.obtain( + args.domains, + key_type="ecdsa", + batch_domains=backend_info.batch_domains, + self_verify=not args.force, + ) + if len(result.issued): + print( + "[ New Certificates ] : ", + ", ".join(flatten_2d_array(sorted([x.domains for x in result.issued]))), + ) + if len(result.existing): + print( + "[ Reuse Existing ] : ", + ", ".join(flatten_2d_array(sorted([x.domains for x in result.existing]))), + ) # Attempt to reload nginx to pick up changes (if running) print("Reloading nginx...") if not nginx.reload(): diff --git a/nginx_proxy/Host.py b/nginx_proxy/Host.py index 1f53216..a68ad20 100644 --- a/nginx_proxy/Host.py +++ b/nginx_proxy/Host.py @@ -77,6 +77,7 @@ def remove_container(self, container_id) -> None: if container_id in self.container_set: for path, location in self.locations.items(): removed = location.remove(container_id) or removed + location.remove_backend_extras(container_id) if location.isempty(): deletions.append(path) self.container_set.remove(container_id) diff --git a/nginx_proxy/Location.py b/nginx_proxy/Location.py index 24a2add..f1dff67 100644 --- a/nginx_proxy/Location.py +++ b/nginx_proxy/Location.py @@ -17,6 +17,13 @@ def __init__(self, name, is_websocket_backend=False, is_http_backend=True): def update_extras(self, extras: Dict[str, Any]): for x in extras: + if x == "injected_by_backend" and isinstance(extras[x], dict): + existing = self.extras.setdefault("injected_by_backend", {}) + for backend_id, directives in extras[x].items(): + normalized = directives if isinstance(directives, list) else [directives] + existing[backend_id] = list(dict.fromkeys(normalized)) + self._sync_injected_from_backend_map() + continue if x in self.extras: data = self.extras[x] if type(data) in (dict, set): @@ -33,6 +40,34 @@ def update_extras(self, extras: Dict[str, Any]): else: self.extras[x] = extras[x] + def _sync_injected_from_backend_map(self): + backend_map = self.extras.get("injected_by_backend") + if not isinstance(backend_map, dict): + return + + merged: list[str] = [] + seen = set() + for directives in backend_map.values(): + if not isinstance(directives, list): + directives = [directives] + for directive in directives: + if directive not in seen: + seen.add(directive) + merged.append(directive) + self.extras["injected"] = merged + + def remove_backend_extras(self, backend_id: str): + backend_map = self.extras.get("injected_by_backend") + if not isinstance(backend_map, dict): + return + if backend_id in backend_map: + del backend_map[backend_id] + self._sync_injected_from_backend_map() + if not backend_map: + del self.extras["injected_by_backend"] + if not self.extras.get("injected"): + self.extras.pop("injected", None) + def add(self, container: BackendTarget): if not any(c.id == container.id for c in self.backends): self.backends.append(container) diff --git a/nginx_proxy/SSL.py b/nginx_proxy/SSL.py deleted file mode 100644 index 847e2c4..0000000 --- a/nginx_proxy/SSL.py +++ /dev/null @@ -1,314 +0,0 @@ -import threading -from datetime import datetime, timezone, date -import logging -import os -import shutil -import sys -import time -import requests -from os.path import join -from typing import List, Dict, Union, Set - -from nginx.Nginx import Nginx -from certapi import CloudflareChallengeSolver, AcmeCertManager, FileSystemKeyStore, CertApiException -from certapi.issuers import AcmeCertIssuer, SelfCertIssuer -from certapi.http.types import IssuedCert -from certapi.crypto import certs_from_pem, Key -from nginx.NginxChallengeSolver import NginxChallengeSolver -from certapi.client.cert_manager_client import CertManagerClient -import traceback as tb -from nginx_proxy.utils.Blacklist import Blacklist - - -class SSL: - - def __init__( - self, ssl_path, nginx: Nginx, update_threshold_seconds: float, server=None, start_ssl_thread: bool = False - ): - self.ssl_path = ssl_path - self.nginx = nginx - ## import only for typing - self.server: "Webserver" = server - self.blacklist = Blacklist() - self.update_threshold_secs = update_threshold_seconds - self.cert_min_renew_threshold_secs = max(self.update_threshold_secs, 10 * 24 * 3600) - - # Internal state for background refresh - self.cache: Dict[str, date] = {} - self.next_ssl_expiry: Union[datetime, None] = None - self.shutdown_requested: bool = False - self.lock: threading.Condition = threading.Condition() - self.certificate_expiry_thread: threading.Thread = threading.Thread( - target=self.ssl_renew_thread_worker, name="SSL-Refresh-Thread" - ) - self.config = server.config if server is not None else {} - - x = os.environ.get("LETSENCRYPT_API") - if x is not None: - if x.startswith("https://") or x.startswith("http://"): - self.api_url = x - else: - self.api_url = "https://acme-staging-v02.api.letsencrypt.org/directory" - else: - self.api_url = "https://acme-v02.api.letsencrypt.org/directory" - - # Check if CertManagerClient should be used - certapi_url = "" - self.use_certapi_server = False - if self.config.get("certapi"): - self.use_certapi_server = True - certapi_url = self.config["certapi"]["url"] - elif os.environ.get("CERTAPI_URL"): - certapi_url = os.environ.get("CERTAPI_URL").strip() - self.use_certapi_server = True - - self.key_store = FileSystemKeyStore(ssl_path, keys_dir_name="private") - - if self.use_certapi_server: - self.certapi_client = CertManagerClient(certapi_url, self.key_store) - self.cert_backend = self.certapi_client - self.cert_manager = None - acme_account_key = self.key_store._get_or_generate_key("acme_account.key", key_type="ecdsa")[0] - - else: - self.certapi_client = None - self.challenge_store = NginxChallengeSolver(nginx.challenge_dir, nginx) - cert_issuer = AcmeCertIssuer.with_keystore(self.key_store, self.challenge_store, acme_url=self.api_url) - - all_stores = [self.challenge_store] - for key, value in os.environ.items(): - if key.startswith("CLOUDFLARE_API_KEY"): - if value: # Ensure the value is not None or empty - cloudflare = CloudflareChallengeSolver(value.strip()) - all_stores.append(cloudflare) - cloudflare.cleanup_old_challenges() - - cert_min_renew_threshold_days = self.cert_min_renew_threshold_secs // (24 * 3600) - self.cert_manager = AcmeCertManager( - self.key_store, cert_issuer, all_stores, renew_threshold_days=cert_min_renew_threshold_days - ) - self.cert_manager.setup() - self.cert_backend = self.cert_manager - acme_account_key = cert_issuer.acme.account_key - - self.self_signer = SelfCertIssuer( - acme_account_key, "NP", "Bagmati", "Buddhanagar", "nginx-proxy", "local.nginx-proxy.com" - ) - - self.certapi_url = certapi_url - self._certapi_waited = False - - if start_ssl_thread: - self.certificate_expiry_thread.start() - - def _wait_for_certapi(self): - if self._certapi_waited: - return - - self._certapi_waited = True - print(f"[SSL] CertAPI enabled, waiting for {self.certapi_url} to be live...") - for i in range(6): - try: - r = requests.get(self.certapi_url.rstrip("/") + "/docs", timeout=5) - print(f"[SSL] CertAPI is live (status {r.status_code})") - return - except (KeyboardInterrupt, SystemExit): - raise - except Exception as e: - print(f"[SSL] CertAPI not live yet (attempt {i+1}/6): {e}") - if i < 5: - time.sleep(10) - - def ssl_renew_thread_worker(self): - while True: - with self.lock: - if self.shutdown_requested: - break - - if self.next_ssl_expiry is None: - print("[SSL Refresh Thread] Looks like there no ssl certificates, Sleeping until there's one") - self.lock.wait() - continue - - now = datetime.now(timezone.utc) - remaining_seconds = (self.next_ssl_expiry - now).total_seconds() - - if remaining_seconds > self.update_threshold_secs: - print("[SSL Refresh Thread] SSL certificate status:") - - max_size = max([len(x) for x in self.cache]) if self.cache else 10 - for host in self.cache: - print( - f" {host:<{max_size + 2}} - {self._format_duration((self.cache[host] - now).total_seconds())}" - ) - - # Sleep until threshold, but cap at 32 days even if expiry is far away - max_sleep_seconds = 32 * 24 * 3600 - # sleep 5 mins more to avoid race condition - sleep_seconds = min(remaining_seconds - self.update_threshold_secs + 300, max_sleep_seconds) - - print( - f"[SSL Refresh Thread] All the certificates are up to date sleeping for {self._format_duration(sleep_seconds)}" - ) - self.lock.wait(sleep_seconds) - continue - - else: - print( - f"[SSL Refresh Thread] Update threshold reached: {self._format_duration(self.update_threshold_secs)}" - ) - for host_name in self.cache: - print( - f"Remaining : {host_name} : {self._format_duration((self.cache[host_name] - now).total_seconds())}" - ) - - # at least gather all the certificates that expires in 10 days - update_threshold_secs = max(self.update_threshold_secs, self.cert_min_renew_threshold_secs) - expired_hosts = [ - host_name - for host_name in self.cache - if (self.cache[host_name] - now).total_seconds() < update_threshold_secs - ] - - for host_name in expired_hosts: - del self.cache[host_name] - - if self.server: - self.server.reload() - - def register_certificate(self, req_domain) -> List[IssuedCert]: - domain = [req_domain] if type(req_domain) is str else req_domain - - result = None - if self.use_certapi_server: - self._wait_for_certapi() - exception = None - - def worker(): - nonlocal result, exception - try: - result = self.cert_backend.issue_certificate(domain, key_type="ecdsa") - except (KeyboardInterrupt, SystemExit): - raise - except Exception as e: - exception = e - - thread = threading.Thread(target=worker) - thread.start() - print("[Cert API Client] Requesting certificates :", ", ".join(domain)) - start_time = time.time() - while thread.is_alive(): - thread.join(timeout=30) - if thread.is_alive(): - print( - f"[Cert API Client] Waiting for response since {int(time.time() - start_time)} seconds" - ) - if exception: - raise exception - else: - result = self.cert_backend.issue_certificate(domain, key_type="ecdsa") - if len(result.issued): - print( - "[ New Certificates ] : ", ", ".join(flatten_2d_array(sorted([x.domains for x in result.issued]))) - ) - if len(result.existing): - print( - "[ Reuse Existing ] : ", - ", ".join(flatten_2d_array(sorted([x.domains for x in result.existing]))), - ) - return result.issued + result.existing - - def is_certificate_fresh(self, domain: str, threshold_seconds: float | None = None) -> bool: - result = self.key_store.find_key_and_cert_by_domain(domain) - if result is None: - return False - - cert = result[2][0] - expiry = cert.not_valid_after_utc - threshold = self.update_threshold_secs if threshold_seconds is None else threshold_seconds - return (expiry - datetime.now(timezone.utc)).total_seconds() > threshold - - def update_expiry_cache(self, certs: List[IssuedCert]): - with self.lock: - for cert in certs: - for domain in cert.domains: - full_chain = certs_from_pem(cert.certificate.encode("utf-8")) - self.cache[domain] = full_chain[0].not_valid_after_utc - - if len(self.cache): - expiry = min(self.cache.values()) - if expiry != self.next_ssl_expiry: - self.next_ssl_expiry = expiry - self.lock.notify() - - def register_certificate_or_selfsign(self, domain, no_self_check=False, ignore_existing=False) -> List[IssuedCert]: - obtained_certificates: List[IssuedCert] = [] - for i in range(0, len(domain), 50): - sub_list = domain[i : i + 50] - # Filter out blacklisted domains from the sublist - valid_list, blacklisted = self.blacklist.partition(sub_list) - - if len(blacklisted) > 0: - print("[Blacklist] ignoring previously failed domain for 3 mins:", ", ".join(blacklisted)) - - try: - # Proceed only with the filtered sublist - obtained: List[IssuedCert] = ( - self.register_certificate( - valid_list, - ) - if valid_list - else [] - ) - - if obtained: - obtained_certificates.extend(obtained) - except CertApiException as e: - tb.print_exception(e) - pass - processed_set = set(flatten_2d_array([c.domains for c in obtained_certificates])).union(set(blacklisted)) - self_signed = [x for x in domain if x not in processed_set] - if self_signed: - # Add the self-signed domains to the blacklist - for domain_item in self_signed: - self.blacklist.add(domain_item) - print("[ Self Signing ] : ", ", ".join(self_signed)) - self.register_certificate_self_sign(self_signed) - - return obtained_certificates - - def register_certificate_self_sign(self, domain): - if type(domain) is str: - domain = [domain] - for d in domain: - if not self.key_store.find_key_by_name(d + ".selfsigned"): - (key, cert) = self.self_signer.generate_key_and_cert_for_domain(d, key_type="ecdsa") - key_id = self.key_store.save_key(key, d + ".selfsigned") - self.key_store.save_cert(key_id, cert, [d], name=d + ".selfsigned") - - def _format_duration(self, seconds: float) -> str: - days, remainder = divmod(int(seconds), 86400) - hours, remainder = divmod(remainder, 3600) - minutes, seconds = divmod(remainder, 60) - - parts = [] - if days > 0: - parts.append(f"{days} days") - if hours > 0 or days > 0: - parts.append(f"{hours} hours") - if minutes > 0 or hours > 0 or days > 0: - parts.append(f"{minutes} minutes") - parts.append(f"{seconds} seconds") - - return ", ".join(parts) - - def shutdown(self): - with self.lock: - self.shutdown_requested = True - self.lock.notify() - if self.certificate_expiry_thread.is_alive(): - self.certificate_expiry_thread.join(timeout=2) - - -def flatten_2d_array(two_d_array): - return [item for sublist in two_d_array for item in sublist] diff --git a/nginx_proxy/certificate_backend.py b/nginx_proxy/certificate_backend.py new file mode 100644 index 0000000..0da59c7 --- /dev/null +++ b/nginx_proxy/certificate_backend.py @@ -0,0 +1,87 @@ +import os +from dataclasses import dataclass +from typing import Any + +from certapi import AcmeCertManager, CloudflareChallengeSolver, FileSystemKeyStore +from certapi.client.cert_manager_client import CertManagerClient +from certapi.issuers import AcmeCertIssuer +from nginx.Nginx import Nginx +from nginx.NginxChallengeSolver import NginxChallengeSolver + + +@dataclass +class CertificateBackend: + backend: Any + key_store: FileSystemKeyStore + certapi_url: str + use_certapi_server: bool + batch_domains: bool + cert_manager: AcmeCertManager | None = None + certapi_client: CertManagerClient | None = None + challenge_store: NginxChallengeSolver | None = None + + +def build_certificate_backend( + ssl_path: str, + nginx: Nginx, + config: dict | None = None, + renew_threshold_days: int = 10, +) -> CertificateBackend: + config = config or {} + batch_domains = os.getenv("CERTAPI_BATCH_DOMAINS", "true").strip().lower() not in {"0", "false", "no", "off"} + certapi_url = "" + use_certapi_server = False + + if config.get("certapi"): + use_certapi_server = True + certapi_url = config["certapi"]["url"] + elif os.environ.get("CERTAPI_URL"): + use_certapi_server = True + certapi_url = os.environ.get("CERTAPI_URL").strip() + + key_store = FileSystemKeyStore(ssl_path, keys_dir_name="private") + + if use_certapi_server: + client = CertManagerClient(certapi_url, key_store) + return CertificateBackend( + backend=client, + key_store=key_store, + certapi_url=certapi_url, + use_certapi_server=True, + batch_domains=batch_domains, + certapi_client=client, + ) + + acme_url = os.environ.get("LETSENCRYPT_API") + if acme_url is not None and not acme_url.startswith(("https://", "http://")): + acme_url = "https://acme-staging-v02.api.letsencrypt.org/directory" + if acme_url is None: + acme_url = "https://acme-v02.api.letsencrypt.org/directory" + + challenge_store = NginxChallengeSolver(nginx.challenge_dir, nginx) + cert_issuer = AcmeCertIssuer.with_keystore(key_store, challenge_store, acme_url=acme_url) + + challenge_solvers = [challenge_store] + for key, value in os.environ.items(): + if key.startswith("CLOUDFLARE_API_KEY") and value: + cloudflare = CloudflareChallengeSolver(value.strip()) + challenge_solvers.append(cloudflare) + cloudflare.cleanup_old_challenges() + + cert_manager = AcmeCertManager( + key_store, + cert_issuer, + challenge_solvers, + renew_threshold_days=renew_threshold_days, + ) + cert_manager.setup() + + return CertificateBackend( + backend=cert_manager, + key_store=key_store, + certapi_url=certapi_url, + use_certapi_server=False, + batch_domains=batch_domains, + cert_manager=cert_manager, + challenge_store=challenge_store, + ) diff --git a/nginx_proxy/post_processors/ssl_certificate_processor.py b/nginx_proxy/post_processors/ssl_certificate_processor.py index 1d7ebe3..29f6daf 100644 --- a/nginx_proxy/post_processors/ssl_certificate_processor.py +++ b/nginx_proxy/post_processors/ssl_certificate_processor.py @@ -1,33 +1,75 @@ -import threading -from datetime import date, datetime, timezone -from typing import List, Dict, Set, Union +from datetime import datetime, timezone +from typing import List +from certapi.client import RenewalManager from nginx.Nginx import Nginx from nginx_proxy import WebServer from nginx_proxy.Host import Host -from nginx_proxy.SSL import SSL -from certapi.http.types import IssuedCert -from certapi.crypto import certs_from_pem - - -import traceback +from nginx_proxy.certificate_backend import build_certificate_backend class SslCertificateProcessor: def __init__( self, nginx: Nginx, server: WebServer, start_ssl_thread=False, ssl_dir="/etc/ssl", update_threshold_days=10 ): - # self.update_threshold = (90 * 24 * 3600) - (3 * 60) - 3600 # Trigger refresh within 4 minutes for testing - self.self_signed: Set[str] = set() self.nginx: Nginx = nginx - self.ssl: SSL = SSL( + self.server: WebServer = server + self.update_threshold_secs = (10 if update_threshold_days is None else update_threshold_days) * 24 * 3600 + self.cert_min_renew_threshold_secs = max(self.update_threshold_secs, 10 * 24 * 3600) + backend_info = build_certificate_backend( ssl_dir, nginx, - update_threshold_seconds=(10 if update_threshold_days is None else update_threshold_days) * 24 * 3600, - server=server, - start_ssl_thread=start_ssl_thread, + config=server.config if server is not None else {}, + renew_threshold_days=self.cert_min_renew_threshold_secs // (24 * 3600), ) - self.server: WebServer = server + self.backend = backend_info.backend + self.key_store = backend_info.key_store + self.certapi_url = backend_info.certapi_url + self.use_certapi_server = backend_info.use_certapi_server + self.certapi_batch_domains = backend_info.batch_domains + self.cert_manager = backend_info.cert_manager + self.certapi_client = backend_info.certapi_client + self.challenge_store = backend_info.challenge_store + self.renewal_manager = RenewalManager( + self.backend, + sync_watch_domains=self.sync_watch_domains, + renew_threshold_days=max(1, int(self.update_threshold_secs // (24 * 3600))), + batch_domains=self.certapi_batch_domains, + ) + + if start_ssl_thread: + self.renewal_manager.start() + + def sync_watch_domains(self): + if self.server is None: + return + domains = sorted({host.hostname for host in self.server.config_data.host_list() if host.secured}) + self.renewal_manager.set_watch_domains(domains) + + def is_certificate_fresh(self, domain: str, threshold_seconds: float | None = None) -> bool: + result = self.key_store.find_key_and_cert_by_domain(domain) + if result is None: + return False + + cert = result[2][0] + expiry = cert.not_valid_after_utc + threshold = self.update_threshold_secs if threshold_seconds is None else threshold_seconds + return (expiry - datetime.now(timezone.utc)).total_seconds() > threshold + + def has_certificate(self, domain: str) -> bool: + return self.key_store.find_key_and_cert_by_domain(domain) is not None + + def has_self_signed_certificate(self, domain: str) -> bool: + return self.key_store.find_key_by_name(domain + ".selfsigned") is not None + + def _ensure_self_signed_certificate(self, domain: str): + if self.has_certificate(domain) or self.has_self_signed_certificate(domain): + return + + register_self_signed = getattr(self.renewal_manager, "_register_self_signed", None) + if register_self_signed is None: + return + register_self_signed(domain) def _prepare_host_for_ssl(self, host: Host): """Sets SSL redirect and port if applicable.""" @@ -35,112 +77,48 @@ def _prepare_host_for_ssl(self, host: Host): host.ssl_redirect = True host.port = 443 - def _wildcard_is_preferred(self, host: Host, wildcard: str, registered: Set[str], warn: bool = False) -> bool: - wildcard_known = (wildcard in registered) or (wildcard in self.ssl.cache) - if not wildcard_known: - return False + def _has_fresh_wildcard_certificate(self, hostname: str) -> bool: + wildcard = self.wildcard_domain_name(hostname) + return wildcard is not None and self.is_certificate_fresh(wildcard) - if self.ssl.is_certificate_fresh(wildcard): - return True - - if warn: - print(f"[SSL] Wildcard {wildcard} is stale; skipping {host.hostname}") - return False + def _host_needs_certificate(self, host: Host) -> bool: + if self.has_certificate(host.hostname): + return False + return not self._has_fresh_wildcard_certificate(host.hostname) - def _assign_existing_cert(self, host: Host, registered: Set[str]) -> bool: - """ - Checks for existing certificate in cache or as wildcard and assigns it. - Returns True if a certificate was assigned, False otherwise. - """ - if host.hostname in self.ssl.cache: - host.ssl_file = host.hostname - registered.add(host.hostname) - return True + def _select_ssl_file(self, host: Host) -> str: + if self.has_certificate(host.hostname): + return host.hostname - # Reuse the wildcard certificate if available and registered wildcard = self.wildcard_domain_name(host.hostname) - if wildcard is not None: - if self._wildcard_is_preferred(host, wildcard, registered, warn=True): - host.ssl_file = wildcard - return True - return False - - def _update_host_ssl_info(self, host: Host, registered: Set[str]): - """ - Updates host.ssl_file based on registration status. - Assumes host.secured is True. - """ - if (host.hostname in registered) or (host.hostname in self.ssl.cache): - host.ssl_file = host.hostname - - else: - wildcard_domain = self.wildcard_domain_name(host.hostname) - if wildcard_domain and self._wildcard_is_preferred(host, wildcard_domain, registered): - host.ssl_file = wildcard_domain - else: - host.ssl_file = host.hostname + ".selfsigned" - self.self_signed.add(host.hostname) + if wildcard is not None and self.is_certificate_fresh(wildcard): + return wildcard + + return host.hostname + ".selfsigned" def process_ssl_certificates(self, hosts: List[Host]): if not hosts: return - registered: Set[str] = set() - new_certs: List[IssuedCert] = [] - non_wildcards: List[Host] = [] - try: - # First pass: Handle wildcard certificates immediately, one by one. - for host in hosts: - if host.secured: - self._prepare_host_for_ssl(host) - if host.hostname.startswith("*."): - if not self._assign_existing_cert(host, registered): - try: - registered_ssl = self.ssl.register_certificate(host.hostname) - if len(registered_ssl) > 0: - registered.add(host.hostname) - new_certs.extend(registered_ssl) - continue - except (KeyboardInterrupt, SystemExit): - raise - except Exception as e: - print(f"Self signing certificate {host.hostname}: {e}") - traceback.print_exception(e) - self.ssl.register_certificate_self_sign(host.hostname) - - else: - non_wildcards.append(host) - - missing_certs: List[str] = [] - - # First read from the cache. - for host in non_wildcards: - if not self._assign_existing_cert(host, registered): - missing_certs.append(host.hostname) - - # Batch process regular certificates - if len(missing_certs) > 0: - try: - new_registrations = self.ssl.register_certificate_or_selfsign(missing_certs) - registered.update(domain for x in new_registrations for domain in x.domains) - new_certs.extend(new_registrations) - except (KeyboardInterrupt, SystemExit): - raise - except Exception as e: - print(f"Error processing certificates for {missing_certs}: {e}") - traceback.print_exception(e) - - # Final pass: Update SSL info for all hosts - for host in hosts: - if host.secured: - self._update_host_ssl_info(host, registered) - if len(new_certs) > 0: - self.ssl.update_expiry_cache(new_certs) - - except (KeyboardInterrupt, SystemExit): - raise - except Exception as e: - print("Unexpected error processing ssl certificates.") - traceback.print_exception(e) + + secured_hosts = [host for host in hosts if host.secured] + if not secured_hosts: + return + + for host in secured_hosts: + self._prepare_host_for_ssl(host) + + secured_domains = sorted({host.hostname for host in secured_hosts}) + self.renewal_manager.set_watch_domains(secured_domains) + + if any(self._host_needs_certificate(host) for host in secured_hosts): + self.renewal_manager.trigger_now() + + for host in secured_hosts: + if self._select_ssl_file(host).endswith(".selfsigned"): + self._ensure_self_signed_certificate(host.hostname) + + for host in secured_hosts: + host.ssl_file = self._select_ssl_file(host) def wildcard_domain_name(self, domain, wild_char="*"): slices = domain.split(".") @@ -149,4 +127,4 @@ def wildcard_domain_name(self, domain, wild_char="*"): return None def shutdown(self): - self.ssl.shutdown() + self.renewal_manager.stop() diff --git a/nginx_proxy/pre_processors/virtual_host_processor.py b/nginx_proxy/pre_processors/virtual_host_processor.py index fd7748f..380233d 100644 --- a/nginx_proxy/pre_processors/virtual_host_processor.py +++ b/nginx_proxy/pre_processors/virtual_host_processor.py @@ -18,6 +18,25 @@ def _default_external_port(schemes): return 443 if has_secure_scheme and not has_insecure_scheme else 80 +def _parse_extra_directive(raw_directive: str): + directive = raw_directive.strip() + if not directive: + return None, None + # Accept "key=value" and "key = value" forms and normalize to nginx form "key value". + if "=" in directive: + key, value = directive.split("=", 1) + key = key.strip() + value = value.strip() + if key: + return key, value if value else None + if " " in directive: + key, value = directive.split(" ", 1) + key = key.strip() + value = value.strip() + return (key, value if value else None) if key else (None, None) + return directive, None + + def process_virtual_hosts(backend: BackendTarget, known_networks: set) -> ProxyConfigData: """ @@ -44,7 +63,12 @@ def process_virtual_hosts(backend: BackendTarget, known_networks: set) -> ProxyC if injections: # Preserve insertion order while avoiding duplicates from repeated hosts deduped = list(dict.fromkeys(injections)) - host.locations[location].update_extras({"injected": deduped}) + host.locations[location].update_extras( + { + "injected": deduped, + "injected_by_backend": {backend.id: deduped}, + } + ) hosts.add_host(host) print( "Valid configuration ", @@ -82,14 +106,9 @@ def _parse_host_entry(entry_string: str): if len(configs) > 1: entry_string = configs[0] for x in configs[1].split(";"): - x = x.strip() - if x: - # Parse as key value, e.g. 'client_max_body_size 200M' - if " " in x: - k, v = x.split(" ", 1) - extras[k.strip()] = v.strip() - else: - extras[x] = None + k, v = _parse_extra_directive(x) + if k: + extras[k] = v host_list = entry_string.strip().split("->") external, internal = host_list if len(host_list) == 2 else (host_list[0], "") external, internal = (split_url(external), split_url(internal)) diff --git a/requirements.txt b/requirements.txt index eb0482b..4d53f5e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,5 +2,5 @@ docker==7.1.0 Jinja2==3.1.6 pydevd==3.1.0 bcrypt==4.3.0 # 5.0.0 requires rust so ignoring -certapi==1.0.5 -requests==2.33.0 \ No newline at end of file +certapi>=1.1.4 +requests==2.33.0 diff --git a/tests/unit/test_backend_target.py b/tests/unit/test_backend_target.py index 67f9a40..197ad20 100644 --- a/tests/unit/test_backend_target.py +++ b/tests/unit/test_backend_target.py @@ -242,3 +242,52 @@ def test_parse_host_entry_with_extras(self): assert h.hostname == "example.com" assert extras["client_max_body_size"] == "100M" assert extras["proxy_read_timeout"] == "120" + + def test_parse_host_entry_with_equals_extra_syntax(self): + h, loc, c, extras = _parse_host_entry("example.com; client_max_body_size=2g; proxy_read_timeout=120") + assert h.hostname == "example.com" + assert extras["client_max_body_size"] == "2g" + assert extras["proxy_read_timeout"] == "120" + + def test_parse_host_entry_with_spaced_equals_extra_syntax(self): + h, loc, c, extras = _parse_host_entry("example.com; client_max_body_size = 2g; proxy_read_timeout = 120") + assert h.hostname == "example.com" + assert extras["client_max_body_size"] == "2g" + assert extras["proxy_read_timeout"] == "120" + + def test_remove_backend_cleans_only_its_injected_directives(self): + known_networks = {"shared-net-id"} + bad_backend = BackendTarget( + id="bad-backend", + name="bad-backend", + env={"VIRTUAL_HOST": "dup.example.com; client_max_body_size=2g; proxy_read_timeout=10"}, + network_settings={"shared": {"NetworkID": "shared-net-id", "IPAddress": "10.0.0.11"}}, + ) + good_backend = BackendTarget( + id="good-backend", + name="good-backend", + env={"VIRTUAL_HOST": "dup.example.com; client_max_body_size 5m; proxy_send_timeout 20"}, + network_settings={"shared": {"NetworkID": "shared-net-id", "IPAddress": "10.0.0.12"}}, + ) + + aggregated = ProxyConfigData() + for backend in (bad_backend, good_backend): + config = process_virtual_hosts(backend, known_networks) + for host in config.host_list(): + aggregated.add_host(host) + + host = aggregated.getHost("dup.example.com") + assert host is not None + injections_before = host.locations["/"].extras.get("injected", []) + assert "client_max_body_size 2g" in injections_before + assert "client_max_body_size 5m" in injections_before + + removed, _ = aggregated.remove_backend("bad-backend") + assert removed + + host_after = aggregated.getHost("dup.example.com") + injections_after = host_after.locations["/"].extras.get("injected", []) + assert "client_max_body_size 2g" not in injections_after + assert "proxy_read_timeout 10" not in injections_after + assert "client_max_body_size 5m" in injections_after + assert "proxy_send_timeout 20" in injections_after diff --git a/tests/unit/test_error_resilience.py b/tests/unit/test_error_resilience.py index c52a410..5a30e90 100644 --- a/tests/unit/test_error_resilience.py +++ b/tests/unit/test_error_resilience.py @@ -9,6 +9,7 @@ import pytest import time from datetime import datetime, timedelta, timezone +from types import SimpleNamespace from unittest.mock import Mock, patch from requests.exceptions import ConnectionError, Timeout, SSLError from certapi import CertApiException @@ -17,6 +18,7 @@ from nginx_proxy.WebServer import WebServer from nginx_proxy.Host import Host from nginx_proxy.NginxProxyApp import NginxProxyAppConfig +from nginx_proxy.post_processors.ssl_certificate_processor import SslCertificateProcessor from nginx.NginxConf import HttpBlock from tests.helpers.docker_test_client import DockerTestClient @@ -56,8 +58,6 @@ def webserver_for_error_tests(docker_client: DockerTestClient): docker_client.close() webserver.cleanup() - if webserver.ssl_processor.ssl.certificate_expiry_thread.is_alive(): - webserver.ssl_processor.ssl.certificate_expiry_thread.join(timeout=2) # ============================================================================ @@ -187,14 +187,13 @@ def test_error_resilience(webserver_for_error_tests, error, hostname): """Test that various errors during certificate issuance don't crash the application and fallback to self-signed""" webserver = webserver_for_error_tests - with patch.object(webserver.ssl_processor.ssl.cert_manager, "issue_certificate", side_effect=error): + with patch.object(webserver.ssl_processor.cert_manager, "obtain", side_effect=error): hosts = [Host(hostname=hostname, port=443, scheme={"https"})] webserver.ssl_processor.process_ssl_certificates(hosts) # Should fallback to self-signed assert hosts[0].ssl_file == f"{hostname}.selfsigned" - assert hostname in webserver.ssl_processor.self_signed # ============================================================================ @@ -231,7 +230,7 @@ def test_webserver_continues_after_errors(docker_client: DockerTestClient, error webserver = WebServer(docker_client, config, nginx_update_throtle_sec=0.1) # Simulate error - with patch.object(webserver.ssl_processor.ssl.cert_manager, "issue_certificate", side_effect=error): + with patch.object(webserver.ssl_processor.cert_manager, "obtain", side_effect=error): hosts = [Host(hostname=hostname, port=443, scheme={"https"})] webserver.ssl_processor.process_ssl_certificates(hosts) @@ -252,8 +251,6 @@ def test_webserver_continues_after_errors(docker_client: DockerTestClient, error container.remove(force=True) docker_client.close() webserver.cleanup() - if webserver.ssl_processor.ssl.certificate_expiry_thread.is_alive(): - webserver.ssl_processor.ssl.certificate_expiry_thread.join(timeout=2) @pytest.mark.parametrize( @@ -279,36 +276,29 @@ def test_fallback_to_selfsigned_on_failure(webserver_for_error_tests, error, hos """Test that system falls back to self-signed certificates on various failures""" webserver = webserver_for_error_tests - with patch.object(webserver.ssl_processor.ssl.cert_manager, "issue_certificate", side_effect=error): + with patch.object(webserver.ssl_processor.cert_manager, "obtain", side_effect=error): hosts = [Host(hostname=hostname, port=443, scheme={"https"})] webserver.ssl_processor.process_ssl_certificates(hosts) # Should have self-signed certificate assert hosts[0].ssl_file == f"{hostname}.selfsigned" - assert hostname in webserver.ssl_processor.self_signed -def test_blacklist_prevents_repeated_failures(webserver_for_error_tests): - """Test that blacklist mechanism prevents repeated certificate request failures""" +def test_failed_initial_request_delegates_fallback_to_renewal_manager(webserver_for_error_tests): + """Initial issuance no longer owns blacklist/retry behavior in nginx-proxy.""" webserver = webserver_for_error_tests - ssl = webserver.ssl_processor.ssl + processor = webserver.ssl_processor - # First attempt should fail and add to blacklist - error = CertApiException("ACME failure", step="Test") - with patch.object(ssl.cert_manager, "issue_certificate", side_effect=error) as mock_issue: - result = ssl.register_certificate_or_selfsign(["test-blacklist.example.com"]) - - # Should have attempted once - assert mock_issue.call_count == 1 + hosts = [Host(hostname="test-fallback-delegated.example.com", port=443, scheme={"https"})] + with patch.object(processor.cert_manager, "obtain") as mock_obtain, patch.object( + processor.renewal_manager, "trigger_now" + ) as mock_trigger, patch.object(processor.renewal_manager, "set_watch_domains") as mock_set_watch_domains: + processor.process_ssl_certificates(hosts) - # Verify domain is in blacklist by checking it won't be retried immediately - # Second attempt within blacklist period should skip the domain - with patch.object(ssl.cert_manager, "issue_certificate", side_effect=error) as mock_issue: - result = ssl.register_certificate_or_selfsign(["test-blacklist.example.com"]) - - # Should not attempt again (blacklisted) - assert mock_issue.call_count == 0 + mock_obtain.assert_not_called() + mock_set_watch_domains.assert_called_once_with(["test-fallback-delegated.example.com"]) + mock_trigger.assert_called_once_with() def test_multiple_simultaneous_cert_errors(webserver_for_error_tests): @@ -317,7 +307,7 @@ def test_multiple_simultaneous_cert_errors(webserver_for_error_tests): error = CertApiException("ACME failure", step="Test") - with patch.object(webserver.ssl_processor.ssl.cert_manager, "issue_certificate", side_effect=error): + with patch.object(webserver.ssl_processor.cert_manager, "obtain", side_effect=error): hosts = [ Host(hostname="test-multi-1.example.com", port=443, scheme={"https"}), Host(hostname="test-multi-2.example.com", port=443, scheme={"https"}), @@ -330,7 +320,6 @@ def test_multiple_simultaneous_cert_errors(webserver_for_error_tests): # All should have self-signed certificates for host in hosts: assert host.ssl_file.endswith(".selfsigned") - assert host.hostname in webserver.ssl_processor.self_signed def test_wildcard_cert_error_handling(webserver_for_error_tests): @@ -339,7 +328,7 @@ def test_wildcard_cert_error_handling(webserver_for_error_tests): error = CertApiException("Wildcard cert failure", step="Test") - with patch.object(webserver.ssl_processor.ssl, "register_certificate", side_effect=error): + with patch.object(webserver.ssl_processor.renewal_manager, "trigger_now") as mock_trigger: hosts = [Host(hostname="*.wildcard-test.example.com", port=443, scheme={"https"})] # Should not crash @@ -347,11 +336,11 @@ def test_wildcard_cert_error_handling(webserver_for_error_tests): # Should fallback to self-signed assert hosts[0].ssl_file == "*.wildcard-test.example.com.selfsigned" + mock_trigger.assert_called_once_with() def test_fresh_wildcard_cert_remains_preferred(webserver_for_error_tests): webserver = webserver_for_error_tests - wildcard_cert = Mock(domains=["*.example.com"]) hosts = [ Host(hostname="*.example.com", port=443, scheme={"https"}), Host(hostname="api.example.com", port=443, scheme={"https"}), @@ -363,24 +352,18 @@ def find_cert(domain): return ("*.example.com", Mock(), [fresh_cert]) return None - with patch.object(webserver.ssl_processor.ssl, "register_certificate", return_value=[wildcard_cert]) as mock_register, \ - patch.object(webserver.ssl_processor.ssl, "register_certificate_or_selfsign") as mock_register_or_selfsign, \ - patch.object(webserver.ssl_processor.ssl.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), \ - patch("builtins.print") as mock_print, \ - patch.object(webserver.ssl_processor.ssl, "update_expiry_cache"): + with patch.object(webserver.ssl_processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), patch.object( + webserver.ssl_processor.renewal_manager, "trigger_now" + ) as mock_trigger: webserver.ssl_processor.process_ssl_certificates(hosts) assert hosts[0].ssl_file == "*.example.com" assert hosts[1].ssl_file == "*.example.com" - mock_register.assert_called_once_with("*.example.com") - mock_register_or_selfsign.assert_not_called() - assert not any("Wildcard *.example.com is stale; skipping api.example.com" in str(call) for call in mock_print.call_args_list) + mock_trigger.assert_not_called() def test_wildcard_near_expiry_is_not_preferred(webserver_for_error_tests): webserver = webserver_for_error_tests - wildcard_cert = Mock(domains=["*.example.com"]) - api_cert = Mock(domains=["api.example.com"]) hosts = [ Host(hostname="*.example.com", port=443, scheme={"https"}), Host(hostname="api.example.com", port=443, scheme={"https"}), @@ -392,20 +375,156 @@ def find_cert(domain): return ("*.example.com", Mock(), [expiring_cert]) return None - with patch.object(webserver.ssl_processor.ssl, "register_certificate", return_value=[wildcard_cert]) as mock_register, \ - patch.object( - webserver.ssl_processor.ssl, "register_certificate_or_selfsign", return_value=[api_cert] - ) as mock_register_or_selfsign, \ - patch.object(webserver.ssl_processor.ssl.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), \ - patch("builtins.print") as mock_print, \ - patch.object(webserver.ssl_processor.ssl, "update_expiry_cache"): + with patch.object(webserver.ssl_processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), patch.object( + webserver.ssl_processor.renewal_manager, "trigger_now" + ) as mock_trigger: webserver.ssl_processor.process_ssl_certificates(hosts) assert hosts[0].ssl_file == "*.example.com" - assert hosts[1].ssl_file == "api.example.com" - mock_register.assert_called_once_with("*.example.com") - mock_register_or_selfsign.assert_called_once_with(["api.example.com"]) - assert any( - "[SSL] Wildcard *.example.com is stale; skipping api.example.com" in " ".join(str(arg) for arg in call.args) - for call in mock_print.call_args_list + assert hosts[1].ssl_file == "api.example.com.selfsigned" + mock_trigger.assert_called_once_with() + + +def test_existing_cert_is_kept_without_nginx_proxy_retry_state(webserver_for_error_tests): + webserver = webserver_for_error_tests + processor = webserver.ssl_processor + hostname = "renew-existing.example.com" + expired_cert = Mock(not_valid_after_utc=datetime.now(timezone.utc) - timedelta(days=1)) + + def find_cert(domain): + if domain == hostname: + return (hostname, Mock(), [expired_cert]) + return None + + with ( + patch.object(processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), + patch.object(processor.cert_manager, "obtain", side_effect=CertApiException("ACME renewal failed", step="Test")) as mock_obtain, + ): + hosts = [Host(hostname=hostname, port=443, scheme={"https"})] + webserver.ssl_processor.process_ssl_certificates(hosts) + + assert hosts[0].ssl_file == hostname + mock_obtain.assert_not_called() + + +def test_initial_failure_for_existing_cert_delegates_retry_to_renewal_manager(webserver_for_error_tests): + webserver = webserver_for_error_tests + processor = webserver.ssl_processor + hostname = "retry-backoff.example.com" + existing_cert = Mock(not_valid_after_utc=datetime.now(timezone.utc) - timedelta(days=1)) + + def find_cert(domain): + if domain == hostname: + return (hostname, Mock(), [existing_cert]) + return None + + with ( + patch.object(processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), + patch.object(processor.cert_manager, "obtain") as mock_obtain, + patch.object(processor.renewal_manager, "trigger_now") as mock_trigger, + ): + processor.process_ssl_certificates([Host(hostname=hostname, port=443, scheme={"https"})]) + + mock_obtain.assert_not_called() + mock_trigger.assert_not_called() + + +def test_failed_wildcard_with_existing_cert_gets_concrete_individual_cert(webserver_for_error_tests): + webserver = webserver_for_error_tests + processor = webserver.ssl_processor + wildcard = "*.example.com" + hosts = [ + Host(hostname=wildcard, port=443, scheme={"https"}), + Host(hostname="api.example.com", port=443, scheme={"https"}), + ] + wildcard_cert = Mock(not_valid_after_utc=datetime.now(timezone.utc) + timedelta(days=3)) + + def find_cert(domain): + if domain == wildcard: + return (wildcard, Mock(), [wildcard_cert]) + return None + + with ( + patch.object(processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), + patch.object(processor.renewal_manager, "trigger_now") as mock_trigger, + ): + webserver.ssl_processor.process_ssl_certificates(hosts) + + assert hosts[0].ssl_file == wildcard + assert hosts[1].ssl_file == "api.example.com.selfsigned" + mock_trigger.assert_called_once_with() + + +def test_failed_wildcard_expiring_within_48h_triggers_individual_dependent_issuance(webserver_for_error_tests): + webserver = webserver_for_error_tests + processor = webserver.ssl_processor + wildcard = "*.example.com" + hosts = [ + Host(hostname=wildcard, port=443, scheme={"https"}), + Host(hostname="api.example.com", port=443, scheme={"https"}), + ] + expiring_in_12h = Mock(not_valid_after_utc=datetime.now(timezone.utc) + timedelta(hours=12)) + + processor.update_threshold_secs = 3600 + + def find_cert(domain): + if domain == wildcard: + return (wildcard, Mock(), [expiring_in_12h]) + return None + + with ( + patch.object(processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), + patch.object(processor.renewal_manager, "trigger_now") as mock_trigger, + ): + processor.process_ssl_certificates(hosts) + + assert hosts[0].ssl_file == wildcard + assert hosts[1].ssl_file == wildcard + mock_trigger.assert_not_called() + + +def test_ssl_uses_renewal_manager_for_background_startup(): + server = SimpleNamespace( + config={ + "certapi": { + "url": "https://certapi.example.com", + "host": "certapi.example.com", + "scheme": "https", + "port": 443, + } + }, + reload=Mock(), + ) + nginx = SimpleNamespace(challenge_dir="./.run_data/acme-challenges/") + backend = Mock() + backend_info = SimpleNamespace( + backend=backend, + key_store=Mock(), + certapi_url="https://certapi.example.com", + use_certapi_server=True, + batch_domains=True, + cert_manager=None, + certapi_client=backend, + challenge_store=None, ) + + with ( + patch( + "nginx_proxy.post_processors.ssl_certificate_processor.build_certificate_backend", + return_value=backend_info, + ), + patch("nginx_proxy.post_processors.ssl_certificate_processor.RenewalManager") as renewal_cls, + ): + renewal = Mock() + renewal_cls.return_value = renewal + processor = SslCertificateProcessor( + nginx, + server=server, + update_threshold_days=1, + ssl_dir="./.run_data", + start_ssl_thread=True, + ) + + renewal.start.assert_called_once_with() + processor.shutdown() + renewal.stop.assert_called_once_with() diff --git a/tests/unit/test_ssl_certapi_batch_domains.py b/tests/unit/test_ssl_certapi_batch_domains.py new file mode 100644 index 0000000..425ebd3 --- /dev/null +++ b/tests/unit/test_ssl_certapi_batch_domains.py @@ -0,0 +1,183 @@ +import runpy +import sys +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import Mock, patch + +from nginx_proxy.Host import Host +from nginx_proxy.post_processors.ssl_certificate_processor import SslCertificateProcessor + + +REPO_ROOT = Path(__file__).resolve().parents[2] + + +def _make_server(): + return SimpleNamespace( + config={ + "certapi": { + "url": "https://certapi.example.com", + "host": "certapi.example.com", + "scheme": "https", + "port": 443, + } + } + ) + + +def _build_processor(monkeypatch, batch_value: str | None, start_ssl_thread=False): + if batch_value is None: + monkeypatch.delenv("CERTAPI_BATCH_DOMAINS", raising=False) + else: + monkeypatch.setenv("CERTAPI_BATCH_DOMAINS", batch_value) + + nginx = SimpleNamespace(challenge_dir="./.run_data/acme-challenges/") + backend = Mock() + backend.obtain.return_value = SimpleNamespace(issued=[], existing=[]) + backend_info = SimpleNamespace( + backend=backend, + key_store=Mock(), + certapi_url="https://certapi.example.com", + use_certapi_server=True, + batch_domains=batch_value is None or batch_value != "false", + cert_manager=None, + certapi_client=backend, + challenge_store=None, + ) + + with ( + patch( + "nginx_proxy.post_processors.ssl_certificate_processor.build_certificate_backend", + return_value=backend_info, + ), + patch("nginx_proxy.post_processors.ssl_certificate_processor.RenewalManager") as renewal_cls, + ): + renewal = Mock() + renewal_cls.return_value = renewal + processor = SslCertificateProcessor( + nginx, + server=_make_server(), + update_threshold_days=1, + ssl_dir="./.run_data", + start_ssl_thread=start_ssl_thread, + ) + processor._test_renewal_cls_call_args = renewal_cls.call_args + return processor, backend, renewal + + +def test_certapi_batch_domains_enabled_by_default(monkeypatch): + processor, backend, _renewal = _build_processor(monkeypatch, None) + + backend.obtain.assert_not_called() + assert processor._test_renewal_cls_call_args.kwargs["batch_domains"] is True + + +def test_certapi_batch_domains_passed_to_renewal_manager(monkeypatch): + processor, backend, _renewal = _build_processor(monkeypatch, "false") + + assert processor.certapi_batch_domains is False + backend.obtain.assert_not_called() + assert processor._test_renewal_cls_call_args.kwargs["batch_domains"] is False + + +def test_processor_does_not_obtain_directly_and_triggers_renewal_once(monkeypatch): + processor, backend, renewal = _build_processor(monkeypatch, None) + processor.key_store.find_key_and_cert_by_domain.return_value = None + + hosts = [Host("api.example.com", 443, {"https"}), Host("www.example.com", 443, {"https"})] + processor.process_ssl_certificates(hosts) + + renewal.set_watch_domains.assert_called_once_with(["api.example.com", "www.example.com"]) + renewal.trigger_now.assert_called_once_with() + backend.obtain.assert_not_called() + assert hosts[0].ssl_file == "api.example.com.selfsigned" + assert hosts[1].ssl_file == "www.example.com.selfsigned" + + +def test_ssl_starts_and_stops_certapi_renewal_manager(monkeypatch): + processor, _backend, renewal = _build_processor(monkeypatch, None, start_ssl_thread=True) + renewal.start.assert_called_once_with() + + processor.shutdown() + renewal.stop.assert_called_once_with() + + +def test_sync_watch_domains_publishes_secured_hosts_to_renewal_manager(monkeypatch): + secured = Host("secure.example.com", 443, {"https"}) + wildcard = Host("*.example.com", 443, {"https"}) + plain = Host("plain.example.com", 80, {"http"}) + server = _make_server() + server.config_data = SimpleNamespace(host_list=lambda: [secured, plain, wildcard]) + processor, _backend, renewal = _build_processor(monkeypatch, None) + processor.server = server + + processor.sync_watch_domains() + + renewal.set_watch_domains.assert_called_once_with(["*.example.com", "secure.example.com"]) + + +def test_getssl_force_passes_self_verify_false_to_remote_backend(monkeypatch, tmp_path): + backend = Mock() + backend.obtain.return_value = SimpleNamespace(issued=[], existing=[]) + backend_info = SimpleNamespace( + backend=backend, + key_store=Mock(), + certapi_url="https://certapi.example.com", + use_certapi_server=True, + batch_domains=True, + cert_manager=None, + certapi_client=backend, + challenge_store=None, + ) + nginx = Mock() + nginx.reload.return_value = True + + monkeypatch.setenv("CERTAPI_URL", "https://certapi.example.com") + monkeypatch.setenv("SSL_DIR", str(tmp_path / "ssl")) + monkeypatch.setenv("NGINX_CONF_DIR", str(tmp_path / "nginx")) + monkeypatch.setenv("CHALLENGE_DIR", str(tmp_path / "challenges")) + monkeypatch.setenv("CERT_RENEW_THRESHOLD_DAYS", "10") + monkeypatch.setattr(sys, "argv", ["getssl", "--force", "api.example.com"]) + + with ( + patch("nginx.Nginx.Nginx", return_value=nginx), + patch("nginx_proxy.certificate_backend.build_certificate_backend", return_value=backend_info), + ): + runpy.run_path(str(REPO_ROOT / "getssl"), run_name="__main__") + + backend.obtain.assert_called_once_with( + ["api.example.com"], key_type="ecdsa", batch_domains=True, self_verify=False + ) + + +def test_getssl_passes_self_verify_true_to_local_backend_by_default(monkeypatch, tmp_path): + backend = Mock() + backend.obtain.return_value = SimpleNamespace(issued=[], existing=[]) + backend_info = SimpleNamespace( + backend=backend, + key_store=Mock(), + certapi_url="", + use_certapi_server=False, + batch_domains=True, + cert_manager=backend, + certapi_client=None, + challenge_store=Mock(), + ) + nginx = Mock() + nginx.reload.return_value = True + + monkeypatch.delenv("CERTAPI_URL", raising=False) + monkeypatch.setenv("SSL_DIR", str(tmp_path / "ssl")) + monkeypatch.setenv("NGINX_CONF_DIR", str(tmp_path / "nginx")) + monkeypatch.setenv("CHALLENGE_DIR", str(tmp_path / "challenges")) + monkeypatch.setenv("CERT_RENEW_THRESHOLD_DAYS", "10") + monkeypatch.setattr(sys, "argv", ["getssl", "api.example.com"]) + + with ( + patch("nginx.Nginx.Nginx", return_value=nginx), + patch("nginx_proxy.certificate_backend.build_certificate_backend", return_value=backend_info), + ): + runpy.run_path(str(REPO_ROOT / "getssl"), run_name="__main__") + + backend.obtain.assert_called_once_with( + ["api.example.com"], key_type="ecdsa", batch_domains=True, self_verify=True + ) diff --git a/tests/unit/test_webserver_events.py b/tests/unit/test_webserver_events.py index 7e6cf30..97dde02 100644 --- a/tests/unit/test_webserver_events.py +++ b/tests/unit/test_webserver_events.py @@ -81,9 +81,7 @@ def create_webserver(docker_client: DockerTestClient, enable_ipv6: bool = False) # Wait for the thread to finish listener_thread.join(timeout=2) - # Stop the SSL refresh thread webserver.cleanup() - webserver.ssl_processor.ssl.certificate_expiry_thread.join(timeout=2) pattern = re.compile(r"^http://\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}:80") @@ -134,7 +132,7 @@ def expect_server_down(nginx: DummyNginx, server_name: str): def test_webserver_initialization(webserver: WebServer, nginx: DummyNginx): - assert isinstance(webserver.ssl_processor.ssl.challenge_store, NginxChallengeSolver) + assert isinstance(webserver.ssl_processor.challenge_store, NginxChallengeSolver) # Check initial default server block config = NginxConfig() full_config_str = f"http {{\n{nginx.current_config}\n}}" From 9e42949c9a8dc0c8e90f6e1bdde77f0d3a0ef9f1 Mon Sep 17 00:00:00 2001 From: Sudip Bhattarai Date: Wed, 29 Apr 2026 18:19:17 +0545 Subject: [PATCH 2/4] Address gemini code-review comments --- .../ssl_certificate_processor.py | 23 ++----------------- .../pre_processors/virtual_host_processor.py | 18 +++++++++------ pyproject.toml | 1 + requirements.txt | 2 +- tests/unit/test_backend_target.py | 11 ++++++--- 5 files changed, 23 insertions(+), 32 deletions(-) diff --git a/nginx_proxy/post_processors/ssl_certificate_processor.py b/nginx_proxy/post_processors/ssl_certificate_processor.py index 29f6daf..d5b5947 100644 --- a/nginx_proxy/post_processors/ssl_certificate_processor.py +++ b/nginx_proxy/post_processors/ssl_certificate_processor.py @@ -44,7 +44,7 @@ def sync_watch_domains(self): if self.server is None: return domains = sorted({host.hostname for host in self.server.config_data.host_list() if host.secured}) - self.renewal_manager.set_watch_domains(domains) + self.renewal_manager.update_watch_domains(domains) def is_certificate_fresh(self, domain: str, threshold_seconds: float | None = None) -> bool: result = self.key_store.find_key_and_cert_by_domain(domain) @@ -59,18 +59,6 @@ def is_certificate_fresh(self, domain: str, threshold_seconds: float | None = No def has_certificate(self, domain: str) -> bool: return self.key_store.find_key_and_cert_by_domain(domain) is not None - def has_self_signed_certificate(self, domain: str) -> bool: - return self.key_store.find_key_by_name(domain + ".selfsigned") is not None - - def _ensure_self_signed_certificate(self, domain: str): - if self.has_certificate(domain) or self.has_self_signed_certificate(domain): - return - - register_self_signed = getattr(self.renewal_manager, "_register_self_signed", None) - if register_self_signed is None: - return - register_self_signed(domain) - def _prepare_host_for_ssl(self, host: Host): """Sets SSL redirect and port if applicable.""" if int(host.port) in (80, 443): @@ -108,14 +96,7 @@ def process_ssl_certificates(self, hosts: List[Host]): self._prepare_host_for_ssl(host) secured_domains = sorted({host.hostname for host in secured_hosts}) - self.renewal_manager.set_watch_domains(secured_domains) - - if any(self._host_needs_certificate(host) for host in secured_hosts): - self.renewal_manager.trigger_now() - - for host in secured_hosts: - if self._select_ssl_file(host).endswith(".selfsigned"): - self._ensure_self_signed_certificate(host.hostname) + self.renewal_manager.update_watch_domains(secured_domains) for host in secured_hosts: host.ssl_file = self._select_ssl_file(host) diff --git a/nginx_proxy/pre_processors/virtual_host_processor.py b/nginx_proxy/pre_processors/virtual_host_processor.py index 380233d..5373936 100644 --- a/nginx_proxy/pre_processors/virtual_host_processor.py +++ b/nginx_proxy/pre_processors/virtual_host_processor.py @@ -1,3 +1,5 @@ +import re + from nginx_proxy import Host, ProxyConfigData from nginx_proxy.BackendTarget import BackendTarget, NoHostConfiguration, UnreachableNetwork from nginx_proxy.utils import split_url @@ -22,18 +24,20 @@ def _parse_extra_directive(raw_directive: str): directive = raw_directive.strip() if not directive: return None, None - # Accept "key=value" and "key = value" forms and normalize to nginx form "key value". - if "=" in directive: + equal_index = directive.find("=") + whitespace_match = re.search(r"\s", directive) + if whitespace_match is not None and (equal_index == -1 or whitespace_match.start() < equal_index): + key, value = re.split(r"\s+", directive, 1) + key = key.strip() + value = value.strip() + return (key, value if value else None) if key else (None, None) + # Accept "key=value" shorthand when the first delimiter is "=". + if equal_index != -1: key, value = directive.split("=", 1) key = key.strip() value = value.strip() if key: return key, value if value else None - if " " in directive: - key, value = directive.split(" ", 1) - key = key.strip() - value = value.strip() - return (key, value if value else None) if key else (None, None) return directive, None diff --git a/pyproject.toml b/pyproject.toml index 7306835..f359857 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,6 +13,7 @@ testpaths = [ [project] name = "nginx-proxy" version = "3.1.0" +requires-python = ">=3.12" [tool.setuptools.packages.find] include = ["nginx*", "cloudflare*"] diff --git a/requirements.txt b/requirements.txt index 4d53f5e..36cc7ca 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,5 +2,5 @@ docker==7.1.0 Jinja2==3.1.6 pydevd==3.1.0 bcrypt==4.3.0 # 5.0.0 requires rust so ignoring -certapi>=1.1.4 +certapi>=1.1.5 requests==2.33.0 diff --git a/tests/unit/test_backend_target.py b/tests/unit/test_backend_target.py index 197ad20..a6b380f 100644 --- a/tests/unit/test_backend_target.py +++ b/tests/unit/test_backend_target.py @@ -249,11 +249,16 @@ def test_parse_host_entry_with_equals_extra_syntax(self): assert extras["client_max_body_size"] == "2g" assert extras["proxy_read_timeout"] == "120" - def test_parse_host_entry_with_spaced_equals_extra_syntax(self): + def test_parse_host_entry_with_whitespace_before_equals_uses_whitespace_syntax(self): h, loc, c, extras = _parse_host_entry("example.com; client_max_body_size = 2g; proxy_read_timeout = 120") assert h.hostname == "example.com" - assert extras["client_max_body_size"] == "2g" - assert extras["proxy_read_timeout"] == "120" + assert extras["client_max_body_size"] == "= 2g" + assert extras["proxy_read_timeout"] == "= 120" + + def test_parse_host_entry_preserves_equals_after_whitespace_splitter(self): + h, loc, c, extras = _parse_host_entry("example.com; proxy_set_header Cookie=session=a=b") + assert h.hostname == "example.com" + assert extras["proxy_set_header"] == "Cookie=session=a=b" def test_remove_backend_cleans_only_its_injected_directives(self): known_networks = {"shared-net-id"} From e4715f25a814113316429c85c88b73b0d7664eea Mon Sep 17 00:00:00 2001 From: Sudip Bhattarai Date: Wed, 29 Apr 2026 19:54:41 +0545 Subject: [PATCH 3/4] Improve renewal architecture --- nginx_proxy/WebServer.py | 3 +- .../ssl_certificate_processor.py | 8 +++- requirements.txt | 2 +- tests/unit/test_error_resilience.py | 38 +++++++++---------- tests/unit/test_ssl_certapi_batch_domains.py | 11 ++++-- tests/unit/test_webserver_events.py | 30 +++++++++------ 6 files changed, 52 insertions(+), 40 deletions(-) diff --git a/nginx_proxy/WebServer.py b/nginx_proxy/WebServer.py index 7a69658..f428839 100644 --- a/nginx_proxy/WebServer.py +++ b/nginx_proxy/WebServer.py @@ -61,7 +61,7 @@ def __init__( self.ssl_processor = post_processors.SslCertificateProcessor( self.nginx, self, - start_ssl_thread=True, + start_ssl_thread=False, ssl_dir=self.config["ssl_dir"], update_threshold_days=self.config["cert_renew_threshold_days"], ) @@ -78,6 +78,7 @@ def __init__( print("Reachable Networks :", self.networks) self.setup_error_config() self.rescan_and_reload(force=True) + self.ssl_processor.start() def setup_error_config(self): # Render error.conf.jinja2 and save it diff --git a/nginx_proxy/post_processors/ssl_certificate_processor.py b/nginx_proxy/post_processors/ssl_certificate_processor.py index d5b5947..082adc1 100644 --- a/nginx_proxy/post_processors/ssl_certificate_processor.py +++ b/nginx_proxy/post_processors/ssl_certificate_processor.py @@ -32,19 +32,23 @@ def __init__( self.challenge_store = backend_info.challenge_store self.renewal_manager = RenewalManager( self.backend, - sync_watch_domains=self.sync_watch_domains, + renewal_callback=self.sync_watch_domains, renew_threshold_days=max(1, int(self.update_threshold_secs // (24 * 3600))), batch_domains=self.certapi_batch_domains, ) if start_ssl_thread: - self.renewal_manager.start() + self.start() + + def start(self): + self.renewal_manager.start() def sync_watch_domains(self): if self.server is None: return domains = sorted({host.hostname for host in self.server.config_data.host_list() if host.secured}) self.renewal_manager.update_watch_domains(domains) + self.server.reload(force=True) def is_certificate_fresh(self, domain: str, threshold_seconds: float | None = None) -> bool: result = self.key_store.find_key_and_cert_by_domain(domain) diff --git a/requirements.txt b/requirements.txt index 36cc7ca..eca6f34 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,5 +2,5 @@ docker==7.1.0 Jinja2==3.1.6 pydevd==3.1.0 bcrypt==4.3.0 # 5.0.0 requires rust so ignoring -certapi>=1.1.5 +certapi>=1.1.6 requests==2.33.0 diff --git a/tests/unit/test_error_resilience.py b/tests/unit/test_error_resilience.py index 5a30e90..a060282 100644 --- a/tests/unit/test_error_resilience.py +++ b/tests/unit/test_error_resilience.py @@ -292,13 +292,12 @@ def test_failed_initial_request_delegates_fallback_to_renewal_manager(webserver_ hosts = [Host(hostname="test-fallback-delegated.example.com", port=443, scheme={"https"})] with patch.object(processor.cert_manager, "obtain") as mock_obtain, patch.object( - processor.renewal_manager, "trigger_now" - ) as mock_trigger, patch.object(processor.renewal_manager, "set_watch_domains") as mock_set_watch_domains: + processor.renewal_manager, "update_watch_domains" + ) as mock_update_watch_domains: processor.process_ssl_certificates(hosts) mock_obtain.assert_not_called() - mock_set_watch_domains.assert_called_once_with(["test-fallback-delegated.example.com"]) - mock_trigger.assert_called_once_with() + mock_update_watch_domains.assert_any_call(["test-fallback-delegated.example.com"]) def test_multiple_simultaneous_cert_errors(webserver_for_error_tests): @@ -328,7 +327,7 @@ def test_wildcard_cert_error_handling(webserver_for_error_tests): error = CertApiException("Wildcard cert failure", step="Test") - with patch.object(webserver.ssl_processor.renewal_manager, "trigger_now") as mock_trigger: + with patch.object(webserver.ssl_processor.renewal_manager, "update_watch_domains") as mock_update_watch_domains: hosts = [Host(hostname="*.wildcard-test.example.com", port=443, scheme={"https"})] # Should not crash @@ -336,7 +335,7 @@ def test_wildcard_cert_error_handling(webserver_for_error_tests): # Should fallback to self-signed assert hosts[0].ssl_file == "*.wildcard-test.example.com.selfsigned" - mock_trigger.assert_called_once_with() + mock_update_watch_domains.assert_any_call(["*.wildcard-test.example.com"]) def test_fresh_wildcard_cert_remains_preferred(webserver_for_error_tests): @@ -353,13 +352,13 @@ def find_cert(domain): return None with patch.object(webserver.ssl_processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), patch.object( - webserver.ssl_processor.renewal_manager, "trigger_now" - ) as mock_trigger: + webserver.ssl_processor.renewal_manager, "update_watch_domains" + ) as mock_update_watch_domains: webserver.ssl_processor.process_ssl_certificates(hosts) assert hosts[0].ssl_file == "*.example.com" assert hosts[1].ssl_file == "*.example.com" - mock_trigger.assert_not_called() + mock_update_watch_domains.assert_called_once_with(["*.example.com", "api.example.com"]) def test_wildcard_near_expiry_is_not_preferred(webserver_for_error_tests): @@ -376,16 +375,16 @@ def find_cert(domain): return None with patch.object(webserver.ssl_processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), patch.object( - webserver.ssl_processor.renewal_manager, "trigger_now" - ) as mock_trigger: + webserver.ssl_processor.renewal_manager, "update_watch_domains" + ) as mock_update_watch_domains: webserver.ssl_processor.process_ssl_certificates(hosts) assert hosts[0].ssl_file == "*.example.com" assert hosts[1].ssl_file == "api.example.com.selfsigned" - mock_trigger.assert_called_once_with() + mock_update_watch_domains.assert_called_once_with(["*.example.com", "api.example.com"]) -def test_existing_cert_is_kept_without_nginx_proxy_retry_state(webserver_for_error_tests): +def test_existing_cert_is_kept_while_renewal_manager_handles_retry(webserver_for_error_tests): webserver = webserver_for_error_tests processor = webserver.ssl_processor hostname = "renew-existing.example.com" @@ -404,7 +403,6 @@ def find_cert(domain): webserver.ssl_processor.process_ssl_certificates(hosts) assert hosts[0].ssl_file == hostname - mock_obtain.assert_not_called() def test_initial_failure_for_existing_cert_delegates_retry_to_renewal_manager(webserver_for_error_tests): @@ -421,12 +419,12 @@ def find_cert(domain): with ( patch.object(processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), patch.object(processor.cert_manager, "obtain") as mock_obtain, - patch.object(processor.renewal_manager, "trigger_now") as mock_trigger, + patch.object(processor.renewal_manager, "update_watch_domains") as mock_update_watch_domains, ): processor.process_ssl_certificates([Host(hostname=hostname, port=443, scheme={"https"})]) mock_obtain.assert_not_called() - mock_trigger.assert_not_called() + mock_update_watch_domains.assert_called_once_with([hostname]) def test_failed_wildcard_with_existing_cert_gets_concrete_individual_cert(webserver_for_error_tests): @@ -446,13 +444,13 @@ def find_cert(domain): with ( patch.object(processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), - patch.object(processor.renewal_manager, "trigger_now") as mock_trigger, + patch.object(processor.renewal_manager, "update_watch_domains") as mock_update_watch_domains, ): webserver.ssl_processor.process_ssl_certificates(hosts) assert hosts[0].ssl_file == wildcard assert hosts[1].ssl_file == "api.example.com.selfsigned" - mock_trigger.assert_called_once_with() + mock_update_watch_domains.assert_called_once_with(["*.example.com", "api.example.com"]) def test_failed_wildcard_expiring_within_48h_triggers_individual_dependent_issuance(webserver_for_error_tests): @@ -474,13 +472,13 @@ def find_cert(domain): with ( patch.object(processor.key_store, "find_key_and_cert_by_domain", side_effect=find_cert), - patch.object(processor.renewal_manager, "trigger_now") as mock_trigger, + patch.object(processor.renewal_manager, "update_watch_domains") as mock_update_watch_domains, ): processor.process_ssl_certificates(hosts) assert hosts[0].ssl_file == wildcard assert hosts[1].ssl_file == wildcard - mock_trigger.assert_not_called() + mock_update_watch_domains.assert_called_once_with(["*.example.com", "api.example.com"]) def test_ssl_uses_renewal_manager_for_background_startup(): diff --git a/tests/unit/test_ssl_certapi_batch_domains.py b/tests/unit/test_ssl_certapi_batch_domains.py index 425ebd3..566e1b9 100644 --- a/tests/unit/test_ssl_certapi_batch_domains.py +++ b/tests/unit/test_ssl_certapi_batch_domains.py @@ -20,7 +20,8 @@ def _make_server(): "scheme": "https", "port": 443, } - } + }, + reload=Mock(), ) @@ -77,6 +78,7 @@ def test_certapi_batch_domains_passed_to_renewal_manager(monkeypatch): assert processor.certapi_batch_domains is False backend.obtain.assert_not_called() assert processor._test_renewal_cls_call_args.kwargs["batch_domains"] is False + assert processor._test_renewal_cls_call_args.kwargs["renewal_callback"] == processor.sync_watch_domains def test_processor_does_not_obtain_directly_and_triggers_renewal_once(monkeypatch): @@ -86,8 +88,8 @@ def test_processor_does_not_obtain_directly_and_triggers_renewal_once(monkeypatc hosts = [Host("api.example.com", 443, {"https"}), Host("www.example.com", 443, {"https"})] processor.process_ssl_certificates(hosts) - renewal.set_watch_domains.assert_called_once_with(["api.example.com", "www.example.com"]) - renewal.trigger_now.assert_called_once_with() + renewal.update_watch_domains.assert_called_once_with(["api.example.com", "www.example.com"]) + renewal.trigger_now.assert_not_called() backend.obtain.assert_not_called() assert hosts[0].ssl_file == "api.example.com.selfsigned" assert hosts[1].ssl_file == "www.example.com.selfsigned" @@ -112,7 +114,8 @@ def test_sync_watch_domains_publishes_secured_hosts_to_renewal_manager(monkeypat processor.sync_watch_domains() - renewal.set_watch_domains.assert_called_once_with(["*.example.com", "secure.example.com"]) + renewal.update_watch_domains.assert_called_once_with(["*.example.com", "secure.example.com"]) + server.reload.assert_called_once_with(force=True) def test_getssl_force_passes_self_verify_false_to_remote_backend(monkeypatch, tmp_path): diff --git a/tests/unit/test_webserver_events.py b/tests/unit/test_webserver_events.py index 97dde02..5fcb8c3 100644 --- a/tests/unit/test_webserver_events.py +++ b/tests/unit/test_webserver_events.py @@ -119,6 +119,21 @@ def expect_server_not_present(nginx: DummyNginx, server_name: str): ), f"Server for {server_name} should not be present. All servers:\n{all_servers_str}" +def expect_servers(nginx: DummyNginx, server_name: str, count: int, timeout: float = 2): + deadline = time.monotonic() + timeout + servers = [] + while time.monotonic() < deadline: + config = HttpBlock.parse(nginx.current_config) + servers = [s for s in config.servers if server_name in s.server_names] + if len(servers) == count: + return servers + time.sleep(0.05) + + config = HttpBlock.parse(nginx.current_config) + all_servers_str = "\n".join([str(s) for s in config.servers]) + assert len(servers) == count, f"Expected {count} server(s) for {server_name}. All servers:\n{all_servers_str}" + + def expect_server_down(nginx: DummyNginx, server_name: str): server = expect_server(nginx, server_name) config = HttpBlock.parse(nginx.current_config) @@ -259,12 +274,9 @@ def test_webserver_add_container_with_ssl(docker_client: DockerTestClient, nginx # Create container with SSL env container = docker_client.containers.run("nginx:alpine", name=container_name, environment=env, network="frontend") - time.sleep(0.2) # Verify that two server blocks are created for SSL - config = HttpBlock.parse(nginx.current_config) - servers = [s for s in config.servers if hostname in s.server_names] - assert len(servers) == 2 + servers = expect_servers(nginx, hostname, 2) # Verify HTTPS server is correctly configured https_server = next((s for s in servers if "443" in s.listen), None) @@ -294,11 +306,8 @@ def test_webserver_ssl_does_not_override_explicit_http_location(docker_client: D } container = docker_client.containers.run("nginx:alpine", name=container_name, environment=env, network="frontend") - time.sleep(0.2) - config = HttpBlock.parse(nginx.current_config) - servers = [s for s in config.servers if hostname in s.server_names] - assert len(servers) == 2 + servers = expect_servers(nginx, hostname, 2) https_server = next((s for s in servers if "443" in s.listen), None) http_server = next((s for s in servers if s.listen == "80"), None) @@ -331,11 +340,8 @@ def test_webserver_ssl_respects_explicit_http_root(docker_client: DockerTestClie } container = docker_client.containers.run("nginx:alpine", name=container_name, environment=env, network="frontend") - time.sleep(0.2) - config = HttpBlock.parse(nginx.current_config) - servers = [s for s in config.servers if hostname in s.server_names] - assert len(servers) == 2 + servers = expect_servers(nginx, hostname, 2) http_server = next((s for s in servers if s.listen == "80"), None) assert http_server is not None From 8bd78b857bb6c48c81d5fa44b44cf6efc6d38475 Mon Sep 17 00:00:00 2001 From: Sudip Bhattarai Date: Wed, 29 Apr 2026 20:19:50 +0545 Subject: [PATCH 4/4] Remove unused blacklist helper --- .gitignore | 1 + nginx_proxy/utils/Blacklist.py | 39 ----------------------------- tests/unit/test_error_resilience.py | 2 +- 3 files changed, 2 insertions(+), 40 deletions(-) delete mode 100644 nginx_proxy/utils/Blacklist.py diff --git a/.gitignore b/.gitignore index eee82aa..c80d866 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ run_data/ .env *.egg-info/ .pytest_cache/ +dist/ \ No newline at end of file diff --git a/nginx_proxy/utils/Blacklist.py b/nginx_proxy/utils/Blacklist.py deleted file mode 100644 index 4c3905e..0000000 --- a/nginx_proxy/utils/Blacklist.py +++ /dev/null @@ -1,39 +0,0 @@ -from typing import Any, List, Tuple - -import time - - -class Blacklist: - def __init__(self, blacklist_duration_secs: int = 180) -> None: - self.blacklisted_items: dict[Any, float] = {} - self.default_duration: int = blacklist_duration_secs - - def _clean_blacklist(self) -> None: - current_time = time.time() - expired_items = [item for item, expiry in self.blacklisted_items.items() if expiry <= current_time] - for item in expired_items: - del self.blacklisted_items[item] - - def filter(self, items: List[Any]) -> List[Any]: - self._clean_blacklist() - return [item for item in items if item not in self.blacklisted_items] - - def add(self, item: Any, duration_seconds: int | None = None) -> None: - if duration_seconds is None: - duration_seconds = self.default_duration - self.blacklisted_items[item] = time.time() + duration_seconds - - def list(self) -> List[Any]: - self._clean_blacklist() - return list(self.blacklisted_items.keys()) - - def partition(self, items: List[Any]) -> Tuple[List[Any], List[Any]]: - self._clean_blacklist() - whitelisted_items: List[Any] = [] - blacklisted_items: List[Any] = [] - for item in items: - if item in self.blacklisted_items: - blacklisted_items.append(item) - else: - whitelisted_items.append(item) - return (whitelisted_items, blacklisted_items) diff --git a/tests/unit/test_error_resilience.py b/tests/unit/test_error_resilience.py index a060282..6af3b50 100644 --- a/tests/unit/test_error_resilience.py +++ b/tests/unit/test_error_resilience.py @@ -286,7 +286,7 @@ def test_fallback_to_selfsigned_on_failure(webserver_for_error_tests, error, hos def test_failed_initial_request_delegates_fallback_to_renewal_manager(webserver_for_error_tests): - """Initial issuance no longer owns blacklist/retry behavior in nginx-proxy.""" + """Initial issuance retry behavior is owned by certapi's renewal manager.""" webserver = webserver_for_error_tests processor = webserver.ssl_processor