diff --git a/python/tank_vendor/__init__.py b/python/tank_vendor/__init__.py index 47bf78756..6ab8f1f14 100644 --- a/python/tank_vendor/__init__.py +++ b/python/tank_vendor/__init__.py @@ -44,6 +44,7 @@ Supported Python versions: 3.9+ """ +import os import pathlib import sys import warnings @@ -199,6 +200,12 @@ def _patched_get_certs_file(ca_certs=None): # If ca_certs explicitly provided, use original behavior if ca_certs is not None: return _original_get_certs_file(ca_certs) + # Preserve shotgun_api3's documented SHOTGUN_API_CACERTS override. + # Without this branch, a network-hosted extracted cert_file can be + # dramatically slower to open than a local override (see SG-44256). + environment_certs = os.environ.get("SHOTGUN_API_CACERTS") + if environment_certs: + return environment_certs # Otherwise return extracted certificate path instead of ZIP path return str(cert_file) diff --git a/tests/core_tests/test_tank_vendor.py b/tests/core_tests/test_tank_vendor.py index 477066ea9..508fd76df 100644 --- a/tests/core_tests/test_tank_vendor.py +++ b/tests/core_tests/test_tank_vendor.py @@ -9,7 +9,11 @@ # not expressly granted therein are reserved by Shotgun Software Inc. import importlib +import os +import pathlib +import shutil import sys +import tempfile import unittest from unittest import mock @@ -265,6 +269,114 @@ def test_cert_file_returns_path(self): self.assertTrue(len(cert_path) > 0) +class TestShotgunAPI3CertPatchPrecedence(ShotgunTestBase): + """ + Test SHOTGUN_API_CACERTS precedence in the shotgun_api3 certs patch (SG-44256). + + Prior to this fix, _patch_shotgun_api3_certs() unconditionally returned the + Core-extracted cacert.pem whenever ca_certs wasn't explicitly passed, silently + ignoring the documented SHOTGUN_API_CACERTS environment variable. When the + extracted cert lives on a slow network share, this made every cold ShotGrid + connection pay a large one-time filesystem cost that a local cert copy avoids. + + These tests call _patch_shotgun_api3_certs() directly against a throwaway + fake "pkgs.zip" layout so they can control both the extracted cert file and + the environment variable without depending on the real build-time layout. + """ + + def setUp(self): + super().setUp() + + import shotgun_api3 + + from tank_vendor import _patch_shotgun_api3_certs + + self._shotgun_api3 = shotgun_api3 + self._patch_shotgun_api3_certs = _patch_shotgun_api3_certs + + # Snapshot the raw descriptor currently installed (already patched + # once at module import time) so we can restore it after each test. + # Reading via __dict__ (not attribute access) preserves the + # staticmethod wrapper itself, rather than the plain function it + # unwraps to -- restoring the unwrapped function directly would + # turn _get_certs_file into a bound instance method for every + # Shotgun() call afterward, breaking unrelated tests with a + # "takes 0 to 1 positional arguments but 2 were given" TypeError. + self._original_get_certs_file = shotgun_api3.Shotgun.__dict__[ + "_get_certs_file" + ] + + # Build a fake pkgs.zip layout with an extracted cert file beside it: + # /pkgs.zip + # /certs/shotgun_api3/lib/certifi/cacert.pem + self._tmp_dir = tempfile.mkdtemp() + cert_dir = ( + pathlib.Path(self._tmp_dir) / "certs" / "shotgun_api3" / "lib" / "certifi" + ) + cert_dir.mkdir(parents=True) + self._extracted_cert_file = cert_dir / "cacert.pem" + self._extracted_cert_file.write_text("fake extracted core cert") + self._fake_zip_path = pathlib.Path(self._tmp_dir) / "pkgs.zip" + + self._env_patcher = mock.patch.dict(os.environ, {}, clear=False) + self._env_patcher.start() + os.environ.pop("SHOTGUN_API_CACERTS", None) + + def tearDown(self): + self._env_patcher.stop() + self._shotgun_api3.Shotgun._get_certs_file = self._original_get_certs_file + shutil.rmtree(self._tmp_dir, ignore_errors=True) + super().tearDown() + + def test_environment_override_takes_precedence_over_extracted_cert(self): + """SHOTGUN_API_CACERTS should win over the Core-extracted cert.""" + local_cert = os.path.join(self._tmp_dir, "local-cacert.pem") + os.environ["SHOTGUN_API_CACERTS"] = local_cert + + self._patch_shotgun_api3_certs(self._fake_zip_path) + + self.assertEqual( + self._shotgun_api3.Shotgun._get_certs_file(), + local_cert, + ) + + def test_falls_back_to_extracted_cert_when_env_var_unset(self): + """With no override set, behavior is unchanged: use the extracted cert.""" + self.assertNotIn("SHOTGUN_API_CACERTS", os.environ) + + self._patch_shotgun_api3_certs(self._fake_zip_path) + + self.assertEqual( + self._shotgun_api3.Shotgun._get_certs_file(), + str(self._extracted_cert_file), + ) + + def test_falls_back_to_extracted_cert_when_env_var_empty(self): + """An empty SHOTGUN_API_CACERTS is treated the same as unset.""" + os.environ["SHOTGUN_API_CACERTS"] = "" + + self._patch_shotgun_api3_certs(self._fake_zip_path) + + self.assertEqual( + self._shotgun_api3.Shotgun._get_certs_file(), + str(self._extracted_cert_file), + ) + + def test_explicit_ca_certs_argument_still_takes_top_precedence(self): + """An explicit ca_certs argument outranks both the env var and the extracted cert.""" + os.environ["SHOTGUN_API_CACERTS"] = os.path.join( + self._tmp_dir, "local-cacert.pem" + ) + + self._patch_shotgun_api3_certs(self._fake_zip_path) + + explicit_cert = "/some/explicit/cert.pem" + self.assertEqual( + self._shotgun_api3.Shotgun._get_certs_file(explicit_cert), + explicit_cert, + ) + + @unittest.skipIf( sys.version_info < (3, 10), "Flow Data SDK requires Python 3.10+ (uses types.UnionType / typing.TypeAlias)",