From bf5301188ac27373d376cb657ad67c5e2a213248 Mon Sep 17 00:00:00 2001 From: Jake Hunsaker Date: Sat, 20 Apr 2024 14:58:18 -0400 Subject: [Global] Resolve empty except alerts from CodeQL This commit resolves the remaining 'Empty except' alerts from CodeQL scanning. Most of these involve logging something during exception handling when we were previously simply `pass`ing. Others, either when logging is unavailable or it makes no sense to log something for, are resolved by placing comments within the except block as the CodeQL alerts should be suppressed if the 'empty except' (`pass`ing) has an explanatory note. Signed-off-by: Jake Hunsaker --- sos/collector/clusters/ocp.py | 2 ++ sos/collector/clusters/ovirt.py | 1 + sos/collector/sosnode.py | 8 ++++---- sos/component.py | 4 ++-- sos/policies/distros/redhat.py | 1 + sos/policies/init_systems/systemd.py | 1 + sos/report/plugins/dnf.py | 1 + sos/report/plugins/gluster.py | 4 ++-- sos/report/plugins/jars.py | 31 +++++++++++++++--------------- sos/report/plugins/pcp.py | 1 + sos/report/plugins/subscription_manager.py | 4 ++-- sos/report/plugins/watchdog.py | 1 + sos/utilities.py | 2 ++ 13 files changed, 36 insertions(+), 25 deletions(-) diff --git a/sos/collector/clusters/ocp.py b/sos/collector/clusters/ocp.py index 020443ef..db47cdad 100644 --- a/sos/collector/clusters/ocp.py +++ b/sos/collector/clusters/ocp.py @@ -226,6 +226,8 @@ class ocp(Cluster): try: idx[state] = statline.index(state.upper()) except Exception: + # label is not available, which is not fatal for our dict + # construction here pass for node in nodelist: _node = node.split() diff --git a/sos/collector/clusters/ovirt.py b/sos/collector/clusters/ovirt.py index 20e79f7d..d7a1b92b 100644 --- a/sos/collector/clusters/ovirt.py +++ b/sos/collector/clusters/ovirt.py @@ -148,6 +148,7 @@ class ovirt(Cluster): v = str(line.split('=')[1].replace('"', '')) conf[k] = v except IndexError: + # not a valid line to parse config values from, ignore pass return conf return False diff --git a/sos/collector/sosnode.py b/sos/collector/sosnode.py index bc8a468f..fad0c378 100644 --- a/sos/collector/sosnode.py +++ b/sos/collector/sosnode.py @@ -367,8 +367,8 @@ class SosNode(): if not is_list: try: res.append(line.split()[0]) - except Exception: - pass + except Exception as err: + self.log_debug(f"Error parsing sos help: {err}") else: r = line.split(',') res.extend(p.strip() for p in r if p.strip()) @@ -475,8 +475,8 @@ class SosNode(): self.log_error('Unable to determine path of sos archive') if self.sos_path: self.retrieved = self.retrieve_sosreport() - except Exception: - pass + except Exception as err: + self.log_error(f"Error during sos execution: {err}") self.cleanup() def _preset_exists(self, preset): diff --git a/sos/component.py b/sos/component.py index b1daa664..9a6b490a 100644 --- a/sos/component.py +++ b/sos/component.py @@ -91,8 +91,8 @@ class SoSComponent(): try: import signal signal.signal(signal.SIGTERM, self.get_exit_handler()) - except Exception: - pass + except Exception as err: + sys.stdout.write(f"Notice: Could not set SIGTERM handler: {err}\n") self.opts = SoSOptions(arg_defaults=self._arg_defaults) if self.load_policy: diff --git a/sos/policies/distros/redhat.py b/sos/policies/distros/redhat.py index 6797d878..5658516b 100644 --- a/sos/policies/distros/redhat.py +++ b/sos/policies/distros/redhat.py @@ -556,6 +556,7 @@ support representative. for line in hfile.read().splitlines(): coreos |= 'Red Hat Enterprise Linux CoreOS' in line except IOError: + # host release file not present, will fallback to RHEL policy check pass return coreos diff --git a/sos/policies/init_systems/systemd.py b/sos/policies/init_systems/systemd.py index 98b9fc49..0f49197c 100644 --- a/sos/policies/init_systems/systemd.py +++ b/sos/policies/init_systems/systemd.py @@ -41,6 +41,7 @@ class SystemdInit(InitSystem): 'config': config } except IndexError: + # not a valid line to extract status info from pass def is_running(self, name, default=False): diff --git a/sos/report/plugins/dnf.py b/sos/report/plugins/dnf.py index af75e680..4a1e52b9 100644 --- a/sos/report/plugins/dnf.py +++ b/sos/report/plugins/dnf.py @@ -113,6 +113,7 @@ class DNFPlugin(Plugin, RedHatPlugin): transactions = int(line.split('|')[0].strip()) break except ValueError: + # not a valid line to extract transactions from, ignore pass for tr_id in range(1, min(transactions+1, 50)): self.add_cmd_output(f"dnf history info {tr_id}", diff --git a/sos/report/plugins/gluster.py b/sos/report/plugins/gluster.py index 404143cd..4f99b5d9 100644 --- a/sos/report/plugins/gluster.py +++ b/sos/report/plugins/gluster.py @@ -52,8 +52,8 @@ class Gluster(Plugin, RedHatPlugin): '/glusterd_state_[0-9]*_[0-9]*')) for name in remove_files: os.remove(name) - except OSError: - pass + except OSError as err: + self._log_error(f"Could not remove statedump files: {err}") def setup(self): self.add_forbidden_path("/var/lib/glusterd/geo-replication/secret.pem") diff --git a/sos/report/plugins/jars.py b/sos/report/plugins/jars.py index ef72f8d1..ca2c0803 100644 --- a/sos/report/plugins/jars.py +++ b/sos/report/plugins/jars.py @@ -64,13 +64,13 @@ class Jars(Plugin, RedHatPlugin): for dirpath, _, filenames in os.walk(location): for filename in filenames: path = self.path_join(dirpath, filename) - if Jars.is_jar(path): + if self.is_jar(path): jar_paths.append(path) # try to extract information about found JARs for jar_path in jar_paths: - maven_id = Jars.get_maven_id(jar_path) - jar_id = Jars.get_jar_id(jar_path) + maven_id = self.get_maven_id(jar_path) + jar_id = self.get_jar_id(jar_path) if maven_id or jar_id: record = {"path": jar_path, "sha1": jar_id, @@ -81,8 +81,7 @@ class Jars(Plugin, RedHatPlugin): results_str = json.dumps(results, indent=4, separators=(",", ": ")) self.add_string_as_file(results_str, "jars.json", plug_dir=True) - @staticmethod - def is_jar(path): + def is_jar(self, path): """Check whether given file is a JAR file. JARs are ZIP files which usually include a manifest @@ -93,12 +92,13 @@ class Jars(Plugin, RedHatPlugin): with zipfile.ZipFile(path) as file: if "META-INF/MANIFEST.MF" in file.namelist(): return True - except (IOError, zipfile.BadZipfile): - pass + except (IOError, zipfile.BadZipfile) as err: + self._log_info( + f"Could not determine if {path} is a JAR: {err}" + ) return False - @staticmethod - def get_maven_id(jar_path): + def get_maven_id(self, jar_path): """Extract Maven coordinates from a given JAR file, if possible. JARs build by Maven (most popular Java build system) contain @@ -123,12 +123,13 @@ class Jars(Plugin, RedHatPlugin): props[key] = value except ValueError: return None - except IOError: - pass + except IOError as err: + self._log_info( + f"Could not extract Maven coordinates from {jar_path}: {err}" + ) return props - @staticmethod - def get_jar_id(jar_path): + def get_jar_id(self, jar_path): """Compute JAR id. Returns sha1 hash of a given JAR file. @@ -140,6 +141,6 @@ class Jars(Plugin, RedHatPlugin): for buf in iter(partial(file.read, 4096), b''): digest.update(buf) jar_id = digest.hexdigest() - except IOError: - pass + except IOError as err: + self._log_info(f"Could not compute JAR id for {jar_path}: {err}") return jar_id diff --git a/sos/report/plugins/pcp.py b/sos/report/plugins/pcp.py index c423b420..836be2be 100644 --- a/sos/report/plugins/pcp.py +++ b/sos/report/plugins/pcp.py @@ -51,6 +51,7 @@ class Pcp(Plugin, RedHatPlugin, DebianPlugin): (key, value) = line.strip().split('=') env_vars[key] = value except (ValueError, KeyError): + # not a line for a key, value pair. Ignore the line. pass try: diff --git a/sos/report/plugins/subscription_manager.py b/sos/report/plugins/subscription_manager.py index a37886c8..6b302b46 100644 --- a/sos/report/plugins/subscription_manager.py +++ b/sos/report/plugins/subscription_manager.py @@ -95,8 +95,8 @@ class SubscriptionManager(Plugin, RedHatPlugin): if no_proxy: env = {'NO_PROXY': no_proxy} except (ModuleNotFoundError, ImportError, NoOptionError, - NoSectionError): - pass + NoSectionError) as err: + self._log_debug(f"Error checking for RHSM cert/proxy issue: {err}") self.add_cmd_output(curlcmd, env=env, timeout=30) def postproc(self): diff --git a/sos/report/plugins/watchdog.py b/sos/report/plugins/watchdog.py index 5094d16d..6ed3c8d8 100644 --- a/sos/report/plugins/watchdog.py +++ b/sos/report/plugins/watchdog.py @@ -44,6 +44,7 @@ class Watchdog(Plugin, RedHatPlugin): if key.strip() == 'log-dir': log_dir = value.strip() except ValueError: + # not a valid key, value line and we can safely ignore pass return log_dir diff --git a/sos/utilities.py b/sos/utilities.py index c8f11993..84419e4e 100644 --- a/sos/utilities.py +++ b/sos/utilities.py @@ -616,10 +616,12 @@ class TempFileUtil(): f.flush() f.close() except Exception: + # file already closed or potentially already removed, ignore pass try: os.unlink(fname) except Exception: + # if the above failed, this is also likely to fail, ignore pass self.files = [] -- cgit