Fix Jinja template cache loading

changelog/62042.fixed

@@ -0,0 +1 @@
+Fix cache checking for Jinja templates

salt/utils/jinja.py

@@ -103,10 +103,18 @@ def file_client(self):
         # If there was no file_client passed to the class, create a cache_client
         # and use that. This avoids opening a new file_client every time this
         # class is instantiated
-        if self._file_client is None:
+        if (
+            self._file_client is None
+            or not hasattr(self._file_client, "opts")
+            or self._file_client.opts["file_roots"] != self.opts["file_roots"]
+        ):
             attr = "_cached_pillar_client" if self.pillar_rend else "_cached_client"
             cached_client = getattr(self, attr, None)
-            if cached_client is None:
+            if (
+                cached_client is None
+                or not hasattr(cached_client, "opts")
+                or cached_client.opts["file_roots"] != self.opts["file_roots"]
+            ):
                 cached_client = salt.fileclient.get_file_client(
                     self.opts, self.pillar_rend
                 )
@@ -119,15 +127,17 @@ def cache_file(self, template):
         Cache a file from the salt master
         """
         saltpath = salt.utils.url.create(template)
-        self.file_client().get_file(saltpath, "", True, self.saltenv)
+        fcl = self.file_client()
+        return fcl.get_file(saltpath, "", True, self.saltenv)
 
     def check_cache(self, template):
         """
         Cache a file only once
         """
         if template not in self.cached:
-            self.cache_file(template)
-            self.cached.append(template)
+            ret = self.cache_file(template)
+            if ret is not False:
+                self.cached.append(template)
 
     def get_source(self, environment, template):
         """
@@ -165,6 +175,9 @@ def get_source(self, environment, template):
                     template,
                 )
                 raise TemplateNotFound(template)
+            # local file clients should pass the dot-expanded relative path
+            if environment.globals.get("opts", {}).get("file_client") == "local":
+                _template = os.path.relpath(_template, base_path)
 
         self.check_cache(_template)
 
@@ -181,25 +194,26 @@ def get_source(self, environment, template):
             }
             environment.globals.update(tpldata)
 
-        # pylint: disable=cell-var-from-loop
-        for spath in self.searchpath:
-            filepath = os.path.join(spath, _template)
-            try:
-                with salt.utils.files.fopen(filepath, "rb") as ifile:
-                    contents = ifile.read().decode(self.encoding)
-                    mtime = os.path.getmtime(filepath)
-
-                    def uptodate():
-                        try:
-                            return os.path.getmtime(filepath) == mtime
-                        except OSError:
-                            return False
-
-                    return contents, filepath, uptodate
-            except OSError:
-                # there is no file under current path
-                continue
-        # pylint: enable=cell-var-from-loop
+        if _template in self.cached:
+            # pylint: disable=cell-var-from-loop
+            for spath in self.searchpath:
+                filepath = os.path.join(spath, _template)
+                try:
+                    with salt.utils.files.fopen(filepath, "rb") as ifile:
+                        contents = ifile.read().decode(self.encoding)
+                        mtime = os.path.getmtime(filepath)
+
+                        def uptodate():
+                            try:
+                                return os.path.getmtime(filepath) == mtime
+                            except OSError:
+                                return False
+
+                        return contents, filepath, uptodate
+                except OSError:
+                    # there is no file under current path
+                    continue
+            # pylint: enable=cell-var-from-loop
 
         # there is no template file within searchpaths
         raise TemplateNotFound(template)

tests/pytests/functional/utils/test_jinja.py

@@ -0,0 +1,56 @@
+import pytest
+import salt.utils.jinja
+from jinja2.exceptions import TemplateNotFound
+from salt.utils.odict import OrderedDict
+
+
+def test_utils_jinja_cache_removed_file_from_root(temp_salt_minion, tmp_path):
+    """
+    this tests for a condition where an included jinja template
+    is removed from the salt filesystem, but is still loaded from
+    the cache.
+    """
+    opts = temp_salt_minion.config.copy()
+    file_root = tmp_path / "root"
+    file_root.mkdir(parents=True, exist_ok=True)
+    cache_root = tmp_path / "cache"
+    cache_root.mkdir(parents=True, exist_ok=True)
+    filename = "jinja_cache"
+    sls_file = file_root / "{}.sls".format(filename)
+    jinja_file = file_root / "{}.jinja".format(filename)
+    sls_file.write_text("{% include '" + filename + ".jinja' %}")
+    jinja_file.write_text("{% set this = 'that' %}")
+
+    # Stop using OrderedDict once we drop Py3.5 support
+    opts["file_roots"] = OrderedDict()
+    opts["file_roots"]["base"] = [str(file_root)]
+    opts["cachedir"] = str(cache_root)
+    opts["master_type"] = "disable"
+    opts["file_client"] = "local"
+
+    loader = salt.utils.jinja.SaltCacheLoader(
+        opts,
+        "base",
+    )
+    # sls file is in the root
+    loader.get_source(None, sls_file.name)
+    # jinja file is in the root
+    loader.get_source(None, jinja_file.name)
+    # both files cached
+    assert len(loader.cached) == 2
+
+    # remove the jinja file and reset loader
+    jinja_file.unlink()
+    loader = salt.utils.jinja.SaltCacheLoader(
+        opts,
+        "base",
+    )
+    # sls file is still in the root
+    loader.get_source(None, sls_file.name)
+    # jinja file is gone from the root
+    with pytest.raises(TemplateNotFound):
+        loader.get_source(None, jinja_file.name)
+    # only one was cached this run
+    assert len(loader.cached) == 1
+    # the cached jinja file is still present, but not used
+    assert (cache_root / "files" / "base" / jinja_file.name).exists()

tests/pytests/unit/utils/jinja/conftest.py

@@ -13,6 +13,7 @@ def __init__(self, loader=None):
         if loader:
             loader._file_client = self
         self.requests = []
+        self.opts = {}
 
     def get_file(self, template, dest="", makedirs=False, saltenv="base"):
         self.requests.append(

tests/pytests/unit/utils/jinja/test_salt_cache_loader.py

@@ -110,6 +110,7 @@ def run_command(opts=None, saltenv="base", **kwargs):
         """
         if opts is None:
             opts = minion_opts
+        mock_file_client.opts = opts
         loader = SaltCacheLoader(opts, saltenv, _file_client=mock_file_client)
         # Create a mock file client and attach it to the loader
         return loader
@@ -221,6 +222,7 @@ def test_file_client_kwarg(minion_opts, mock_file_client):
     A file client can be passed to SaltCacheLoader overriding the any
     cached file client
     """
+    mock_file_client.opts = minion_opts
     loader = SaltCacheLoader(minion_opts, _file_client=mock_file_client)
     assert loader._file_client is mock_file_client
 
@@ -231,6 +233,7 @@ def test_cache_loader_shutdown(minion_opts, mock_file_client):
     file_client does not have a destroy method
     """
     assert not hasattr(mock_file_client, "destroy")
+    mock_file_client.opts = minion_opts
     loader = SaltCacheLoader(minion_opts, _file_client=mock_file_client)
     assert loader._file_client is mock_file_client
     # Shutdown method should not raise any exceptions