From 8a3fce85c0076e21f22304f0a45d44aa38f8959b Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 07:05:49 +0200 Subject: [PATCH 01/14] Fix undefined names (F821) Every one of these raises NameError if its code path is reached: - nodeapply: run_automation accumulated into an exitcode that only existed in run(), so any automation error crashed instead of being reported. It now keeps and returns its own, tracked separately from the exit code of the ssh commands: the early exit after the spawn loop tests that one, and folding automation failures into it would exit with children already running and their pipes abandoned. Both are reported at the real exits. - nodeconsole: redraw() reads firstnodename, which was local to do_screenshot(); promote it to a module global like the other drawing state. - nodedeploy: the redeploy path appended to a lockednodes list that did not exist yet. The block that follows re-reads the same lock state and acts on it, so drop the dead duplicate. - samples/nodeattrib_from_switch.py, misc/filterpasswd: missing import sys. - xcc3: fixuuid was never imported. xcc imports xcc3, so take a local copy the way the smm handler does instead of creating an import cycle. - httpapi: the async session call still passed the WSGI-era env and an extra argument to handle_async(), which has taken only querydict since the aiohttp port. Calling it correctly exposed that handle_async() registers an AsyncSession before raising on the discontinued long poll path, so every request to it would leak a session that is never reaped. It now only creates one when there is a websocket handler to yield it to. - messages: the InputFirmwareUpdate.filename property checked self.filebynode[node] with no node in scope. __init__ already validates every expanded path and nodefile() rechecks per node, so drop the checks. - pam: drop the python2 branches referencing unicode and raw_input. The server has been python3 only since the asyncio port. - cooltera: the sensor-name listing referenced a nonexistent sensors dict. The available sensors depend on the model, which is only known after reading the device, so list them from the same status data the readings use. - deltapdu, eatonpdu, geist: the not-implemented response in update() used node outside the loop, unlike retrieve() in the same files and unlike raritan/enlogic. - confluentdbgcli: stray self. on a module-level socket connect. --- confluent_client/bin/nodeapply | 15 ++++++++++----- confluent_client/bin/nodeconsole | 2 ++ confluent_client/bin/nodedeploy | 5 ----- .../samples/nodeattrib_from_switch.py | 1 + confluent_server/confluent/asynchttp.py | 10 +++++----- .../confluent/discovery/handlers/xcc3.py | 16 ++++++++++++++++ confluent_server/confluent/httpapi.py | 4 +--- confluent_server/confluent/messages.py | 8 ++------ confluent_server/confluent/pam.py | 18 +++--------------- .../plugins/hardwaremanagement/cooltera.py | 13 ++++++++----- .../plugins/hardwaremanagement/deltapdu.py | 3 ++- .../plugins/hardwaremanagement/eatonpdu.py | 3 ++- .../plugins/hardwaremanagement/geist.py | 3 ++- confluent_server/confluentdbgcli.py | 2 +- misc/filterpasswd | 2 ++ 15 files changed, 57 insertions(+), 48 deletions(-) diff --git a/confluent_client/bin/nodeapply b/confluent_client/bin/nodeapply index 7547895f..6d619f57 100755 --- a/confluent_client/bin/nodeapply +++ b/confluent_client/bin/nodeapply @@ -37,6 +37,7 @@ import confluent.sortutil as sortutil devnull = None def run_automation(noderange, category, c): + exitcode = 0 automationbynode = {} for res in c.update('/noderange/{0}/deployment/remote_config/run'.format(noderange), { 'category': category, @@ -64,7 +65,8 @@ def run_automation(noderange, category, c): if res.get('complete', False): del automationbynode[node] sys.stdout.write('{0}: Automation complete\n'.format(node)) - + return exitcode + def run(): global devnull @@ -104,10 +106,13 @@ def run(): pipedesc = {} pendingexecs = deque() exitcode = 0 + # Kept apart from exitcode: a failed automation run must not trip the + # early exit below, which would abandon ssh children already spawned. + autoexitcode = 0 c.stop_if_noderange_over(args[0], options.maxnodes) if options.automation: - run_automation(args[0], options.automation, c) + autoexitcode = run_automation(args[0], options.automation, c) nodemap = {} cmdparms = [] @@ -124,7 +129,7 @@ def run(): cmdstorun.append(['run_remote', script]) if not cmdstorun: if options.automation: - sys.exit(0) + sys.exit(autoexitcode) argparser.print_help() sys.exit(1) for res in c.read('/noderange/{0}/nodes/'.format(args[0])): @@ -145,7 +150,7 @@ def run(): else: pendingexecs.append((sshnode, cmdv)) if not all or exitcode: - sys.exit(exitcode) + sys.exit(exitcode | autoexitcode) rdy = poller.poll(10) while all: pernodeout = {} @@ -193,7 +198,7 @@ def run(): sys.stdout.flush() if all: rdy = poller.poll(10) - sys.exit(exitcode) + sys.exit(exitcode | autoexitcode) def run_cmdv(node, cmdv, all, poller, pipedesc): diff --git a/confluent_client/bin/nodeconsole b/confluent_client/bin/nodeconsole index fe6decc9..800da71c 100755 --- a/confluent_client/bin/nodeconsole +++ b/confluent_client/bin/nodeconsole @@ -790,6 +790,7 @@ numrows = 0 cwidth = 0 cheight = 0 imagedatabynode = {} +firstnodename = None def redraw(): for node in imagedatabynode: @@ -818,6 +819,7 @@ async def do_screenshot(): global streaming global resized global numrows + global firstnodename sess = client.Command() if streaming: asyncio.create_task(watch_input()) diff --git a/confluent_client/bin/nodedeploy b/confluent_client/bin/nodedeploy index 47b607eb..e47f8d35 100755 --- a/confluent_client/bin/nodedeploy +++ b/confluent_client/bin/nodedeploy @@ -133,11 +133,6 @@ def main(args): curr = nodeinfo[attr].get('value', '') if curr and node not in profilebynode: profilebynode[node] = curr - for lockinfo in c.read('/noderange/{0}/deployment/lock'.format(args.noderange)): - for node in lockinfo.get('databynode', {}): - lockstate = lockinfo['databynode'][node]['lock']['value'] - if lockstate == 'locked': - lockednodes.append(node) if args.profile and profilebynode: sys.stderr.write('The -r/--redeploy option cannot be used with a profile, it redeploys the current or pending profile\n') return 1 diff --git a/confluent_client/samples/nodeattrib_from_switch.py b/confluent_client/samples/nodeattrib_from_switch.py index eff3e394..ba133846 100644 --- a/confluent_client/samples/nodeattrib_from_switch.py +++ b/confluent_client/samples/nodeattrib_from_switch.py @@ -13,6 +13,7 @@ import confluent.client as cl import socket import struct +import sys c = cl.Command() macs = [] interface = sys.argv[1] diff --git a/confluent_server/confluent/asynchttp.py b/confluent_server/confluent/asynchttp.py index 29c937d5..76458f8d 100644 --- a/confluent_server/confluent/asynchttp.py +++ b/confluent_server/confluent/asynchttp.py @@ -119,12 +119,12 @@ def handle_async(querydict, wshandler=None): # This may be one of two things, a request for a new async stream # or a request for next data from async stream # httpapi otherwise handles requests an injecting them to queue - if 'asyncid' not in querydict or not querydict['asyncid']: + if wshandler and ('asyncid' not in querydict or not querydict['asyncid']): # This is a new request, create a new multiplexer - currsess = AsyncSession(wshandler) - if wshandler: - yield currsess - return + yield AsyncSession(wshandler) + return + # Without a websocket handler there is nobody to hand a session to, so do + # not register one that would never be reaped. raise Exception("Long polling asynchttp is discontinued") diff --git a/confluent_server/confluent/discovery/handlers/xcc3.py b/confluent_server/confluent/discovery/handlers/xcc3.py index 0a1c7590..e2d57095 100644 --- a/confluent_server/confluent/discovery/handlers/xcc3.py +++ b/confluent_server/confluent/discovery/handlers/xcc3.py @@ -12,11 +12,27 @@ # See the License for the specific language governing permissions and # limitations under the License. +import codecs import confluent.discovery.handlers.redfishbmc as redfishbmc +import confluent.util as util import socket +import struct import aiohmi.util.webclient as webclient +# Duplicated from the xcc handler rather than imported: xcc imports this +# module, so importing it back would be circular. smm carries its own copy of +# this for the same reason. +def fixuuid(baduuid): + # SMM dumps it out in hex + uuidprefix = (baduuid[:8], baduuid[9:13], baduuid[14:18]) + a = codecs.encode(struct.pack('= (3,): - if isinstance(username, str): username = username.encode(encoding) - if isinstance(service, str): service = service.encode(encoding) - else: - if isinstance(username, unicode): - username = username.encode(encoding) - if isinstance(password, unicode): - password = password.encode(encoding) - if isinstance(service, unicode): - service = service.encode(encoding) + if isinstance(username, str): username = username.encode(encoding) + if isinstance(service, str): service = service.encode(encoding) if b'\x00' in username or b'\x00' in service: self.code = 4 # PAM_SYSTEM_ERR in Linux-PAM @@ -242,11 +234,7 @@ if __name__ == "__main__": readline.redisplay() readline.set_pre_input_hook(hook) - if sys.version_info >= (3,): - getinput = input - else: - getinput = raw_input - result = getinput(prompt) + result = input(prompt) readline.set_pre_input_hook() return result diff --git a/confluent_server/confluent/plugins/hardwaremanagement/cooltera.py b/confluent_server/confluent/plugins/hardwaremanagement/cooltera.py index 528d79c6..b5238577 100644 --- a/confluent_server/confluent/plugins/hardwaremanagement/cooltera.py +++ b/confluent_server/confluent/plugins/hardwaremanagement/cooltera.py @@ -151,12 +151,9 @@ _sensors_by_node = {} async def read_sensors(element, node, configmanager): category, name = element[-2:] if len(element) == 3: - # just get names + # the request is for the names under a category, so that is the last + # element rather than the one before it category = name - name = 'all' - for sensor in sensors: - yield msg.ChildCollection(simplify_name(sensors[sensor][0])) - return if category in ('leds, fans'): return sn = _sensors_by_node.get(node, None) @@ -166,6 +163,12 @@ async def read_sensors(element, node, configmanager): statinfo = xml2stateinfo(statdata) _sensors_by_node[node] = (statinfo, time.time() + 1) sn = _sensors_by_node.get(node, None) + if len(element) == 3: + # the names are only known after reading the device, as the sensor + # set depends on the model + for sensor in sn[0] if sn else (): + yield msg.ChildCollection(simplify_name(sensor['name'])) + return if sn: yield msg.SensorReadings(sn[0], name=node) diff --git a/confluent_server/confluent/plugins/hardwaremanagement/deltapdu.py b/confluent_server/confluent/plugins/hardwaremanagement/deltapdu.py index e3da3786..02df6406 100644 --- a/confluent_server/confluent/plugins/hardwaremanagement/deltapdu.py +++ b/confluent_server/confluent/plugins/hardwaremanagement/deltapdu.py @@ -195,7 +195,8 @@ async def retrieve(nodes, element, configmanager, inputdata): async def update(nodes, element, configmanager, inputdata): if 'outlets' not in element: - yield msg.ConfluentResourceUnavailable(node, 'Not implemented') + for node in nodes: + yield msg.ConfluentResourceUnavailable(node, 'Not implemented') return timeout = 4 for node in nodes: diff --git a/confluent_server/confluent/plugins/hardwaremanagement/eatonpdu.py b/confluent_server/confluent/plugins/hardwaremanagement/eatonpdu.py index f26bd0bd..f6c0a3ac 100644 --- a/confluent_server/confluent/plugins/hardwaremanagement/eatonpdu.py +++ b/confluent_server/confluent/plugins/hardwaremanagement/eatonpdu.py @@ -327,7 +327,8 @@ async def retrieve(nodes, element, configmanager, inputdata): async def update(nodes, element, configmanager, inputdata): if 'outlets' not in element: - yield msg.ConfluentResourceUnavailable(node, 'Not implemented') + for node in nodes: + yield msg.ConfluentResourceUnavailable(node, 'Not implemented') return for node in nodes: gc = PDUClient(node, configmanager) diff --git a/confluent_server/confluent/plugins/hardwaremanagement/geist.py b/confluent_server/confluent/plugins/hardwaremanagement/geist.py index 9a3e0a8e..cb7bea1e 100644 --- a/confluent_server/confluent/plugins/hardwaremanagement/geist.py +++ b/confluent_server/confluent/plugins/hardwaremanagement/geist.py @@ -336,7 +336,8 @@ async def retrieve(nodes, element, configmanager, inputdata): async def update(nodes, element, configmanager, inputdata): if 'outlets' not in element: - yield msg.ConfluentResourceUnavailable(node, 'Not implemented') + for node in nodes: + yield msg.ConfluentResourceUnavailable(node, 'Not implemented') return for node in nodes: gc = GeistClient(node, configmanager) diff --git a/confluent_server/confluentdbgcli.py b/confluent_server/confluentdbgcli.py index 6c804cc3..eb7d5f45 100644 --- a/confluent_server/confluentdbgcli.py +++ b/confluent_server/confluentdbgcli.py @@ -20,7 +20,7 @@ import readline import socket connection = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) -self.connection.connect('/var/run/confluent/dbg.sock') +connection.connect('/var/run/confluent/dbg.sock') readline.parse_and_bind("tab: complete") readline.parse_and_bind("set bell-style none") diff --git a/misc/filterpasswd b/misc/filterpasswd index 1d2785bd..11adc3a8 100644 --- a/misc/filterpasswd +++ b/misc/filterpasswd @@ -1,3 +1,5 @@ +import sys + uidmin = 1000 uidmax = 60000 gidmin = 1000 From 5d9e30de7b5c1fe41bc1e1db82bb614401d5d043 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 07:07:05 +0200 Subject: [PATCH 02/14] Stop loop variables from shadowing what they iterate (B020) Each of these loops rebinds the name that holds the iterable. They work today because the iterable is evaluated once before the loop starts, but the name is then gone, so any later use reads a loop item instead of the collection. - nodeinventory: `for arg in args` / `for arg in arg.split(',')`. - confignet (common and debian copies): iname holds the comma separated interface list and is then reused for each interface in it. - xcc _get_agentless_firmware: adata holds the adapter query response and is then reused for each adapter. No behaviour change, just distinct names for distinct things. --- confluent_client/bin/nodeinventory | 4 ++-- confluent_osdeploy/common/profile/scripts/confignet | 12 ++++++------ .../debian/profiles/default/scripts/confignet | 12 ++++++------ confluent_server/aiohmi/redfish/oem/lenovo/xcc.py | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/confluent_client/bin/nodeinventory b/confluent_client/bin/nodeinventory index 6551c1b5..59eb57f3 100755 --- a/confluent_client/bin/nodeinventory +++ b/confluent_client/bin/nodeinventory @@ -121,8 +121,8 @@ if len(args) > 1: os.execlp('nodefirmware', 'nodefirmware', noderange) else: url = '/noderange/{0}/inventory/hardware/all/system' - for arg in args: - for arg in arg.split(','): + for rawarg in args: + for arg in rawarg.split(','): if arg == 'serial': filters.append(re.compile('serial number')) elif arg == 'model': diff --git a/confluent_osdeploy/common/profile/scripts/confignet b/confluent_osdeploy/common/profile/scripts/confignet index 0108c540..9a972d55 100644 --- a/confluent_osdeploy/common/profile/scripts/confignet +++ b/confluent_osdeploy/common/profile/scripts/confignet @@ -625,18 +625,18 @@ if __name__ == '__main__': time.sleep(1) continue dc = json.loads(dc) - iname = get_interface_name(idxmap[curridx], nc.get('default', {})) - if iname: - for iname in iname.split(','): + inames = get_interface_name(idxmap[curridx], nc.get('default', {})) + if inames: + for iname in inames.split(','): if 'default' in netname_to_interfaces: netname_to_interfaces['default']['interfaces'].add(iname) else: netname_to_interfaces['default'] = {'interfaces': set([iname]), 'settings': nc['default']} for netname in nc.get('extranets', {}): uname = '_' + netname - iname = get_interface_name(idxmap[curridx], nc['extranets'][netname]) - if iname: - for iname in iname.split(','): + inames = get_interface_name(idxmap[curridx], nc['extranets'][netname]) + if inames: + for iname in inames.split(','): if uname in netname_to_interfaces: netname_to_interfaces[uname]['interfaces'].add(iname) else: diff --git a/confluent_osdeploy/debian/profiles/default/scripts/confignet b/confluent_osdeploy/debian/profiles/default/scripts/confignet index 91734b5a..61d8d4ff 100644 --- a/confluent_osdeploy/debian/profiles/default/scripts/confignet +++ b/confluent_osdeploy/debian/profiles/default/scripts/confignet @@ -545,18 +545,18 @@ if __name__ == '__main__': time.sleep(1) continue dc = json.loads(dc) - iname = get_interface_name(idxmap[curridx], nc.get('default', {})) - if iname: - for iname in iname.split(','): + inames = get_interface_name(idxmap[curridx], nc.get('default', {})) + if inames: + for iname in inames.split(','): if 'default' in netname_to_interfaces: netname_to_interfaces['default']['interfaces'].add(iname) else: netname_to_interfaces['default'] = {'interfaces': set([iname]), 'settings': nc['default']} for netname in nc.get('extranets', {}): uname = '_' + netname - iname = get_interface_name(idxmap[curridx], nc['extranets'][netname]) - if iname: - for iname in iname.split(','): + inames = get_interface_name(idxmap[curridx], nc['extranets'][netname]) + if inames: + for iname in inames.split(','): if uname in netname_to_interfaces: netname_to_interfaces[uname]['interfaces'].add(iname) else: diff --git a/confluent_server/aiohmi/redfish/oem/lenovo/xcc.py b/confluent_server/aiohmi/redfish/oem/lenovo/xcc.py index f41a252c..a8343cd1 100644 --- a/confluent_server/aiohmi/redfish/oem/lenovo/xcc.py +++ b/confluent_server/aiohmi/redfish/oem/lenovo/xcc.py @@ -623,12 +623,12 @@ class OEMHandler(generic.OEMHandler): async def _get_agentless_firmware(self, components): skipkeys = set([]) wc = await self.wc() - adata = await wc.grab_json_response( + adapterdata = await wc.grab_json_response( '/api/dataset/imm_adapters?params=pci_GetAdapters') fdata = await wc.grab_json_response( '/api/function/adapter_update?params=pci_GetAdapterListAndFW') anames = set() - for adata in adata.get('items', []): + for adata in adapterdata.get('items', []): baseaname = adata['adapterName'] aname = baseaname idx = 1 From b119de345b731549c16a55695aff40ec28735bf6 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 07:08:39 +0200 Subject: [PATCH 03/14] Remove shadowed duplicate definitions (F811) Three names were defined twice in the same scope, so the first definition was unreachable: - lenovo OEM handler: two set_user_access methods, the second silently replacing the first. That made the SMM privilege update dead code. The conditions are mutually exclusive (is_fpc returns None once has_xcc is true), so merge both into the surviving method. - redfish plugin handle_cert_authorities and prepfish disable_host_interface: byte identical copies, drop the redundant one. --- .../aiohmi/ipmi/oem/lenovo/handler.py | 6 ++--- .../plugins/hardwaremanagement/redfish.py | 22 ------------------- misc/prepfish.py | 4 ---- 3 files changed, 2 insertions(+), 30 deletions(-) diff --git a/confluent_server/aiohmi/ipmi/oem/lenovo/handler.py b/confluent_server/aiohmi/ipmi/oem/lenovo/handler.py index 2999ebdf..adea804e 100755 --- a/confluent_server/aiohmi/ipmi/oem/lenovo/handler.py +++ b/confluent_server/aiohmi/ipmi/oem/lenovo/handler.py @@ -409,6 +409,8 @@ class OEMHandler(generic.OEMHandler): privilege_level): if await self.is_fpc() and self._fpc_variant != 6: await self.smmhandler.set_user_priv(uid, privilege_level) + if await self.has_xcc(): + await self.immhandler.set_user_access(uid, privilege_level) async def is_fpc(self): """True if the target is a Lenovo nextscale fan power controller""" @@ -1361,10 +1363,6 @@ class OEMHandler(generic.OEMHandler): return await self.immhandler.get_user_privilege_level(uid) return None - async def set_user_access(self, uid, channel, callback, link_auth, ipmi_msg, privilege_level): - if await self.has_xcc(): - await self.immhandler.set_user_access(uid, privilege_level) - async def process_zero_fru(self, zerofru): if (self.oemid['manufacturer_id'] == 19046 and self.oemid['product_id'] == 13616): diff --git a/confluent_server/confluent/plugins/hardwaremanagement/redfish.py b/confluent_server/confluent/plugins/hardwaremanagement/redfish.py index 51a4fe4b..ebc64d8f 100644 --- a/confluent_server/confluent/plugins/hardwaremanagement/redfish.py +++ b/confluent_server/confluent/plugins/hardwaremanagement/redfish.py @@ -637,28 +637,6 @@ class IpmiHandler: os.unlink(certname) await self.ipmicmd.install_bmc_certificate(cert) - async def handle_cert_authorities(self): - if len(self.element) == 3: - if self.op == 'read': - async for cert in self.ipmicmd.get_trusted_cas(): - await self.output.put(msg.ChildCollection(cert['id'])) - elif self.op == 'update': - cert = self.inputdata.get_pem(self.node) - await self.ipmicmd.add_trusted_ca(cert) - elif len(self.element) == 4: - certid = self.element[-1] - if self.op == 'read': - async for certdata in self.ipmicmd.get_trusted_cas(): - if certdata['id'] == certid: - await self.output.put(msg.CertificateAuthority( - pem=certdata['pem'], - node=self.node, - subject=certdata['subject'], - san=certdata.get('san', None))) - elif self.op == 'delete': - await self.ipmicmd.del_trusted_ca(certid) - return - async def handle_alerts(self): if self.element[3] == 'destinations': if len(self.element) == 4: diff --git a/misc/prepfish.py b/misc/prepfish.py index 8b9bc914..79861114 100644 --- a/misc/prepfish.py +++ b/misc/prepfish.py @@ -232,10 +232,6 @@ def dotwait(): sys.stderr.flush() time.sleep(0.5) -def disable_host_interface(): - s = Session('/dev/ipmi0') - s.raw_command(netfn=0xc, command=1, data=(1, 0xc1, 0)) - def get_redfish_creds(): os.makedirs('/run/redfish', exist_ok=True, mode=0o700) try: From fcacaca79d85b61209a37bcddd68520604241f64 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 07:09:15 +0200 Subject: [PATCH 04/14] Use a raw string for a regex escape (W605) '\s' is not a recognised string escape. Python still accepts it today but warns, and it becomes a syntax error in a future release. --- confluent_osdeploy/esxi7/profiles/hypervisor/scripts/makeksnet | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/confluent_osdeploy/esxi7/profiles/hypervisor/scripts/makeksnet b/confluent_osdeploy/esxi7/profiles/hypervisor/scripts/makeksnet index 7dcf966a..c6c310b2 100644 --- a/confluent_osdeploy/esxi7/profiles/hypervisor/scripts/makeksnet +++ b/confluent_osdeploy/esxi7/profiles/hypervisor/scripts/makeksnet @@ -2,7 +2,7 @@ import re import subprocess import json -uplinkmatch = re.compile('^\s*Uplinks:\s*(.*)') +uplinkmatch = re.compile(r'^\s*Uplinks:\s*(.*)') nodename = None for inf in open('/etc/confluent/confluent.info', 'r').read().split('\n'): if inf.startswith('NODENAME: '): From 9984bff9092b3170f10f961be7924f8dade23aa1 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 07:09:26 +0200 Subject: [PATCH 05/14] Fix the dedicated hotspare drive list (B035) The DedicatedSpareDrives payload was built as a set containing a list containing a dict comprehension with a constant key, so it collapsed to a single entry and then raised TypeError on the unhashable list. Build a list of drive references, the same shape as the Drives list just above it. --- confluent_server/aiohmi/redfish/oem/lenovo/xcc3.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/confluent_server/aiohmi/redfish/oem/lenovo/xcc3.py b/confluent_server/aiohmi/redfish/oem/lenovo/xcc3.py index fb715b4a..41f8dc42 100644 --- a/confluent_server/aiohmi/redfish/oem/lenovo/xcc3.py +++ b/confluent_server/aiohmi/redfish/oem/lenovo/xcc3.py @@ -684,8 +684,8 @@ class OEMHandler(generic.OEMHandler): "Drives":[ {'@odata.id': f'/redfish/v1/Systems/1/Storage/{cid}/Drives/{did}'} for did in spec_disks]}} if spec_hotspares: - request_data["Links"]["DedicatedSpareDrives"] = {[ - {'@odata.id': f'/redfish/v1/Systems/1/Storage/{cid}/Drives/{hid}' for hid in spec_hotspares}]} + request_data["Links"]["DedicatedSpareDrives"] = [ + {'@odata.id': f'/redfish/v1/Systems/1/Storage/{cid}/Drives/{hid}'} for hid in spec_hotspares] if volsize: request_data["CapacityBytes"] = volsize if stripsize: From 607845bacf345d03a23e25f870c10d7818c7a8aa Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 07:09:46 +0200 Subject: [PATCH 06/14] Stop the plugin loader from shadowing the plugin module (F402) load_plugins() used `plugin` as the loop variable for plugin file names, which shadows `import confluent.plugin as plugin` for the whole function. Nothing in the function needed the module, so this was latent rather than broken, but the next line that does need it would have failed oddly. --- confluent_server/confluent/core.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/confluent_server/confluent/core.py b/confluent_server/confluent/core.py index 46e3d223..0c4491ca 100644 --- a/confluent_server/confluent/core.py +++ b/confluent_server/confluent/core.py @@ -131,24 +131,24 @@ def load_plugins(): continue sys.path.insert(1, plugindir) # two passes, to avoid adding both py and pyc files - for plugin in os.listdir(plugindir): - if plugin.startswith('.'): + for pluginname in os.listdir(plugindir): + if pluginname.startswith('.'): continue - if '__pycache__' in plugin: + if '__pycache__' in pluginname: continue - (plugin, plugtype) = os.path.splitext(plugin) + (pluginname, plugtype) = os.path.splitext(pluginname) if plugtype == '.sh': - pluginmap[plugin] = shellmodule.Plugin( - os.path.join(plugindir, plugin + '.sh')) - elif "__init__" not in plugin: - plugins.add(plugin) - for plugin in plugins: - tmpmod = __import__(plugin) + pluginmap[pluginname] = shellmodule.Plugin( + os.path.join(plugindir, pluginname + '.sh')) + elif "__init__" not in pluginname: + plugins.add(pluginname) + for pluginname in plugins: + tmpmod = __import__(pluginname) if 'plugin_names' in tmpmod.__dict__: for name in tmpmod.plugin_names: pluginmap[name] = tmpmod else: - pluginmap[plugin] = tmpmod + pluginmap[pluginname] = tmpmod _register_resource(tmpmod) plugins.clear() # restore path to not include the plugindir From 938070c5b7a2dec43b6deb80da021ffb2717c244 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 07:23:49 +0200 Subject: [PATCH 07/14] Skip pending nodes that have no handler The guard evaluated the `next` builtin and discarded it, which does nothing, so a pending node with no handler fell through to None.NodeHandler(...). The AttributeError was caught by the enclosing except and logged as "Unexpected error during discovery", turning a node that should have been quietly skipped into a spurious error in the log. --- confluent_server/confluent/discovery/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/confluent_server/confluent/discovery/core.py b/confluent_server/confluent/discovery/core.py index d028842a..20f388ec 100644 --- a/confluent_server/confluent/discovery/core.py +++ b/confluent_server/confluent/discovery/core.py @@ -714,7 +714,7 @@ async def _recheck_nodes_backend(nodeattribs, configmanager): info = pending_nodes[nodename] try: if info['handler'] is None: - next + continue handler = info['handler'].NodeHandler(info, configmanager) tasks.spawn(eval_node(configmanager, handler, info, nodename)) except Exception: From 3d79c3535d7ec0009ba62f26b1f64bfe2ef1f087 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 07:23:49 +0200 Subject: [PATCH 08/14] Add ruff configuration and a CI job The enforced rule set is deliberately narrow: undefined names, statements in impossible positions, duplicate definitions, invalid escapes and a couple of bugbear checks that only fire on genuine defects. No style rules, and the tree is clean under it as of the preceding commits. Discovery needs help. Ruff only walks *.py, and about a quarter of the Python here has no extension: every node* CLI tool, the server bin tools, the osdeploy scripts (some of which carry no shebang either) and the setup.py templates. extend-include lists them, and *.sh is excluded so the shell scripts sharing those directories are not parsed as Python. The CI job pins both the action and the ruff version, since there is no pyproject.toml for the action to read a version from and an unpinned `latest` would let a new ruff release fail an unchanged branch. --- .github/workflows/ci.yml | 19 +++++++++++ ruff.toml | 71 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 ruff.toml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6a07cc4b..c30d00e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,6 +27,25 @@ jobs: done } | sort -u | xargs -d '\n' shellcheck --severity=error --exclude=SC2148 + ruff: + name: Ruff + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + # Pinned to an exact release: unlike actions/checkout, ruff-action + # publishes no moving major tag past v3, so @v4 does not resolve. + - uses: astral-sh/ruff-action@v4.1.0 + with: + # There is no pyproject.toml for the action to read a version from, + # so pin it here: an unpinned ruff would resolve to `latest` and a + # new release could turn a green branch red on its own. Bump this + # deliberately, together with the rule set in ruff.toml. + version: 0.15.21 + # Rule selection, file discovery (the many extensionless Python + # executables) and exclusions all live in ruff.toml, so the whole + # workspace can be handed over as-is. + args: check --output-format=github + python-compileall: name: Python compileall runs-on: ubuntu-latest diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 00000000..5c19ab7d --- /dev/null +++ b/ruff.toml @@ -0,0 +1,71 @@ +# Ruff configuration for confluent. +# +# py37 is the oldest version ruff can target. The oldest interpreter parts of +# this tree still run on is 3.6 (el8, sles15) +target-version = "py37" + +line-length = 120 + +# Ruff discovers *.py only: it does not read shebangs when walking a tree, so +# without the patterns below it silently skips every CLI tool in +# confluent_client/bin and confluent_server/bin, the osdeploy deploy scripts, +# and the loose misc/ utilities. +# Some of the osdeploy scripts carry no shebang at all. +extend-include = [ + "confluent_client/bin/*", + "confluent_server/bin/*", + # Generated into setup.py at build time by makesetup; #VERSION# only ever + # appears inside a string literal, so the template itself is valid Python. + "**/setup.py.tmpl", + "imgutil/imgutil", + "misc/filterpasswd", + "misc/getipsfromswitchport", + "misc/getusbnicaddr", + # confluent_osdeploy: per-profile deploy scripts, matched by name because + # each profile directory mixes Python and shell. + "**/bfb-autoinstall", + "**/nodedeploy-bfb", + "**/opt/confluent/bin/apiclient", + "**/scripts/add_local_repositories", + "**/scripts/autoconsole", + "**/scripts/configbmc", + "**/scripts/confignet", + "**/scripts/getinstalldisk", + "**/scripts/makeksnet", + "**/scripts/mergetime", + "**/scripts/syncfileclient", +] + +extend-exclude = [ + # Shell scripts that live in the directories included wholesale above. + "*.sh", +] + +# Honour exclusions even when CI passes an explicit file list. +force-exclude = true + +# `ruff format` is deliberately not adopted: the tree uses single quotes almost +# everywhere and reformatting it would bury real changes. Preserve quotes so an +# accidental run does less damage. +[format] +quote-style = "preserve" + +[lint] +select = [ + "E9", # unparseable file + "F63", # `is` against a literal, assert on a tuple, bad print/if-tuple + "F7", # statements in impossible positions: return/yield outside a + # function, break/continue outside a loop, except clause not last + "F81", # redefinition of an unused name (shadowed def/class) + "F82", # undefined name, undefined name in __all__, use before assignment + "F402", # import shadowed by a loop variable + "PLE", # pylint errors: bad string format, invalid returns, ... + "T100", # forgotten pdb/breakpoint call + "W6", # invalid escape sequence in a non-raw string, and any future + # deprecated-construct warning pycodestyle adds + "B020", # loop control variable overrides the iterable it iterates + "B035", # dict comprehension with a static key +] + +# No ignores and no per-file exemptions: every rule selected above is expected +# to stay at zero on its own. From 644843b8926eb0f6c5652b464da7c00fcb1f4012 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 14:12:50 +0200 Subject: [PATCH 09/14] Remove unused imports and pointless f-string prefixes (F401, F541, E713) Entirely mechanical, produced by `ruff check --fix --select F401,F541,E713` and reviewed rather than taken on faith: deleting an import is only safe if nothing imports it for its side effects or re-exports it. None of the 19 removed names is referenced anywhere in its file, none appears in any string literal, and none of the touched files uses eval, exec, globals() or __import__, so there is no dynamic lookup that could reach them. --- confluent_client/bin/confetty | 1 - confluent_client/bin/confluent2hosts | 1 - confluent_client/bin/nodebmcpassword | 2 +- confluent_client/bin/nodeconsole | 2 +- confluent_client/bin/nodediscover | 1 - confluent_client/bin/nodelicense | 1 - confluent_client/bin/nodersync | 1 - .../common/initramfs/opt/confluent/bin/apiclient | 2 -- confluent_osdeploy/common/profile/scripts/syncfileclient | 1 - .../el7-diskless/profiles/default/scripts/syncfileclient | 1 - .../el7/profiles/default/scripts/add_local_repositories | 1 - .../el7/profiles/default/scripts/syncfileclient | 1 - .../el8/profiles/default/scripts/add_local_repositories | 1 - .../esxi7/profiles/hypervisor/scripts/getinstalldisk | 4 ++-- .../ubuntu22.04/profiles/default/scripts/mergetime | 1 - confluent_server/bin/confluent | 1 - confluent_server/bin/confluent_selfcheck | 2 -- confluent_server/confluent/plugins/console/tsmsol.py | 1 - confluent_server/setup.py.tmpl | 1 - misc/getipsfromswitchport | 4 ++-- misc/getusbnicaddr | 1 - 21 files changed, 6 insertions(+), 25 deletions(-) diff --git a/confluent_client/bin/confetty b/confluent_client/bin/confetty index 4fe8ebb8..11272271 100755 --- a/confluent_client/bin/confetty +++ b/confluent_client/bin/confetty @@ -41,7 +41,6 @@ # esc-( would interfere with normal esc use too much # ~ I will not use for now... -import math import getpass import optparse import os diff --git a/confluent_client/bin/confluent2hosts b/confluent_client/bin/confluent2hosts index 66af0f48..5a212a33 100644 --- a/confluent_client/bin/confluent2hosts +++ b/confluent_client/bin/confluent2hosts @@ -14,7 +14,6 @@ path = os.path.realpath(os.path.join(path, '..', 'lib', 'python')) if path.startswith('/opt'): sys.path.append(path) import confluent.client as client -import confluent.sortutil as sortutil def partitionhostsline(line): comment = '' diff --git a/confluent_client/bin/nodebmcpassword b/confluent_client/bin/nodebmcpassword index 0b8883bd..c2f0343c 100755 --- a/confluent_client/bin/nodebmcpassword +++ b/confluent_client/bin/nodebmcpassword @@ -92,7 +92,7 @@ for rsp in session.read('/noderange/{0}/configuration/management_controller/user continue for user in rsp['databynode'][node]['users']: if user['username'] == username: - if not user['uid'] in uid_dict: + if user['uid'] not in uid_dict: uid_dict[user['uid']] = node continue uid_dict[user['uid']] = uid_dict[user['uid']] + ',{}'.format(node) diff --git a/confluent_client/bin/nodeconsole b/confluent_client/bin/nodeconsole index 800da71c..da274319 100755 --- a/confluent_client/bin/nodeconsole +++ b/confluent_client/bin/nodeconsole @@ -894,7 +894,7 @@ async def do_screenshot(): imgdata = res['databynode'][node].get('image', {}).get('imgdata', None) if imgdata: if len(imgdata) < 32: # We were subjected to error - errorstr = f'Unable to get screenshot' + errorstr = 'Unable to get screenshot' if errorstr or imgdata: imgdata = base64.b64decode(imgdata) draw_node(node, imgdata, errorstr, firstnodename, cwidth, cheight) diff --git a/confluent_client/bin/nodediscover b/confluent_client/bin/nodediscover index 4c45a933..cc2ffee6 100755 --- a/confluent_client/bin/nodediscover +++ b/confluent_client/bin/nodediscover @@ -27,7 +27,6 @@ if path.startswith('/opt'): sys.path.append(path) import confluent.asynclient as client -import confluent.sortutil as sortutil defcolumns = ['Node', 'Model', 'Serial', 'UUID', 'Mac Address', 'Type', 'Current IP Addresses'] diff --git a/confluent_client/bin/nodelicense b/confluent_client/bin/nodelicense index b13140da..cb5fb136 100755 --- a/confluent_client/bin/nodelicense +++ b/confluent_client/bin/nodelicense @@ -19,7 +19,6 @@ import optparse import os import signal import sys -import time try: signal.signal(signal.SIGPIPE, signal.SIG_DFL) diff --git a/confluent_client/bin/nodersync b/confluent_client/bin/nodersync index 8d316bab..0c427e86 100755 --- a/confluent_client/bin/nodersync +++ b/confluent_client/bin/nodersync @@ -35,7 +35,6 @@ if path.startswith('/opt'): import confluent.client as client import confluent.screensqueeze as sq -import confluent.sortutil as sortutil def run(): diff --git a/confluent_osdeploy/common/initramfs/opt/confluent/bin/apiclient b/confluent_osdeploy/common/initramfs/opt/confluent/bin/apiclient index 1ae22689..e08097a9 100644 --- a/confluent_osdeploy/common/initramfs/opt/confluent/bin/apiclient +++ b/confluent_osdeploy/common/initramfs/opt/confluent/bin/apiclient @@ -6,14 +6,12 @@ except ImportError: import base64 import ctypes import ctypes.util -import glob import os import select import socket import subprocess import ssl import sys -import struct import time import re import hashlib diff --git a/confluent_osdeploy/common/profile/scripts/syncfileclient b/confluent_osdeploy/common/profile/scripts/syncfileclient index 99687df2..7688bd13 100644 --- a/confluent_osdeploy/common/profile/scripts/syncfileclient +++ b/confluent_osdeploy/common/profile/scripts/syncfileclient @@ -2,7 +2,6 @@ import random import time import subprocess -import importlib import tempfile import json import os diff --git a/confluent_osdeploy/el7-diskless/profiles/default/scripts/syncfileclient b/confluent_osdeploy/el7-diskless/profiles/default/scripts/syncfileclient index 69283a13..ca5a47a6 100644 --- a/confluent_osdeploy/el7-diskless/profiles/default/scripts/syncfileclient +++ b/confluent_osdeploy/el7-diskless/profiles/default/scripts/syncfileclient @@ -1,6 +1,5 @@ #!/usr/bin/python import time -import importlib import tempfile import json import os diff --git a/confluent_osdeploy/el7/profiles/default/scripts/add_local_repositories b/confluent_osdeploy/el7/profiles/default/scripts/add_local_repositories index 12cecbc1..efe677d8 100644 --- a/confluent_osdeploy/el7/profiles/default/scripts/add_local_repositories +++ b/confluent_osdeploy/el7/profiles/default/scripts/add_local_repositories @@ -4,7 +4,6 @@ except ImportError: import ConfigParser as configparser import cStringIO import imp -import sys apiclient = imp.load_source('apiclient', '/etc/confluent/apiclient') repo = None server = None diff --git a/confluent_osdeploy/el7/profiles/default/scripts/syncfileclient b/confluent_osdeploy/el7/profiles/default/scripts/syncfileclient index c17cf52e..e79c77ed 100644 --- a/confluent_osdeploy/el7/profiles/default/scripts/syncfileclient +++ b/confluent_osdeploy/el7/profiles/default/scripts/syncfileclient @@ -1,5 +1,4 @@ #!/usr/bin/python -import importlib import tempfile import json import os diff --git a/confluent_osdeploy/el8/profiles/default/scripts/add_local_repositories b/confluent_osdeploy/el8/profiles/default/scripts/add_local_repositories index c3bc7e68..64b88aba 100644 --- a/confluent_osdeploy/el8/profiles/default/scripts/add_local_repositories +++ b/confluent_osdeploy/el8/profiles/default/scripts/add_local_repositories @@ -5,7 +5,6 @@ except ImportError: import cStringIO import importlib.util import importlib.machinery -import sys import glob modloader = importlib.machinery.SourceFileLoader('apiclient', '/opt/confluent/bin/apiclient') modspec = importlib.util.spec_from_file_location('apiclient', '/opt/confluent/bin/apiclient', loader=modloader) diff --git a/confluent_osdeploy/esxi7/profiles/hypervisor/scripts/getinstalldisk b/confluent_osdeploy/esxi7/profiles/hypervisor/scripts/getinstalldisk index 419f5224..8e725b08 100644 --- a/confluent_osdeploy/esxi7/profiles/hypervisor/scripts/getinstalldisk +++ b/confluent_osdeploy/esxi7/profiles/hypervisor/scripts/getinstalldisk @@ -229,8 +229,8 @@ def main(): sc.write(f'install --drive={nd[0]} --overwritevmfs\n') else: with open('/tmp/storagecfg', 'w') as sc: - sc.write(f'clearpart --firstdisk --overwritevmfs\n') - sc.write(f'install --firstdisk --overwritevmfs\n') + sc.write('clearpart --firstdisk --overwritevmfs\n') + sc.write('install --firstdisk --overwritevmfs\n') if __name__ == '__main__': diff --git a/confluent_osdeploy/ubuntu22.04/profiles/default/scripts/mergetime b/confluent_osdeploy/ubuntu22.04/profiles/default/scripts/mergetime index 7edb2632..261b1f88 100644 --- a/confluent_osdeploy/ubuntu22.04/profiles/default/scripts/mergetime +++ b/confluent_osdeploy/ubuntu22.04/profiles/default/scripts/mergetime @@ -1,6 +1,5 @@ #!/usr/bin/python3 import yaml -import os ainst = {} with open('/autoinstall.yaml', 'r') as allin: diff --git a/confluent_server/bin/confluent b/confluent_server/bin/confluent index 5eebfd75..6f8d2d1f 100755 --- a/confluent_server/bin/confluent +++ b/confluent_server/bin/confluent @@ -32,7 +32,6 @@ import confluent.main #p = cProfile.Profile(time.clock) #p.enable() #try: -import multiprocessing def main(): confluent.main.run(sys.argv) diff --git a/confluent_server/bin/confluent_selfcheck b/confluent_server/bin/confluent_selfcheck index e787f211..3afd3e36 100755 --- a/confluent_server/bin/confluent_selfcheck +++ b/confluent_server/bin/confluent_selfcheck @@ -17,8 +17,6 @@ import confluent.certutil as certutil import confluent.client as client import confluent.config.configmanager as configmanager import confluent.netutil as netutil -import tempfile -import shutil import pwd import signal import confluent.collective.manager as collective diff --git a/confluent_server/confluent/plugins/console/tsmsol.py b/confluent_server/confluent/plugins/console/tsmsol.py index cbafe533..e022a4a9 100644 --- a/confluent_server/confluent/plugins/console/tsmsol.py +++ b/confluent_server/confluent/plugins/console/tsmsol.py @@ -26,7 +26,6 @@ import confluent.tasks as tasks import confluent.util as util import aiohmi.exceptions as pygexc import aiohmi.redfish.command as rcmd -import aiohmi.util.webclient as webclient import aiohttp class CustomVerifier(aiohttp.Fingerprint): diff --git a/confluent_server/setup.py.tmpl b/confluent_server/setup.py.tmpl index fc481798..b9e36932 100644 --- a/confluent_server/setup.py.tmpl +++ b/confluent_server/setup.py.tmpl @@ -1,5 +1,4 @@ from setuptools import setup -import os setup( name='confluent_server', diff --git a/misc/getipsfromswitchport b/misc/getipsfromswitchport index aed45e26..216e2a17 100644 --- a/misc/getipsfromswitchport +++ b/misc/getipsfromswitchport @@ -171,11 +171,11 @@ async def main(switch, port): portname = portcandidate if not portname: await ping_everywhere() - async for rsp in client.update(f'/networking/macs/rescan', {'rescan': 'start'}): + async for rsp in client.update('/networking/macs/rescan', {'rescan': 'start'}): pass scanning = True while scanning: - async for rsp in client.read(f'/networking/macs/rescan'): + async for rsp in client.read('/networking/macs/rescan'): if 'scanning' in rsp: scanning = rsp['scanning'] if scanning: diff --git a/misc/getusbnicaddr b/misc/getusbnicaddr index 206d7e64..2e947b51 100644 --- a/misc/getusbnicaddr +++ b/misc/getusbnicaddr @@ -1,7 +1,6 @@ #!/usr/bin/python3 import glob import os -import select import socket From c6c2d3112ea5c9b96964bada9e8321639616a522 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 14:14:18 +0200 Subject: [PATCH 10/14] Tidy comparisons, statement layout and a redundant alias (E711, E712, E701, PLC0414) Hand written rather than autofixed, since three of the four need the surrounding code read to be sure they are equivalent: - confetty: `powerstate == None` -> `is None`. - nodeconfig: `setmode != True` / `!= False` -> `not setmode` / `setmode`. Safe because setmode only ever holds None, True or False, and the two lines above each test normalise None away first. - pam: split two `if cond: stmt` one-liners. - imgutil: `from shutil import copytree as copytree`, an alias that renames nothing. Not a re-export marker, this is a script. --- confluent_client/bin/confetty | 2 +- confluent_client/bin/nodeconfig | 4 ++-- confluent_server/confluent/pam.py | 6 ++++-- imgutil/imgutil | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/confluent_client/bin/confetty b/confluent_client/bin/confetty index 11272271..d32ad5d2 100755 --- a/confluent_client/bin/confetty +++ b/confluent_client/bin/confetty @@ -1042,7 +1042,7 @@ def main(): except IOError: pass if powerstate is None or powertime < time.time() - 10: # Check powerstate every 10 seconds - if powerstate == None: + if powerstate is None: powerstate = True powertime = time.time() check_power_state() diff --git a/confluent_client/bin/nodeconfig b/confluent_client/bin/nodeconfig index ba1b861e..4746c4b6 100755 --- a/confluent_client/bin/nodeconfig +++ b/confluent_client/bin/nodeconfig @@ -170,7 +170,7 @@ def parse_config_line(arguments, single=False): if '=' in param or param[-1] == ':' or forceset: if setmode is None: setmode = True - if setmode != True: + if not setmode: bailout('Cannot do set and query in same command: Query detected but "{0}" appears to be set'.format(param)) if '=' in param: key, _, value = param.partition('=') @@ -182,7 +182,7 @@ def parse_config_line(arguments, single=False): else: if setmode is None: setmode = False - if setmode != False: + if setmode: bailout('Cannot do set and query in same command: Set mode detected but "{0}" appears to be a query'.format(param)) if '.' not in param: if param == 'bmc': diff --git a/confluent_server/confluent/pam.py b/confluent_server/confluent/pam.py index 4d613dc1..4823358d 100644 --- a/confluent_server/confluent/pam.py +++ b/confluent_server/confluent/pam.py @@ -178,8 +178,10 @@ class pam(): return 0 # python3 ctypes prefers bytes - if isinstance(username, str): username = username.encode(encoding) - if isinstance(service, str): service = service.encode(encoding) + if isinstance(username, str): + username = username.encode(encoding) + if isinstance(service, str): + service = service.encode(encoding) if b'\x00' in username or b'\x00' in service: self.code = 4 # PAM_SYSTEM_ERR in Linux-PAM diff --git a/imgutil/imgutil b/imgutil/imgutil index 58f022ab..118e9b29 100644 --- a/imgutil/imgutil +++ b/imgutil/imgutil @@ -5,7 +5,7 @@ import ctypes import ctypes.util import datetime import inspect -from shutil import copytree as copytree +from shutil import copytree if hasattr(inspect, 'getfullargspec') and 'dirs_exist_ok' in inspect.getfullargspec(copytree).args: def copy_tree(src, dst): copytree(src, dst, dirs_exist_ok=True) From d31dba40293401d2a51b50fd8803fa6c9f4613d2 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 14:16:38 +0200 Subject: [PATCH 11/14] Enforce the rules the tree is now clean under Adds the checks whose findings were cleared in the two preceding commits (F401, F541, E701, E711, E712, E713, PLC0414) plus three that were already at zero and cost nothing to lock in: E401, B015 and B023. B905 is deliberately left out even though it also reads as clean: it only reports on py310+, and satisfying it would mean adding a keyword the oldest interpreters this tree runs on cannot parse. --- ruff.toml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/ruff.toml b/ruff.toml index 5c19ab7d..bf49679c 100644 --- a/ruff.toml +++ b/ruff.toml @@ -58,14 +58,30 @@ select = [ # function, break/continue outside a loop, except clause not last "F81", # redefinition of an unused name (shadowed def/class) "F82", # undefined name, undefined name in __all__, use before assignment + "F401", # unused import "F402", # import shadowed by a loop variable + "F541", # f-string with no placeholders + "E401", # several imports on one line + "E701", # several statements on one line + "E711", # comparison to None with == rather than is + "E712", # comparison to True/False with == rather than truthiness + "E713", # `not x in y` rather than `x not in y` + "PLC0414", # import alias that renames nothing "PLE", # pylint errors: bad string format, invalid returns, ... "T100", # forgotten pdb/breakpoint call "W6", # invalid escape sequence in a non-raw string, and any future # deprecated-construct warning pycodestyle adds + "B015", # comparison whose result is discarded "B020", # loop control variable overrides the iterable it iterates + "B023", # closure captures a loop variable, so every closure sees the + # last value rather than the one from its iteration "B035", # dict comprehension with a static key ] +# Deliberately not selected, though currently at zero: B905 (zip without an +# explicit strict=). It only reports on py310+, so it reads as clean here, and +# "fixing" it would mean adding a keyword the oldest supported interpreters +# cannot parse. + # No ignores and no per-file exemptions: every rule selected above is expected # to stay at zero on its own. From 6a6d559d8136e0ba1829cb5d2c2a64ae7f12f1d2 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 15:11:24 +0200 Subject: [PATCH 12/14] Speedup ShellCheck --- .github/workflows/ci.yml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c30d00e3..82ec0679 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,14 +18,24 @@ jobs: # files are sourced fragments or dracut hooks; ShellCheck then # falls back to checking them as bash. run: | + shebang_re='^#!.*[/ ](sh|bash|dash|ash|ksh)([[:blank:]]|$)' { git ls-files '*.sh' git ls-files | while IFS= read -r f; do [ -f "$f" ] || continue - head -c 200 "$f" | head -n 1 | \ - grep -qE '^#!.*[/ ](sh|bash|dash|ash|ksh)([ \t]|$)' && echo "$f" + firstline= + # An empty file makes read fail, which under -e would end the run. + IFS= read -r -n 200 firstline < "$f" 2>/dev/null || true + if [[ $firstline =~ $shebang_re ]]; then + printf '%s\n' "$f" + fi done - } | sort -u | xargs -d '\n' shellcheck --severity=error --exclude=SC2148 + } | sort -u > /tmp/shfiles + # A selection that quietly comes up empty would check nothing and + # still pass, so say how many files there are and insist on some. + echo "$(wc -l < /tmp/shfiles) shell files" + [ -s /tmp/shfiles ] || { echo '::error::No shell files found'; exit 1; } + xargs -d '\n' shellcheck --severity=error --exclude=SC2148 < /tmp/shfiles ruff: name: Ruff From dc141d8a086ed9bab246a713cb133c7559d81dbb Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Sun, 9 Aug 2026 23:37:47 +0200 Subject: [PATCH 13/14] Compile the Python files that are not named *.py compileall only ever compiles *.py. Handed anything else, even by name on the command line, it skips the file and still exits 0, so the job has been checking 218 of the 298 Python files in this tree and reporting success for the rest. Everything without the extension went unchecked: the whole of confluent_client/bin and confluent_server/bin, the osdeploy deploy scripts, the loose misc utilities and the setup.py.tmpl templates. Those files now go to py_compile, which compiles what it is given. The list is built from python shebangs, read with the shell builtin rather than by forking head and grep per file, plus four patterns for the files that carry no shebang at all and so cannot be detected: the setup templates, configbmc, add_local_repositories and misc/filterpasswd. It comes to the same 298 files ruff.toml arrives at through extend-include, and wants keeping in step with it. The shebang test matches python anywhere in the line rather than after a slash or space, because several tools use /usr/libexec/platform-python. --- .github/workflows/ci.yml | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82ec0679..2150c004 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,15 +77,45 @@ jobs: - uses: actions/setup-python@v6 with: python-version: ${{ env.PYTHON_VERSIONS }} + - name: List the Python files without a .py name + # compileall only ever compiles *.py: handed anything else, even by + # name, it skips it and still exits 0. That leaves every extensionless + # CLI tool, deploy script and setup.py.tmpl unchecked, so collect them + # here and feed them to py_compile, which does compile what it is + # given. The first line is read with the shell builtin rather than + # forking head and grep per file. + run: | + shebang_re='^#!.*python' + { + git ls-files | while IFS= read -r f; do + [ -f "$f" ] || continue + case "$f" in *.py) continue ;; esac + firstline= + # An empty file makes read fail, which under -e would end the run. + IFS= read -r -n 200 firstline < "$f" 2>/dev/null || true + if [[ $firstline =~ $shebang_re ]]; then + printf '%s\n' "$f" + fi + done + # Python that carries no shebang at all, so nothing can detect it. + # Kept in step with extend-include in ruff.toml. + git ls-files '*/setup.py.tmpl' '*/scripts/configbmc' \ + '*/scripts/add_local_repositories' 'misc/filterpasswd' + } | sort -u > /tmp/pyfiles + # An empty list would leave py_compile with nothing to do and the + # job green, which is the very hole this step exists to close. + echo "$(wc -l < /tmp/pyfiles) files without a .py name" + [ -s /tmp/pyfiles ] || { echo '::error::No such files found'; exit 1; } - name: Compile all Python files run: | rc=0 for v in $PYTHON_VERSIONS; do echo "::group::Python $v" - if "python$v" -W error -m compileall -q -x '/\.git/' .; then - echo "::endgroup::" - else - echo "::endgroup::" + ok=0 + "python$v" -W error -m compileall -q -x '/\.git/' . || ok=1 + xargs -d '\n' "python$v" -W error -m py_compile < /tmp/pyfiles || ok=1 + echo "::endgroup::" + if [ "$ok" -ne 0 ]; then echo "::error::Python $v compileall failed" rc=1 fi From 3fe70363e2f8045d9d3e89ef6587077b8c375f71 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Mon, 10 Aug 2026 04:46:08 +0200 Subject: [PATCH 14/14] Use latest ruff version --- .github/workflows/ci.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2150c004..62708ed7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,11 +46,7 @@ jobs: # publishes no moving major tag past v3, so @v4 does not resolve. - uses: astral-sh/ruff-action@v4.1.0 with: - # There is no pyproject.toml for the action to read a version from, - # so pin it here: an unpinned ruff would resolve to `latest` and a - # new release could turn a green branch red on its own. Bump this - # deliberately, together with the rule set in ruff.toml. - version: 0.15.21 + version: latest # Rule selection, file discovery (the many extensionless Python # executables) and exclusions all live in ruff.toml, so the whole # workspace can be handed over as-is.