cephadm: simplify loading jinja2 templates from zipapp

The saga of failures in teuthology brought on by the combination of
zipapp and jinja2 template loading continues in this episode. Remove
some of the path handling that was taken from the original jinja2
implementation and try to replace it with something inherently simpler
such that we always construct paths relative to the zip archive and pass
that to the zipimporter. The hope here is that by doing fewer
manipulations of the "path" we are more likely to match the items
tracked inside the zipimporter class.

We do not worry about some of the concerns that regular jinja2 has as we
never expect cephadm to be run on something other than linux and we have
control over what sources we expect to load templates from.  We do take
one precaution and that is to reject any paths that contain "." and ".."
as components. Less to avoid malicious attempts to read files it should
not (but that would be nice) and more to nudge toward simple template
references recorded in our Templates enum.

Signed-off-by: John Mulligan <jmulligan@redhat.com>

diff --git a/src/cephadm/cephadmlib/templating.py b/src/cephadm/cephadmlib/templating.py
index ceef32ff9fedd64a9266be8537e7d45ec61b43dc..3e852b2d397de96c3c8f01829c86ab85a50d1ba2 100644 (file)
--- a/src/cephadm/cephadmlib/templating.py
+++ b/src/cephadm/cephadmlib/templating.py
@@ -34,27 +34,18 @@ class TemplateNotFoundInZipApp(jinja2.TemplateNotFound):
         self,
         template: str,
         *,
-        path: str = '',
         relative_path: str = '',
-        archive_norm_path: str = '',
-        archive_path: str = ''
+        archive_path: str = '',
     ) -> None:
         super().__init__(template)
-        self.path = path
         self.relative_path = relative_path
-        self.archive_norm_path = archive_norm_path
         self.archive_path = archive_path
 
     def __str__(self) -> str:
-        msg = self.message
-        msg2 = ''
-        if self.path or self.relative_path:
-            msg2 += f' path [{self.path!r}, rel={self.relative_path!r}] not found'
-        if self.archive_norm_path or self.archive_path:
-            msg2 += f' in [{self.archive_norm_path!r}, orig={self.archive_path!r}]'
-        if msg2:
-            msg2 = ':' + msg2
-        return f'{msg}{msg2}'
+        return (
+            f'{self.message}: path {self.relative_path!r}'
+            f' not found in {self.archive_path!r}'
+        )
 
 
 class _PackageLoader(jinja2.PackageLoader):
@@ -86,27 +77,23 @@ class _PackageLoader(jinja2.PackageLoader):
 
     def _get_archive_source(self, template: str) -> Tuple[str, str, None]:
         assert isinstance(self._loader, zipimport.zipimporter)
-        path = arelpath = os.path.normpath(
-            posixpath.join(
-                self._template_root,
-                *jinja2.loaders.split_template_path(template)
-            )
+        arelpath = posixpath.join(
+            self.package_name, self.package_path, template
         )
-        archive_path = os.path.normpath(self._loader.archive)
-        if arelpath.startswith(archive_path + '/'):
-            plen = len(archive_path) + 1
-            arelpath = arelpath[plen:]
+        if any(p == '.' or p == '..' for p in arelpath.split(posixpath.sep)):
+            raise ValueError('template path contains invalid components')
         try:
             source = cast(bytes, self._loader.get_data(arelpath))
         except OSError as e:
             not_found = TemplateNotFoundInZipApp(
                 template,
-                path=path,
                 relative_path=arelpath,
-                archive_norm_path=archive_path,
                 archive_path=self._loader.archive,
             )
             raise not_found from e
+        path = os.path.normpath(
+            posixpath.join(self._loader.archive, arelpath)
+        )
         return source.decode(self.encoding), path, None