Merge pull request #395 from thomas-mangin/T2417

validator: T2417: try to make the code clearer

7 files changed, 163 insertions, 96 deletions

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
new file mode 100755
index 000000000..37d9d0364
--- /dev/null
+++ b/src/validators/numeric
@@ -0,0 +1,66 @@
+#!/usr/bin/env python3
+#
+# numeric value validator
+#
+# Copyright (C) 2017 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# 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 sys
+import argparse
+
+
+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:
+        number = converter(args.number)
+    except:
+        sys.exit(f'{args.number} is not a valid {name} number')
+
+    if args.range:
+        try:
+            for r in args.range:
+                _lower, _upper = r.split('-')
+                lower = int(_lower)
+                upper = int(_upper)
+        except:
+            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}')
+
+    elif args.non_negative and number < 0:
+        sys.exit('Number should be non-negative')
+
+    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)