validator: T2417: try to make the code clearer

8 files changed, 134 insertions, 148 deletions

diff --git a/python/vyos/util.py b/python/vyos/util.py
index e598e0ff3..381cd0358 100644
--- a/python/vyos/util.py
+++ b/python/vyos/util.py
@@ -389,12 +389,9 @@ def get_cfg_group_id():
 
 def file_is_persistent(path):
     import re
-    if not re.match(r'^(/config|/opt/vyatta/etc/config)', os.path.dirname(path)):
-        warning = "Warning: file {0} is outside the /config directory\n".format(path)
-        warning += "It will not be automatically migrated to a new image on system update"
-        return (False, warning)
-    else:
-        return (True, None)
+    location = r'^(/config|/opt/vyatta/etc/config)'
+    absolute = os.path.abspath(os.path.dirname(path))
+    return re.match(location,absolute)
 
 
 def commit_in_progress():
diff --git a/src/validators/file-exists b/src/validators/file-exists
index e179805ed..5cef6b199 100755
--- a/src/validators/file-exists
+++ b/src/validators/file-exists
@@ -23,40 +23,39 @@ import os
 import sys
 import argparse
 
-parser = argparse.ArgumentParser()
-parser.add_argument("-d", "--directory", type=str, help="File must be present in this directory.")
-parser.add_argument("-e", "--error", action="store_true", help="Tread warnings as errors - change exit code to '1'")
-parser.add_argument("file", type=str, help="Path of file to validate")
 
-args = parser.parse_args()
-
-msg_prefix = "WARNING: "
-if args.error:
-    msg_prefix = "ERROR: "
-
-#
-# Always check if the given file exists
-#
-if not os.path.exists(args.file):
-    print(msg_prefix + "File '{}' not found".format(args.file))
-    if args.error:
-        sys.exit(1)
-    else:
-        sys.exit(0)
-
-#
-# Optional check if the file is under a certain directory path
-#
-if args.directory:
-    # remove directory path from path to verify
-    rel_filename = args.file.replace(args.directory, '').lstrip('/')
-
-    if not os.path.exists(args.directory + '/' + rel_filename):
-        print(msg_prefix + "'{}' lies outside of '{}' directory.\n" \
-              "It will not get preserved during image upgrade!".format(args.file, args.directory))
-        if args.error:
-            sys.exit(1)
-        else:
-            sys.exit(0)
-
-sys.exit(0)
+def exit(strict, message):
+    if strict:
+        sys.exit(f'ERROR: {message}')
+    print(f'WARNING: {message}', file=sys.stderr)
+    sys.exit()
+
+
+if __name__ == '__main__':
+    parser = argparse.ArgumentParser()
+    parser.add_argument("-d", "--directory", type=str, help="File must be present in this directory.")
+    parser.add_argument("-e", "--error", action="store_true", help="Tread warnings as errors - change exit code to '1'")
+    parser.add_argument("file", type=str, help="Path of file to validate")
+
+    args = parser.parse_args()
+
+    #
+    # Always check if the given file exists
+    #
+    if not os.path.exists(args.file):
+        exit(args.error, f"File '{args.file}' not found")
+
+    #
+    # Optional check if the file is under a certain directory path
+    #
+    if args.directory:
+        # remove directory path from path to verify
+        rel_filename = args.file.replace(args.directory, '').lstrip('/')
+
+        if not os.path.exists(args.directory + '/' + rel_filename):
+            exit(args.error,
+                f"'{args.file}' lies outside of '{args.directory}' directory.\n"
+                  "It will not get preserved during image upgrade!"
+            )
+
+    sys.exit()
diff --git a/src/validators/fqdn b/src/validators/fqdn
index 9f4ed764f..347ffda42 100755
--- a/src/validators/fqdn
+++ b/src/validators/fqdn
@@ -14,14 +14,17 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from re import match
-from sys import argv,exit
+import re
+import sys
 
-if len(argv) == 2:
-  # pattern copied from: https://www.regextester.com/103452
-  pattern = "(?=^.{4,253}$)(^((?!-)[a-zA-Z0-9-]{0,62}[a-zA-Z0-9]\.)+[a-zA-Z]{2,63}$)"
-  if match(pattern, argv[1]):
-    exit(0)
-  else:
-    exit(1)
 
+# pattern copied from: https://www.regextester.com/103452
+pattern = "(?=^.{4,253}$)(^((?!-)[a-zA-Z0-9-]{0,62}[a-zA-Z0-9]\.)+[a-zA-Z]{2,63}$)"
+
+
+if __name__ == '__main__':
+    if len(sys.argv) != 2:
+        sys.exit(1)
+    if not re.match(pattern, sys.argv[1]):
+        sys.exit(1)
+    sys.exit(0)
diff --git a/src/validators/mac-address b/src/validators/mac-address
index 435920b84..b2d3496f4 100755
--- a/src/validators/mac-address
+++ b/src/validators/mac-address
@@ -15,12 +15,15 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import re
-from sys import exit, argv
+import sys
 
-if len(argv) == 2:
-    pattern = "^([0-9A-Fa-f]{2}[:]){5}([0-9A-Fa-f]{2})$"
-    if re.match(pattern, argv[1]):
-        exit(0)
-    else:
-        exit(1)
 
+pattern = "^([0-9A-Fa-f]{2}[:]){5}([0-9A-Fa-f]{2})$"
+
+
+if __name__ == '__main__':
+    if len(sys.argv) != 2:
+        sys.exit(1)
+    if not re.match(pattern, sys.argv[1]):
+        sys.exit(1)
+    sys.exit(0)
diff --git a/src/validators/numeric b/src/validators/numeric
index 2cd5178b9..37d9d0364 100755
--- a/src/validators/numeric
+++ b/src/validators/numeric
@@ -20,59 +20,47 @@
 import sys
 import argparse
 
-parser = argparse.ArgumentParser()
-parser.add_argument("-f", "--float", action="store_true", help="Accept floating point values")
-group = parser.add_mutually_exclusive_group()
-group.add_argument("-r", "--range", type=str, help="Check if the number is within range (inclusive), example: 1024-65535", action='append')
-group.add_argument("-n", "--non-negative", action="store_true", help="Check if the number is non-negative (>= 0)")
-group.add_argument("-p", "--positive", action="store_true", help="Check if the number is positive (> 0)")
-parser.add_argument("number", type=str, help="Number to validate")
 
-args = parser.parse_args()
+if __name__ == '__main__':
+    parser = argparse.ArgumentParser()
+    parser.add_argument("-f", "--float", action="store_true", help="Accept floating point values")
+    group = parser.add_mutually_exclusive_group()
+    group.add_argument("-r", "--range", type=str, help="Check if the number is within range (inclusive), example: 1024-65535", action='append')
+    group.add_argument("-n", "--non-negative", action="store_true", help="Check if the number is non-negative (>= 0)")
+    group.add_argument("-p", "--positive", action="store_true", help="Check if the number is positive (> 0)")
+    parser.add_argument("number", type=str, help="Number to validate")
+
+    args = parser.parse_args()
+
+    if args.float:
+        converter = float
+        name = 'floating point'
+    else:
+        converter = int
+        name = 'integer'
 
-# Try to load the argument
-number = None
-if args.float:
-    try:
-        number = float(args.number)
-    except:
-        print("{0} is not a valid floating point number".format(args.number), file=sys.stderr)
-        sys.exit(1)
-else:
     try:
-        number = int(args.number)
+        number = converter(args.number)
     except:
-        print("{0} is not a valid integer number".format(args.number), file=sys.stderr)
-        sys.exit(1)
+        sys.exit(f'{args.number} is not a valid {name} number')
 
-if args.range:
-    valid = False
-    for r in args.range:
+    if args.range:
         try:
-            list = r.split('-')
-            lower = int(list[0])
-            upper = int(list[1])
+            for r in args.range:
+                _lower, _upper = r.split('-')
+                lower = int(_lower)
+                upper = int(_upper)
         except:
-            print("{0} is not a valid number range",format(args.range), file=sys.stderr)
-            sys.exit(1)
+            sys.exit(f'{args.range} is not a valid number range')
+
+        if number < lower or number > upper:
+            span = args.range if len(args.range) > 1 else args.range[0]
+            sys.exit('Number {number} is not in the range {span}')
 
-        if (number >= lower) and (number <= upper):
-            valid = True
-    # end for
+    elif args.non_negative and number < 0:
+        sys.exit('Number should be non-negative')
 
-    if not valid:
-        if len(args.range) > 1:
-            err_msg = "Number {0} is not in any of the ranges {1}".format(number, args.range)
-        else:
-            err_msg = "Number {0} is not in the range {1}".format(number, args.range[0])
-        print(err_msg, file=sys.stderr)
-        sys.exit(1)
-elif args.non_negative:
-    if number < 0:
-        print("Number should be non-negative", file=sys.stderr)
-        sys.exit(1)
-elif args.positive:
-    if number <= 0:
-        print("Number should be positive", file=sys.stderr)
-        sys.exit(1)
+    elif args.positive and number <= 0:
+        sys.exit('Number should be positive')
 
+    sys.exit()
diff --git a/src/validators/script b/src/validators/script
index 689296a73..2665ec1f6 100755
--- a/src/validators/script
+++ b/src/validators/script
@@ -17,7 +17,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; If not, see <http://www.gnu.org/licenses/>.
 
-import re
 import os
 import sys
 import shlex
@@ -25,22 +24,22 @@ import shlex
 import vyos.util
 
 
-if len(sys.argv) < 2:
-    print("Please specify script file to check")
-    sys.exit(1)
+if __name__ == '__main__':
+    if len(sys.argv) < 2:
+        sys.exit('Please specify script file to check')
 
-#if the "script" is a script+ stowaway arugments, this removes the aguements
-script = shlex.split(sys.argv[1])[0]
+    # if the "script" is a script+ stowaway arguments, this removes the aguements
+    script = shlex.split(sys.argv[1])[0]
 
-if not os.path.exists(script):
-    print("File {0} does not exist".format(script))
-    sys.exit(1)
+    if not os.path.exists(script):
+        sys.exit(f'File {script} does not exist')
 
-if not (os.path.isfile(script) and os.access(script, os.X_OK)):
-    print("File {0} is not an executable file".format(script))
-    sys.exit(1)
+    if not (os.path.isfile(script) and os.access(script, os.X_OK)):
+        sys.exit('File {script} is not an executable file')
 
-# File outside the config dir is just a warning
-res, warning = vyos.util.file_is_persistent(script)
-if not res:
-    print(warning)
+    # File outside the config dir is just a warning
+    if not vyos.util.file_is_persistent(script):
+        sys.exit(
+            'Warning: file {path} is outside the / config directory\n'
+            'It will not be automatically migrated to a new image on system update'
+        )
diff --git a/src/validators/timezone b/src/validators/timezone
index ec845e755..7e4534b57 100755
--- a/src/validators/timezone
+++ b/src/validators/timezone
@@ -15,25 +15,19 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import argparse
-
-from sys import exit
+import sys
 
 from vyos.util import cmd
 
-parser = argparse.ArgumentParser()
-parser.add_argument("--validate", action="store", help="Check if timezone is valid")
 
 if __name__ == '__main__':
+    parser = argparse.ArgumentParser()
+    parser.add_argument("--validate", action="store", required=True, help="Check if timezone is valid")
     args = parser.parse_args()
 
-    if args.validate:
-        tz_data = cmd('find /usr/share/zoneinfo/posix -type f -or -type l | sed -e s:/usr/share/zoneinfo/posix/::')
-        tz_data = tz_data.split('\n')
-        # if timezone can't be found in list it's invalid
-        if args.validate not in tz_data:
-            exit(1)
-    else:
-        parser.print_help()
-        exit(1)
+    tz_data = cmd('find /usr/share/zoneinfo/posix -type f -or -type l | sed -e s:/usr/share/zoneinfo/posix/::')
+    tz_data = tz_data.split('\n')
 
-    exit(0)
+    if args.validate not in tz_data:
+        sys.exit("the timezone can't be found in the timezone list')
+    sys.exit()
diff --git a/src/validators/vrf-name b/src/validators/vrf-name
index 11c453f4d..878893c46 100755
--- a/src/validators/vrf-name
+++ b/src/validators/vrf-name
@@ -14,27 +14,30 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+
 import re
-from sys import exit, argv
+import sys
+
+
+if __name__ == '__main__':
+    if len(sys.argv) != 2:
+        sys.exit(1)
+
+    vrf = sys.argv[1]
+    length = len(vrf)
 
-if len(argv) == 2:
-    len = len(argv[1])
-    # VRF instance name must be 16 characters or less, python range needs to be
-    # extended by one
-    if not len in range(1, 17):
-        exit(1)
+    if length not in range(1, 17):
+        sys.exit('VRF instance name must be 16 characters or less')
 
     # Treat loopback interface "lo" explicitly. Adding "lo" explicitly to the
     # following regex pattern would deny any VRF name starting with lo - thuse
     # local-vrf would be illegal - and that we do not want.
-    if argv[1] == "lo":
-        exit(1)
+    if vrf == "lo":
+        exit(f'"{vrf}" is invalid as VRF name as it is an interface name')
 
-    # VRF instances should not be named after regular interface names like bond0,
-    # br10 and so on - this can cause a lot of confusion/trouble
     pattern = "^(?!(bond|br|dum|eth|lan|eno|ens|enp|enx|gnv|ipoe|l2tp|l2tpeth|" \
               "vtun|ppp|pppoe|peth|tun|vti|vxlan|wg|wlan|wlm)\d+(\.\d+(v.+)?)?$).*$"
-    if re.match(pattern, argv[1]):
-        exit(0)
+    if not re.match(pattern, vrf):
+        sys.exit(f'"{vrf}" is invalid as VRF name as it is an interface name')
 
-exit(1)
+    sys.exit(0)