fix(openshift): key the reachable cache on the client, not a dropped method

src/uds/services/OpenShift/openshift/client.py

@@ -102,12 +102,11 @@ def cache_key(self) -> str:
     def get_token(self) -> str | None:
         try:
             url = f"{self._cluster_url}/oauth/authorize?client_id=openshift-challenging-client&response_type=token"
-            r = requests.get(
+            r = security.secure_requests_session(verify=self._verify_ssl).get(
                 url,
                 auth=(self._username, self._password),
                 timeout=15,
                 allow_redirects=False,
-                verify=self._verify_ssl,
             )
             if r.status_code not in (301, 302, 303, 307, 308):
                 raise exceptions.OpenshiftAuthError(

src/uds/services/OpenShift/provider.py

@@ -52,6 +52,13 @@
 logger = logging.getLogger(__name__)
 
 
+def cache_key_helper(self: "OpenshiftProvider") -> str:
+    """
+    Helper function to generate cache keys for the OpenshiftProvider class
+    """
+    return self.api.cache_key()
+
+
 class OpenshiftProvider(ServiceProvider):
     offers = [OpenshiftService, OpenshiftServiceFixed]
     type_name = _("Openshift Provider")
@@ -131,7 +138,7 @@ def api(self) -> "client.OpenshiftClient":
     def test_connection(self) -> bool:
         return self.api.test()
 
-    @cached("reachable", consts.cache.SHORT_CACHE_TIMEOUT, key_helper=lambda x: x.connection_key())
+    @cached("reachable", consts.cache.SHORT_CACHE_TIMEOUT, key_helper=cache_key_helper)
     def is_available(self) -> bool:
         return self.api.test()
 

tests/services/openshift/fixtures.py

@@ -142,8 +142,7 @@ def inner(*args: typing.Any, **kwargs: typing.Any) -> T:
 # Connection identity matching PROVIDER_VALUES_DICT, as OpenshiftClient.cache_key() would build it
 CLIENT_CACHE_KEY = (
     f'{PROVIDER_VALUES_DICT["cluster_url"]}|{PROVIDER_VALUES_DICT["api_url"]}|'
-    f'{PROVIDER_VALUES_DICT["username"]}|{PROVIDER_VALUES_DICT["namespace"]}|'
-    f'{PROVIDER_VALUES_DICT["verify_ssl"]}'
+    f'{PROVIDER_VALUES_DICT["username"]}|{PROVIDER_VALUES_DICT["namespace"]}'
 )
 
 # Service values

tests/services/openshift/test_client.py

@@ -42,6 +42,8 @@
 
 logger = logging.getLogger(__name__)
 
+SECURE_SESSION = "uds.services.OpenShift.openshift.client.security.secure_requests_session"
+
 
 class TestOpenshiftClient(UDSTransactionTestCase):
     """Tests for operations with OpenShiftClient."""
@@ -196,29 +198,24 @@ def _redirect_response(self, token: str = "a-token") -> mock.Mock:
 
     def test_get_token_from_redirect_fragment(self) -> None:
         client = self._client()
-        with mock.patch("requests.get", return_value=self._redirect_response()) as requests_get:
+        with mock.patch(SECURE_SESSION) as secure_session:
+            secure_session.return_value.get.return_value = self._redirect_response()
             self.assertEqual(client.get_token(), "a-token")
             # verify_ssl must be honored, not hardcoded to False
-            self.assertFalse(requests_get.call_args.kwargs["verify"])
-            self.assertFalse(requests_get.call_args.kwargs["allow_redirects"])
+            self.assertFalse(secure_session.call_args.kwargs["verify"])
+            self.assertFalse(secure_session.return_value.get.call_args.kwargs["allow_redirects"])
 
     def test_get_token_on_non_redirect_raises_auth_error(self) -> None:
         response = mock.Mock()
         response.status_code = 401
         response.headers = {}
         client = self._client()
-        with mock.patch("requests.get", return_value=response):
+        with mock.patch(SECURE_SESSION) as secure_session:
+            secure_session.return_value.get.return_value = response
             with self.assertRaises(openshift_exceptions.OpenshiftAuthError):
                 client.get_token()
 
-    def test_connect_reuses_session_and_token(self) -> None:
-        """``connect()`` rebuilds the requests.Session and re-fetches the token
-        every call (the comment in ``OpenshiftClient.connect`` notes that the
-        class is short-lived and the session is ALWAY cleaned soon). The only
-        way to truly reuse a session is to skip ``connect()`` and rely on
-        ``do_request`` setting ``self._session = None`` on a 401 to force the
-        next ``connect()`` to start over.
-        """
+    def test_session_reconnects_on_every_access(self) -> None:
         client = self._client()
         with mock.patch.object(client, "get_token", return_value="a-token") as get_token:
             first = client.session

tests/services/openshift/test_provider.py

@@ -107,16 +107,8 @@ def test_provider_test(self) -> None:
     def test_provider_is_available(self) -> None:
         """
         Test the provider is_available method and cache behavior.
-
-        The OpenshiftProvider identity is stable once the provider is loaded
-        (``connection_key`` is intentionally absent), so the @cached decorator
-        keys purely on the method name. We patch ``connection_key`` on the
-        instance to provide a stable identity without reintroducing a method.
         """
         with fixtures.patched_provider() as provider:
-            # Provide the identity the @cached key_helper expects, without
-            # adding the method back to the provider itself.
-            provider.connection_key = lambda: "test-cache-key"  # type: ignore[attr-defined, unused-ignore]
             api = typing.cast(mock.MagicMock, provider.api)
             # First, true result
             self.assertEqual(provider.is_available(), True)
@@ -131,6 +123,21 @@ def test_provider_is_available(self) -> None:
             self.assertEqual(provider.is_available(), False)
             api.test.assert_called_once_with()
 
+    def test_provider_is_available_keyed_on_connection_identity(self) -> None:
+        """
+        The cached entry belongs to the connection identity, not to the method name: a provider
+        pointing to another cluster must not read the previous one's answer.
+        """
+        with fixtures.patched_provider() as provider:
+            api = typing.cast(mock.MagicMock, provider.api)
+            self.assertEqual(provider.is_available(), True)
+            api.test.reset_mock()
+
+            api.cache_key.return_value = 'https://other-cluster.example.com|https://other-api:6443|kubeadmin|default'
+            api.test.return_value = False
+            self.assertEqual(provider.is_available(), False)
+            api.test.assert_called_once_with()
+
     # --- Provider API Methods ---
     def test_provider_api_methods(self) -> None:
         """
@@ -168,26 +175,21 @@ def test_initialize_resets_cached_api(self) -> None:
         provider.initialize({})
         self.assertIsNone(provider._cached_api)
 
-    def test_api_recreates_client_when_config_changed(self) -> None:
+    def test_api_keeps_cached_client_when_config_changed(self) -> None:
         """
-        The api property does NOT re-check the cached client against the
-        current configuration: ``_cached_api`` is reused verbatim until
-        :py:meth:`initialize` clears it. The provider's identity is
-        considered stable for the lifetime of the loaded provider.
+        api property does not inspect the connection params: only initialize() drops the cached
+        client, which is what runs when the configuration changes.
         """
         provider = fixtures.create_provider()
         old_client = fixtures.create_client_mock()
         old_client.cache_key.return_value = (
-            "https://old-cluster.example.com|https://old-api.example.com:6443|kubeadmin|default|False"
+            "https://old-cluster.example.com|https://old-api.example.com:6443|kubeadmin|default"
         )
         provider._cached_api = old_client
 
         with mock.patch("uds.services.OpenShift.provider.client.OpenshiftClient") as MockClient:
-            result = provider.api
-            # The cached client is reused, no new OpenshiftClient is built,
-            # even though the cache_key differs from the current config.
+            self.assertIs(provider.api, old_client)
             MockClient.assert_not_called()
-            self.assertIs(result, old_client)
 
     def test_api_reuses_client_when_config_unchanged(self) -> None:
         """

tests/services/openshift/test_service.py

@@ -104,13 +104,8 @@ def test_service_methods(self) -> None:
     def test_service_is_available(self) -> None:
         """
         Check service availability and cache handling.
-
-        The provider identity is stable once loaded, so we patch
-        ``connection_key`` on the instance to feed the @cached decorator's
-        key_helper without reintroducing the method on the provider class.
         """
         service, provider, provider_ctx = self._create_service_with_provider()
-        provider.connection_key = lambda: "test-cache-key"  # type: ignore[attr-defined, unused-ignore]
         api = typing.cast(mock.MagicMock, provider.api)
         self.assertTrue(service.is_available())
         api.test.assert_called_with()

tests/services/openshift/test_service_fixed.py

@@ -64,13 +64,8 @@ def setUp(self) -> None:
     def test_service_is_available(self) -> None:
         """
         Test provider availability and cache logic.
-
-        The provider identity is stable once loaded, so we patch
-        ``connection_key`` on the instance to feed the @cached decorator's
-        key_helper without reintroducing the method on the provider class.
         """
         service, provider, provider_ctx = self._create_service_fixed_with_provider()
-        provider.connection_key = lambda: "test-cache-key"  # type: ignore[attr-defined, unused-ignore]
         api = typing.cast(mock.MagicMock, provider.api)
         self.assertTrue(service.is_available())
         api.test.assert_called_with()