diff --git a/README.md b/README.md index 1084308a..b52a5762 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,34 @@ 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 | + +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`. + **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 +559,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 +605,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/nodescraper/pluginexecutor.py b/nodescraper/pluginexecutor.py index 4571ab03..5d02b669 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 ( @@ -251,6 +255,35 @@ def discover_os_info(self) -> None: if inband_connection is not None: inband_connection.disconnect() + 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 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", + plugin_name, + ) + return None + def _get_connection_manager_for_plugin( self, plugin_class: type, @@ -264,6 +297,11 @@ 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`). Plugins declaring + ``RedfishSshProxyConnectionManager`` always use the proxy. """ if not plugin_class.CONNECTION_TYPE: return None @@ -279,6 +317,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/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_redfish_connection_selection.py b/test/unit/framework/test_redfish_connection_selection.py new file mode 100644 index 00000000..27ec1e8e --- /dev/null +++ b/test/unit/framework/test_redfish_connection_selection.py @@ -0,0 +1,106 @@ +############################################################################### +# +# 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, Optional + +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 + +REDFISH_PLUGIN = "RedfishEndpointPlugin" +PROXY_PLUGIN = "AmcRedfishDiagPlugin" + +DIRECT_CONFIG: dict[str, Any] = { + "RedfishConnectionManager": {"host": "bmc.example.com", "username": "root", "password": "x"} +} +PROXY_CONFIG: dict[str, Any] = { + "RedfishSshProxyConnectionManager": { + "host": "192.0.2.10", + "ssh": {"hostname": "bmc.example.com", "username": "root", "password": "x"}, + } +} +BOTH_CONFIG: dict[str, Any] = {**DIRECT_CONFIG, **PROXY_CONFIG} + + +def _executor(connections: dict) -> PluginExecutor: + return PluginExecutor( + plugin_configs=[PluginConfig(plugins={REDFISH_PLUGIN: {}})], + connections=connections, + system_info=SystemInfo( + name="test", location=SystemLocation.REMOTE, os_family=OSFamily.LINUX + ), + ) + + +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({}) + 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) + 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( + REDFISH_PLUGIN, ex.plugin_config.plugins[REDFISH_PLUGIN], [], queue_callback=None + ) + ) + + def test_plugin_declaring_proxy_keeps_proxy_when_both_configured(self): + ex = _executor(BOTH_CONFIG) + 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__": + 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..8dbf4ec3 --- /dev/null +++ b/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py @@ -0,0 +1,158 @@ +############################################################################### +# +# 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. + +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 +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] = { + "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