From 3365207c987c91a86d44b788aa9a07a0bdbc0dd4 Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Fri, 23 Oct 2026 08:39:01 +0200 Subject: [PATCH] fix: what bandit and the remaining lint rules found - driver.py: the logger is defined after the imports (E402). The lazy paramiko client is annotated Any, as _node_ssh already is; the quoted paramiko.SSHClient named a module that is never imported there (F821). - Unused locals are gone: get_facts' hostname, get_users' groups (both read a value nobody used), the ARP flags column is _flags; the output is unchanged. - Long lines are wrapped: the apt-get and snmpd.conf commands are split into adjacent string literals, the power-action error message and the system mixin's docstring are wrapped; the text is unchanged. - get_vm_status re-raises with raise ... from e (B904). - The two except Exception: pass (closing the SSH session, the is_alive probe) now log at debug level (B110), so a swallowed failure leaves a trace. - nosec for the empty "password" field of NAPALM's get_users shape and the ssh_password_auth flag (B105); all three are false positives. - tests: unused loop variables. --- napalm_proxmox/driver.py | 12 ++++++------ napalm_proxmox/interfaces_mixin.py | 2 +- napalm_proxmox/system_mixin.py | 21 ++++++++++++++------- napalm_proxmox/vm_mixin.py | 4 +++- napalm_proxmox/vm_provision_mixin.py | 2 +- tests/test_misc.py | 6 +++--- 6 files changed, 28 insertions(+), 19 deletions(-) diff --git a/napalm_proxmox/driver.py b/napalm_proxmox/driver.py index f2021e0..f759947 100644 --- a/napalm_proxmox/driver.py +++ b/napalm_proxmox/driver.py @@ -32,8 +32,6 @@ from __future__ import annotations import logging from typing import Any -logger = logging.getLogger(__name__) - from napalm.base.exceptions import ConnectionException from napalm_device_types import ( AptProxyMixin, @@ -68,6 +66,8 @@ from napalm_proxmox.vm_mixin import ProxmoxVMMixin from napalm_proxmox.vm_provision_mixin import ProxmoxVMProvisionMixin from napalm_proxmox.vm_snapshot_mixin import ProxmoxVMSnapshotMixin +logger = logging.getLogger(__name__) + # --------------------------------------------------------------------------- # # Type aliases # --------------------------------------------------------------------------- # @@ -156,7 +156,8 @@ class ProxmoxDriver( self._api: ProxmoxAPI | None = None self._node_name: str = "" - self._ssh_client: paramiko.SSHClient | None = None + # paramiko.SSHClient; paramiko is imported lazily (see _node_ssh). + self._ssh_client: Any = None # Candidate config (merge/replace) self._candidate_config: str = "" @@ -257,7 +258,7 @@ class ProxmoxDriver( try: self._ssh_client.close() except Exception: - pass + logger.debug("%s: closing the SSH session: ignored", self.hostname, exc_info=True) self._ssh_client = None def is_alive(self) -> _JsonDict: @@ -274,7 +275,7 @@ class ProxmoxDriver( self._api.version.get() alive = True except Exception: - pass + logger.debug("%s: liveness probe failed", self.hostname, exc_info=True) return {"is_alive": alive} # ------------------------------------------------------------------ # @@ -432,7 +433,6 @@ class ProxmoxDriver( uptime = float(status.get("uptime", 0)) dns_search = dns.get("search", "") - hostname = self._node_name fqdn = f"{self._node_name}.{dns_search}" if dns_search else self.hostname iface_list = sorted(iface["iface"] for iface in network if iface.get("iface")) diff --git a/napalm_proxmox/interfaces_mixin.py b/napalm_proxmox/interfaces_mixin.py index 8eb7ec8..c8a19f9 100644 --- a/napalm_proxmox/interfaces_mixin.py +++ b/napalm_proxmox/interfaces_mixin.py @@ -174,7 +174,7 @@ class ProxmoxInterfaceMixin: parts = line.split() if len(parts) < 6: continue - ip_addr, _, flags, mac, _, iface = ( + ip_addr, _, _flags, mac, _, iface = ( parts[0], parts[1], parts[2], diff --git a/napalm_proxmox/system_mixin.py b/napalm_proxmox/system_mixin.py index d26c45b..0ce4cb6 100644 --- a/napalm_proxmox/system_mixin.py +++ b/napalm_proxmox/system_mixin.py @@ -12,7 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""System-level NAPALM getters for Proxmox VE nodes (environment, NTP, SNMP, users, packages, services, updates, disk SMART).""" +"""System-level NAPALM getters for Proxmox VE nodes. + +Environment, NTP, SNMP, users, packages, services, updates and disk SMART. +""" from __future__ import annotations @@ -224,7 +227,6 @@ class ProxmoxSystemMixin: if not uid: continue # Proxmox roles: Administrator -> 15, otherwise 1 - groups = user.get("groups", "") or "" level = 1 try: roles = self._api.access.users(uid).get() or {} # type: ignore[union-attr] @@ -234,7 +236,8 @@ class ProxmoxSystemMixin: logger.debug("Failed to fetch roles for user %s: %s", uid, exc) result[uid] = { "level": level, - "password": "", + # NAPALM's get_users shape; the password is never read. + "password": "", # nosec B105 "sshkeys": [], } @@ -252,7 +255,8 @@ class ProxmoxSystemMixin: if uname not in result and uid_int < 1000 or uid_int == 0: result[uname] = { "level": 15 if uid_int == 0 else 0, - "password": "", + # NAPALM's get_users shape; the password is never read. + "password": "", # nosec B105 "sshkeys": [], } @@ -445,7 +449,8 @@ class ProxmoxSystemMixin: raise ValueError(f"Invalid package name: {pkg!r}") pkg_args = " ".join(packages) output = self._exec_ssh_command( - f"DEBIAN_FRONTEND=noninteractive apt-get install --only-upgrade -y {pkg_args} 2>&1 || true" + "DEBIAN_FRONTEND=noninteractive apt-get install --only-upgrade -y " + f"{pkg_args} 2>&1 || true" ) return {"success": True, "output": output} @@ -475,7 +480,8 @@ class ProxmoxSystemMixin: port = 161 try: conf = self._exec_ssh_command( - "grep -E '^[[:space:]]*(ro|rw)?community' /etc/snmp/snmpd.conf 2>/dev/null | head -5" + "grep -E '^[[:space:]]*(ro|rw)?community' /etc/snmp/snmpd.conf " + "2>/dev/null | head -5" ) for line in conf.splitlines(): parts = line.split() @@ -809,7 +815,8 @@ class ProxmoxSystemMixin: "timezone": None, "ntp_servers": [], "ssh_port": 22, - "ssh_password_auth": False, + # A flag, not a password. + "ssh_password_auth": False, # nosec B105 } # Cluster name and node list from /cluster/status diff --git a/napalm_proxmox/vm_mixin.py b/napalm_proxmox/vm_mixin.py index 626acd7..b4379f9 100644 --- a/napalm_proxmox/vm_mixin.py +++ b/napalm_proxmox/vm_mixin.py @@ -387,7 +387,9 @@ class ProxmoxVMMixin: if action not in allowed: return { "success": False, - "error": f"Action '{action}' not supported for {vm_type} (allowed: {sorted(allowed)})", + "error": ( + f"Action '{action}' not supported for {vm_type} (allowed: {sorted(allowed)})" + ), } try: vm_api = self._node_api().qemu(vmid) if vm_type == "vm" else self._node_api().lxc(vmid) diff --git a/napalm_proxmox/vm_provision_mixin.py b/napalm_proxmox/vm_provision_mixin.py index b09d85d..6f339cc 100644 --- a/napalm_proxmox/vm_provision_mixin.py +++ b/napalm_proxmox/vm_provision_mixin.py @@ -939,7 +939,7 @@ class ProxmoxVMProvisionMixin: raise except Exception as e: _logger.exception(f"Error getting status for VM {vmid}: {e}") - raise RuntimeError(f"Failed to get VM {vmid} status: {e}") + raise RuntimeError(f"Failed to get VM {vmid} status: {e}") from e def get_network_targets(self) -> list[NetworkTargetDict]: """ diff --git a/tests/test_misc.py b/tests/test_misc.py index ebe03e2..a8fd6c2 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -86,7 +86,7 @@ class TestGetUsers: def test_user_has_required_keys(self, driver): driver._node_api().execute.post.return_value = {"data": ""} result = driver.get_users() - for user, data in result.items(): + for _user, data in result.items(): assert "level" in data assert "password" in data assert "sshkeys" in data @@ -249,7 +249,7 @@ class TestGetRouteTo: def test_route_fields(self, driver): driver._node_api().execute.post.return_value = {"data": self.ROUTE_OUTPUT} result = driver.get_route_to() - for prefix, routes in result.items(): + for _prefix, routes in result.items(): for route in routes: assert "protocol" in route assert "next_hop" in route @@ -258,7 +258,7 @@ class TestGetRouteTo: def test_protocol_filter(self, driver): driver._node_api().execute.post.return_value = {"data": self.ROUTE_OUTPUT} result = driver.get_route_to(protocol="dhcp") - for prefix, routes in result.items(): + for _prefix, routes in result.items(): for route in routes: assert "dhcp" in route["protocol"].lower()