From 1dc3625fabea7331570f713fd1c87ac812d72d92 Mon Sep 17 00:00:00 2001

From: Jake Hunsaker <jhunsake@redhat.com>

Date: Wed, 18 May 2022 13:39:38 -0400

Subject: [PATCH] [Plugin] Make forbidden path checks more efficient

Forbidden path checks have up until now worked by taking a given file

path (potentially with globs), expanding that against all discovered

files that actually exist on the system, and then comparing a potential

collection path against that list.

While this works, and works reasonably fast for most scenarios, it isn't

very efficient and causes significant slow downs when a non-standard

configuration is in play - e.g. thousands of block devices which sos

would individually have to compare against tens of thousands of paths

for every path the `block` plugin wants to collect.

Improve this by first not expanding the forbidden path globs, but taking

them as distinct patterns, translating from shell-style (to maintain

historical precedent of using globs to specify paths to be skipped) to

python regex patterns as needed. Second, use `re` to handle our pattern

matching for comparison against the distinct patterns provided by a

plugin to skip.

Closes: #2938

Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>

---

 sos/report/plugins/__init__.py | 20 +++++++++-----------

 sos/report/plugins/cgroups.py  |  6 ++----

 sos/report/plugins/pulpcore.py |  2 +-

 sos/report/plugins/rhui.py     |  2 +-

 4 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/sos/report/plugins/__init__.py b/sos/report/plugins/__init__.py

index 2a42e6b0a..ba1397a8a 100644

--- a/sos/report/plugins/__init__.py

+++ b/sos/report/plugins/__init__.py

@@ -46,11 +46,6 @@ def _mangle_command(command, name_max):

     return mangledname

-def _path_in_path_list(path, path_list):

-    return any((p == path or path.startswith(os.path.abspath(p)+os.sep)

-                for p in path_list))

-

-

 def _node_type(st):

     """ return a string indicating the type of special node represented by

     the stat buffer st (block, character, fifo, socket).

@@ -1407,7 +1402,9 @@ def _get_dest_for_srcpath(self, srcpath):

         return None

     def _is_forbidden_path(self, path):

-        return _path_in_path_list(path, self.forbidden_paths)

+        return any(

+            re.match(forbid, path) for forbid in self.forbidden_paths

+        )

     def _is_policy_forbidden_path(self, path):

         return any([

@@ -1495,14 +1492,12 @@ def _do_copy_path(self, srcpath, dest=None):

             'symlink': "no"

         })

-    def add_forbidden_path(self, forbidden, recursive=False):

+    def add_forbidden_path(self, forbidden):

         """Specify a path, or list of paths, to not copy, even if it's part of

         an ``add_copy_spec()`` call

         :param forbidden: A filepath to forbid collection from

         :type forbidden: ``str`` or a ``list`` of strings

-

-        :param recursive: Should forbidden glob be applied recursively

         """

         if isinstance(forbidden, str):

             forbidden = [forbidden]

@@ -1512,8 +1507,11 @@ def add_forbidden_path(self, forbidden, recursive=False):

         for forbid in forbidden:

             self._log_info("adding forbidden path '%s'" % forbid)

-            for path in glob.glob(forbid, recursive=recursive):

-                self.forbidden_paths.append(path)

+            if "*" in forbid:

+                # calling translate() here on a dir-level path will break the

+                # re.match() call during path comparison

+                forbid = fnmatch.translate(forbid)

+            self.forbidden_paths.append(forbid)

     def set_option(self, optionname, value):

         """Set the named option to value. Ensure the original type of the

diff --git a/sos/report/plugins/pulpcore.py b/sos/report/plugins/pulpcore.py

index 6c4237cae..f6bc194c7 100644

--- a/sos/report/plugins/pulpcore.py

+++ b/sos/report/plugins/pulpcore.py

@@ -89,7 +89,7 @@ class PulpCore(Plugin, IndependentPlugin

             "/etc/pki/pulp/*"

         ])

         # skip collecting certificate keys

-        self.add_forbidden_path("/etc/pki/pulp/**/*.key", recursive=True)

+        self.add_forbidden_path("/etc/pki/pulp/**/*.key")

         self.add_cmd_output("rq info -u redis://localhost:6379/8",

                             env={"LC_ALL": "en_US.UTF-8"},

diff --git a/sos/report/plugins/rhui.py b/sos/report/plugins/rhui.py

index add024613..8063fd51c 100644

--- a/sos/report/plugins/rhui.py

+++ b/sos/report/plugins/rhui.py

@@ -30,7 +30,7 @@ def setup(self):

             "/var/log/rhui/*",

         ])

         # skip collecting certificate keys

-        self.add_forbidden_path("/etc/pki/rhui/**/*.key", recursive=True)

+        self.add_forbidden_path("/etc/pki/rhui/**/*.key")

         # call rhui-manager commands with 1m timeout and

         # with an env. variable ensuring that "RHUI Username:"