fix: reuse ssl context in the sync client (#222)

* fix: reuse ssl context in the sync client This brings ssl context handling in line with the async client. Importantly, openssl has a pretty signifigant performance regression in creating ssl contexts v3.0+ that is mitigated by paying the context creation tax once, instead of for every request. Based on testing, this reduces the openssl v3 performance penalty from ~200ms per connection to 9ms per connection. Original PR: openfga/sdk-generator#607 * chore: add tests for ssl context reuse --------- Co-authored-by: Walt Della <walt.della@zapier.com>

Author: rhamzeh

openfga_sdk/sync/rest.py

@@ -155,10 +155,18 @@ def __init__(
  :param pools_size: The number of connection pools to use.
  :param maxsize: The maximum number of connections per pool.
  """
- if hasattr(configuration, "verify_ssl") and configuration.verify_ssl:
- cert_reqs = ssl.CERT_REQUIRED
- else:
- cert_reqs = ssl.CERT_NONE
+ # Reuse SSL context to mitigate OpenSSL 3.0+ performance issues
+ # See: https://github.com/openssl/openssl/issues/17064
+ ssl_context = ssl.create_default_context(cafile=configuration.ssl_ca_cert)
+
+ if configuration.cert_file:
+ ssl_context.load_cert_chain(
+ configuration.cert_file, keyfile=configuration.key_file
+ )
+
+ if not configuration.verify_ssl:
+ ssl_context.check_hostname = False
+ ssl_context.verify_mode = ssl.CERT_NONE
 
  addition_pool_args = {}
 
@@ -193,10 +201,7 @@ def __init__(
  urllib3.ProxyManager(
  num_pools=pools_size,
  maxsize=maxsize,
- cert_reqs=cert_reqs,
- ca_certs=configuration.ssl_ca_cert,
- cert_file=configuration.cert_file,
- key_file=configuration.key_file,
+ ssl_context=ssl_context,
  proxy_url=configuration.proxy,
  proxy_headers=configuration.proxy_headers,
  **addition_pool_args,
@@ -208,10 +213,7 @@ def __init__(
  self.pool_manager = urllib3.PoolManager(
  num_pools=pools_size,
  maxsize=maxsize,
- cert_reqs=cert_reqs,
- ca_certs=configuration.ssl_ca_cert,
- cert_file=configuration.cert_file,
- key_file=configuration.key_file,
+ ssl_context=ssl_context,
  **addition_pool_args,
  )
 

test/sync/rest_test.py

@@ -11,8 +11,9 @@
 """
 
 import json
+import ssl
 
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 import pytest
 
@@ -531,3 +532,204 @@ def release_conn(self):
  # Exception is logged, we yield nothing
  assert results == []
  mock_pool_manager.request.assert_called_once()
+
+
+# Tests for SSL Context Reuse (fix for OpenSSL 3.0+ performance issues)
+@patch('ssl.create_default_context')
+@patch('urllib3.PoolManager')
+def test_ssl_context_created_with_ca_cert(mock_pool_manager, mock_create_context):
+ """Test that SSL context is created with CA certificate file."""
+ mock_ssl_context = MagicMock()
+ mock_create_context.return_value = mock_ssl_context
+
+ mock_config = MagicMock()
+ mock_config.ssl_ca_cert = "/path/to/ca.pem"
+ mock_config.cert_file = None
+ mock_config.key_file = None
+ mock_config.verify_ssl = True
+ mock_config.connection_pool_maxsize = 4
+ mock_config.timeout_millisec = 5000
+ mock_config.proxy = None
+
+ RESTClientObject(configuration=mock_config)
+
+ # Verify SSL context was created with CA file
+ mock_create_context.assert_called_once_with(cafile="/path/to/ca.pem")
+
+ # Verify SSL context was passed to PoolManager
+ mock_pool_manager.assert_called_once()
+ call_kwargs = mock_pool_manager.call_args[1]
+ assert call_kwargs['ssl_context'] == mock_ssl_context
+
+
+@patch('ssl.create_default_context')
+@patch('urllib3.PoolManager')
+def test_ssl_context_loads_client_certificate(mock_pool_manager, mock_create_context):
+ """Test that SSL context loads client certificate and key when provided."""
+ mock_ssl_context = MagicMock()
+ mock_create_context.return_value = mock_ssl_context
+
+ mock_config = MagicMock()
+ mock_config.ssl_ca_cert = None
+ mock_config.cert_file = "/path/to/client.pem"
+ mock_config.key_file = "/path/to/client.key"
+ mock_config.verify_ssl = True
+ mock_config.connection_pool_maxsize = 4
+ mock_config.timeout_millisec = 5000
+ mock_config.proxy = None
+
+ RESTClientObject(configuration=mock_config)
+
+ # Verify SSL context was created
+ mock_create_context.assert_called_once_with(cafile=None)
+
+ # Verify client certificate was loaded
+ mock_ssl_context.load_cert_chain.assert_called_once_with(
+ "/path/to/client.pem", keyfile="/path/to/client.key"
+ )
+
+ # Verify SSL context was passed to PoolManager
+ mock_pool_manager.assert_called_once()
+ call_kwargs = mock_pool_manager.call_args[1]
+ assert call_kwargs['ssl_context'] == mock_ssl_context
+
+
+@patch('ssl.create_default_context')
+@patch('urllib3.PoolManager')
+def test_ssl_context_disables_verification_when_verify_ssl_false(mock_pool_manager, mock_create_context):
+ """Test that SSL context disables verification when verify_ssl=False."""
+ mock_ssl_context = MagicMock()
+ mock_create_context.return_value = mock_ssl_context
+
+ mock_config = MagicMock()
+ mock_config.ssl_ca_cert = None
+ mock_config.cert_file = None
+ mock_config.key_file = None
+ mock_config.verify_ssl = False
+ mock_config.connection_pool_maxsize = 4
+ mock_config.timeout_millisec = 5000
+ mock_config.proxy = None
+
+ RESTClientObject(configuration=mock_config)
+
+ # Verify SSL context was created
+ mock_create_context.assert_called_once_with(cafile=None)
+
+ # Verify SSL verification was disabled
+ assert mock_ssl_context.check_hostname is False
+ assert mock_ssl_context.verify_mode == ssl.CERT_NONE
+
+ # Verify SSL context was passed to PoolManager
+ mock_pool_manager.assert_called_once()
+ call_kwargs = mock_pool_manager.call_args[1]
+ assert call_kwargs['ssl_context'] == mock_ssl_context
+
+
+@patch('ssl.create_default_context')
+@patch('urllib3.ProxyManager')
+def test_ssl_context_used_with_proxy_manager(mock_proxy_manager, mock_create_context):
+ """Test that SSL context is passed to ProxyManager when proxy is configured."""
+ mock_ssl_context = MagicMock()
+ mock_create_context.return_value = mock_ssl_context
+
+ mock_config = MagicMock()
+ mock_config.ssl_ca_cert = "/path/to/ca.pem"
+ mock_config.cert_file = "/path/to/client.pem"
+ mock_config.key_file = "/path/to/client.key"
+ mock_config.verify_ssl = True
+ mock_config.connection_pool_maxsize = 4
+ mock_config.timeout_millisec = 5000
+ mock_config.proxy = "http://proxy:8080"
+ mock_config.proxy_headers = {"Proxy-Auth": "token"}
+
+ RESTClientObject(configuration=mock_config)
+
+ # Verify SSL context was created with CA file
+ mock_create_context.assert_called_once_with(cafile="/path/to/ca.pem")
+
+ # Verify client certificate was loaded
+ mock_ssl_context.load_cert_chain.assert_called_once_with(
+ "/path/to/client.pem", keyfile="/path/to/client.key"
+ )
+
+ # Verify SSL context was passed to ProxyManager
+ mock_proxy_manager.assert_called_once()
+ call_kwargs = mock_proxy_manager.call_args[1]
+ assert call_kwargs['ssl_context'] == mock_ssl_context
+ assert call_kwargs['proxy_url'] == "http://proxy:8080"
+ assert call_kwargs['proxy_headers'] == {"Proxy-Auth": "token"}
+
+
+@patch('ssl.create_default_context')
+@patch('urllib3.PoolManager')
+def test_ssl_context_reuse_performance_optimization(mock_pool_manager, mock_create_context):
+ """Test that SSL context creation is called only once per client instance."""
+ mock_ssl_context = MagicMock()
+ mock_create_context.return_value = mock_ssl_context
+
+ mock_config = MagicMock()
+ mock_config.ssl_ca_cert = "/path/to/ca.pem"
+ mock_config.cert_file = None
+ mock_config.key_file = None
+ mock_config.verify_ssl = True
+ mock_config.connection_pool_maxsize = 4
+ mock_config.timeout_millisec = 5000
+ mock_config.proxy = None
+
+ # Create client instance
+ client = RESTClientObject(configuration=mock_config)
+
+ # Verify SSL context was created exactly once
+ mock_create_context.assert_called_once_with(cafile="/path/to/ca.pem")
+
+ # Verify the same SSL context instance is reused
+ mock_pool_manager.assert_called_once()
+ call_kwargs = mock_pool_manager.call_args[1]
+ assert call_kwargs['ssl_context'] is mock_ssl_context
+
+ # Verify context was not created again during subsequent operations
+ mock_create_context.reset_mock()
+
+ # Build a request (this should not trigger SSL context creation)
+ client.build_request("GET", "https://example.com")
+
+ # SSL context should not be created again
+ mock_create_context.assert_not_called()
+
+
+@patch('ssl.create_default_context')
+@patch('urllib3.PoolManager')
+def test_ssl_context_with_all_ssl_options(mock_pool_manager, mock_create_context):
+ """Test SSL context creation with all SSL configuration options set."""
+ mock_ssl_context = MagicMock()
+ mock_create_context.return_value = mock_ssl_context
+
+ mock_config = MagicMock()
+ mock_config.ssl_ca_cert = "/path/to/ca.pem"
+ mock_config.cert_file = "/path/to/client.pem"
+ mock_config.key_file = "/path/to/client.key"
+ mock_config.verify_ssl = True
+ mock_config.connection_pool_maxsize = 8
+ mock_config.timeout_millisec = 10000
+ mock_config.proxy = None
+
+ RESTClientObject(configuration=mock_config)
+
+ # Verify SSL context was created with CA file
+ mock_create_context.assert_called_once_with(cafile="/path/to/ca.pem")
+
+ # Verify client certificate was loaded
+ mock_ssl_context.load_cert_chain.assert_called_once_with(
+ "/path/to/client.pem", keyfile="/path/to/client.key"
+ )
+
+ # Verify SSL verification settings were NOT modified (verify_ssl=True)
+ # check_hostname and verify_mode should remain at their default secure values
+ assert not hasattr(mock_ssl_context, 'check_hostname') or mock_ssl_context.check_hostname
+ assert not hasattr(mock_ssl_context, 'verify_mode') or mock_ssl_context.verify_mode != ssl.CERT_NONE
+
+ # Verify SSL context was passed to PoolManager
+ mock_pool_manager.assert_called_once()
+ call_kwargs = mock_pool_manager.call_args[1]
+ assert call_kwargs['ssl_context'] == mock_ssl_context
+ assert call_kwargs['maxsize'] == 8