From 48b7af9bde1e7ce222975ce94af2b41fc570a85d Mon Sep 17 00:00:00 2001 From: Anson Yim Date: Wed, 28 Oct 2026 14:01:43 -0400 Subject: [PATCH 1/4] Add regex/exists checks to RedfishEndpointPlugin and per-plugin connection override - RedfishEndpointPlugin analyzer: new 'regex' and 'exists' constraint keys - PluginExecutor: plugin config 'connection' key selects a configured connection manager by name (e.g. RedfishSshProxyConnectionManager) instead of CONNECTION_TYPE - Tests for the new constraints, the override, and RedfishEndpointPlugin over the SSH proxy --- nodescraper/pluginexecutor.py | 39 ++++- .../ooband/redfish_endpoint/analyzer_args.py | 4 + .../redfish_endpoint/endpoint_analyzer.py | 16 +- .../test_plugin_connection_override.py | 99 +++++++++++ .../plugin/test_redfish_endpoint_analyzer.py | 60 +++++++ .../plugin/test_redfish_endpoint_ssh_proxy.py | 159 ++++++++++++++++++ 6 files changed, 375 insertions(+), 2 deletions(-) create mode 100644 test/unit/framework/test_plugin_connection_override.py create mode 100644 test/unit/plugin/test_redfish_endpoint_ssh_proxy.py diff --git a/nodescraper/pluginexecutor.py b/nodescraper/pluginexecutor.py index 4571ab03..4329b443 100644 --- a/nodescraper/pluginexecutor.py +++ b/nodescraper/pluginexecutor.py @@ -251,10 +251,34 @@ def discover_os_info(self) -> None: if inband_connection is not None: inband_connection.disconnect() + def _resolve_connection_override( + self, plugin_name: str, connection_override: str + ) -> Optional[ConnectionManager]: + """Return the prebuilt manager named by a plugin's ``connection`` config key. + + Args: + plugin_name: Registry key for the plugin (for logging). + connection_override: Connection manager name; must also appear in the connection config. + + Returns: + The connection manager instance, or None if it is not configured. + """ + mgr_impl = self.plugin_registry.connection_managers.get(connection_override) + conn_mgr = self.connection_library.get(mgr_impl) if mgr_impl else None + if conn_mgr is None: + self.logger.error( + "%s: connection %r must be a registered connection manager present in the " + "connection config", + plugin_name, + connection_override, + ) + return conn_mgr + def _get_connection_manager_for_plugin( self, plugin_class: type, plugin_name: str, + connection_override: Optional[str] = None, ) -> Optional[ConnectionManager]: """Resolve and (if needed) initialise the connection manager for *plugin_class*. @@ -264,10 +288,17 @@ def _get_connection_manager_for_plugin( The resolved manager is stored in ``self.connection_library`` so that it can be shared across plugins that require the same type. + + *connection_override* (a plugin config ``connection`` key) selects a configured + connection manager by name instead of the plugin's ``CONNECTION_TYPE``, e.g. to run + ``RedfishEndpointPlugin`` through ``RedfishSshProxyConnectionManager``. """ if not plugin_class.CONNECTION_TYPE: return None + if connection_override: + return self._resolve_connection_override(plugin_name, connection_override) + if issubclass(plugin_class, OOBSSHDataPlugin): mgr_impl = OobSshConnectionManager connection_args = self.connection_configs.get("RedfishConnectionManager") @@ -342,6 +373,10 @@ def _run_single_plugin( plugin_class = self.plugin_registry.plugins[plugin_name] + # "connection" selects the connection manager; it is not a run() argument. + plugin_args = dict(plugin_args) + connection_override = plugin_args.pop("connection", None) + init_payload = { "system_info": self.system_info, "logger": self.logger, @@ -351,7 +386,9 @@ def _run_single_plugin( } if plugin_class.CONNECTION_TYPE: - conn_mgr = self._get_connection_manager_for_plugin(plugin_class, plugin_name) + conn_mgr = self._get_connection_manager_for_plugin( + plugin_class, plugin_name, connection_override=connection_override + ) if conn_mgr is None: return False init_payload["connection_manager"] = conn_mgr diff --git a/nodescraper/plugins/ooband/redfish_endpoint/analyzer_args.py b/nodescraper/plugins/ooband/redfish_endpoint/analyzer_args.py index 28f115e5..5b495bb5 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/analyzer_args.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/analyzer_args.py @@ -42,6 +42,8 @@ class ConstraintKey(str, Enum): MIN = "min" MAX = "max" ANY_OF = "anyOf" + REGEX = "regex" + EXISTS = "exists" RedfishConstraint = Union[int, float, str, bool, dict[str, Any]] @@ -62,6 +64,8 @@ class RedfishEndpointAnalyzerArgs(AnalyzerArgs): "'min' — value must be numeric and >= the given number. " "'max' — value must be numeric and <= the given number. " "'anyOf' — value must be in the given list (OR; any match passes). " + "'regex' — value must be a string matching the pattern (Python re.search; anchor with ^ and $ for a full match). " + "'exists' — true: property must be present and not null; false: property must be absent or null. " 'Example: { "/redfish/v1/Systems/1": { "Status/Health": { "anyOf": ["OK", "Warning"] }, "PowerState": "On" }, "*": { "Status/Health": { "anyOf": ["OK"] } } }.' ), ) diff --git a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py index 3b783fe3..d0e51b1b 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py @@ -23,6 +23,7 @@ # SOFTWARE. # ############################################################################### +import re from typing import Any, Optional from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus @@ -59,6 +60,19 @@ def _get_by_path(obj: Any, path: str) -> Any: def _check_constraint(actual: Any, constraint: RedfishConstraint) -> tuple[bool, str]: """Compare actual value to constraint.""" if isinstance(constraint, dict): + if ConstraintKey.EXISTS in constraint: + want = bool(constraint[ConstraintKey.EXISTS]) + ok = (actual is not None) == want + return ok, f"expected {'present' if want else 'absent'}, got {actual!r}" + if ConstraintKey.REGEX in constraint: + pattern = constraint[ConstraintKey.REGEX] + if not isinstance(actual, str): + return False, f"expected string matching {pattern!r}, got {actual!r}" + try: + ok = re.search(pattern, actual) is not None + except re.error as exc: + return False, f"invalid regex {pattern!r}: {exc}" + return ok, f"expected match for {pattern!r}, got {actual!r}" if ConstraintKey.EQ in constraint: ok = actual == constraint[ConstraintKey.EQ] return ok, f"expected eq {constraint[ConstraintKey.EQ]}, got {actual!r}" @@ -91,7 +105,7 @@ class RedfishEndpointAnalyzer(DataAnalyzer[RedfishEndpointDataModel, RedfishEndp DOCUMENTATION_ANALYSIS_ITEMS: tuple[str, ...] = ( "For each entry in analysis_args.checks, reads JSON paths in collected responses and " - "compares values to constraints (eq, min/max, anyOf, regex, etc.).", + "compares values to constraints (eq, min/max, anyOf, regex, exists).", "URI key `*` runs checks against every collected response body.", ) diff --git a/test/unit/framework/test_plugin_connection_override.py b/test/unit/framework/test_plugin_connection_override.py new file mode 100644 index 00000000..165cd261 --- /dev/null +++ b/test/unit/framework/test_plugin_connection_override.py @@ -0,0 +1,99 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +import unittest +from typing import Any +from unittest.mock import patch + +from nodescraper.connection.redfish import RedfishSshProxyConnectionManager +from nodescraper.enums import OSFamily, SystemLocation +from nodescraper.models import PluginConfig, SystemInfo +from nodescraper.pluginexecutor import PluginExecutor + +PLUGIN = "RedfishEndpointPlugin" +OVERRIDE = "RedfishSshProxyConnectionManager" + +PROXY_CONNECTION: dict[str, Any] = { + OVERRIDE: { + "host": "192.0.2.10", + "ssh": {"hostname": "bmc", "username": "root", "password": "x"}, + } +} + + +def _executor(plugin_args: dict, connections: dict) -> PluginExecutor: + return PluginExecutor( + plugin_configs=[PluginConfig(plugins={PLUGIN: plugin_args})], + connections=connections, + system_info=SystemInfo( + name="test", location=SystemLocation.REMOTE, os_family=OSFamily.LINUX + ), + ) + + +class TestPluginConnectionOverride(unittest.TestCase): + def test_override_uses_configured_manager(self): + ex = _executor({"connection": OVERRIDE}, PROXY_CONNECTION) + plugin_cls = ex.plugin_registry.plugins[PLUGIN] + self.assertIsNot(plugin_cls.CONNECTION_TYPE, RedfishSshProxyConnectionManager) + + cm = ex._get_connection_manager_for_plugin(plugin_cls, PLUGIN, connection_override=OVERRIDE) + + self.assertIs(cm, ex.connection_library[RedfishSshProxyConnectionManager]) + + def test_unregistered_name_returns_none(self): + ex = _executor({}, {}) + plugin_cls = ex.plugin_registry.plugins[PLUGIN] + self.assertIsNone( + ex._get_connection_manager_for_plugin( + plugin_cls, PLUGIN, connection_override="NotARealManager" + ) + ) + + def test_registered_but_unconfigured_returns_none(self): + ex = _executor({}, {}) + plugin_cls = ex.plugin_registry.plugins[PLUGIN] + self.assertIsNone( + ex._get_connection_manager_for_plugin(plugin_cls, PLUGIN, connection_override=OVERRIDE) + ) + + def test_connection_key_not_passed_to_run(self): + ex = _executor( + {"collection": False, "analysis": False, "connection": OVERRIDE}, + PROXY_CONNECTION, + ) + plugin_cls = ex.plugin_registry.plugins[PLUGIN] + + with patch.object(plugin_cls, "run", autospec=True) as mock_run: + ran = ex._run_single_plugin( + PLUGIN, ex.plugin_config.plugins[PLUGIN], [], queue_callback=None + ) + + self.assertTrue(ran) + self.assertNotIn("connection", mock_run.call_args.kwargs) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/unit/plugin/test_redfish_endpoint_analyzer.py b/test/unit/plugin/test_redfish_endpoint_analyzer.py index 55163824..1d389b05 100644 --- a/test/unit/plugin/test_redfish_endpoint_analyzer.py +++ b/test/unit/plugin/test_redfish_endpoint_analyzer.py @@ -129,6 +129,66 @@ def test_check_constraint_literal_match(): assert ok is False +def test_check_constraint_regex_pass(): + ok, _ = _check_constraint("123e4567-e89b-12d3-a456-426614174000", {"regex": r"^[0-9a-f-]{36}$"}) + assert ok is True + ok, _ = _check_constraint("SN1234", {"regex": r"\d"}) + assert ok is True + + +def test_check_constraint_regex_fail(): + ok, msg = _check_constraint("000000000000", {"regex": r"^(?!0+$).+$"}) + assert ok is False + assert "expected match" in msg + + +def test_check_constraint_regex_non_string_fails(): + for actual in (None, 42, {"a": 1}): + ok, msg = _check_constraint(actual, {"regex": ".*"}) + assert ok is False + assert "expected string" in msg + + +def test_check_constraint_regex_invalid_pattern_fails(): + ok, msg = _check_constraint("abc", {"regex": "("}) + assert ok is False + assert "invalid regex" in msg + + +def test_check_constraint_exists(): + for present in ("x", 0, False, {}, {"@odata.id": "/redfish/v1/Chassis/1/Sensors"}): + assert _check_constraint(present, {"exists": True})[0] is True + assert _check_constraint(present, {"exists": False})[0] is False + assert _check_constraint(None, {"exists": True})[0] is False + assert _check_constraint(None, {"exists": False})[0] is True + + +def test_redfish_endpoint_analyzer_regex_and_exists(redfish_endpoint_analyzer): + data = RedfishEndpointDataModel( + responses={ + "/redfish/v1/Chassis/1": { + "UUID": "123e4567-e89b-12d3-a456-426614174000", + "SerialNumber": "000000000000", + "Sensors": {"@odata.id": "/redfish/v1/Chassis/1/Sensors"}, + }, + } + ) + args = RedfishEndpointAnalyzerArgs( + checks={ + "/redfish/v1/Chassis/1": { + "UUID": {"regex": r"^[0-9a-fA-F-]{36}$"}, + "SerialNumber": {"regex": r"^(?!0+$).+$"}, + "Sensors": {"exists": True}, + "Assembly": {"exists": True}, + }, + } + ) + result = redfish_endpoint_analyzer.analyze_data(data, args=args) + assert result.status == ExecutionStatus.ERROR + failures = result.events[-1].data["failures"] + assert sorted(f["path"] for f in failures) == ["Assembly", "SerialNumber"] + + def test_redfish_endpoint_analyzer_no_checks(redfish_endpoint_analyzer): data = RedfishEndpointDataModel(responses={"/redfish/v1": {}}) result = redfish_endpoint_analyzer.analyze_data(data, args=None) diff --git a/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py b/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py new file mode 100644 index 00000000..7b0dfe58 --- /dev/null +++ b/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py @@ -0,0 +1,159 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +"""RedfishEndpointPlugin run against a host behind a BMC via RedfishSshProxyConnectionManager. + +Only the BMC SSH session and the curl call are faked; the connection manager, executor +connection override, collector and analyzer are the real implementations. +""" +import json +from typing import Any, cast +from unittest.mock import MagicMock, patch + +import pytest + +from nodescraper.connection.redfish.ssh_proxy_connection import ( + CurlResponse, + SshProxyRedfishConnection, +) +from nodescraper.enums import ExecutionStatus, OSFamily, SystemLocation +from nodescraper.models import PluginConfig, SystemInfo +from nodescraper.pluginexecutor import PluginExecutor + +BASE = "http://192.0.2.10:80" + +CHASSIS = { + "/redfish/v1": {"@odata.id": "/redfish/v1"}, + "/redfish/v1/Chassis": {"Members@odata.count": 2}, + "/redfish/v1/Chassis/1": { + "Id": "1", + "UUID": "123e4567-e89b-12d3-a456-426614174000", + "SerialNumber": "SN0001", + "Sensors": {"@odata.id": "/redfish/v1/Chassis/1/Sensors"}, + "Status": {"Health": "OK", "State": "Enabled"}, + }, + "/redfish/v1/Chassis/2": { + "Id": "2", + "UUID": "not-a-uuid", + "SerialNumber": "000000000000", + "Status": {"Health": "OK", "State": "Enabled"}, + }, +} + +PLUGIN_ARGS: dict[str, Any] = { + "connection": "RedfishSshProxyConnectionManager", + "collection_args": {"uris": ["/redfish/v1/Chassis", "/redfish/v1/Chassis/1"]}, + "analysis_args": { + "checks": { + "/redfish/v1/Chassis": {"Members@odata.count": 2}, + "/redfish/v1/Chassis/1": { + "UUID": {"regex": r"^[0-9a-fA-F-]{36}$"}, + "SerialNumber": {"regex": r"^(?!0+$).+$"}, + "Sensors": {"exists": True}, + "Status/Health": "OK", + }, + } + }, +} + + +def _target(hostname: str) -> dict[str, Any]: + return { + "host": "192.0.2.10", + "ssh": {"hostname": hostname, "username": "root", "password": "x"}, + } + + +def _fake_curl(self: SshProxyRedfishConnection, url: str, extra_flags: str = "") -> CurlResponse: + cast(MagicMock, self._shell).urls.append(url) + path = url[len(BASE) :].rstrip("/") or "/" + body = CHASSIS.get(path) + if body is None: + return CurlResponse(404, b"") + return CurlResponse(200, json.dumps(body).encode()) + + +def _run(connection_args: dict[str, Any], plugin_args: dict[str, Any]): + shells: dict[str, MagicMock] = {} + + def make_shell(ssh_params): + shell = MagicMock(name=str(ssh_params.hostname)) + shell.urls = [] + shells[str(ssh_params.hostname)] = shell + return shell + + with ( + patch( + "nodescraper.connection.redfish.ssh_proxy_manager.RemoteShell", side_effect=make_shell + ), + patch.object(SshProxyRedfishConnection, "_curl", autospec=True, side_effect=_fake_curl), + ): + executor = PluginExecutor( + plugin_configs=[PluginConfig(plugins={"RedfishEndpointPlugin": plugin_args})], + connections={"RedfishSshProxyConnectionManager": connection_args}, + system_info=SystemInfo( + name="test", location=SystemLocation.REMOTE, os_family=OSFamily.LINUX + ), + ) + results = executor.run_queue() + return results, shells + + +@pytest.fixture(autouse=True) +def _no_log_dir(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + + +def test_single_target_collects_and_checks_through_proxy(): + results, shells = _run(_target("bmc-a"), PLUGIN_ARGS) + + assert len(results) == 1 + assert results[0].status == ExecutionStatus.OK, results[0].message + assert f"{BASE}/redfish/v1/Chassis/1" in shells["bmc-a"].urls + + +def test_single_target_reports_failed_checks(): + plugin_args = json.loads(json.dumps(PLUGIN_ARGS)) + plugin_args["collection_args"]["uris"] = ["/redfish/v1/Chassis/2"] + plugin_args["analysis_args"]["checks"] = { + "/redfish/v1/Chassis/2": { + "UUID": {"regex": r"^[0-9a-fA-F-]{36}$"}, + "SerialNumber": {"regex": r"^(?!0+$).+$"}, + "Sensors": {"exists": True}, + } + } + + results, _ = _run(_target("bmc-a"), plugin_args) + + assert results[0].status == ExecutionStatus.ERROR + assert "3 check(s) failed" in results[0].message + + +def test_multi_target_collects_from_each_bmc(): + results, shells = _run({"targets": [_target("bmc-a"), _target("bmc-b")]}, PLUGIN_ARGS) + + assert results[0].status == ExecutionStatus.OK, results[0].message + for host in ("bmc-a", "bmc-b"): + assert f"{BASE}/redfish/v1/Chassis/1" in shells[host].urls From 44e7a56d6fc39d7e3ce0978659aa44c8e48743b9 Mon Sep 17 00:00:00 2001 From: Anson Yim Date: Thu, 29 Oct 2026 12:07:55 -0400 Subject: [PATCH 2/4] Fall back to RedfishSshProxyConnectionManager for Redfish plugins Redfish plugins (CONNECTION_TYPE = RedfishConnectionManager) now pick their transport from the connection config: - RedfishConnectionManager present: use it (also when both are present) - only RedfishSshProxyConnectionManager present: use the SSH proxy - neither present: skip the plugin with an error The plugin config 'connection' key still forces a specific manager. OOB SSH plugins are unchanged. Adds config/connection-config_redfish_ssh_proxy.example.json and README docs. --- README.md | 36 +++++++- ...tion-config_redfish_ssh_proxy.example.json | 10 +++ nodescraper/pluginexecutor.py | 39 ++++++++- .../test_plugin_connection_override.py | 86 ++++++++++++------- .../plugin/test_redfish_endpoint_ssh_proxy.py | 5 +- 5 files changed, 138 insertions(+), 38 deletions(-) create mode 100644 config/connection-config_redfish_ssh_proxy.example.json diff --git a/README.md b/README.md index 1084308a..f4203b89 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,8 @@ system debug. For details on what data is collected and analyzed, see the [plugi - [Execution Methods](#execution-methods) - [Example: Remote Execution](#example-remote-execution) - [Example: connection\_config.json](#example-connection_configjson) + - [Redfish multi-target](#redfish-multi-target) + - [Redfish through a BMC SSH proxy](#redfish-through-a-bmc-ssh-proxy) - [Subcommands](#subcommands) - [**'describe' subcommand**](#describe-subcommand) - [**'run-plugins' sub command**](#run-plugins-sub-command) @@ -263,6 +265,36 @@ Per-target results are written to `///`. Single-t A ready-to-edit sample is in `config/connection-config_redfish_multi_target.example.json`. Replace the example hosts and password, then pass it with `--connection-config`. +#### Redfish through a BMC SSH proxy + +When the Redfish service is only reachable from the BMC, use `RedfishSshProxyConnectionManager`. node-scraper opens SSH to the BMC (`ssh`) and runs `curl` from there against `host`. + +```json +{ + "RedfishSshProxyConnectionManager": { + "host": "192.0.2.10", + "ssh": { + "hostname": "bmc.example.com", + "username": "root", + "password": "placeholder" + } + } +} +``` + +Redfish plugins (those based on `OOBandDataPlugin`) pick their connection from the connection config: + +| `RedfishConnectionManager` | `RedfishSshProxyConnectionManager` | Connection used | +|---|---|---| +| present | absent | direct Redfish | +| absent | present | SSH proxy | +| present | present | direct Redfish | +| absent | absent | plugin is skipped | + +To force the proxy for one plugin while both blocks are configured, set `"connection": "RedfishSshProxyConnectionManager"` in that plugin's entry in the plugin config. The proxy also accepts a `targets` list, in the same shape as Redfish multi-target. OOB SSH plugins are unaffected and still require `RedfishConnectionManager`. + +A sample is in `config/connection-config_redfish_ssh_proxy.example.json`. + **Notes:** - If using SSH keys, specify `key_filename` instead of `password`. - The remote user must have permissions to run the requested plugins and access required files. If needed, use the `--skip-sudo` argument to skip plugins requiring sudo. @@ -529,7 +561,7 @@ The RedfishEndpointPlugin collects Redfish URIs (GET responses) and optionally r **How to run** -1. Create a connection config (e.g. `connection-config.json`) with `RedfishConnectionManager` and your BMC host (single target) or `targets` list (multi-target). +1. Create a connection config (e.g. `connection-config.json`) with `RedfishConnectionManager` and your BMC host (single target) or `targets` list (multi-target). If Redfish is only reachable through the BMC, use `RedfishSshProxyConnectionManager` instead (see [Redfish through a BMC SSH proxy](#redfish-through-a-bmc-ssh-proxy)). 2. Create a plugin config with `uris` to collect and optional `checks` for analysis (see example below). For example save as `plugin_config_redfish_endpoint.json`. 3. Run: ```sh @@ -575,7 +607,7 @@ The RedfishEndpointPlugin collects Redfish URIs (GET responses) and optionally r - **`max_pages`**: Optional (default `200`). Safety cap on the number of pages to follow per URI when `follow_next_link` is enabled. **`analysis_args`** -- **`checks`**: Optional. Map of URI to expected values or constraints for analysis. Supports exact match (e.g. `"PowerState": "On"`), `anyOf`, `min`/`max`, etc. +- **`checks`**: Optional. Map of URI to expected values or constraints for analysis. Supports exact match (e.g. `"PowerState": "On"`), `anyOf`, `min`/`max`, `regex` (e.g. `{"regex": "^[0-9a-fA-F-]{36}$"}`), and `exists` (`{"exists": true}` requires the property to be present and non-null). #### **'summary' sub command** The 'summary' subcommand can be used to combine results from multiple runs of node-scraper to a diff --git a/config/connection-config_redfish_ssh_proxy.example.json b/config/connection-config_redfish_ssh_proxy.example.json new file mode 100644 index 00000000..4b4a57c8 --- /dev/null +++ b/config/connection-config_redfish_ssh_proxy.example.json @@ -0,0 +1,10 @@ +{ + "RedfishSshProxyConnectionManager": { + "host": "192.0.2.10", + "ssh": { + "hostname": "bmc.example.com", + "username": "root", + "password": "placeholder" + } + } +} diff --git a/nodescraper/pluginexecutor.py b/nodescraper/pluginexecutor.py index 4329b443..94f296a0 100644 --- a/nodescraper/pluginexecutor.py +++ b/nodescraper/pluginexecutor.py @@ -39,6 +39,10 @@ from nodescraper.connection.inband import InBandConnectionManager from nodescraper.connection.inband.osdetection import discover_and_write_os_family from nodescraper.connection.oob_ssh import OobSshConnectionManager +from nodescraper.connection.redfish import ( + RedfishConnectionManager, + RedfishSshProxyConnectionManager, +) from nodescraper.constants import DEFAULT_LOGGER from nodescraper.enums import ExecutionStatus, OSFamily from nodescraper.helpers.plugin_execution_target import ( @@ -274,6 +278,31 @@ def _resolve_connection_override( ) return conn_mgr + def _resolve_redfish_manager(self, plugin_name: str) -> Optional[ConnectionManager]: + """Pick the Redfish transport for a plugin declaring ``RedfishConnectionManager``. + + The direct ``RedfishConnectionManager`` wins when configured; otherwise + ``RedfishSshProxyConnectionManager`` is used. With neither configured the plugin is skipped. + + Args: + plugin_name: Registry key for the plugin (for logging). + + Returns: + The connection manager instance, or None if no Redfish connection is configured. + """ + for name in ( + RedfishConnectionManager.__name__, + RedfishSshProxyConnectionManager.__name__, + ): + if name in self.connection_configs: + return self._resolve_connection_override(plugin_name, name) + self.logger.error( + "%s requires RedfishConnectionManager or RedfishSshProxyConnectionManager in the " + "connection config", + plugin_name, + ) + return None + def _get_connection_manager_for_plugin( self, plugin_class: type, @@ -289,9 +318,13 @@ def _get_connection_manager_for_plugin( The resolved manager is stored in ``self.connection_library`` so that it can be shared across plugins that require the same type. + Plugins declaring ``RedfishConnectionManager`` fall back to + ``RedfishSshProxyConnectionManager`` when only the proxy is configured + (see :meth:`_resolve_redfish_manager`). + *connection_override* (a plugin config ``connection`` key) selects a configured - connection manager by name instead of the plugin's ``CONNECTION_TYPE``, e.g. to run - ``RedfishEndpointPlugin`` through ``RedfishSshProxyConnectionManager``. + connection manager by name instead, e.g. to force the SSH proxy when both Redfish + connections are configured. """ if not plugin_class.CONNECTION_TYPE: return None @@ -310,6 +343,8 @@ def _get_connection_manager_for_plugin( return None else: connection_manager_class: Type[ConnectionManager] = plugin_class.CONNECTION_TYPE + if connection_manager_class is RedfishConnectionManager: + return self._resolve_redfish_manager(plugin_name) if connection_manager_class.__name__ in self.plugin_registry.connection_managers: mgr_impl = self.plugin_registry.connection_managers[ connection_manager_class.__name__ diff --git a/test/unit/framework/test_plugin_connection_override.py b/test/unit/framework/test_plugin_connection_override.py index 165cd261..e716b6d3 100644 --- a/test/unit/framework/test_plugin_connection_override.py +++ b/test/unit/framework/test_plugin_connection_override.py @@ -24,28 +24,37 @@ # ############################################################################### import unittest -from typing import Any +from typing import Any, Optional from unittest.mock import patch -from nodescraper.connection.redfish import RedfishSshProxyConnectionManager +from nodescraper.connection.redfish import ( + RedfishConnectionManager, + RedfishSshProxyConnectionManager, +) from nodescraper.enums import OSFamily, SystemLocation +from nodescraper.interfaces import ConnectionManager from nodescraper.models import PluginConfig, SystemInfo from nodescraper.pluginexecutor import PluginExecutor PLUGIN = "RedfishEndpointPlugin" -OVERRIDE = "RedfishSshProxyConnectionManager" +DIRECT = "RedfishConnectionManager" +PROXY = "RedfishSshProxyConnectionManager" -PROXY_CONNECTION: dict[str, Any] = { - OVERRIDE: { +DIRECT_CONFIG: dict[str, Any] = { + DIRECT: {"host": "bmc.example.com", "username": "root", "password": "x"} +} +PROXY_CONFIG: dict[str, Any] = { + PROXY: { "host": "192.0.2.10", - "ssh": {"hostname": "bmc", "username": "root", "password": "x"}, + "ssh": {"hostname": "bmc.example.com", "username": "root", "password": "x"}, } } +BOTH_CONFIG: dict[str, Any] = {**DIRECT_CONFIG, **PROXY_CONFIG} -def _executor(plugin_args: dict, connections: dict) -> PluginExecutor: +def _executor(connections: dict, plugin_args: Optional[dict] = None) -> PluginExecutor: return PluginExecutor( - plugin_configs=[PluginConfig(plugins={PLUGIN: plugin_args})], + plugin_configs=[PluginConfig(plugins={PLUGIN: plugin_args or {}})], connections=connections, system_info=SystemInfo( name="test", location=SystemLocation.REMOTE, os_family=OSFamily.LINUX @@ -53,37 +62,52 @@ def _executor(plugin_args: dict, connections: dict) -> PluginExecutor: ) -class TestPluginConnectionOverride(unittest.TestCase): - def test_override_uses_configured_manager(self): - ex = _executor({"connection": OVERRIDE}, PROXY_CONNECTION) - plugin_cls = ex.plugin_registry.plugins[PLUGIN] - self.assertIsNot(plugin_cls.CONNECTION_TYPE, RedfishSshProxyConnectionManager) +def _resolve(ex: PluginExecutor, override: Optional[str] = None) -> Optional[ConnectionManager]: + plugin_cls = ex.plugin_registry.plugins[PLUGIN] + return ex._get_connection_manager_for_plugin(plugin_cls, PLUGIN, connection_override=override) - cm = ex._get_connection_manager_for_plugin(plugin_cls, PLUGIN, connection_override=OVERRIDE) - self.assertIs(cm, ex.connection_library[RedfishSshProxyConnectionManager]) +class TestRedfishConnectionSelection(unittest.TestCase): + def test_plugin_declares_direct_redfish(self): + ex = _executor({}) + self.assertIs(ex.plugin_registry.plugins[PLUGIN].CONNECTION_TYPE, RedfishConnectionManager) - def test_unregistered_name_returns_none(self): - ex = _executor({}, {}) - plugin_cls = ex.plugin_registry.plugins[PLUGIN] - self.assertIsNone( - ex._get_connection_manager_for_plugin( - plugin_cls, PLUGIN, connection_override="NotARealManager" - ) + def test_direct_only_uses_direct(self): + ex = _executor(DIRECT_CONFIG) + self.assertIs(_resolve(ex), ex.connection_library[RedfishConnectionManager]) + + def test_proxy_only_falls_back_to_proxy(self): + ex = _executor(PROXY_CONFIG) + self.assertIs(_resolve(ex), ex.connection_library[RedfishSshProxyConnectionManager]) + + def test_both_prefers_direct(self): + ex = _executor(BOTH_CONFIG) + self.assertIs(_resolve(ex), ex.connection_library[RedfishConnectionManager]) + + def test_neither_skips_plugin(self): + ex = _executor({}) + self.assertIsNone(_resolve(ex)) + self.assertNotIn(RedfishConnectionManager, ex.connection_library) + self.assertFalse( + ex._run_single_plugin(PLUGIN, ex.plugin_config.plugins[PLUGIN], [], queue_callback=None) ) + +class TestPluginConnectionOverride(unittest.TestCase): + def test_override_forces_proxy_when_both_configured(self): + ex = _executor(BOTH_CONFIG) + self.assertIs(_resolve(ex, PROXY), ex.connection_library[RedfishSshProxyConnectionManager]) + + def test_unregistered_name_returns_none(self): + ex = _executor(BOTH_CONFIG) + self.assertIsNone(_resolve(ex, "NotARealManager")) + def test_registered_but_unconfigured_returns_none(self): - ex = _executor({}, {}) - plugin_cls = ex.plugin_registry.plugins[PLUGIN] - self.assertIsNone( - ex._get_connection_manager_for_plugin(plugin_cls, PLUGIN, connection_override=OVERRIDE) - ) + ex = _executor(DIRECT_CONFIG) + self.assertIsNone(_resolve(ex, PROXY)) def test_connection_key_not_passed_to_run(self): - ex = _executor( - {"collection": False, "analysis": False, "connection": OVERRIDE}, - PROXY_CONNECTION, - ) + ex = _executor(BOTH_CONFIG, {"collection": False, "analysis": False, "connection": PROXY}) plugin_cls = ex.plugin_registry.plugins[PLUGIN] with patch.object(plugin_cls, "run", autospec=True) as mock_run: diff --git a/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py b/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py index 7b0dfe58..8dbf4ec3 100644 --- a/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py +++ b/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py @@ -25,8 +25,8 @@ ############################################################################### """RedfishEndpointPlugin run against a host behind a BMC via RedfishSshProxyConnectionManager. -Only the BMC SSH session and the curl call are faked; the connection manager, executor -connection override, collector and analyzer are the real implementations. +The plugin config names no connection: the executor falls back to the SSH proxy because it is +the only Redfish connection configured. Only the BMC SSH session and the curl call are faked. """ import json from typing import Any, cast @@ -63,7 +63,6 @@ } PLUGIN_ARGS: dict[str, Any] = { - "connection": "RedfishSshProxyConnectionManager", "collection_args": {"uris": ["/redfish/v1/Chassis", "/redfish/v1/Chassis/1"]}, "analysis_args": { "checks": { From d6a6afc4c49db55151e28d1608e06c0e1c581de4 Mon Sep 17 00:00:00 2001 From: Anson Yim Date: Thu, 29 Oct 2026 13:06:19 -0400 Subject: [PATCH 3/4] Remove plugin-config connection override; select Redfish transport from connection config Redfish plugins choose their connection only from the connection config: - RedfishConnectionManager present (alone or with the proxy): direct Redfish - only RedfishSshProxyConnectionManager present: SSH proxy - neither present: plugin skipped with an error Plugins whose CONNECTION_TYPE is RedfishSshProxyConnectionManager always use the proxy. The plugin config 'connection' key is removed. --- README.md | 2 +- nodescraper/pluginexecutor.py | 50 +++------------- ...y => test_redfish_connection_selection.py} | 57 +++++++------------ 3 files changed, 30 insertions(+), 79 deletions(-) rename test/unit/framework/{test_plugin_connection_override.py => test_redfish_connection_selection.py} (62%) diff --git a/README.md b/README.md index f4203b89..738195ea 100644 --- a/README.md +++ b/README.md @@ -291,7 +291,7 @@ Redfish plugins (those based on `OOBandDataPlugin`) pick their connection from t | present | present | direct Redfish | | absent | absent | plugin is skipped | -To force the proxy for one plugin while both blocks are configured, set `"connection": "RedfishSshProxyConnectionManager"` in that plugin's entry in the plugin config. The proxy also accepts a `targets` list, in the same shape as Redfish multi-target. OOB SSH plugins are unaffected and still require `RedfishConnectionManager`. +Plugins that declare `RedfishSshProxyConnectionManager` as their connection type (such as `AmcRedfishDiagPlugin`) always use the proxy, even when both blocks are configured. The proxy also accepts a `targets` list, in the same shape as Redfish multi-target. OOB SSH plugins are unaffected and still require `RedfishConnectionManager`. A sample is in `config/connection-config_redfish_ssh_proxy.example.json`. diff --git a/nodescraper/pluginexecutor.py b/nodescraper/pluginexecutor.py index 94f296a0..5d02b669 100644 --- a/nodescraper/pluginexecutor.py +++ b/nodescraper/pluginexecutor.py @@ -255,29 +255,6 @@ def discover_os_info(self) -> None: if inband_connection is not None: inband_connection.disconnect() - def _resolve_connection_override( - self, plugin_name: str, connection_override: str - ) -> Optional[ConnectionManager]: - """Return the prebuilt manager named by a plugin's ``connection`` config key. - - Args: - plugin_name: Registry key for the plugin (for logging). - connection_override: Connection manager name; must also appear in the connection config. - - Returns: - The connection manager instance, or None if it is not configured. - """ - mgr_impl = self.plugin_registry.connection_managers.get(connection_override) - conn_mgr = self.connection_library.get(mgr_impl) if mgr_impl else None - if conn_mgr is None: - self.logger.error( - "%s: connection %r must be a registered connection manager present in the " - "connection config", - plugin_name, - connection_override, - ) - return conn_mgr - def _resolve_redfish_manager(self, plugin_name: str) -> Optional[ConnectionManager]: """Pick the Redfish transport for a plugin declaring ``RedfishConnectionManager``. @@ -294,8 +271,12 @@ def _resolve_redfish_manager(self, plugin_name: str) -> Optional[ConnectionManag RedfishConnectionManager.__name__, RedfishSshProxyConnectionManager.__name__, ): - if name in self.connection_configs: - return self._resolve_connection_override(plugin_name, name) + if name not in self.connection_configs: + continue + mgr_impl = self.plugin_registry.connection_managers.get(name) + conn_mgr = self.connection_library.get(mgr_impl) if mgr_impl else None + if conn_mgr is not None: + return conn_mgr self.logger.error( "%s requires RedfishConnectionManager or RedfishSshProxyConnectionManager in the " "connection config", @@ -307,7 +288,6 @@ def _get_connection_manager_for_plugin( self, plugin_class: type, plugin_name: str, - connection_override: Optional[str] = None, ) -> Optional[ConnectionManager]: """Resolve and (if needed) initialise the connection manager for *plugin_class*. @@ -320,18 +300,12 @@ def _get_connection_manager_for_plugin( Plugins declaring ``RedfishConnectionManager`` fall back to ``RedfishSshProxyConnectionManager`` when only the proxy is configured - (see :meth:`_resolve_redfish_manager`). - - *connection_override* (a plugin config ``connection`` key) selects a configured - connection manager by name instead, e.g. to force the SSH proxy when both Redfish - connections are configured. + (see :meth:`_resolve_redfish_manager`). Plugins declaring + ``RedfishSshProxyConnectionManager`` always use the proxy. """ if not plugin_class.CONNECTION_TYPE: return None - if connection_override: - return self._resolve_connection_override(plugin_name, connection_override) - if issubclass(plugin_class, OOBSSHDataPlugin): mgr_impl = OobSshConnectionManager connection_args = self.connection_configs.get("RedfishConnectionManager") @@ -408,10 +382,6 @@ def _run_single_plugin( plugin_class = self.plugin_registry.plugins[plugin_name] - # "connection" selects the connection manager; it is not a run() argument. - plugin_args = dict(plugin_args) - connection_override = plugin_args.pop("connection", None) - init_payload = { "system_info": self.system_info, "logger": self.logger, @@ -421,9 +391,7 @@ def _run_single_plugin( } if plugin_class.CONNECTION_TYPE: - conn_mgr = self._get_connection_manager_for_plugin( - plugin_class, plugin_name, connection_override=connection_override - ) + conn_mgr = self._get_connection_manager_for_plugin(plugin_class, plugin_name) if conn_mgr is None: return False init_payload["connection_manager"] = conn_mgr diff --git a/test/unit/framework/test_plugin_connection_override.py b/test/unit/framework/test_redfish_connection_selection.py similarity index 62% rename from test/unit/framework/test_plugin_connection_override.py rename to test/unit/framework/test_redfish_connection_selection.py index e716b6d3..27ec1e8e 100644 --- a/test/unit/framework/test_plugin_connection_override.py +++ b/test/unit/framework/test_redfish_connection_selection.py @@ -25,7 +25,6 @@ ############################################################################### import unittest from typing import Any, Optional -from unittest.mock import patch from nodescraper.connection.redfish import ( RedfishConnectionManager, @@ -36,15 +35,14 @@ from nodescraper.models import PluginConfig, SystemInfo from nodescraper.pluginexecutor import PluginExecutor -PLUGIN = "RedfishEndpointPlugin" -DIRECT = "RedfishConnectionManager" -PROXY = "RedfishSshProxyConnectionManager" +REDFISH_PLUGIN = "RedfishEndpointPlugin" +PROXY_PLUGIN = "AmcRedfishDiagPlugin" DIRECT_CONFIG: dict[str, Any] = { - DIRECT: {"host": "bmc.example.com", "username": "root", "password": "x"} + "RedfishConnectionManager": {"host": "bmc.example.com", "username": "root", "password": "x"} } PROXY_CONFIG: dict[str, Any] = { - PROXY: { + "RedfishSshProxyConnectionManager": { "host": "192.0.2.10", "ssh": {"hostname": "bmc.example.com", "username": "root", "password": "x"}, } @@ -52,9 +50,9 @@ BOTH_CONFIG: dict[str, Any] = {**DIRECT_CONFIG, **PROXY_CONFIG} -def _executor(connections: dict, plugin_args: Optional[dict] = None) -> PluginExecutor: +def _executor(connections: dict) -> PluginExecutor: return PluginExecutor( - plugin_configs=[PluginConfig(plugins={PLUGIN: plugin_args or {}})], + plugin_configs=[PluginConfig(plugins={REDFISH_PLUGIN: {}})], connections=connections, system_info=SystemInfo( name="test", location=SystemLocation.REMOTE, os_family=OSFamily.LINUX @@ -62,15 +60,16 @@ def _executor(connections: dict, plugin_args: Optional[dict] = None) -> PluginEx ) -def _resolve(ex: PluginExecutor, override: Optional[str] = None) -> Optional[ConnectionManager]: - plugin_cls = ex.plugin_registry.plugins[PLUGIN] - return ex._get_connection_manager_for_plugin(plugin_cls, PLUGIN, connection_override=override) +def _resolve(ex: PluginExecutor, plugin_name: str = REDFISH_PLUGIN) -> Optional[ConnectionManager]: + plugin_cls = ex.plugin_registry.plugins[plugin_name] + return ex._get_connection_manager_for_plugin(plugin_cls, plugin_name) class TestRedfishConnectionSelection(unittest.TestCase): def test_plugin_declares_direct_redfish(self): ex = _executor({}) - self.assertIs(ex.plugin_registry.plugins[PLUGIN].CONNECTION_TYPE, RedfishConnectionManager) + plugin_cls = ex.plugin_registry.plugins[REDFISH_PLUGIN] + self.assertIs(plugin_cls.CONNECTION_TYPE, RedfishConnectionManager) def test_direct_only_uses_direct(self): ex = _executor(DIRECT_CONFIG) @@ -89,34 +88,18 @@ def test_neither_skips_plugin(self): self.assertIsNone(_resolve(ex)) self.assertNotIn(RedfishConnectionManager, ex.connection_library) self.assertFalse( - ex._run_single_plugin(PLUGIN, ex.plugin_config.plugins[PLUGIN], [], queue_callback=None) + ex._run_single_plugin( + REDFISH_PLUGIN, ex.plugin_config.plugins[REDFISH_PLUGIN], [], queue_callback=None + ) ) - -class TestPluginConnectionOverride(unittest.TestCase): - def test_override_forces_proxy_when_both_configured(self): - ex = _executor(BOTH_CONFIG) - self.assertIs(_resolve(ex, PROXY), ex.connection_library[RedfishSshProxyConnectionManager]) - - def test_unregistered_name_returns_none(self): + def test_plugin_declaring_proxy_keeps_proxy_when_both_configured(self): ex = _executor(BOTH_CONFIG) - self.assertIsNone(_resolve(ex, "NotARealManager")) - - def test_registered_but_unconfigured_returns_none(self): - ex = _executor(DIRECT_CONFIG) - self.assertIsNone(_resolve(ex, PROXY)) - - def test_connection_key_not_passed_to_run(self): - ex = _executor(BOTH_CONFIG, {"collection": False, "analysis": False, "connection": PROXY}) - plugin_cls = ex.plugin_registry.plugins[PLUGIN] - - with patch.object(plugin_cls, "run", autospec=True) as mock_run: - ran = ex._run_single_plugin( - PLUGIN, ex.plugin_config.plugins[PLUGIN], [], queue_callback=None - ) - - self.assertTrue(ran) - self.assertNotIn("connection", mock_run.call_args.kwargs) + plugin_cls = ex.plugin_registry.plugins[PROXY_PLUGIN] + self.assertIs(plugin_cls.CONNECTION_TYPE, RedfishSshProxyConnectionManager) + self.assertIs( + _resolve(ex, PROXY_PLUGIN), ex.connection_library[RedfishSshProxyConnectionManager] + ) if __name__ == "__main__": From c2f361c76ea7e8d3a7372fed0f54c231e3d5d0fa Mon Sep 17 00:00:00 2001 From: Anson Yim Date: Thu, 29 Oct 2026 13:37:28 -0400 Subject: [PATCH 4/4] Remove Redfish SSH-proxy example connection config The README section documents the connection block inline. --- README.md | 2 -- .../connection-config_redfish_ssh_proxy.example.json | 10 ---------- 2 files changed, 12 deletions(-) delete mode 100644 config/connection-config_redfish_ssh_proxy.example.json diff --git a/README.md b/README.md index 738195ea..b52a5762 100644 --- a/README.md +++ b/README.md @@ -293,8 +293,6 @@ Redfish plugins (those based on `OOBandDataPlugin`) pick their connection from t Plugins that declare `RedfishSshProxyConnectionManager` as their connection type (such as `AmcRedfishDiagPlugin`) always use the proxy, even when both blocks are configured. The proxy also accepts a `targets` list, in the same shape as Redfish multi-target. OOB SSH plugins are unaffected and still require `RedfishConnectionManager`. -A sample is in `config/connection-config_redfish_ssh_proxy.example.json`. - **Notes:** - If using SSH keys, specify `key_filename` instead of `password`. - The remote user must have permissions to run the requested plugins and access required files. If needed, use the `--skip-sudo` argument to skip plugins requiring sudo. diff --git a/config/connection-config_redfish_ssh_proxy.example.json b/config/connection-config_redfish_ssh_proxy.example.json deleted file mode 100644 index 4b4a57c8..00000000 --- a/config/connection-config_redfish_ssh_proxy.example.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "RedfishSshProxyConnectionManager": { - "host": "192.0.2.10", - "ssh": { - "hostname": "bmc.example.com", - "username": "root", - "password": "placeholder" - } - } -}