Fix Redfish credential loading for 'redfish list'#2430
Draft
ideaship wants to merge 1 commit into
Draft
Conversation
The `osism redfish list <host> ...` command connected to the BMC with
empty credentials. It builds a direct sushy connection via
get_redfish_connection(), which is the only path that re-derives BMC
credentials from the NetBox device "secrets" custom field through
_get_conductor_redfish_credentials(). That helper silently returned
None, None, so the connection fell back to anonymous auth and failed
against any BMC that requires authentication -- with no exception
raised and nothing logged. The configured Redfish address was
similarly dropped, falling back to the default https://{hostname}.
The Ironic sync path is unaffected: it calls _prepare_node_attributes()
directly (ironic.py:823), already unpacks the tuple, and pushes the
same secrets-derived credentials into Ironic. So BMC access that goes
through Ironic (deploy, clean, power) kept working; only the standalone
redfish list command, which re-derives the credentials on its own, was
broken.
The cause is in conductor/utils.py. _prepare_node_attributes() returns
a (node_attributes, template_vars) 2-tuple, but
_get_conductor_redfish_credentials() and
_get_conductor_redfish_address() still bound the whole tuple to
node_attributes and then tested `"driver_info" in node_attributes`. On
a tuple that is a membership test against the two tuple elements, which
is always False, so both helpers short-circuited before .get() and
returned None, None / None regardless of the device configuration. The
tuple return was introduced when ironic.py was refactored; these two
callers were never updated and had no test coverage.
Unpack the tuple at both call sites and add regression tests for both
helpers covering the redfish happy path, the None-device guard, the
non-redfish driver, missing driver_info, and swallowed exceptions. The
happy-path tests fail against the pre-fix code, pinning the behaviour.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Roger Luethi <luethi@osism.tech>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
osism redfish list <host> ...connects to the BMC with empty credentials and fails against any BMC that requires authentication — silently, with no exception raised and nothing logged. The configured Redfish address is likewise dropped, falling back to the defaulthttps://{hostname}.get_redfish_connection()is the only path that re-derives BMC credentials from the NetBox devicesecretscustom field, via_get_conductor_redfish_credentials()._prepare_node_attributes()returns a(node_attributes, template_vars)tuple, but that helper (and_get_conductor_redfish_address()) still bound the whole tuple tonode_attributesand then tested"driver_info" in node_attributes. On a tuple that is a membership test against the two tuple elements — alwaysFalse— so both helpers short-circuited before.get()and returnedNone.The tuple return was introduced when
ironic.pywas refactored; these twoutils.pycallers were never updated and had no test coverage.Scope
The Ironic sync path is unaffected — it calls
_prepare_node_attributes()directly (ironic.py:823), already unpacks the tuple, and pushes the same secrets-derived credentials into Ironic. So BMC access that goes through Ironic (deploy, clean, power) kept working; only the standaloneredfish listcommand, which re-derives credentials on its own, was broken. This is why the defect went unnoticed.Fix
conductor/utils.py.None-device guard, non-redfish driver, missingdriver_info, and swallowed exceptions. The happy-path tests fail against the pre-fix code, pinning the behaviour.Context
🤖 Generated with Claude Code