[global] Fix alerts for mixing implicit and explicit returns

This commit resolves the alerts generated by GitHub code scanning for
mixing implicit and explicit returns. In most cases this is simply
changing empty or implicit returns to `return None`, while in a few
other places the return logic is slightly reworked.

Signed-off-by: Jake Hunsaker <[email protected]>

sos/cleaner/__init__.py

@@ -428,6 +428,7 @@ def execute(self):
                          "representative and keep the mapping file private")
 
         self.cleanup()
+        return None
 
     def rebuild_nested_archive(self):
         """Handles repacking the nested tarball, now containing only obfuscated
@@ -750,7 +751,7 @@ def obfuscate_file(self, filename, short_name=None, arc_name=None):
         """
         if not filename:
             # the requested file doesn't exist in the archive
-            return
+            return None
         subs = 0
         if not short_name:
             short_name = filename.split('/')[-1]

sos/cleaner/mappings/__init__.py

@@ -41,6 +41,7 @@ def ignore_item(self, item):
         for skip in self.ignore_matches:
             if re.match(skip, item, re.I):
                 return True
+        return False
 
     def add(self, item):
         """Add a particular item to the map, generating an obfuscated pair

sos/cleaner/mappings/hostname_map.py

@@ -215,6 +215,7 @@ def sanitize_item(self, item):
             if all([h.isupper() for h in host]):
                 _fqdn = _fqdn.upper()
             return _fqdn
+        return None
 
     def sanitize_short_name(self, hostname):
         """Obfuscate the short name of the host with an incremented counter

sos/cleaner/mappings/mac_map.py

@@ -77,3 +77,4 @@ def sanitize_item(self, item):
         # match 48-bit IPv4 MAC addresses
         if re.match('([0-9a-fA-F][:_]?){12}', item):
             return self.mac_template % hextets
+        return None

sos/collector/__init__.py

@@ -598,6 +598,7 @@ def _validate_option(self, default, cli):
                     return True
                 else:
                     return False
+        self.exit(f"Unknown option type: {cli.opt_type}")
 
     def log_info(self, msg):
         """Log info messages to both console and log file"""

sos/collector/clusters/ocp.py

@@ -127,7 +127,7 @@ def setup(self):
         collection via a container image
         """
         if not self.set_transport_type() == 'oc':
-            return
+            return None
 
         out = self.exec_primary_cmd(self.fmt_oc_cmd("auth can-i '*' '*'"))
         self.oc_cluster_admin = out['status'] == 0

sos/collector/sosnode.py

@@ -213,6 +213,7 @@ def create_sos_container(self):
                 self.log_error("Could not create container on host: %s"
                                % res['output'])
                 raise Exception
+        return False
 
     def get_container_auth(self):
         """Determine what the auth string should be to pull the image used to
@@ -331,6 +332,7 @@ def _load_sos_info(self):
             self._load_sos_plugins(sosinfo['output'])
         if self.check_sos_version('3.6'):
             self._load_sos_presets()
+        return None
 
     def _load_sos_presets(self):
         cmd = '%s --list-presets' % self.sos_bin

sos/collector/transports/__init__.py

@@ -102,6 +102,7 @@ def display_help(cls, section):
             section.add_text(
                 'Detailed information not available for this transport'
             )
+        return None
 
     @classmethod
     def display_self_help(cls, section):
@@ -299,6 +300,11 @@ def _run_command_with_pexpect(self, cmd, timeout, need_root, env):
             return {'status': result.exitstatus, 'output': out}
         elif index == 1:
             raise CommandTimeoutException(cmd)
+        # if we somehow manage to flow to this point, use this bogus exit code
+        # as a signal to debugging efforts that whatever went sideways did so
+        # as part of the above block
+        self.log_debug(f"Unexpected index {index} from pexpect: {result}")
+        return {'status': 999, 'output': ''}
 
     def _send_pexpect_password(self, index, result):
         """Handle password prompts for sudo and su usage for non-root SSH users

sos/collector/transports/control_persist.py

@@ -176,6 +176,7 @@ def _disconnect(self):
                 return False
         self.log_debug("Control socket not present when attempting to "
                        "terminate session")
+        return False
 
     @property
     def connected(self):

sos/component.py

@@ -182,6 +182,7 @@ def check_listing_options(self):
         opts = [o for o in self.opts.dict().keys() if o.startswith('list')]
         if opts:
             return any([getattr(self.opts, opt) for opt in opts])
+        return False
 
     @classmethod
     def add_parser_options(cls, parser):

sos/help/__init__.py

@@ -126,6 +126,7 @@ def _get_collect_transport(self):
         _transport = self.opts.topic.split('.')[-1]
         if _transport in TRANSPORTS:
             return TRANSPORTS[_transport]
+        return None
 
     def _get_collect_cluster(self):
         from sos.collector import SoSCollector
@@ -135,6 +136,7 @@ def _get_collect_cluster(self):
         for cluster in clusters:
             if cluster[0] == self.opts.topic.split('.')[-1]:
                 return cluster[1]
+        return None
 
     def _get_plugin_variant(self):
         mod = importlib.import_module('sos.' + self.opts.topic)
@@ -145,6 +147,7 @@ def _get_plugin_variant(self):
             if plugin.__subclasses__():
                 cls = self.policy.match_plugin(plugin.__subclasses__())
                 return cls
+        return None
 
     def _get_policy_by_name(self):
         _topic = self.opts.topic.split('.')[-1]
@@ -157,6 +160,7 @@ def _get_policy_by_name(self):
                 _p = policy.__name__.lower().replace('policy', '')
                 if _p == _topic:
                     return policy
+        return None
 
     def display_self_help(self):
         """Displays the help information for this component directly, that is

sos/policies/__init__.py

@@ -467,6 +467,7 @@ def display_results(self, archive, directory, checksum, archivestat=None,
             self.ui_log.info(
                 _("\nPlease send this file to your support representative.\n")
             )
+        return None
 
     def get_msg(self):
         """This method is used to prepare the preamble text to display to

sos/report/__init__.py

@@ -1385,6 +1385,7 @@ def collect_plugin(self, plugin):
             self.handle_exception(plugname, "collect")
         except Exception:
             self.handle_exception(plugname, "collect")
+        return None
 
     def ui_progress(self, status_line):
         if self.opts.verbosity == 0 and not self.opts.batch:

sos/report/plugins/__init__.py

@@ -61,6 +61,7 @@ def _node_type(st):
     for t in _types:
         if t[0](st.st_mode):
             return t[1]
+    return ''
 
 
 _certmatch = re.compile("-*BEGIN.*?-*END", re.DOTALL)
@@ -187,6 +188,10 @@ def _check_required_state(self, items, required):
             return all(items)
         elif required == 'none':
             return not any(items)
+        raise ValueError(
+            f"predicate requires must be 'any', 'all', or 'none' "
+            f"not {required}"
+        )
 
     def _failed_or_forbidden(self, test, item):
         """Helper to direct failed predicates to provide the proper messaging
@@ -1458,11 +1463,11 @@ def _do_copy_path(self, srcpath, dest=None, force=False):
         saved for use later in preparing a report.
         """
         if self._timeout_hit:
-            return
+            return None
 
         if self._is_forbidden_path(srcpath):
             self._log_debug("skipping forbidden path '%s'" % srcpath)
-            return ''
+            return None
 
         if not dest:
             dest = srcpath
@@ -1474,27 +1479,27 @@ def _do_copy_path(self, srcpath, dest=None, force=False):
             st = os.lstat(srcpath)
         except (OSError, IOError):
             self._log_info("failed to stat '%s'" % srcpath)
-            return
+            return None
 
         if stat.S_ISLNK(st.st_mode):
             self._copy_symlink(srcpath)
-            return
+            return None
         else:
             if stat.S_ISDIR(st.st_mode) and os.access(srcpath, os.R_OK):
                 # copy empty directory
                 if not self.listdir(srcpath):
                     self.archive.add_dir(dest)
-                    return
+                    return None
                 self._copy_dir(srcpath)
-                return
+                return None
 
         # handle special nodes (block, char, fifo, socket)
         if not (stat.S_ISREG(st.st_mode) or stat.S_ISDIR(st.st_mode)):
             ntype = _node_type(st)
             self._log_debug("creating %s node at archive:'%s'"
                             % (ntype, dest))
             self._copy_node(dest, st)
-            return
+            return None
 
         # if we get here, it's definitely a regular file (not a symlink or dir)
         self._log_debug("copying path '%s' to archive:'%s'" % (srcpath, dest))
@@ -1512,7 +1517,7 @@ def _do_copy_path(self, srcpath, dest=None, force=False):
             'symlink': "no"
         })
 
-        return
+        return None
 
     def add_forbidden_path(self, forbidden):
         """Specify a path, or list of paths, to not copy, even if it's part of
@@ -1695,7 +1700,7 @@ def add_copy_spec(self, copyspecs, sizelimit=None, maxage=None,
         if not self.test_predicate(pred=pred):
             self._log_info("skipped copy spec '%s' due to predicate (%s)" %
                            (copyspecs, self.get_predicate(pred=pred)))
-            return
+            return None
 
         if sizelimit is None:
             sizelimit = self.get_option("log_size")
@@ -1723,11 +1728,12 @@ def get_filename_tag(fname):
             mangled to _conf or similar.
             """
             if fname.startswith(('/proc', '/sys')):
-                return
+                return None
             _fname = fname.split('/')[-1]
             _fname = _fname.replace('-', '_')
             if _fname.endswith(('.conf', '.log', '.txt')):
                 return _fname.replace('.', '_')
+            return None
 
         for copyspec in copyspecs:
             if not (copyspec and len(copyspec)):
@@ -1890,6 +1896,7 @@ def time_filter(path):
                         'files_copied': _manifest_files,
                         'tags': _spec_tags
                     })
+        return None
 
     def add_device_cmd(self, cmds, devices, timeout=None, sizelimit=None,
                        chroot=True, runat=None, env=None, binary=False,
@@ -2322,7 +2329,7 @@ def _collect_cmd_output(self, cmd, suggest_filename=None,
 
         """
         if self._timeout_hit:
-            return
+            return None
 
         if timeout is None:
             timeout = self.cmdtimeout

sos/report/plugins/autofs.py

@@ -40,6 +40,7 @@ def getdaemondebug(self):
                                   *self.files)
         for i in debugout:
             return i[1]
+        return None
 
     def setup(self):
         self.add_copy_spec("/etc/auto*")

sos/report/plugins/ipa.py

@@ -37,14 +37,17 @@ def check_ipa_server_version(self):
 
     def ca_installed(self):
         # Follow the same checks as IPA CA installer code
-        if self.path_exists("%s/conf/ca/CS.cfg" % self.pki_tomcat_dir_v4) \
-                or self.path_exists("%s/conf/CS.cfg" % self.pki_tomcat_dir_v3):
-            return True
+        return any(
+            self.path_exists(path) for path in [
+                f"{self.pki_tomcat_dir_v4}/conf/ca/CS.cfg",
+                f"{self.pki_tomcat_dir_v3}/conf/CS.cfg"
+            ]
+        )
 
     def ipa_server_installed(self):
-        if self.is_installed("ipa-server") \
-                or self.is_installed("freeipa-server"):
-            return True
+        return any(
+            self.is_installed(pkg) for pkg in ['ipa-server', 'freeipa-server']
+        )
 
     def retrieve_pki_logs(self, ipa_version):
         if ipa_version == "v4":

sos/report/plugins/ovn_central.py

@@ -51,12 +51,12 @@ def get_tables_from_schema(self, filename, skip=[]):
             if res['status'] != 0:
                 self._log_error("Could not retrieve DB schema file from "
                                 "container %s" % self._container_name)
-                return
+                return None
             try:
                 db = json.loads(res['output'])
             except Exception:
                 self._log_error("Cannot parse JSON file %s" % filename)
-                return
+                return None
         else:
             try:
                 with open(self.path_join(filename), 'r') as f:
@@ -65,16 +65,17 @@ def get_tables_from_schema(self, filename, skip=[]):
                     except Exception:
                         self._log_error(
                             "Cannot parse JSON file %s" % filename)
-                        return
+                        return None
             except IOError as ex:
                 self._log_error(
                     "Could not open DB schema file %s: %s" % (filename, ex))
-                return
+                return None
         try:
             return [table for table in dict.keys(
                 db['tables']) if table not in skip]
         except AttributeError:
             self._log_error("DB schema %s has no 'tables' key" % filename)
+        return None
 
     def add_database_output(self, tables, cmds, ovn_cmd):
         if not tables:

sos/utilities.py

@@ -533,8 +533,7 @@ def _find_plugins_in_dir(self, path):
             pnames = self._get_plugins_from_list(py_files)
             if pnames:
                 return pnames
-            else:
-                return []
+        return []
 
     def get_modules(self):
         """Returns the list of importable modules in the configured python