rekor: Make create_entry() conservative WRT session use

* Session thread safety is ambiguous: it may now be safe but situation
  is unclear
* Use a single Session per create_entry() call: This may have a little
  more overhead but seems like the safest option
* Note that RekorLog (v1) other methods may not be thread safe: I only
  reviewed the create_entry() flow

Signed-off-by: Jussi Kukkonen <[email protected]>

Author: jku

sigstore/_internal/rekor/client.py

@@ -74,6 +74,8 @@ def from_response(cls, dict_: dict[str, Any]) -> RekorLogInfo:
 
 class _Endpoint(ABC):
     def __init__(self, url: str, session: requests.Session) -> None:
+        # Note that _Endpoint may not be thread be safe if the same Session is provided
+        # to an _Endpoint in multiple threads
         self.url = url
         self.session = session
 
@@ -210,20 +212,19 @@ def __init__(self, url: str) -> None:
         Create a new `RekorClient` from the given URL.
         """
         self.url = f"{url}/api/v1"
-        self.session = requests.Session()
-        self.session.headers.update(
+
+    def _session(self) -> requests.Session:
+        # We do not use a long living session to avoid potential thread safety issues:
+        # submitting entries via create_entry() should be thread safe.
+        session = requests.Session()
+        session.headers.update(
             {
                 "Content-Type": "application/json",
                 "Accept": "application/json",
                 "User-Agent": USER_AGENT,
             }
         )
-
-    def __del__(self) -> None:
-        """
-        Terminates the underlying network session.
-        """
-        self.session.close()
+        return session
 
     @classmethod
     def production(cls) -> RekorClient:
@@ -246,7 +247,10 @@ def log(self) -> RekorLog:
         """
         Returns a `RekorLog` adapter for making requests to a Rekor log.
         """
-        return RekorLog(f"{self.url}/log", session=self.session)
+
+        # Each RekorLog gets their own session
+        with self._session() as s:
+            return RekorLog(f"{self.url}/log", session=s)
 
     def create_entry(self, request: EntryRequestBody) -> LogEntry:
         """

sigstore/_internal/rekor/client_v2.py

@@ -55,20 +55,6 @@ def __init__(self, base_url: str) -> None:
         Create a new `RekorV2Client` from the given URL.
         """
         self.url = f"{base_url}/api/v2"
-        self.session = requests.Session()
-        self.session.headers.update(
-            {
-                "Content-Type": "application/json",
-                "Accept": "application/json",
-                "User-Agent": USER_AGENT,
-            }
-        )
-
-    def __del__(self) -> None:
-        """
-        Terminates the underlying network session.
-        """
-        self.session.close()
 
     def create_entry(self, payload: EntryRequestBody) -> LogEntry:
         """
@@ -79,7 +65,19 @@ def create_entry(self, payload: EntryRequestBody) -> LogEntry:
         https://github.com/sigstore/rekor-tiles/blob/main/CLIENTS.md#handling-longer-requests
         """
         _logger.debug(f"proposed: {json.dumps(payload)}")
-        resp = self.session.post(
+
+        # Use a short lived session to avoid potential issues with multi-threading:
+        # Session thread-safety is ambiguous
+        session = requests.Session()
+        session.headers.update(
+            {
+                "Content-Type": "application/json",
+                "Accept": "application/json",
+                "User-Agent": USER_AGENT,
+            }
+        )
+
+        resp = session.post(
             f"{self.url}/log/entries",
             json=payload,
         )