Repository: sanic-org/sanic
Pull request: Add GenericCreator for loading SSL certs in processes
Resolves #2577 

Problem statement:
Certificates not created with `mkcert` or `trustme` raise a RuntimeError
The `CertLoader` class in `sanic-org/sanic/sanic/worker/loader.py` checks the creator of the certificate. If the creator is not `mkcert` or `trustme` then it raises a `RuntimeError`. This will prevent Sanic from running with certificates from any other sources.

Hints from issue discussion:

Commit message: Add to unit test

Patch to review:
diff --git a/sanic/worker/loader.py b/sanic/worker/loader.py
index abdcc9870a..309e930f75 100644
--- a/sanic/worker/loader.py
+++ b/sanic/worker/loader.py
@@ -16,6 +16,7 @@
     cast,
 )
 
+from sanic.http.tls.context import process_to_context
 from sanic.http.tls.creators import CertCreator, MkcertCreator, TrustmeCreator
 
 
@@ -109,18 +110,28 @@ class CertLoader:
     _creator_class: Type[CertCreator]
 
     def __init__(self, ssl_data: Dict[str, Union[str, os.PathLike]]):
+        self._ssl_data = ssl_data
+
         creator_name = ssl_data.get("creator")
-        if creator_name not in ("mkcert", "trustme"):
-            raise RuntimeError(f"Unknown certificate creator: {creator_name}")
-        elif creator_name == "mkcert":
-            self._creator_class = MkcertCreator
-        elif creator_name == "trustme":
-            self._creator_class = TrustmeCreator
+        if not creator_name:
+            return
+        else:
+            if creator_name not in ("mkcert", "trustme"):
+                raise RuntimeError(
+                    f"Unknown certificate creator: {creator_name}"
+                )
+            elif creator_name == "mkcert":
+                self._creator_class = MkcertCreator
+            elif creator_name == "trustme":
+                self._creator_class = TrustmeCreator
 
         self._key = ssl_data["key"]
         self._cert = ssl_data["cert"]
         self._localhost = cast(str, ssl_data["localhost"])
 
     def load(self, app: SanicApp):
+        if not hasattr(self, "_creator_class"):
+            return process_to_context(self._ssl_data)
+
         creator = self._creator_class(app, self._key, self._cert)
         return creator.generate_cert(self._localhost)
diff --git a/tests/test_tls.py b/tests/test_tls.py
index 6c369f9284..497b67deaf 100644
--- a/tests/test_tls.py
+++ b/tests/test_tls.py
@@ -4,6 +4,7 @@
 import subprocess
 
 from contextlib import contextmanager
+from multiprocessing import Event
 from pathlib import Path
 from unittest.mock import Mock, patch
 from urllib.parse import urlparse
@@ -636,3 +637,29 @@ def test_sanic_ssl_context_create():
 
     assert sanic_context is context
     assert isinstance(sanic_context, SanicSSLContext)
+
+
+def test_ssl_in_multiprocess_mode(app: Sanic, caplog):
+
+    ssl_dict = {"cert": localhost_cert, "key": localhost_key}
+    event = Event()
+
+    @app.main_process_start
+    async def main_start(app: Sanic):
+        app.shared_ctx.event = event
+
+    @app.after_server_start
+    async def shutdown(app):
+        app.shared_ctx.event.set()
+        app.stop()
+
+    assert not event.is_set()
+    with caplog.at_level(logging.INFO):
+        app.run(ssl=ssl_dict)
+    assert event.is_set()
+
+    assert (
+        "sanic.root",
+        logging.INFO,
+        "Goin' Fast @ https://127.0.0.1:8000",
+    ) in caplog.record_tuples


Code review comments:
diff_hunk: @@ -109,18 +110,28 @@ class CertLoader:
     _creator_class: Type[CertCreator]
 
     def __init__(self, ssl_data: Dict[str, Union[str, os.PathLike]]):
+        self._ssl_data = ssl_data
+
         creator_name = ssl_data.get("creator")
-        if creator_name not in ("mkcert", "trustme"):
-            raise RuntimeError(f"Unknown certificate creator: {creator_name}")
-        elif creator_name == "mkcert":
-            self._creator_class = MkcertCreator
-        elif creator_name == "trustme":
-            self._creator_class = TrustmeCreator
+        if not creator_name:
+            return
+        else:
path: sanic/worker/loader.py
text: @user1:
Don't use `else` if the `if` returns/throws.

diff_hunk: @@ -109,18 +110,28 @@ class CertLoader:
     _creator_class: Type[CertCreator]
 
     def __init__(self, ssl_data: Dict[str, Union[str, os.PathLike]]):
+        self._ssl_data = ssl_data
+
         creator_name = ssl_data.get("creator")
-        if creator_name not in ("mkcert", "trustme"):
-            raise RuntimeError(f"Unknown certificate creator: {creator_name}")
-        elif creator_name == "mkcert":
-            self._creator_class = MkcertCreator
-        elif creator_name == "trustme":
-            self._creator_class = TrustmeCreator
+        if not creator_name:
+            return
+        else:
+            if creator_name not in ("mkcert", "trustme"):
+                raise RuntimeError(
+                    f"Unknown certificate creator: {creator_name}"
+                )
+            elif creator_name == "mkcert":
+                self._creator_class = MkcertCreator
+            elif creator_name == "trustme":
+                self._creator_class = TrustmeCreator
path: sanic/worker/loader.py
text: @user1:
Maybe use `dict(mkcert=MkcertCreator, trustme=TrustmeCreator).get(creator_name)` instead of this whole thing? Less code, no repetition.

@author:
Was thinking about that, but was keeping my code changes minimal.

diff_hunk: @@ -109,18 +110,28 @@ class CertLoader:
     _creator_class: Type[CertCreator]
 
     def __init__(self, ssl_data: Dict[str, Union[str, os.PathLike]]):
+        self._ssl_data = ssl_data
+
         creator_name = ssl_data.get("creator")
-        if creator_name not in ("mkcert", "trustme"):
-            raise RuntimeError(f"Unknown certificate creator: {creator_name}")
-        elif creator_name == "mkcert":
-            self._creator_class = MkcertCreator
-        elif creator_name == "trustme":
-            self._creator_class = TrustmeCreator
+        if not creator_name:
+            return
+        else:
+            if creator_name not in ("mkcert", "trustme"):
+                raise RuntimeError(
+                    f"Unknown certificate creator: {creator_name}"
+                )
+            elif creator_name == "mkcert":
+                self._creator_class = MkcertCreator
+            elif creator_name == "trustme":
+                self._creator_class = TrustmeCreator
 
         self._key = ssl_data["key"]
         self._cert = ssl_data["cert"]
         self._localhost = cast(str, ssl_data["localhost"])
 
     def load(self, app: SanicApp):
+        if not hasattr(self, "_creator_class"):
path: sanic/worker/loader.py
text: @user1:
Might prefer member variables set to `None` instead of using `hasattr`.


@author:
I didn't want to have to deal with typing issues of `Optional` and another branch of complexity in `load`.