Fix ssl renewal callback

Author: mesudip

nginx_proxy/DockerEventListener.py

@@ -73,11 +73,6 @@ class Reload:
     force: bool = False
 
 
-@dataclass(frozen=True)
-class SyncSslWatchDomains:
-    pass
-
-
 _STOP = object()
 
 
@@ -184,8 +179,6 @@ def _dispatch(self, command):
             self.web_server._do_reload(command.force)
         elif isinstance(command, Reload):
             self.web_server._do_reload(command.force)
-        elif isinstance(command, SyncSslWatchDomains):
-            self._sync_ssl_watch_domains()
         elif callable(command):
             command()
         else:
@@ -451,12 +444,6 @@ def _process_backend_activation(self, container_id: str, generation: int):
         self._pending_backend_generations.pop(container_id, None)
         self._activate_backend_if_running(container_id)
 
-    def _sync_ssl_watch_domains(self):
-        try:
-            self.web_server._do_reload(True)
-        finally:
-            self.web_server.ssl_processor._dispatcher_ssl_reload_pending = False
-
     @staticmethod
     def _container_has_healthcheck(container) -> bool:
         healthcheck = container.attrs.get("Config", {}).get("Healthcheck")
@@ -497,7 +484,8 @@ def _container_name(self, container=None, container_id: str | None = None, attri
             return None
         except (KeyboardInterrupt, SystemExit):
             raise
-        except Exception:
+        except Exception as e:
+            self._log_unexpected_error(f"Could not resolve container name for {container_id}", e)
             return None
         name = getattr(resolved, "name", None) or resolved.attrs.get("Name")
         return name.lstrip("/") if isinstance(name, str) else None
@@ -532,10 +520,12 @@ def _should_forward_network_connect(self, container_id: str) -> bool:
         except docker.errors.NotFound:
             return False
         except ValueError:
+            print(f"WARN: Ignoring network connect for invalid container id {container_id!r}", file=sys.stderr)
             return False
         except (KeyboardInterrupt, SystemExit):
             raise
-        except Exception:
+        except Exception as e:
+            self._log_unexpected_error(f"Could not inspect container {container_id} for network connect", e)
             return False
 
         if not self._container_is_running(container):
@@ -563,5 +553,11 @@ def _load_started_container_ids(self) -> set[str]:
             }
         except (KeyboardInterrupt, SystemExit):
             raise
-        except Exception:
+        except Exception as e:
+            self._log_unexpected_error("Could not load running containers during Docker event listener startup", e)
             return set()
+
+    @staticmethod
+    def _log_unexpected_error(message: str, error: Exception):
+        print(f"WARN: {message}: {error.__class__.__name__} -> {error}", file=sys.stderr)
+        traceback.print_exc(limit=10)

nginx_proxy/WebServer.py

@@ -240,17 +240,17 @@ def reload(self, immediate=False, force=False) -> bool:
         Returns True if a reload was initiated or scheduled.
         """
 
-        def run_or_enqueue():
-            if self._reload_dispatcher is None:
-                return self._do_reload(force)
-            if self._is_reload_dispatcher_thread is not None and self._is_reload_dispatcher_thread():
-                return self._do_reload(force)
+        return self.throttler.throttle(lambda: self._do_reload(force), immediate=immediate or force)
 
-            from nginx_proxy.DockerEventListener import Reload
+    def enqueue_reload(self, force=False) -> bool:
+        if self._reload_dispatcher is None:
+            return self.reload(immediate=force, force=force)
+        if self._is_reload_dispatcher_thread is not None and self._is_reload_dispatcher_thread():
+            return self.reload(immediate=force, force=force)
 
-            return self._reload_dispatcher(Reload(force))
+        from nginx_proxy.DockerEventListener import Reload
 
-        return self.throttler.throttle(run_or_enqueue, immediate=immediate or force)
+        return self._reload_dispatcher(Reload(force))
 
     def disconnect(self, network, container, scope):
         if self.id is not None and container == self.id:

nginx_proxy/post_processors/ssl_certificate_processor.py

@@ -30,10 +30,9 @@ def __init__(
         self.cert_manager = backend_info.cert_manager
         self.certapi_client = backend_info.certapi_client
         self.challenge_store = backend_info.challenge_store
-        self._dispatcher_ssl_reload_pending = False
         self.renewal_manager = RenewalManager(
             self.backend,
-            renewal_callback=self.sync_watch_domains,
+            renewal_callback=self.ssl_renewal_callback,
             renew_threshold_days=max(1, int(self.update_threshold_secs // (24 * 3600))),
             batch_domains=self.certapi_batch_domains,
         )
@@ -44,21 +43,11 @@ def __init__(
     def start(self):
         self.renewal_manager.start()
 
-    def sync_watch_domains(self):
+    def ssl_renewal_callback(self):
+        print("[SSL] Renewal callback triggered")
         if self.server is None:
             return
-        listener = getattr(self.server, "docker_event_listener", None)
-        if listener is not None and listener.is_dispatcher_running():
-            if self._dispatcher_ssl_reload_pending:
-                return
-            from nginx_proxy.DockerEventListener import SyncSslWatchDomains
-
-            self._dispatcher_ssl_reload_pending = True
-            listener.enqueue(SyncSslWatchDomains())
-            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)
+        self.server.enqueue_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)

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.10
+certapi>=1.1.11
 requests==2.33.0

tests/conftest.py

@@ -0,0 +1,18 @@
+def pytest_collection_modifyitems(items):
+    original_order = {item: index for index, item in enumerate(items)}
+    swarm_mode_order = {
+        "enable": 0,
+        "exclude": 1,
+        "ignore": 2,
+        "prefer-local": 3,
+        "strict": 4,
+    }
+
+    def swarm_mode_sort_key(item):
+        callspec = getattr(item, "callspec", None)
+        swarm_mode = callspec.params.get("swarm_mode") if callspec is not None else None
+        if swarm_mode is None:
+            return (0, 0, original_order[item])
+        return (1, swarm_mode_order.get(swarm_mode, len(swarm_mode_order)), original_order[item])
+
+    items.sort(key=swarm_mode_sort_key)

tests/integration/test_nginx_proxy.py

@@ -5,6 +5,8 @@
 import requests
 import websocket
 import time
+import re
+import hashlib
 from datetime import datetime, timezone
 from unittest.mock import patch
 
@@ -42,6 +44,12 @@ def _hostname_mode_token(swarm_mode):
     return {"prefer-local": "pl"}.get(swarm_mode, swarm_mode)
 
 
+def _hostname_slug(value):
+    slug = re.sub(r"[^a-z0-9]+", "-", value.lower()).strip("-")
+    digest = hashlib.sha1(value.encode("utf-8")).hexdigest()[:10]
+    return f"{slug[:36].strip('-')}-{digest}"
+
+
 def _has_proxy_server(config_str, server_name):
     config = HttpBlock.parse(config_str)
     for server in config.servers:
@@ -125,7 +133,8 @@ def test_http_routing_discovery(
     """
     Test HTTP routing discovery for various swarm modes and backend types.
     """
-    hostname = f"{backend_type}.{swarm_mode}.routing.example.com"
+    case_slug = _hostname_slug(request.node.name)
+    hostname = f"{backend_type}.{_hostname_mode_token(swarm_mode)}.{case_slug}.routing.example.com"
     should_be_reachable = is_reachable(swarm_mode, backend_type)
 
     env = {"VIRTUAL_HOST": hostname + virtual_host_path, "VIRTUAL_PORT": "8080"}

tests/unit/test_ssl_certapi_batch_domains.py

@@ -5,7 +5,6 @@
 from unittest.mock import Mock, patch
 
 from nginx_proxy.Host import Host
-from nginx_proxy.DockerEventListener import SyncSslWatchDomains
 from nginx_proxy.post_processors.ssl_certificate_processor import SslCertificateProcessor
 
 
@@ -23,6 +22,7 @@ def _make_server():
             }
         },
         reload=Mock(),
+        enqueue_reload=Mock(),
     )
 
 
@@ -79,7 +79,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
+    assert processor._test_renewal_cls_call_args.kwargs["renewal_callback"] == processor.ssl_renewal_callback
 
 
 def test_processor_does_not_obtain_directly_and_triggers_renewal_once(monkeypatch):
@@ -104,34 +104,25 @@ def test_ssl_starts_and_stops_certapi_renewal_manager(monkeypatch):
     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"})
+def test_ssl_renewal_callback_enqueues_server_reload(monkeypatch):
     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()
+    processor.ssl_renewal_callback()
 
-    renewal.update_watch_domains.assert_called_once_with(["*.example.com", "secure.example.com"])
-    server.reload.assert_called_once_with(force=True)
+    renewal.update_watch_domains.assert_not_called()
+    server.enqueue_reload.assert_called_once_with(force=True)
+    server.reload.assert_not_called()
 
 
-def test_sync_watch_domains_enqueues_when_dispatcher_is_running(monkeypatch):
-    server = _make_server()
-    listener = Mock()
-    listener.is_dispatcher_running.return_value = True
-    server.docker_event_listener = listener
+def test_ssl_renewal_callback_ignores_missing_server(monkeypatch):
     processor, _backend, renewal = _build_processor(monkeypatch, None)
-    processor.server = server
+    processor.server = None
 
-    processor.sync_watch_domains()
+    processor.ssl_renewal_callback()
 
     renewal.update_watch_domains.assert_not_called()
-    server.reload.assert_not_called()
-    listener.enqueue.assert_called_once_with(SyncSslWatchDomains())
 
 
 def test_getssl_force_passes_self_verify_false_to_remote_backend(monkeypatch, tmp_path):

tests/unit/test_web_server.py

@@ -3,6 +3,7 @@
 from datetime import datetime, timedelta, timezone
 
 from nginx_proxy.BackendTarget import BackendTarget
+from nginx_proxy.DockerEventListener import Reload
 from nginx_proxy.ProxyConfigData import ProxyConfigData
 from nginx_proxy.WebServer import WebServer
 
@@ -206,6 +207,17 @@ def test_reload_force_runs_immediately(web_server):
         assert mock_throttle.call_args.kwargs["immediate"] is True
 
 
+def test_enqueue_reload_uses_dispatcher_when_running(web_server):
+    dispatcher = MagicMock(return_value=True)
+    web_server.set_reload_dispatcher(dispatcher, lambda: False)
+
+    assert web_server.enqueue_reload(force=True) is True
+
+    command = dispatcher.call_args.args[0]
+    assert isinstance(command, Reload)
+    assert command.force is True
+
+
 def test_should_register_container_now_skips_unhealthy_healthcheck(web_server):
     container = MagicMock()
     container.status = "running"