From cfa06db648f5371bd2178088eb9c4f4dc49c898c Mon Sep 17 00:00:00 2001 From: Roopa Prabhu Date: Sun, 22 Nov 2015 16:26:14 -0800 Subject: [PATCH] ifupdown2: syntax-check: exit with 1 if syntax errors Ticket: CM-7995 Reviewed By: CCR-3850 Testing Done: Tested exit code on syntax errors This patch adds members 'errors' and 'warns' to networkinterfaces.py to track errors and warns during parsing interfaces file. This patch also adds --syntax-check option to ifreload given people seem to use ifreload more than ifup these days. $ ifreload --syntax-check -a error: /etc/network/interfaces: iface swp1.200: unsupported keyword (roopa-attr) $ echo $? 1 (cherry picked from commit e643a136fcf5d387ff0f9a31cb6a6af4983e1012) --- ifupdown/ifupdownmain.py | 43 +++++++++++++++++++++++++++-------- ifupdown/networkinterfaces.py | 11 ++++++++- man.rst/ifreload.8.rst | 4 +++- man.rst/ifup.8.rst | 4 +++- sbin/ifupdown | 4 ++++ 5 files changed, 54 insertions(+), 12 deletions(-) diff --git a/ifupdown/ifupdownmain.py b/ifupdown/ifupdownmain.py index 9dfb3a7..57a9faf 100644 --- a/ifupdown/ifupdownmain.py +++ b/ifupdown/ifupdownmain.py @@ -603,7 +603,7 @@ class ifupdownMain(ifupdownBase): return False def _ifaceobj_syntax_checker(self, ifaceobj): - err = False + ret = True for attrname, attrvalue in ifaceobj.config.items(): found = False for k, v in self.module_attrs.items(): @@ -611,14 +611,15 @@ class ifupdownMain(ifupdownBase): found = True break if not found: - err = True + ret = False self.logger.warn('%s: unsupported attribute \'%s\'' \ % (ifaceobj.name, attrname)) continue - return err + return ret def read_iface_config(self): """ Reads default network interface config /etc/network/interfaces. """ + ret = True nifaces = networkInterfaces(self.interfacesfile, self.interfacesfileiobuf, self.interfacesfileformat, @@ -629,6 +630,9 @@ class ifupdownMain(ifupdownBase): self._iface_configattr_syntax_checker) nifaces.subscribe('validateifaceobj', self._ifaceobj_syntax_checker) nifaces.load() + if nifaces.errors or nifaces.warns: + ret = False + return ret def read_old_iface_config(self): """ Reads the saved iface config instead of default iface config. @@ -963,7 +967,7 @@ class ifupdownMain(ifupdownBase): self.ALL = True self.WITH_DEPENDS = True try: - self.read_iface_config() + iface_read_ret = self.read_iface_config() except Exception: raise @@ -987,8 +991,12 @@ class ifupdownMain(ifupdownBase): else: self.populate_dependency_info(ops) - # If only syntax check was requested, return here + # If only syntax check was requested, return here. + # return here because we want to make sure most + # errors above are caught and reported. if syntaxcheck: + if not iface_read_ret: + raise Exception() return try: @@ -1134,14 +1142,14 @@ class ifupdownMain(ifupdownBase): def _reload_currentlyup(self, upops, downops, auto=True, allow=None, ifacenames=None, excludepats=None, usecurrentconfig=False, - **extra_args): + syntaxcheck=False, **extra_args): """ reload currently up interfaces """ new_ifaceobjdict = {} # Override auto to true auto = True try: - self.read_iface_config() + iface_read_ret = self.read_iface_config() except: raise if not self.ifaceobjdict: @@ -1155,6 +1163,15 @@ class ifupdownMain(ifupdownBase): # generate dependency graph of interfaces self.populate_dependency_info(upops) + + # If only syntax check was requested, return here. + # return here because we want to make sure most + # errors above are caught and reported. + if syntaxcheck: + if not iface_read_ret: + raise Exception() + return + if (not usecurrentconfig and self.STATEMANAGER_ENABLE and self.statemanager.ifaceobjdict): already_up_ifacenames = self.statemanager.ifaceobjdict.keys() @@ -1215,12 +1232,12 @@ class ifupdownMain(ifupdownBase): def _reload_default(self, upops, downops, auto=False, allow=None, ifacenames=None, excludepats=None, usecurrentconfig=False, - **extra_args): + syntaxcheck=False, **extra_args): """ reload interface config """ new_ifaceobjdict = {} try: - self.read_iface_config() + iface_read_ret = self.read_iface_config() except: raise @@ -1235,6 +1252,14 @@ class ifupdownMain(ifupdownBase): # generate dependency graph of interfaces self.populate_dependency_info(upops) + # If only syntax check was requested, return here. + # return here because we want to make sure most + # errors above are caught and reported. + if syntaxcheck: + if not iface_read_ret: + raise Exception() + return + if (not usecurrentconfig and self.STATEMANAGER_ENABLE and self.statemanager.ifaceobjdict): # Save a copy of new iface objects and dependency_graph diff --git a/ifupdown/networkinterfaces.py b/ifupdown/networkinterfaces.py index 95bb6d2..76cad5b 100644 --- a/ifupdown/networkinterfaces.py +++ b/ifupdown/networkinterfaces.py @@ -64,6 +64,9 @@ class networkInterfaces(): self._currentfile_has_template = False self._ws_split_regex = re.compile(r'[\s\t]\s*') + self.errors = 0 + self.warns = 0 + @property def _currentfile(self): try: @@ -76,12 +79,14 @@ class networkInterfaces(): self.logger.error('%s: %s' %(filename, msg)) else: self.logger.error('%s: line%d: %s' %(filename, lineno, msg)) + self.errors += 1 def _parse_warn(self, filename, lineno, msg): if lineno == -1 or self._currentfile_has_template: self.logger.warn('%s: %s' %(filename, msg)) else: self.logger.warn('%s: line%d: %s' %(filename, lineno, msg)) + self.warns += 1 def _validate_addr_family(self, ifaceobj, lineno=-1): if ifaceobj.addr_family: @@ -421,12 +426,15 @@ class networkInterfaces(): if not isinstance(ifacedicts,list): ifacedicts = [ifacedicts] + errors = 0 for ifacedict in ifacedicts: ifaceobj = ifaceJsonDecoder.json_to_ifaceobj(ifacedict) if ifaceobj: self._validate_addr_family(ifaceobj) - self.callbacks.get('validateifaceobj')(ifaceobj) + if not self.callbacks.get('validateifaceobj')(ifaceobj): + errors += 1 self.callbacks.get('iface_found')(ifaceobj) + self.errors += errors def load(self): """ This member function loads the networkinterfaces file. @@ -437,6 +445,7 @@ class networkInterfaces(): if not self.interfacesfile and not self.interfacesfileiobuf: self.logger.warn('no terminal line stdin used or ') self.logger.warn('no network interfaces file defined.') + self.warns += 1 return if self.interfacesfileformat == 'json': diff --git a/man.rst/ifreload.8.rst b/man.rst/ifreload.8.rst index 0d46083..8cd150b 100644 --- a/man.rst/ifreload.8.rst +++ b/man.rst/ifreload.8.rst @@ -14,7 +14,7 @@ reload network interface configuration SYNOPSIS ======== - ifreload [-h] (-a|-c) [-v] [-d] [-f] [-n] + ifreload [-h] (-a|-c) [-v] [-d] [-f] [-n] [-s] DESCRIPTION =========== @@ -61,6 +61,8 @@ OPTIONS then each dependent interface must be specified in order to be excluded. + -s, --syntax-check Only run the interfaces file parser + EXAMPLES ======== diff --git a/man.rst/ifup.8.rst b/man.rst/ifup.8.rst index 7ac4541..bee77f9 100644 --- a/man.rst/ifup.8.rst +++ b/man.rst/ifup.8.rst @@ -22,7 +22,7 @@ SYNOPSIS ======== ifup [-h] [-a] [-v] [-d] [--allow CLASS] [--with-depends] - **[-X EXCLUDEPATS] [-f] [-n] [--print-dependency {list,dot}]** + **[-X EXCLUDEPATS] [-f] [-n] [-s] [--print-dependency {list,dot}]** **[IFACE [IFACE ...]]** ifdown [-h] [-a] [-v] [-d] [--allow CLASS] [--with-depends] @@ -115,6 +115,8 @@ OPTIONS your state file is corrupted or you want down to use the latest from the interfaces file + -s, --syntax-check Only run the interfaces file parser + EXAMPLES ======== # bringing up all interfaces diff --git a/sbin/ifupdown b/sbin/ifupdown index e89c04b..b3415b8 100755 --- a/sbin/ifupdown +++ b/sbin/ifupdown @@ -145,6 +145,7 @@ def run_reload(args): auto=args.all, allow=args.CLASS, ifacenames=None, excludepats=args.excludepats, usecurrentconfig=args.usecurrentconfig, + syntaxcheck=args.syntaxcheck, currentlyup=args.currentlyup) except: raise @@ -378,6 +379,9 @@ def update_ifreload_argparser(argparser): argparser.add_argument('-f', '--force', dest='force', action='store_true', help='force run all operations') + argparser.add_argument('-s', '--syntax-check', dest='syntaxcheck', + action='store_true', + help='Only run the interfaces file parser') def parse_args(argsv, op): if op == 'query':