From 9a0ab16793a8388b2c3d3909fd3a087c5b6296d4 Mon Sep 17 00:00:00 2001 From: Pavel Moravec Date: Fri, 1 Nov 2019 12:13:23 -0400 Subject: [PATCH 01/10] [Plugin] remove invalid {strip/join}_sysroot() Do not strip the sysroot path prefix when calling _do_copy_path() for a symlink target and do not add the sysroot prefix when testing for a forbidden path. Related: #1842 Signed-off-by: Pavel Moravec Signed-off-by: Bryn M. Reeves --- sos/plugins/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py index e75ec82e..4f1b73ce 100644 --- a/sos/plugins/__init__.py +++ b/sos/plugins/__init__.py @@ -731,7 +731,7 @@ class Plugin(object): # skip recursive copying of symlink pointing to itself. if (absdest != srcpath): - self._do_copy_path(self.strip_sysroot(absdest)) + self._do_copy_path(absdest) else: self._log_debug("link '%s' points to itself, skipping target..." % linkdest) @@ -758,8 +758,6 @@ class Plugin(object): return None def _is_forbidden_path(self, path): - if self.use_sysroot(): - path = self.join_sysroot(path) return _path_in_path_list(path, self.forbidden_paths) def _copy_node(self, path, st): -- 2.21.0 From aeeebf126fc9fdb0fd8c3b01418bef742bce78c3 Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Fri, 1 Nov 2019 12:22:51 -0400 Subject: [PATCH 02/10] [Plugin] fix destination paths in _do_copy_path() The path used to copy special device nodes and directories in _do_copy_path() should be the destination path in the archive (without sysroot prefix), and not the source path in the host file system that includes this prefix. Related: #1842 Signed-off-by: Bryn M. Reeves --- sos/plugins/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py index 4f1b73ce..60fbeaf7 100644 --- a/sos/plugins/__init__.py +++ b/sos/plugins/__init__.py @@ -651,9 +651,13 @@ class Plugin(object): self._copy_symlink(srcpath) return else: - if stat.S_ISDIR(st.st_mode) and os.access(srcpath, os.R_OK): - self._copy_dir(srcpath) - return + if stat.S_ISDIR(st.st_mode) and os.access(srcpath, os.R_OK): + # copy empty directory + if not os.listdir(srcpath): + self.archive.add_dir(dest) + return + self._copy_dir(dest) + return # handle special nodes (block, char, fifo, socket) if not (stat.S_ISREG(st.st_mode) or stat.S_ISDIR(st.st_mode)): @@ -808,7 +808,7 @@ class Plugin(object): ntype = _node_type(st) self._log_debug("creating %s node at archive:'%s'" % (ntype, dest)) - self._copy_node(srcpath, st) + self._copy_node(dest, st) return # if we get here, it's definitely a regular file (not a symlink or dir) -- 2.21.0 From 05f3d5bda8f548459fabcd38f2d087d6ecef98a2 Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Fri, 1 Nov 2019 12:25:09 -0400 Subject: [PATCH 03/10] [kernel] remove trailing directory globs in forbidden paths Since the forbidden path test now uses an exact match the trailing globs ("/some/directory/path/to/exclude/*") used to exclude trace related directories from collection lead to a failure to properly blacklist these files: The glob is expanded, for e.g.: "/sys/kernel/debug/tracing/per_cpu/*" Expands to unclude a 'cpuN' sub-directory for each CPU present on the machine. These expanded paths are then added to the forbidden paths list for the plugin: /sys/kernel/debug/tracing/per_cpu/cpu0 /sys/kernel/debug/tracing/per_cpu/cpu1 ... When an attempt is made to collect the entire "per_cpu" directory a check is made for the full "/sys/kernel/debug/tracing/per_cpu" path against each entry in the forbidden paths list. Since this is a prefix of the actual paths stored no match is returned and the collection is permitted. Remove the trailing globs from these directory paths and prevent any collection of the directories they reference by the plugin. Related: #1842 Signed-off-by: Bryn M. Reeves --- sos/plugins/kernel.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sos/plugins/kernel.py b/sos/plugins/kernel.py index 88b14689..5c852143 100644 --- a/sos/plugins/kernel.py +++ b/sos/plugins/kernel.py @@ -89,9 +89,9 @@ class Kernel(Plugin, RedHatPlugin, DebianPlugin, UbuntuPlugin): self.add_forbidden_path([ '/sys/kernel/debug/tracing/trace_pipe', '/sys/kernel/debug/tracing/README', - '/sys/kernel/debug/tracing/trace_stat/*', - '/sys/kernel/debug/tracing/per_cpu/*', - '/sys/kernel/debug/tracing/events/*', + '/sys/kernel/debug/tracing/trace_stat', + '/sys/kernel/debug/tracing/per_cpu', + '/sys/kernel/debug/tracing/events', '/sys/kernel/debug/tracing/free_buffer', '/sys/kernel/debug/tracing/trace_marker', '/sys/kernel/debug/tracing/trace_marker_raw', -- 2.21.0 From 801c71b33dcfeaa980baa9f377b721bdd26aa5e8 Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Fri, 1 Nov 2019 16:53:29 +0000 Subject: [PATCH 04/10] [tests] fix test_copy_dir_forbidden_path Rather than call just Plugin.setup() and Plugin._do_copy_path(), add an add_copy_spec() call to the mock plugin setup() method, and invoke copying by calling the Plugin.collect() method. Related: #1845 Signed-off-by: Bryn M. Reeves --- tests/plugin_tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/plugin_tests.py b/tests/plugin_tests.py index b8760429..6522fe14 100644 --- a/tests/plugin_tests.py +++ b/tests/plugin_tests.py @@ -81,6 +81,7 @@ class ForbiddenMockPlugin(Plugin): plugin_name = "forbidden" def setup(self): + self.add_copy_spec("tests") self.add_forbidden_path("tests") @@ -235,7 +236,7 @@ class PluginTests(unittest.TestCase): }) p.archive = MockArchive() p.setup() - p._do_copy_path("tests") + p.collect() self.assertEquals(p.archive.m, {}) -- 2.21.0 From c4182ebd52af523261d2e7ef75affbb88eaf31fb Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Mon, 4 Nov 2019 10:45:15 +0000 Subject: [PATCH 05/10] [Plugin] use correct source path when copying directories Signed-off-by: Bryn M. Reeves --- sos/plugins/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py index 60fbeaf7..240fe9f1 100644 --- a/sos/plugins/__init__.py +++ b/sos/plugins/__init__.py @@ -656,7 +656,7 @@ class Plugin(object): if not os.listdir(srcpath): self.archive.add_dir(dest) return - self._copy_dir(dest) + self._copy_dir(srcpath) return # handle special nodes (block, char, fifo, socket) -- 2.21.0 From 68f4d7cc7adde00171af842b5bc808f41d888a87 Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Mon, 4 Nov 2019 10:48:01 +0000 Subject: [PATCH 06/10] [Plugin] improve _copy_dir() variable naming Directory entries found in _copy_dir() may be either files or sub-directories: reflect this in the names of local variables. Signed-off-by: Bryn M. Reeves --- sos/plugins/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py index 240fe9f1..1a1464c1 100644 --- a/sos/plugins/__init__.py +++ b/sos/plugins/__init__.py @@ -738,10 +738,11 @@ class Plugin(object): def _copy_dir(self, srcpath): try: - for afile in os.listdir(srcpath): + for name in os.listdir(srcpath): self._log_debug("recursively adding '%s' from '%s'" - % (afile, srcpath)) - self._do_copy_path(os.path.join(srcpath, afile), dest=None) + % (name, srcpath)) + path = os.path.join(srcpath, name) + self._do_copy_path(path) except OSError as e: if e.errno == errno.ELOOP: msg = "Too many levels of symbolic links copying" -- 2.21.0 From ad3adef07c32aee5bdd438706c6c1d4590ff8297 Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Mon, 4 Nov 2019 14:13:00 +0000 Subject: [PATCH 07/10] [ceph] fix directory blacklist style Plugins must use 'path/to/exclude' rather than 'path/to/exclude/*' in order to omit a directory and all its content from the report. Signed-off-by: Bryn M. Reeves --- sos/plugins/ceph.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sos/plugins/ceph.py b/sos/plugins/ceph.py index 6e340c69..43284bc8 100644 --- a/sos/plugins/ceph.py +++ b/sos/plugins/ceph.py @@ -103,8 +103,8 @@ class Ceph(Plugin, RedHatPlugin, UbuntuPlugin): "/var/lib/ceph/*keyring*", "/var/lib/ceph/*/*keyring*", "/var/lib/ceph/*/*/*keyring*", - "/var/lib/ceph/osd/*", - "/var/lib/ceph/mon/*", + "/var/lib/ceph/osd", + "/var/lib/ceph/mon", # Excludes temporary ceph-osd mount location like # /var/lib/ceph/tmp/mnt.XXXX from sos collection. "/var/lib/ceph/tmp/*mnt*", -- 2.21.0 From 4d1576b04d35902ce44d26d6a5b2219e6f9c175a Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Mon, 4 Nov 2019 14:15:55 +0000 Subject: [PATCH 09/10] [openstack_octavia] fix directory blacklist style Plugins must use 'path/to/exclude' rather than 'path/to/exclude/*' in order to omit a directory and all its content from the report. Signed-off-by: Bryn M. Reeves --- sos/plugins/openstack_octavia.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sos/plugins/openstack_octavia.py b/sos/plugins/openstack_octavia.py index b97c83fa..ccdcd4c9 100644 --- a/sos/plugins/openstack_octavia.py +++ b/sos/plugins/openstack_octavia.py @@ -30,7 +30,7 @@ class OpenStackOctavia(Plugin): ]) # don't collect certificates - self.add_forbidden_path("/etc/octavia/certs/") + self.add_forbidden_path("/etc/octavia/certs") # logs if self.get_option("all_logs"): -- 2.21.0 From 1fd194191a56c51052f0c24ddeb3bbf9088ae0ca Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Mon, 4 Nov 2019 14:16:13 +0000 Subject: [PATCH 10/10] [vdsm] fix directory blacklist style Plugins must use 'path/to/exclude' rather than 'path/to/exclude/*' in order to omit a directory and all its content from the report. Signed-off-by: Bryn M. Reeves --- sos/plugins/vdsm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sos/plugins/vdsm.py b/sos/plugins/vdsm.py index b2a1ca58..69672643 100644 --- a/sos/plugins/vdsm.py +++ b/sos/plugins/vdsm.py @@ -60,9 +60,9 @@ class Vdsm(Plugin, RedHatPlugin): plugin_name = 'vdsm' def setup(self): - self.add_forbidden_path('/etc/pki/vdsm/keys/*') + self.add_forbidden_path('/etc/pki/vdsm/keys') self.add_forbidden_path('/etc/pki/vdsm/libvirt-spice/*-key.*') - self.add_forbidden_path('/etc/pki/libvirt/private/*') + self.add_forbidden_path('/etc/pki/libvirt/private') self.add_cmd_output('service vdsmd status') self.add_cmd_output('service supervdsmd status') -- 2.21.0