fix(cp): fall back to __opts__ when __file_client__ is not packed

changelog/69734.fixed.md

@@ -0,0 +1 @@
+Fixed ``cp._client`` raising ``LoaderError`` (surfaced as ``KeyError: '__file_client__'``) when the executing loader has not packed a ``__file_client__`` context. It now falls back to building a file client from ``__opts__``, so ``cp.cache_file`` and other ``salt://`` fetches work under loaders that do not pack a file client.

salt/modules/cp.py

@@ -20,7 +20,7 @@
 import salt.utils.path
 import salt.utils.templates
 import salt.utils.url
-from salt.exceptions import CommandExecutionError
+from salt.exceptions import CommandExecutionError, LoaderError
 from salt.loader.context import NamedLoaderContext
 from salt.loader.dunder import (
     __context__,
@@ -173,8 +173,20 @@ def _client():
     If the __file_client__ context is set return it, otherwize create a new
     file client using __opts__.
     """
-    if __file_client__:
-        return __file_client__.value()
+    # ``__file_client__`` is a NamedLoaderContext. When the loader executing
+    # this module has not packed a file client (e.g. loaders other than
+    # minion_mods, such as the resource module loader), evaluating the context
+    # raises ``LoaderError`` instead of yielding a falsey value -- so the
+    # ``__opts__`` fallback below was never reached and callers such as
+    # ``cp.cache_file`` (and therefore every ``salt://`` fetch) failed with
+    # ``KeyError: '__file_client__'``. Guard the lookup so a missing/None
+    # context falls back to building a client from ``__opts__``.
+    try:
+        file_client = __file_client__.value()
+    except LoaderError:
+        file_client = None
+    if file_client:
+        return file_client
     return salt.fileclient.get_file_client(__opts__.value())
 
 

tests/pytests/unit/modules/test_cp.py

@@ -9,7 +9,7 @@
 import salt.utils.files
 import salt.utils.platform
 import salt.utils.templates as templates
-from salt.exceptions import CommandExecutionError
+from salt.exceptions import CommandExecutionError, LoaderError
 from tests.support.mock import MagicMock, Mock, mock_open, patch
 
 
@@ -18,6 +18,41 @@ def configure_loader_modules():
     return {cp: {"__opts__": {"saltenv": None}}}
 
 
+def test__client_returns_packed_file_client():
+    """
+    _client() returns the file client from the __file_client__ context when
+    one is packed.
+    """
+    packed_client = Mock()
+    ctx = MagicMock()
+    ctx.value.return_value = packed_client
+    with patch.object(cp, "__file_client__", ctx, create=True):
+        with patch("salt.fileclient.get_file_client") as get_file_client:
+            assert cp._client() is packed_client
+            get_file_client.assert_not_called()
+
+
+def test__client_falls_back_when_file_client_not_packed():
+    """
+    When the executing loader has not packed __file_client__, evaluating the
+    context raises LoaderError. _client() must fall back to building a client
+    from __opts__ instead of propagating the error.
+    """
+    opts = {"saltenv": None}
+    opts_ctx = MagicMock()
+    opts_ctx.value.return_value = opts
+    file_client_ctx = MagicMock()
+    file_client_ctx.value.side_effect = LoaderError("__file_client__ not packed")
+    built_client = Mock()
+    with patch.object(cp, "__file_client__", file_client_ctx, create=True):
+        with patch.object(cp, "__opts__", opts_ctx, create=True):
+            with patch(
+                "salt.fileclient.get_file_client", return_value=built_client
+            ) as get_file_client:
+                assert cp._client() is built_client
+                get_file_client.assert_called_once_with(opts)
+
+
 def test__render_filenames_undefined_template():
     """
     Test if _render_filenames fails upon getting a template not in