Merge pull request #4613 from c-po/add-pylint

T7648: Set up a linter check to check complete files for syntax errors and missing imports

23 files changed, 76 insertions, 91 deletions

diff --git a/Makefile b/Makefile
index 218fc4ae1..3235c7214 100644
--- a/Makefile
+++ b/Makefile
@@ -7,7 +7,6 @@ LIBS := -lzmq
 CFLAGS :=
 BUILD_ARCH := $(shell dpkg-architecture -q DEB_BUILD_ARCH)
 J2LINT := $(shell command -v j2lint 2> /dev/null)
-PYLINT_FILES := $(shell git ls-files *.py src/migration-scripts src/services)
 LIBVYOSCONFIG_BUILD_PATH := /tmp/libvyosconfig/_build/libvyosconfig.so
 LIBVYOSCONFIG_STATUS := $(shell git submodule status)
 
@@ -89,7 +88,7 @@ vyshim:
 	$(MAKE) -C $(SHIM_DIR)
 
 .PHONY: all
-all: clean copyright libvyosconfig interface_definitions op_mode_definitions test j2lint vyshim generate-configd-include-json
+all: clean copyright pylint libvyosconfig interface_definitions op_mode_definitions test j2lint vyshim generate-configd-include-json
 
 .PHONY: copyright
 copyright:
@@ -116,6 +115,11 @@ check_migration_scripts_executable:
 	@echo "Checking if migration scripts have executable bit set..."
 	find src/migration-scripts -type f -not -executable -print -exec false {} + || sh -c 'echo "Found files that are not executable! Add permissions." && exit 1'
 
+.PHONE: pylint
+pylint:
+	@echo Running "pylint --errors-only ..."
+	@PYTHONPATH=python/ pylint --errors-only $(shell git ls-files python/vyos/ifconfig/*.py python/vyos/utils/*.py src/conf_mode/*.py src/op_mode/*.py src/migration-scripts src/services/vyos*)
+
 .PHONY: j2lint
 j2lint:
 ifndef J2LINT
@@ -129,7 +133,7 @@ sonar:
 
 .PHONY: unused-imports
 unused-imports:
-	@pylint --disable=all --enable=W0611 $(PYLINT_FILES)
+	@pylint --disable=all --enable=W0611 $(shell git ls-files *.py src/migration-scripts src/services)
 
 deb:
 	dpkg-buildpackage -uc -us -tc -b
diff --git a/debian/control b/debian/control
index e58822e9f..90158b1db 100644
--- a/debian/control
+++ b/debian/control
@@ -10,8 +10,17 @@ Build-Depends:
   iproute2,
   libzmq3-dev,
   python3 (>= 3.10),
-# For QA
+# For QA/Lint
   pylint,
+  python3-vici (>= 5.7.2),
+  python3-fastapi,
+  python3-pyudev,
+  python3-systemd,
+  python3-certbot-nginx,
+  python3-pam,
+  python3-dbus,
+  python3-pyroute2,
+  python3-voluptuous,
 # For generating command definitions
   python3-lxml,
   python3-xmltodict,
diff --git a/python/vyos/ifconfig/control.py b/python/vyos/ifconfig/control.py
index e5672ed50..3cf3297b4 100644
--- a/python/vyos/ifconfig/control.py
+++ b/python/vyos/ifconfig/control.py
@@ -29,6 +29,8 @@ class Control(Section):
     _command_get = {}
     _command_set = {}
     _signature = {}
+    config = {}
+    ifname = None
 
     def __init__(self, **kargs):
         # some commands (such as operation comands - show interfaces, etc.)
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 787364483..3b3536301 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -23,10 +23,9 @@ from glob import glob
 
 from ipaddress import IPv4Network
 from ipaddress import IPv6Interface
-from netifaces import ifaddresses
-# this is not the same as socket.AF_INET/INET6
-from netifaces import AF_INET
-from netifaces import AF_INET6
+from netifaces import ifaddresses # pylint: disable = no-name-in-module
+from socket import AF_INET
+from socket import AF_INET6
 from netaddr import EUI
 from netaddr import mac_unix_expanded
 
diff --git a/python/vyos/utils/dict.py b/python/vyos/utils/dict.py
index e6ef943c6..ff21f0677 100644
--- a/python/vyos/utils/dict.py
+++ b/python/vyos/utils/dict.py
@@ -347,8 +347,6 @@ class FixedDict(dict):
     raise ConfigError(f'Option "{k}" has no defined default')
     """
 
-    from vyos import ConfigError
-
     def __init__(self, **options):
         self._allowed = options.keys()
         super().__init__(**options)
@@ -368,6 +366,7 @@ class FixedDict(dict):
         >>> d
         {'key': 'value'}
         """
+        from vyos import ConfigError
         if k not in self._allowed:
             raise ConfigError(f'Option "{k}" has no defined default')
         super().__setitem__(k, v)
diff --git a/python/vyos/utils/file.py b/python/vyos/utils/file.py
index 31c2361df..1e2de2b39 100644
--- a/python/vyos/utils/file.py
+++ b/python/vyos/utils/file.py
@@ -92,36 +92,6 @@ def read_json(fname, defaultonfailure=None):
             return defaultonfailure
         raise e
 
-def chown(path, user=None, group=None, recursive=False):
-    """ change file/directory owner """
-    from pwd import getpwnam
-    from grp import getgrnam
-
-    if user is None and group is None:
-        return False
-
-    # path may also be an open file descriptor
-    if not isinstance(path, int) and not os.path.exists(path):
-        return False
-
-    # keep current value if not specified otherwise
-    uid = -1
-    gid = -1
-
-    if user:
-        uid = getpwnam(user).pw_uid
-    if group:
-        gid = getgrnam(group).gr_gid
-
-    if recursive:
-        for dirpath, dirnames, filenames in os.walk(path):
-            os.chown(dirpath, uid, gid)
-            for filename in filenames:
-                os.chown(os.path.join(dirpath, filename), uid, gid)
-    else:
-        os.chown(path, uid, gid)
-    return True
-
 
 def chmod(path, bitmask):
     # path may also be an open file descriptor
@@ -175,12 +145,6 @@ def file_permissions(path):
     """ Return file permissions in string format, e.g '0755' """
     return oct(os.stat(path).st_mode)[4:]
 
-def makedir(path, user=None, group=None):
-    if os.path.exists(path):
-        return
-    os.makedirs(path, mode=0o755)
-    chown(path, user, group)
-
 def wait_for_inotify(file_path, pre_hook=None, event_type=None, timeout=None, sleep_interval=0.1):
     """ Waits for an inotify event to occur """
     if not os.path.dirname(file_path):
diff --git a/python/vyos/utils/misc.py b/python/vyos/utils/misc.py
index 0ffd82696..c7f7e7343 100644
--- a/python/vyos/utils/misc.py
+++ b/python/vyos/utils/misc.py
@@ -48,7 +48,7 @@ def install_into_config(conf, config_paths, override_prompt=True):
 
     for path in config_paths:
         if override_prompt and conf.exists(path) and not conf.is_multi(path):
-            if not ask_yes_no(f'Config node "{node}" already exists. Do you want to overwrite it?'):
+            if not ask_yes_no(f'Config node "{path}" already exists. Do you want to overwrite it?'):
                 continue
 
         try:
diff --git a/python/vyos/utils/network.py b/python/vyos/utils/network.py
index 2182642dd..0e2cc58cf 100644
--- a/python/vyos/utils/network.py
+++ b/python/vyos/utils/network.py
@@ -13,9 +13,10 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
+from socket import AF_INET
+from socket import AF_INET6
+
 def _are_same_ip(one, two):
-    from socket import AF_INET
-    from socket import AF_INET6
     from socket import inet_pton
     from vyos.template import is_ipv4
     # compare the binary representation of the IP
@@ -347,7 +348,7 @@ def is_ipv6_link_local(addr):
 
 def is_addr_assigned(ip_address, vrf=None, return_ifname=False, include_vrf=False) -> bool | str:
     """ Verify if the given IPv4/IPv6 address is assigned to any interface """
-    from netifaces import interfaces
+    from netifaces import interfaces # pylint: disable = no-name-in-module
     from vyos.utils.network import get_interface_config
     from vyos.utils.dict import dict_search
 
@@ -445,10 +446,8 @@ def is_subnet_connected(subnet, primary=False):
     from ipaddress import ip_address
     from ipaddress import ip_network
 
-    from netifaces import ifaddresses
-    from netifaces import interfaces
-    from netifaces import AF_INET
-    from netifaces import AF_INET6
+    from netifaces import ifaddresses # pylint: disable = no-name-in-module
+    from netifaces import interfaces # pylint: disable = no-name-in-module
 
     from vyos.template import is_ipv6
 
@@ -482,9 +481,7 @@ def is_subnet_connected(subnet, primary=False):
 def is_afi_configured(interface: str, afi):
     """ Check if given address family is configured, or in other words - an IP
     address is assigned to the interface. """
-    from netifaces import ifaddresses
-    from netifaces import AF_INET
-    from netifaces import AF_INET6
+    from netifaces import ifaddresses # pylint: disable = no-name-in-module
 
     if afi not in [AF_INET, AF_INET6]:
         raise ValueError('Address family must be in [AF_INET, AF_INET6]')
diff --git a/python/vyos/utils/permission.py b/python/vyos/utils/permission.py
index efd44bfeb..08bb7bdbc 100644
--- a/python/vyos/utils/permission.py
+++ b/python/vyos/utils/permission.py
@@ -15,23 +15,37 @@
 
 import os
 
-def chown(path, user, group):
+def chown(path, user=None, group=None, recursive=False):
     """ change file/directory owner """
     from pwd import getpwnam
     from grp import getgrnam
 
-    if user is None or group is None:
+    if user is None and group is None:
         return False
 
     # path may also be an open file descriptor
     if not isinstance(path, int) and not os.path.exists(path):
         return False
 
-    uid = getpwnam(user).pw_uid
-    gid = getgrnam(group).gr_gid
-    os.chown(path, uid, gid)
+    # keep current value if not specified otherwise
+    uid = -1
+    gid = -1
+
+    if user:
+        uid = getpwnam(user).pw_uid
+    if group:
+        gid = getgrnam(group).gr_gid
+
+    if recursive:
+        for dirpath, dirnames, filenames in os.walk(path):
+            os.chown(dirpath, uid, gid)
+            for filename in filenames:
+                os.chown(os.path.join(dirpath, filename), uid, gid)
+    else:
+        os.chown(path, uid, gid)
     return True
 
+
 def chmod(path, bitmask):
     # path may also be an open file descriptor
     if not isinstance(path, int) and not os.path.exists(path):
diff --git a/src/conf_mode/qos.py b/src/conf_mode/qos.py
index 326d617a2..4ea900bb8 100755
--- a/src/conf_mode/qos.py
+++ b/src/conf_mode/qos.py
@@ -15,7 +15,8 @@
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 
 from sys import exit
-from netifaces import interfaces
+
+from netifaces import interfaces # pylint: disable = no-name-in-module
 
 from vyos.base import Warning
 from vyos.config import Config
diff --git a/src/conf_mode/service_broadcast-relay.py b/src/conf_mode/service_broadcast-relay.py
index f7f2875f5..b3f38dd21 100755
--- a/src/conf_mode/service_broadcast-relay.py
+++ b/src/conf_mode/service_broadcast-relay.py
@@ -17,7 +17,7 @@
 import os
 
 from glob import glob
-from netifaces import AF_INET
+from socket import AF_INET
 from sys import exit
 
 from vyos.config import Config
diff --git a/src/conf_mode/service_dhcp-server.py b/src/conf_mode/service_dhcp-server.py
index 6bbe72e90..b404b1708 100755
--- a/src/conf_mode/service_dhcp-server.py
+++ b/src/conf_mode/service_dhcp-server.py
@@ -29,9 +29,9 @@ from vyos.template import render
 from vyos.utils.dict import dict_search
 from vyos.utils.dict import dict_search_args
 from vyos.utils.file import chmod_775
-from vyos.utils.file import chown
 from vyos.utils.file import makedir
 from vyos.utils.file import write_file
+from vyos.utils.permission import chown
 from vyos.utils.process import call
 from vyos.utils.network import interface_exists
 from vyos.utils.network import is_subnet_connected
diff --git a/src/conf_mode/service_dhcpv6-server.py b/src/conf_mode/service_dhcpv6-server.py
index 07dd08337..8344f99b4 100755
--- a/src/conf_mode/service_dhcpv6-server.py
+++ b/src/conf_mode/service_dhcpv6-server.py
@@ -25,11 +25,11 @@ from vyos.config import Config
 from vyos.template import render
 from vyos.utils.process import call
 from vyos.utils.file import chmod_775
-from vyos.utils.file import chown
 from vyos.utils.file import makedir
 from vyos.utils.file import write_file
 from vyos.utils.dict import dict_search
 from vyos.utils.network import is_subnet_connected
+from vyos.utils.permission import chown
 from vyos import ConfigError
 from vyos import airbag
 airbag.enable()
diff --git a/src/conf_mode/service_mdns_repeater.py b/src/conf_mode/service_mdns_repeater.py
index 360b94588..aa10a687e 100755
--- a/src/conf_mode/service_mdns_repeater.py
+++ b/src/conf_mode/service_mdns_repeater.py
@@ -18,7 +18,9 @@ import os
 
 from json import loads
 from sys import exit
-from netifaces import ifaddresses, AF_INET, AF_INET6
+from socket import AF_INET
+from socket import AF_INET6
+from netifaces import ifaddresses # pylint: disable = no-name-in-module
 
 from vyos.config import Config
 from vyos.configverify import verify_interface_exists
diff --git a/src/conf_mode/service_stunnel.py b/src/conf_mode/service_stunnel.py
index eb1d4dea2..5ea5b88b4 100644
--- a/src/conf_mode/service_stunnel.py
+++ b/src/conf_mode/service_stunnel.py
@@ -19,7 +19,7 @@ from shutil import rmtree
 
 from sys import exit
 
-from netifaces import AF_INET
+from socket import AF_INET
 from psutil import net_if_addrs
 
 from vyos.config import Config
diff --git a/src/conf_mode/system_login.py b/src/conf_mode/system_login.py
index 00cacccd1..234b24770 100755
--- a/src/conf_mode/system_login.py
+++ b/src/conf_mode/system_login.py
@@ -37,7 +37,7 @@ from vyos.utils.auth import get_current_user
 from vyos.utils.configfs import delete_cli_node
 from vyos.utils.configfs import add_cli_node
 from vyos.utils.dict import dict_search
-from vyos.utils.file import chown
+from vyos.utils.permission import chown
 from vyos.utils.process import cmd
 from vyos.utils.process import call
 from vyos.utils.process import run
diff --git a/src/migration-scripts/interfaces/0-to-1 b/src/migration-scripts/interfaces/0-to-1
index 6ed5f118f..c4417c4ca 100644
--- a/src/migration-scripts/interfaces/0-to-1
+++ b/src/migration-scripts/interfaces/0-to-1
@@ -20,6 +20,8 @@
 
 from vyos.configtree import ConfigTree
 
+base = ['interfaces', 'bridge']
+
 def migrate_bridge(config, tree, intf):
     # check if bridge-group exists
     tree_bridge = tree + ['bridge-group']
@@ -49,8 +51,6 @@ def migrate_bridge(config, tree, intf):
 
 
 def migrate(config: ConfigTree) -> None:
-    base = ['interfaces', 'bridge']
-
     if not config.exists(base):
         # Nothing to do
         return
diff --git a/src/migration-scripts/interfaces/5-to-6 b/src/migration-scripts/interfaces/5-to-6
index 45a6daef7..8f406b5c8 100644
--- a/src/migration-scripts/interfaces/5-to-6
+++ b/src/migration-scripts/interfaces/5-to-6
@@ -56,7 +56,7 @@ def copy_rtradv(c, old_base, interface):
         # cleanup boolean nodes in individual route
         route_base = new_base + ['route']
         if c.exists(route_base):
-            for route in config.list_nodes(route_base):
+            for route in c.list_nodes(route_base):
                 if c.exists(route_base + [route, 'remove-route']):
                     tmp = c.return_value(route_base + [route, 'remove-route'])
                     c.delete(route_base + [route, 'remove-route'])
@@ -66,7 +66,7 @@ def copy_rtradv(c, old_base, interface):
         # cleanup boolean nodes in individual prefix
         prefix_base = new_base + ['prefix']
         if c.exists(prefix_base):
-            for prefix in config.list_nodes(prefix_base):
+            for prefix in c.list_nodes(prefix_base):
                 if c.exists(prefix_base + [prefix, 'autonomous-flag']):
                     tmp = c.return_value(prefix_base + [prefix, 'autonomous-flag'])
                     c.delete(prefix_base + [prefix, 'autonomous-flag'])
diff --git a/src/migration-scripts/quagga/2-to-3 b/src/migration-scripts/quagga/2-to-3
index b619d1edc..2494abdae 100644
--- a/src/migration-scripts/quagga/2-to-3
+++ b/src/migration-scripts/quagga/2-to-3
@@ -15,6 +15,8 @@
 
 from vyos.configtree import ConfigTree
 
+# Just to avoid writing it so many times
+af_path = ['address-family', 'ipv4-unicast']
 
 def migrate_neighbor(config, neighbor_path, neighbor):
     if config.exists(neighbor_path):
@@ -106,9 +108,6 @@ def migrate(config: ConfigTree) -> None:
         # Nothing to do
         return
 
-    # Just to avoid writing it so many times
-    af_path = ['address-family', 'ipv4-unicast']
-
     # Check if BGP is actually configured and obtain the ASN
     asn_list = config.list_nodes(['protocols', 'bgp'])
     if asn_list:
diff --git a/src/migration-scripts/vrrp/1-to-2 b/src/migration-scripts/vrrp/1-to-2
index a717df29c..706537813 100644
--- a/src/migration-scripts/vrrp/1-to-2
+++ b/src/migration-scripts/vrrp/1-to-2
@@ -25,7 +25,7 @@ from vyos.configtree import ConfigTree
 # It was supported only under ethernet and bonding and their
 # respective vif, vif-s, and vif-c subinterfaces
 
-def get_vrrp_group(path):
+def get_vrrp_group(config, path):
     group = {"preempt": True, "rfc_compatibility": False, "disable": False}
 
     if config.exists(path + ["advertise-interval"]):
@@ -115,7 +115,7 @@ def migrate(config: ConfigTree) -> None:
             if config.exists(parent_path + vg_path):
                 pgroups = config.list_nodes(parent_path + vg_path)
                 for pg in pgroups:
-                    g = get_vrrp_group(parent_path + vg_path + [pg])
+                    g = get_vrrp_group(config, parent_path + vg_path + [pg])
                     g["interface"] = pi
                     g["vrid"] = pg
                     groups.append(g)
@@ -132,7 +132,7 @@ def migrate(config: ConfigTree) -> None:
                     if config.exists(parent_path + vif_vg_path):
                         vifgroups = config.list_nodes(parent_path + vif_vg_path)
                         for vif_group in vifgroups:
-                            g = get_vrrp_group(parent_path + vif_vg_path + [vif_group])
+                            g = get_vrrp_group(config, parent_path + vif_vg_path + [vif_group])
                             g["interface"] = "{0}.{1}".format(pi, vif)
                             g["vrid"] = vif_group
                             groups.append(g)
@@ -147,7 +147,7 @@ def migrate(config: ConfigTree) -> None:
                     if config.exists(parent_path + vifs_vg_path):
                         vifsgroups = config.list_nodes(parent_path + vifs_vg_path)
                         for vifs_group in vifsgroups:
-                            g = get_vrrp_group(parent_path + vifs_vg_path + [vifs_group])
+                            g = get_vrrp_group(config, parent_path + vifs_vg_path + [vifs_group])
                             g["interface"] = "{0}.{1}".format(pi, vif_s)
                             g["vrid"] = vifs_group
                             groups.append(g)
@@ -161,7 +161,7 @@ def migrate(config: ConfigTree) -> None:
                              vifc_vg_path = [pi, "vif-s", vif_s, "vif-c", vif_c, "vrrp", "vrrp-group"]
                              vifcgroups = config.list_nodes(parent_path + vifc_vg_path)
                              for vifc_group in vifcgroups:
-                                  g = get_vrrp_group(parent_path + vifc_vg_path + [vifc_group])
+                                  g = get_vrrp_group(config, parent_path + vifc_vg_path + [vifc_group])
                                   g["interface"] = "{0}.{1}.{2}".format(pi, vif_s, vif_c)
                                   g["vrid"] = vifc_group
                                   groups.append(g)
@@ -173,7 +173,7 @@ def migrate(config: ConfigTree) -> None:
         return
 
     # Otherwise, there is VRRP to convert
-     
+
     # Now convert the collected groups to the new syntax
     base_group_path = ["high-availability", "vrrp", "group"]
     sync_path = ["high-availability", "vrrp", "sync-group"]
diff --git a/src/op_mode/connect_disconnect.py b/src/op_mode/connect_disconnect.py
index 9035ce072..117e65713 100755
--- a/src/op_mode/connect_disconnect.py
+++ b/src/op_mode/connect_disconnect.py
@@ -18,6 +18,7 @@ import os
 import argparse
 
 from psutil import process_iter
+from time import sleep
 
 from vyos.configquery import ConfigTreeQuery
 from vyos.utils.process import call
@@ -68,7 +69,7 @@ def connect(interface):
             if ( count % 60 == 0 ):
                 print(f'Commit still in progress after {count}s - waiting')
             count += 1
-            time.sleep(1)
+            sleep(1)
         call('/usr/libexec/vyos/conf_mode/qos.py')
 
 def disconnect(interface):
diff --git a/src/op_mode/pki.py b/src/op_mode/pki.py
index 2ee57a458..0f144443b 100755
--- a/src/op_mode/pki.py
+++ b/src/op_mode/pki.py
@@ -417,7 +417,7 @@ def parse_san_string(san_string):
             output.append(ipaddress.IPv6Address(value))
         elif tag == 'dns' or tag == 'rfc822':
             output.append(value)
-    return
+    return output
 
 
 def generate_certificate_request(
diff --git a/src/op_mode/storage.py b/src/op_mode/storage.py
index 7e869e9d9..0f3fabe32 100755
--- a/src/op_mode/storage.py
+++ b/src/op_mode/storage.py
@@ -13,10 +13,8 @@
 #
 # 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 vyos.opmode
 
 from jinja2 import Template
@@ -28,9 +26,6 @@ Used:       {{used}} ({{use_percentage}}%)
 Available:  {{avail}} ({{avail_percentage}}%)
 """
 
-def _get_formatted_output():
-    return _get_system_storage()
-
 def show(raw: bool):
     from vyos.utils.disk import get_persistent_storage_stats
 
@@ -49,7 +44,7 @@ def show(raw: bool):
             tmpl = Template(output_tmpl)
             return tmpl.render(data).strip()
 
-    return output
+    return None
 
 if __name__ == '__main__':
     try:
@@ -59,4 +54,3 @@ if __name__ == '__main__':
     except (ValueError, vyos.opmode.Error) as e:
         print(e)
         sys.exit(1)
-