From 75d759066e8ee0a469abc37f48f7bfcdfe8182b5 Mon Sep 17 00:00:00 2001
From: "Bryn M. Reeves" <bmr@redhat.com>
Date: Fri, 31 Aug 2018 12:58:01 +0100
Subject: [archive] replace FileCacheArchive._makedirs()

The Python os.makedirs() implementation is inadequate for sos's
needs: it will create leading directories given an intended path
destination, but it is not able to reflect cases where some of
the intermediate paths are actually symbolic links.

Replace the use of os.makedirs() with a method that walks over
the path, and either creates directories, or symbolic links (and
their directory target) to better correspond with the content of
the host file system.

This fixes a situation where two plugins can race in the archive,
leading to an exception in the plugin that runs last:

 - /foo/bar exists and is a link to /foo/bar.qux
 - One plugin attempts to collect /foo/bar
 - Another plugin attempts to collect a link /foo/qux -> /foo/bar/baz

If the 2nd plugin happens to run first it will create the path
"/foo/bar" as a _directory_ (via _makedirs()). Since the archive
now checks for matching object types when a path collision occurs,
the first plugin will arrive at add_dir(), note that "/foo/bar" is
present and is not a symbolic link, and will raise an exception.

Correct this by ensuring that whichever plugin executes first, the
correct link/directory path structure will be set up.

Signed-off-by: Bryn M. Reeves <bmr@redhat.com>
---
 sos/archive.py | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 64 insertions(+), 8 deletions(-)

diff --git a/sos/archive.py b/sos/archive.py
index 903cc672..11afa7aa 100644
--- a/sos/archive.py
+++ b/sos/archive.py
@@ -159,6 +159,67 @@ class FileCacheArchive(Archive):
             name = name.lstrip(os.sep)
         return (os.path.join(self._archive_root, name))
 
+    def _make_leading_paths(self, src, mode=0o700):
+        """Create leading path components
+
+            The standard python `os.makedirs` is insufficient for our
+            needs: it will only create directories, and ignores the fact
+            that some path components may be symbolic links.
+        """
+        self.log_debug("Making leading paths for %s" % src)
+        root = self._archive_root
+
+        def in_archive(path):
+            """Test whether path ``path`` is inside the archive.
+            """
+            return path.startswith(os.path.join(root, ""))
+
+        if not src.startswith("/"):
+            # Sos archive path (sos_commands, sos_logs etc.)
+            src_dir = src
+        else:
+            # Host file path
+            src_dir = src if os.path.isdir(src) else os.path.split(src)[0]
+
+        # Build a list of path components in root-to-leaf order.
+        path = src_dir
+        path_comps = []
+        while path != '/' and path != '':
+            head, tail = os.path.split(path)
+            path_comps.append(tail)
+            path = head
+        path_comps.reverse()
+
+        abs_path = root
+        rel_path = ""
+
+        # Check and create components as needed
+        for comp in path_comps:
+            abs_path = os.path.join(abs_path, comp)
+
+            if not in_archive(abs_path):
+                continue
+
+            rel_path = os.path.join(rel_path, comp)
+            src_path = os.path.join("/", rel_path)
+
+            if not os.path.exists(abs_path):
+                self.log_debug("Making path %s" % abs_path)
+                if os.path.islink(src_path) and os.path.isdir(src_path):
+                    target = os.readlink(src_path)
+                    abs_target = os.path.join(root, target)
+
+                    # Recursively create leading components of target
+                    self._make_leading_paths(abs_target, mode=mode)
+
+                    self.log_debug("Making symlink '%s' -> '%s'" %
+                                   (abs_path, target))
+                    target = os.path.relpath(target)
+                    os.symlink(target, abs_path)
+                else:
+                    self.log_debug("Making directory %s" % abs_path)
+                    os.mkdir(abs_path, mode)
+
     def _check_path(self, src, path_type, dest=None, force=False):
         """Check a new destination path in the archive.
 
@@ -203,7 +264,8 @@ class FileCacheArchive(Archive):
             raise ValueError("path '%s' exists and is not a directory" %
                              dest_dir)
         elif not os.path.exists(dest_dir):
-            self._makedirs(dest_dir)
+            src_dir = src if path_type == P_DIR else os.path.split(src)[0]
+            self._make_leading_paths(src_dir)
 
         def is_special(mode):
             return any([
@@ -326,10 +388,7 @@ class FileCacheArchive(Archive):
 
     def add_dir(self, path):
         with self._path_lock:
-            dest = self._check_path(path, P_DIR)
-            if not dest:
-                return
-            self.makedirs(path)
+            self._check_path(path, P_DIR)
 
     def add_node(self, path, mode, device):
         dest = self._check_path(path, P_NODE)
@@ -347,9 +406,6 @@ class FileCacheArchive(Archive):
                 raise e
             shutil.copystat(path, dest)
 
-    def _makedirs(self, path, mode=0o700):
-        os.makedirs(path, mode)
-
     def name_max(self):
         if 'PC_NAME_MAX' in os.pathconf_names:
             pc_name_max = os.pathconf_names['PC_NAME_MAX']
-- 
cgit