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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
This allows a user to opt into pcrs if they understand what they are doing.
Some PCRs are sensitive to firmware updates and some are sensitive to boot loader, kernel, boot config, or initramfs. All of these are an opportunity for an unsuspecting update to remove access to the boot volume. There are update processes that can be put into place to make this work,
but it is up to the OS update process to address that, and
OS update processes are likely not to address that at this time.
reply_dhcp4 logs the insecure mode remediation hint on every DHCP
discover it refuses. A node in this state never receives a reply, so it
retries for as long as it is powered on and the same message repeats
every few seconds.
Rate limit it per hardware address the way the neighbouring boot attempt
messages already do, reusing the ignoremacs window that check_reply uses
for the missing profile hint.
The per-MAC 90 second log throttle in proxydhcp has been inert: the
`skiplogging = True` reset sat in relay_proxydhcp, where it is a dead
local, while the loop in proxydhcp only ever assigns False. Once the
first packet is handled the flag stays False for the life of the
process, so every retransmitted boot request logs again even though
ignoredisco is updated to suppress it.
Reset the flag at the top of each loop iteration instead, next to the
timestamp check it belongs to, and drop the dead assignment.
reply_dhcp4 declines to answer a PXE boot request unless
deployment.useinsecureprotocols is set to firmware or always, but
proxydhcp had no such check. A node left at the default of never was
therefore still offered a TFTP bootfile and a plain http boot.ipxe URL
whenever the request arrived on port 4011 rather than port 67, so the
attribute silently did nothing in ProxyDHCP deployments alongside an
independent DHCP server.
Apply the same gate, including the UEFI HTTP boot exemption, and log the
same remediation hint. The node attributes are now fetched once and
passed through to get_deployment_profile instead of being looked up
again there.
Requests whose architecture could not be determined are ignored rather
than falling through to the reply. opts_to_dict stops parsing before the
client architecture option whenever the message type is not a request,
and such a packet would otherwise reach the iPXE branch and be handed a
plain http boot.ipxe URL without ever passing the gate.
The default 'cp' is /lbin/cp, but that fails, use full path to the cp that works.
Perform the hmac registration of api key that was missing.
Remove assumption that the ip will be ipv6, wrapping it only if a : is present in address.
The rsync push carried no preservation flags, so files arrived with their
special permission bits explicitly disabled and a setuid/setgid entry could
only be honored by the permissions= chmod on the client side.
Preservation was turned on once before in e52a9ff70f ("Have syncfiles
attempt to preserve more") and rolled back the same day in c0287e93ed
("Roll back rsync ownership"), because rsync also applied the staging copy's
attributes to the parent directories it merely traversed on the way to the
synced files, clobbering the permissions of system directories such as /etc.
Naming every staged file explicitly through --files-from and adding
--no-implied-dirs confines preservation to the content actually being
synchronized, leaving traversed directories alone and creating missing ones
with default attributes.
Two details follow from the way the staging tree is built. Files are staged as
symlinks, so rsync reads their attributes through to the real file, but
directories are staged as directories and need the source attributes copied
onto them for the otherwise empty ones that have to be named explicitly.
Ownership is mapped from the account the daemon runs as to root, since that
account generally does not exist on the node and would otherwise arrive as a
meaningless numeric id.
--xattrs from that earlier attempt is deliberately left out: with --copy-links
rsync reads xattrs off the symlink rather than its referent, so it transfers
nothing here while adding a failure mode on hosts without xattr support.
chown() clears the setuid bit of a file on Linux (and its setgid bit, if
the file is group-executable), even when run by root and even when the
owner/group are unchanged. Since the owner/group chown ran after the
permissions chmod, any syncfiles entry combining owner=/group= with a
setuid/setgid permissions= value silently lost the special bits.
get_syncresult() caught the sync task's exception, logged a repr server
side and returned 200 OK with a null body. The node then called
.get('options') on that null resulted in:
c1: 'NoneType' object has no attribute 'get'
and syncfileclient still exited 0 as if syncing had succeeded.
Return the error to the requestor as a 500 with an error payload. On
the node, unwrap the body that grab_url_with_status raises for a
non-success status, print it once and exit non-zero. Only a failure the
server deliberately reported for this sync is terminal. Anything else,
such as a dropped connection, is re-raised so the existing retry loop
handles it as before. The same case now reports
c1: Error performing syncfiles: Syncing failed due to unreadable files: /etc/dangling.conf
c1: 'syncfileclient' exited with code 1
nodeattrib/nodegroupattrib -e replaced '.' with '_' in the attribute name
before handing it to the server, not just when looking up the environment
variable. Any attribute with a dot in it was therefore rejected, e.g.
$ export info_note=test
$ nodeattrib -e gpu1 info.note
Traceback (most recent call last):
File "/opt/confluent/bin/nodeattrib", line 97, in <module>
exitcode=client.updateattrib(session,args,nodetype, noderange, options, argassign)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/confluent/lib/python/confluent/client.py", line 688, in updateattrib
key, os.environ[key.upper()])
~~~~~~~~~~^^^^^^^^^^^^^
File "<frozen os>", line 714, in __getitem__
KeyError: 'INFO_NOTE'
$ export INFO_NOTE=test
$ nodeattrib -e gpu1 info.note
Error: Bad Request - info_note attribute on node gpu1 is invalid
Keep the attribute name intact and derive the environment variable name
from it separately. A missing environment variable now reports which
variable names were looked for instead of raising a bare KeyError
traceback.
source_remote imageboot.sh is the last thing the diskless cmdline hook
runs, so returning early on a failed extraction ended the hook and left
dracut to time out. Falling through instead reaches the existing
/sysroot/sbin/init guard, which reports the failure and holds the node so
it stays reachable over ssh, as it did before extraction was checked.
imageboot falls back to cp when unsquashfs is unavailable, but the build
side did not: a bare dracut_install/copy_exec aborts initramfs generation
when the binary is absent, and the capture prerequisite check refused to
capture the image at all.
Mark the initramfs copies optional and report the missing package as an
advisory rather than a hard prerequisite, so such images still build and
capture, just without the faster extraction path. Widen the EL check to
every release past el8 so future ones inherit it.
The copy ran after the cd to the repo root, so it read ../LICENSE from
outside the checkout and never placed the file in confluent_osdeploy/. The
tarball went out without it and the spec's %install, which does
"cp LICENSE" after %setup cds into the unpacked directory, failed.
imgutil/buildrpm already copies before its cd; do the same here.
The aarch64 spec has its LICENSE lines commented out, so only the x86_64
build broke, but the copy was equally wrong in both scripts.
Release tags do not live on master: 3.15.2 through 3.15.6 were tagged on branch
3.15, so git describe reaches only 3.15.1 and dev builds were stamped
3.15.2.dev<n>. Besides being confusing, rpm and dpkg both rank the released
3.15.6 above that, so a dev package will not install over a released one.
Add a top-level VERSION file naming the release the branch is working toward
(4.0.0 on master) and a mkversion helper that stamps packages from it, keeping
the tag-derived value as a floor so a forgotten bump cannot go backwards.
mkversion also replaces the block copy-pasted into seven build scripts, and
makesetup no longer writes a per-package VERSION file, so the stale checked-in
confluent_common/VERSION goes with it.
confluent_imgutil does not depend on confluent_server, and the yaml
import is optional too, yet both capture and pack dereference osimage
and yaml unconditionally when writing manifest.yaml. With
confluent_osdeploy present but the server absent that raises rather
than producing a profile.
Guard the manifest on both being importable and say so, since rebase
is what the manifest exists for. The yaml fallback now binds None
instead of leaving the name undefined.
The two call sites carried the manifest write verbatim in both, so fold
them into one function rather than duplicate the guard as well.
osimport polls import progress from a coroutine, so a blocking sleep
between reads stalls the whole client loop. It was the only use of time
in the script, so the import goes with it.
Markus Hilger
Jarrod Johnson
Jarrod Johnson