diff --git a/nodescraper/connection/redfish/redfish_connection.py b/nodescraper/connection/redfish/redfish_connection.py index 8711ff4..449b4ed 100644 --- a/nodescraper/connection/redfish/redfish_connection.py +++ b/nodescraper/connection/redfish/redfish_connection.py @@ -183,6 +183,18 @@ def run_get(self, path: Union[str, RedfishPath]) -> RedfishGetResult: status_code=None, ) + def copy(self) -> "RedfishConnection": + """Return a new connection with the same config and its own session (for concurrent use).""" + return RedfishConnection( + base_url=self.base_url, + username=self.username, + password=self.password, + timeout=self.timeout, + use_session_auth=self.use_session_auth, + verify_ssl=self.verify_ssl, + api_root=self.api_root, + ) + def get_service_root(self) -> dict[str, Any]: """GET service root (e.g. /redfish/v1/).""" return self.get(RedfishPath(self.api_root)) diff --git a/nodescraper/models/event.py b/nodescraper/models/event.py index df6a61e..33cf280 100644 --- a/nodescraper/models/event.py +++ b/nodescraper/models/event.py @@ -28,12 +28,28 @@ import re import uuid from enum import Enum -from typing import Optional, Union +from typing import Any, Optional, Union from pydantic import BaseModel, Field, field_serializer, field_validator from nodescraper.enums import EventPriority + +def _data_to_json_safe(obj: Any) -> Any: + """Recursively convert event data to JSON-serializable form (e.g. exceptions -> str).""" + if isinstance(obj, BaseException): + return str(obj) + if isinstance(obj, dict): + return {k: _data_to_json_safe(v) for k, v in obj.items()} + if isinstance(obj, (list, tuple)): + return [_data_to_json_safe(v) for v in obj] + if isinstance(obj, (str, int, float, bool, type(None))): + return obj + if isinstance(obj, (Enum, datetime.datetime, uuid.UUID)): + return str(obj) + return str(obj) + + LOG_LEVEL_MAP = { logging.INFO: EventPriority.INFO, logging.WARNING: EventPriority.WARNING, @@ -127,6 +143,11 @@ def serialize_priority(self, priority: EventPriority, _info) -> str: """ return priority.name + @field_serializer("data") + def serialize_data(self, data: dict, _info) -> dict: + """Ensure data is JSON-serializable (e.g. convert nested exceptions to str).""" + return _data_to_json_safe(data) + @field_validator("data") @classmethod def validate_data(cls, data: dict) -> dict: diff --git a/nodescraper/plugins/ooband/redfish_endpoint/collector_args.py b/nodescraper/plugins/ooband/redfish_endpoint/collector_args.py index c63cd8d..662caff 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/collector_args.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/collector_args.py @@ -27,9 +27,34 @@ class RedfishEndpointCollectorArgs(BaseModel): - """Collection args: uris to GET.""" + """Collection args: uris to GET (or discover from tree), optional concurrency and tree discovery.""" - uris: list[str] = Field(default_factory=list) + uris: list[str] = Field( + default_factory=list, + description="Redfish URIs to GET. Ignored when discover_tree is True.", + ) + discover_tree: bool = Field( + default=False, + description="If True, discover endpoints from the BMC Redfish tree (service root and links) instead of using uris.", + ) + tree_max_depth: int = Field( + default=2, + ge=1, + le=10, + description="When discover_tree is True: max traversal depth (1=service root only, 2=root + collections, 3=+ members).", + ) + tree_max_endpoints: int = Field( + default=0, + ge=0, + le=10_000, + description="When discover_tree is True: max endpoints to discover (0=no limit).", + ) + max_workers: int = Field( + default=1, + ge=1, + le=32, + description="Max concurrent GETs (1=sequential). Use >1 for async endpoint fetches.", + ) @field_validator("uris", mode="before") @classmethod diff --git a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py index 6323a43..59dd7a8 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py @@ -129,10 +129,10 @@ def analyze_data( ) if failed: - details = "; ".join(f"{f['uri']} {f['path']}: {f['reason']}" for f in failed) + description = f"Redfish endpoint checks failed: {len(failed)} failure(s)" self._log_event( category=EventCategory.TELEMETRY, - description=f"Redfish endpoint checks failed: {len(failed)} failure(s) — {details}", + description=description, data={"failures": failed}, priority=EventPriority.WARNING, console_log=True, diff --git a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_collector.py b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_collector.py index 39dacf7..0a1305a 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_collector.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_collector.py @@ -23,15 +23,99 @@ # SOFTWARE. # ############################################################################### -from typing import Optional +from collections import deque +from concurrent.futures import ThreadPoolExecutor, as_completed +from typing import Any, Optional +from urllib.parse import urlparse from nodescraper.base import RedfishDataCollector +from nodescraper.connection.redfish import RedfishConnection, RedfishGetResult from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus from nodescraper.models import TaskResult from .collector_args import RedfishEndpointCollectorArgs from .endpoint_data import RedfishEndpointDataModel +ODATA_ID = "@odata.id" +MEMBERS = "Members" + + +def _normalize_path(odata_id: str, api_root: str) -> str: + """Convert @odata.id value (URL or path) to a normalized path under api_root.""" + if not odata_id or not isinstance(odata_id, str): + return "" + s = odata_id.strip() + if s.startswith(("http://", "https://")): + parsed = urlparse(s) + s = parsed.path or "/" + if not s.startswith("/"): + s = "/" + s + s = s.rstrip("/") or "/" + api_root_norm = api_root.strip("/") + if api_root_norm and not s.startswith("/" + api_root_norm): + return "" + return s + + +def _extract_odata_ids(obj: Any) -> list[str]: + """Recursively extract all @odata.id values from a Redfish JSON body.""" + out: list[str] = [] + if isinstance(obj, dict): + if ODATA_ID in obj and isinstance(obj[ODATA_ID], str): + out.append(obj[ODATA_ID]) + for k, v in obj.items(): + if k == MEMBERS and isinstance(v, list): + for item in v: + if ( + isinstance(item, dict) + and ODATA_ID in item + and isinstance(item[ODATA_ID], str) + ): + out.append(item[ODATA_ID]) + elif isinstance(v, dict): + out.extend(_extract_odata_ids(v)) + elif isinstance(v, list): + for item in v: + if isinstance(item, dict): + out.extend(_extract_odata_ids(item)) + return out + + +def _discover_tree( + connection: RedfishConnection, + api_root: str, + max_depth: int, + max_endpoints: int, +) -> tuple[list[str], dict[str, dict], list[RedfishGetResult]]: + """ + Traverse the Redfish resource tree from the service root. + + max_depth matches collection_args.tree_max_depth: 1 = service root only; 2 = root + one link + level; child links are only enqueued when depth + 1 < max_depth (root is depth 0). + """ + root_path = _normalize_path(api_root, api_root) or ("/" + api_root.strip("/")) + seen: set[str] = set() + to_visit: deque[tuple[str, int]] = deque([(root_path, 0)]) + responses: dict[str, dict] = {} + results: list[RedfishGetResult] = [] + while to_visit: + if max_endpoints and len(seen) >= max_endpoints: + break + path, depth = to_visit.popleft() + if path in seen or depth > max_depth: + continue + seen.add(path) + res = connection.run_get(path) + results.append(res) + if res.success and res.data is not None: + responses[path] = res.data + for odata_id in _extract_odata_ids(res.data): + link_path = _normalize_path(odata_id, api_root) + # Follow only if the child depth stays strictly below max_depth (1 = root only). + if link_path and link_path not in seen and depth + 1 < max_depth: + to_visit.append((link_path, depth + 1)) + return sorted(seen), responses, results + def _uris_from_args(args: Optional[RedfishEndpointCollectorArgs]) -> list[str]: """Return list of URIs from collector args.uris.""" @@ -40,6 +124,18 @@ def _uris_from_args(args: Optional[RedfishEndpointCollectorArgs]) -> list[str]: return list(args.uris) if args.uris else [] +def _discover_tree_enabled(args: Optional[RedfishEndpointCollectorArgs]) -> bool: + """True only when tree discovery is explicitly enabled (avoids string/other truthy junk).""" + if args is None: + return False + return getattr(args, "discover_tree", False) is True + + +def _fetch_one(connection_copy: RedfishConnection, path: str) -> RedfishGetResult: + """Run a single GET on a connection copy (used from worker threads).""" + return connection_copy.run_get(path) + + class RedfishEndpointCollector( RedfishDataCollector[RedfishEndpointDataModel, RedfishEndpointCollectorArgs] ): @@ -50,30 +146,106 @@ class RedfishEndpointCollector( def collect_data( self, args: Optional[RedfishEndpointCollectorArgs] = None ) -> tuple[TaskResult, Optional[RedfishEndpointDataModel]]: - """GET each configured Redfish URI via _run_redfish_get() and store the JSON response.""" + """Collect via tree discovery, or via explicit URIs, or skip if neither is configured.""" uris = _uris_from_args(args) + use_tree = _discover_tree_enabled(args) + + # 1) Tree discovery: only when discover_tree is explicitly true + if use_tree: + api_root = getattr(self.connection, "api_root", "redfish/v1") + max_depth = getattr(args, "tree_max_depth", 2) if args else 2 + max_endpoints = (getattr(args, "tree_max_endpoints", 0) or 0) if args else 0 + _paths, responses, results = _discover_tree( + self.connection, + api_root, + max_depth=max_depth, + max_endpoints=max_endpoints, + ) + for res in results: + self.result.artifacts.append(res) + if not res.success and res.error: + self._log_event( + category=EventCategory.RUNTIME, + description=f"Redfish GET failed during tree discovery for {res.path}: {res.error}", + priority=EventPriority.WARNING, + console_log=True, + ) + if not responses: + self.result.message = "No Redfish endpoints discovered from tree" + self.result.status = ExecutionStatus.ERROR + return self.result, None + data = RedfishEndpointDataModel(responses=responses) + self.result.message = f"Collected {len(responses)} Redfish endpoint(s) from tree" + self.result.status = ExecutionStatus.OK + return self.result, data + + # 2) URI list: when discover_tree is false/absent and uris are provided if not uris: - self.result.message = "No Redfish URIs configured" + self.result.message = ( + "No collection mode configured: set collection_args.discover_tree to true " + "or provide collection_args.uris" + ) self.result.status = ExecutionStatus.NOT_RAN return self.result, None - responses: dict[str, dict] = {} + paths = [] for uri in uris: - path = uri + path = uri.strip() if uri else "" if not path: continue if not path.startswith("/"): path = "/" + path - res = self._run_redfish_get(path, log_artifact=True) - if res.success and res.data is not None: - responses[res.path] = res.data - else: - self._log_event( - category=EventCategory.RUNTIME, - description=f"Redfish GET failed for {path}: {res.error or 'unknown'}", - priority=EventPriority.WARNING, - console_log=True, - ) + paths.append(path) + + max_workers = getattr(args, "max_workers", 1) if args else 1 + max_workers = min(max_workers, len(paths)) + + if max_workers <= 1: + # Sequential + responses = {} + for path in paths: + res = self._run_redfish_get(path, log_artifact=True) + if res.success and res.data is not None: + responses[res.path] = res.data + else: + self._log_event( + category=EventCategory.RUNTIME, + description=f"Redfish GET failed for {path}: {res.error or 'unknown'}", + priority=EventPriority.WARNING, + console_log=True, + ) + else: + # Concurrent: one connection copy per worker, collect results in main thread + responses = {} + results = [] + with ThreadPoolExecutor(max_workers=max_workers) as executor: + futures = {} + for path in paths: + conn = self.connection.copy() + futures[executor.submit(_fetch_one, conn, path)] = path + for future in as_completed(futures): + path = futures[future] + try: + res = future.result() + results.append(res) + if res.success and res.data is not None: + responses[res.path] = res.data + else: + self._log_event( + category=EventCategory.RUNTIME, + description=f"Redfish GET failed for {path}: {res.error or 'unknown'}", + priority=EventPriority.WARNING, + console_log=True, + ) + except Exception as e: + self._log_event( + category=EventCategory.RUNTIME, + description=f"Redfish GET failed for {path}: {e!s}", + priority=EventPriority.WARNING, + console_log=True, + ) + for res in results: + self.result.artifacts.append(res) if not responses: self.result.message = "No Redfish endpoints could be read" diff --git a/test/functional/conftest.py b/test/functional/conftest.py index 77ded95..0e8f78a 100644 --- a/test/functional/conftest.py +++ b/test/functional/conftest.py @@ -28,9 +28,24 @@ import subprocess import sys from typing import List +from unittest.mock import MagicMock import pytest +from nodescraper.models.systeminfo import OSFamily, SystemInfo + + +@pytest.fixture +def system_info(): + """Minimal SystemInfo for collectors that require it (same shape as test/unit/conftest).""" + return SystemInfo(name="test_host", platform="X", os_family=OSFamily.LINUX, sku="TEST") + + +@pytest.fixture +def redfish_conn_mock(): + """MagicMock Redfish connection for Redfish plugin tests.""" + return MagicMock() + @pytest.fixture def run_cli_command(): diff --git a/test/functional/fixtures/redfish_endpoint_plugin_config_full_args.json b/test/functional/fixtures/redfish_endpoint_plugin_config_full_args.json new file mode 100644 index 0000000..099057c --- /dev/null +++ b/test/functional/fixtures/redfish_endpoint_plugin_config_full_args.json @@ -0,0 +1,26 @@ +{ + "plugins": { + "RedfishEndpointPlugin": { + "collection_args": { + "uris": ["/redfish/v1", "/redfish/v1/Systems"], + "discover_tree": false, + "tree_max_depth": 3, + "tree_max_endpoints": 100, + "max_workers": 2 + }, + "analysis_args": { + "checks": { + "/redfish/v1/Systems/1": { + "Status/Health": { "anyOf": ["OK", "Warning"] }, + "PowerState": { "eq": "On" }, + "PowerConsumedWatts": { "min": 0, "max": 500 } + }, + "*": { + "Status/State": { "eq": "Enabled" }, + "Status/Health": { "anyOf": ["OK", "Warning"] } + } + } + } + } + } +} diff --git a/test/functional/test_redfish_endpoint_plugin.py b/test/functional/test_redfish_endpoint_plugin.py index 2a25043..f2513b6 100644 --- a/test/functional/test_redfish_endpoint_plugin.py +++ b/test/functional/test_redfish_endpoint_plugin.py @@ -23,92 +23,368 @@ # SOFTWARE. # ############################################################################### -from pathlib import Path +from unittest.mock import MagicMock import pytest +from nodescraper.connection.redfish import RedfishGetResult +from nodescraper.enums import EventCategory, ExecutionStatus +from nodescraper.plugins.ooband.redfish_endpoint import ( + RedfishEndpointCollector, + RedfishEndpointCollectorArgs, +) +from nodescraper.plugins.ooband.redfish_endpoint import endpoint_collector as ec + @pytest.fixture -def fixtures_dir(): - """Return path to functional test fixtures directory.""" - return Path(__file__).parent / "fixtures" +def redfish_endpoint_collector(system_info, redfish_conn_mock): + return RedfishEndpointCollector( + system_info=system_info, + connection=redfish_conn_mock, + ) -@pytest.fixture -def redfish_plugin_config(fixtures_dir): - """Path to RedfishEndpointPlugin config (URIs + checks).""" - return fixtures_dir / "redfish_endpoint_plugin_config.json" +def test_redfish_endpoint_collector_no_uris(redfish_endpoint_collector): + result, data = redfish_endpoint_collector.collect_data() + assert result.status == ExecutionStatus.NOT_RAN + assert ( + result.message + == "No collection mode configured: set collection_args.discover_tree to true or provide collection_args.uris" + ) + assert data is None -@pytest.fixture -def redfish_connection_config(fixtures_dir): - """Path to Redfish connection config (RedfishConnectionManager).""" - return fixtures_dir / "redfish_connection_config.json" +def test_redfish_endpoint_collector_no_uris_with_args(redfish_endpoint_collector): + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs(uris=[]) + ) + assert result.status == ExecutionStatus.NOT_RAN + assert data is None -def test_redfish_endpoint_plugin_with_config_and_connection( - run_cli_command, redfish_plugin_config, redfish_connection_config, tmp_path -): - assert redfish_plugin_config.exists(), f"Config not found: {redfish_plugin_config}" - assert redfish_connection_config.exists(), f"Config not found: {redfish_connection_config}" +def test_redfish_endpoint_collector_one_uri_success(redfish_endpoint_collector, redfish_conn_mock): + redfish_conn_mock.run_get.return_value = RedfishGetResult( + path="/redfish/v1", + success=True, + data={"Name": "Root"}, + status_code=200, + ) + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs(uris=["/redfish/v1"]) + ) + assert result.status == ExecutionStatus.OK + assert result.message == "Collected 1 Redfish endpoint(s)" + assert data is not None + assert data.responses["/redfish/v1"]["Name"] == "Root" + redfish_conn_mock.run_get.assert_called_once() + call_path = redfish_conn_mock.run_get.call_args[0][0] + assert call_path == "/redfish/v1" or call_path.strip("/") == "redfish/v1" - log_path = str(tmp_path / "logs_redfish") - result = run_cli_command( - [ - "--log-path", - log_path, - "--connection-config", - str(redfish_connection_config), - "--plugin-configs=" + str(redfish_plugin_config), - "run-plugins", - "RedfishEndpointPlugin", - ], - check=False, + +def test_redfish_endpoint_collector_uri_normalized_with_leading_slash( + redfish_endpoint_collector, redfish_conn_mock +): + redfish_conn_mock.run_get.return_value = RedfishGetResult( + path="/redfish/v1/Systems", + success=True, + data={"Members": []}, + status_code=200, + ) + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs(uris=["redfish/v1/Systems"]) ) + assert result.status == ExecutionStatus.OK + assert data is not None + assert "/redfish/v1/Systems" in data.responses or "redfish/v1/Systems" in data.responses - output = result.stdout + result.stderr - assert "RedfishEndpointPlugin" in output or "Redfish" in output +def test_redfish_endpoint_collector_one_fail_no_success( + redfish_endpoint_collector, redfish_conn_mock +): + redfish_conn_mock.run_get.return_value = RedfishGetResult( + path="/redfish/v1", + success=False, + error="Connection refused", + status_code=None, + ) + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs(uris=["/redfish/v1"]) + ) + assert result.status == ExecutionStatus.ERROR + assert result.message.startswith("No Redfish endpoints could be read") + assert data is None + assert len(result.events) >= 1 + assert any( + e.category == EventCategory.RUNTIME.value or "Redfish GET failed" in (e.description or "") + for e in result.events + ) -def test_redfish_endpoint_plugin_plugin_config_only( - run_cli_command, redfish_plugin_config, tmp_path + +def test_redfish_endpoint_collector_mixed_success_fail( + redfish_endpoint_collector, redfish_conn_mock ): - assert redfish_plugin_config.exists() + def run_get_side_effect(path): + path_str = str(path) + if "Systems" in path_str: + return RedfishGetResult( + path=path_str if path_str.startswith("/") else "/" + path_str, + success=True, + data={"Id": "1"}, + status_code=200, + ) + return RedfishGetResult( + path=path_str if path_str.startswith("/") else "/" + path_str, + success=False, + error="Not Found", + status_code=404, + ) - log_path = str(tmp_path / "logs_redfish_noconn") - result = run_cli_command( - [ - "--log-path", - log_path, - "--plugin-configs=" + str(redfish_plugin_config), - "run-plugins", - "RedfishEndpointPlugin", - ], - check=False, + redfish_conn_mock.run_get.side_effect = run_get_side_effect + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs(uris=["/redfish/v1/Systems", "/redfish/v1/Bad"]) ) + assert result.status == ExecutionStatus.OK + assert data is not None + assert len(data.responses) == 1 + keys = list(data.responses.keys()) + assert any("Systems" in k for k in keys) + assert list(data.responses.values())[0].get("Id") == "1" - output = result.stdout + result.stderr - assert "RedfishEndpointPlugin" in output or "Redfish" in output +def test_normalize_path_empty_or_invalid(): + api = "redfish/v1" + assert ec._normalize_path("", api) == "" + assert ec._normalize_path(None, api) == "" # type: ignore[arg-type] + assert ec._normalize_path(" ", api) == "" -def test_redfish_endpoint_plugin_default_subcommand( - run_cli_command, redfish_plugin_config, redfish_connection_config, tmp_path -): - assert redfish_plugin_config.exists() - assert redfish_connection_config.exists() - - log_path = str(tmp_path / "logs_redfish_default") - result = run_cli_command( - [ - "--log-path", - log_path, - "--connection-config", - str(redfish_connection_config), - "--plugin-configs=" + str(redfish_plugin_config), - "RedfishEndpointPlugin", - ], - check=False, - ) - - output = result.stdout + result.stderr - assert "RedfishEndpointPlugin" in output or "Redfish" in output + +def test_normalize_path_relative_path(): + api = "redfish/v1" + assert ec._normalize_path("/redfish/v1", api) == "/redfish/v1" + assert ec._normalize_path("/redfish/v1/", api) == "/redfish/v1" + assert ec._normalize_path("redfish/v1/Systems", api) == "/redfish/v1/Systems" + assert ec._normalize_path(" /redfish/v1/Chassis ", api) == "/redfish/v1/Chassis" + + +def test_normalize_path_full_url(): + api = "redfish/v1" + assert ec._normalize_path("https://host/redfish/v1/Systems", api) == "/redfish/v1/Systems" + assert ec._normalize_path("http://bmc/redfish/v1", api) == "/redfish/v1" + + +def test_normalize_path_outside_api_root(): + api = "redfish/v1" + assert ec._normalize_path("/other/root", api) == "" + assert ec._normalize_path("https://host/other/path", api) == "" + + +def test_extract_odata_ids_empty(): + assert ec._extract_odata_ids({}) == [] + assert ec._extract_odata_ids([]) == [] + assert ec._extract_odata_ids("x") == [] + + +def test_extract_odata_ids_single(): + assert ec._extract_odata_ids({"@odata.id": "/redfish/v1"}) == ["/redfish/v1"] + assert ec._extract_odata_ids({"@odata.id": "https://host/redfish/v1"}) == [ + "https://host/redfish/v1" + ] + + +def test_extract_odata_ids_members(): + body = { + "Members": [ + {"@odata.id": "/redfish/v1/Systems/1"}, + {"@odata.id": "/redfish/v1/Systems/2"}, + ] + } + assert set(ec._extract_odata_ids(body)) == {"/redfish/v1/Systems/1", "/redfish/v1/Systems/2"} + + +def test_extract_odata_ids_nested_and_members(): + body = { + "@odata.id": "/redfish/v1", + "Systems": {"@odata.id": "/redfish/v1/Systems", "Members": []}, + "Chassis": { + "Members": [{"@odata.id": "/redfish/v1/Chassis/1"}], + }, + } + ids = ec._extract_odata_ids(body) + assert "/redfish/v1" in ids + assert "/redfish/v1/Systems" in ids + assert "/redfish/v1/Chassis/1" in ids + + +def test_uris_from_args_none(): + assert ec._uris_from_args(None) == [] + + +def test_uris_from_args_empty(): + assert ec._uris_from_args(RedfishEndpointCollectorArgs(uris=[])) == [] + + +def test_uris_from_args_with_uris(): + args = RedfishEndpointCollectorArgs(uris=["/redfish/v1", "/redfish/v1/Systems"]) + assert ec._uris_from_args(args) == ["/redfish/v1", "/redfish/v1/Systems"] + + +def test_fetch_one_calls_run_get(): + conn = MagicMock() + conn.run_get.return_value = RedfishGetResult( + path="/redfish/v1", success=True, data={}, status_code=200 + ) + out = ec._fetch_one(conn, "/redfish/v1") + conn.run_get.assert_called_once_with("/redfish/v1") + assert out.success is True + assert out.path == "/redfish/v1" + + +def test_discover_tree_single_root(): + conn = MagicMock() + conn.run_get.return_value = RedfishGetResult( + path="/redfish/v1", + success=True, + data={"@odata.id": "/redfish/v1", "Name": "Root"}, + status_code=200, + ) + paths, responses, results = ec._discover_tree( + conn, api_root="redfish/v1", max_depth=2, max_endpoints=0 + ) + assert paths == ["/redfish/v1"] + assert list(responses.keys()) == ["/redfish/v1"] + assert responses["/redfish/v1"]["Name"] == "Root" + assert len(results) == 1 + conn.run_get.assert_called_once_with("/redfish/v1") + + +def test_discover_tree_follows_links(): + conn = MagicMock() + root_data = { + "@odata.id": "/redfish/v1", + "Systems": {"@odata.id": "/redfish/v1/Systems"}, + } + systems_data = { + "@odata.id": "/redfish/v1/Systems", + "Members": [{"@odata.id": "/redfish/v1/Systems/1"}], + } + system1_data = {"@odata.id": "/redfish/v1/Systems/1", "Id": "1"} + + def run_get(path): + if path == "/redfish/v1": + return RedfishGetResult(path=path, success=True, data=root_data, status_code=200) + if path == "/redfish/v1/Systems": + return RedfishGetResult(path=path, success=True, data=systems_data, status_code=200) + if path == "/redfish/v1/Systems/1": + return RedfishGetResult(path=path, success=True, data=system1_data, status_code=200) + return RedfishGetResult(path=path, success=False, error="Not Found", status_code=404) + + conn.run_get.side_effect = run_get + paths, responses, results = ec._discover_tree( + conn, api_root="redfish/v1", max_depth=3, max_endpoints=0 + ) + assert "/redfish/v1" in paths + assert "/redfish/v1/Systems" in paths + assert "/redfish/v1/Systems/1" in paths + assert responses["/redfish/v1"]["@odata.id"] == "/redfish/v1" + assert responses["/redfish/v1/Systems"]["@odata.id"] == "/redfish/v1/Systems" + assert responses["/redfish/v1/Systems/1"]["Id"] == "1" + assert len(results) >= 3 + + +def test_discover_tree_respects_max_depth(): + conn = MagicMock() + root_data = {"@odata.id": "/redfish/v1", "Systems": {"@odata.id": "/redfish/v1/Systems"}} + systems_data = { + "@odata.id": "/redfish/v1/Systems", + "Members": [{"@odata.id": "/redfish/v1/Systems/1"}], + } + + def run_get(path): + if path == "/redfish/v1": + return RedfishGetResult(path=path, success=True, data=root_data, status_code=200) + if path == "/redfish/v1/Systems": + return RedfishGetResult(path=path, success=True, data=systems_data, status_code=200) + return RedfishGetResult(path=path, success=True, data={}, status_code=200) + + conn.run_get.side_effect = run_get + paths, responses, results = ec._discover_tree( + conn, api_root="redfish/v1", max_depth=1, max_endpoints=0 + ) + assert "/redfish/v1" in paths + assert "/redfish/v1/Systems" not in paths + assert len(responses) == 1 + + +def test_discover_tree_respects_max_endpoints(): + conn = MagicMock() + conn.run_get.return_value = RedfishGetResult( + path="/redfish/v1", + success=True, + data={"@odata.id": "/redfish/v1", "Systems": {"@odata.id": "/redfish/v1/Systems"}}, + status_code=200, + ) + paths, responses, results = ec._discover_tree( + conn, api_root="redfish/v1", max_depth=5, max_endpoints=1 + ) + assert len(paths) == 1 + assert len(responses) == 1 + conn.run_get.assert_called_once() + + +def test_collect_data_discover_tree_success(redfish_endpoint_collector, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.return_value = RedfishGetResult( + path="/redfish/v1", + success=True, + data={"@odata.id": "/redfish/v1", "Name": "Root"}, + status_code=200, + ) + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs(discover_tree=True) + ) + assert result.status == ExecutionStatus.OK + assert result.message == "Collected 1 Redfish endpoint(s) from tree" + assert data is not None + assert "/redfish/v1" in data.responses + assert data.responses["/redfish/v1"]["Name"] == "Root" + + +def test_collect_data_discover_tree_no_responses(redfish_endpoint_collector, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.return_value = RedfishGetResult( + path="/redfish/v1", success=False, error="Connection refused", status_code=None + ) + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs(discover_tree=True) + ) + assert result.status == ExecutionStatus.ERROR + assert "No Redfish endpoints discovered from tree" in result.message + assert data is None + + +def test_collect_data_concurrent_two_uris(redfish_endpoint_collector, redfish_conn_mock): + redfish_conn_mock.copy.return_value = redfish_conn_mock + call_count = 0 + + def run_get(path): + nonlocal call_count + call_count += 1 + return RedfishGetResult( + path=path if path.startswith("/") else "/" + path, + success=True, + data={"path": path}, + status_code=200, + ) + + redfish_conn_mock.run_get.side_effect = run_get + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs( + uris=["/redfish/v1", "/redfish/v1/Systems"], max_workers=2 + ) + ) + assert result.status == ExecutionStatus.OK + assert data is not None + assert len(data.responses) == 2 + assert "/redfish/v1" in data.responses or "redfish/v1" in data.responses + assert any("Systems" in k for k in data.responses) + assert redfish_conn_mock.copy.called diff --git a/test/unit/plugin/test_redfish_endpoint_collector.py b/test/unit/plugin/test_redfish_endpoint_collector.py index c2893ee..7a78640 100644 --- a/test/unit/plugin/test_redfish_endpoint_collector.py +++ b/test/unit/plugin/test_redfish_endpoint_collector.py @@ -23,6 +23,8 @@ # SOFTWARE. # ############################################################################### +from unittest.mock import MagicMock + import pytest from nodescraper.connection.redfish import RedfishGetResult @@ -31,6 +33,7 @@ RedfishEndpointCollector, RedfishEndpointCollectorArgs, ) +from nodescraper.plugins.ooband.redfish_endpoint import endpoint_collector as ec @pytest.fixture @@ -44,7 +47,7 @@ def redfish_endpoint_collector(system_info, redfish_conn_mock): def test_redfish_endpoint_collector_no_uris(redfish_endpoint_collector): result, data = redfish_endpoint_collector.collect_data() assert result.status == ExecutionStatus.NOT_RAN - assert result.message == "No Redfish URIs configured" + assert "No collection mode configured" in result.message assert data is None @@ -53,6 +56,7 @@ def test_redfish_endpoint_collector_no_uris_with_args(redfish_endpoint_collector args=RedfishEndpointCollectorArgs(uris=[]) ) assert result.status == ExecutionStatus.NOT_RAN + assert "No collection mode configured" in result.message assert data is None @@ -143,3 +147,242 @@ def run_get_side_effect(path): keys = list(data.responses.keys()) assert any("Systems" in k for k in keys) assert list(data.responses.values())[0].get("Id") == "1" + + +def test_normalize_path_empty_or_invalid(): + api = "redfish/v1" + assert ec._normalize_path("", api) == "" + assert ec._normalize_path(None, api) == "" # type: ignore[arg-type] + assert ec._normalize_path(" ", api) == "" + + +def test_normalize_path_relative_path(): + api = "redfish/v1" + assert ec._normalize_path("/redfish/v1", api) == "/redfish/v1" + assert ec._normalize_path("/redfish/v1/", api) == "/redfish/v1" + assert ec._normalize_path("redfish/v1/Systems", api) == "/redfish/v1/Systems" + assert ec._normalize_path(" /redfish/v1/Chassis ", api) == "/redfish/v1/Chassis" + + +def test_normalize_path_full_url(): + api = "redfish/v1" + assert ec._normalize_path("https://host/redfish/v1/Systems", api) == "/redfish/v1/Systems" + assert ec._normalize_path("http://bmc/redfish/v1", api) == "/redfish/v1" + + +def test_normalize_path_outside_api_root(): + api = "redfish/v1" + assert ec._normalize_path("/other/root", api) == "" + assert ec._normalize_path("https://host/other/path", api) == "" + + +def test_extract_odata_ids_empty(): + assert ec._extract_odata_ids({}) == [] + assert ec._extract_odata_ids([]) == [] + assert ec._extract_odata_ids("x") == [] + + +def test_extract_odata_ids_single(): + assert ec._extract_odata_ids({"@odata.id": "/redfish/v1"}) == ["/redfish/v1"] + assert ec._extract_odata_ids({"@odata.id": "https://host/redfish/v1"}) == [ + "https://host/redfish/v1" + ] + + +def test_extract_odata_ids_members(): + body = { + "Members": [ + {"@odata.id": "/redfish/v1/Systems/1"}, + {"@odata.id": "/redfish/v1/Systems/2"}, + ] + } + assert set(ec._extract_odata_ids(body)) == {"/redfish/v1/Systems/1", "/redfish/v1/Systems/2"} + + +def test_extract_odata_ids_nested_and_members(): + body = { + "@odata.id": "/redfish/v1", + "Systems": {"@odata.id": "/redfish/v1/Systems", "Members": []}, + "Chassis": { + "Members": [{"@odata.id": "/redfish/v1/Chassis/1"}], + }, + } + ids = ec._extract_odata_ids(body) + assert "/redfish/v1" in ids + assert "/redfish/v1/Systems" in ids + assert "/redfish/v1/Chassis/1" in ids + + +def test_uris_from_args_none(): + assert ec._uris_from_args(None) == [] + + +def test_uris_from_args_empty(): + assert ec._uris_from_args(RedfishEndpointCollectorArgs(uris=[])) == [] + + +def test_uris_from_args_with_uris(): + args = RedfishEndpointCollectorArgs(uris=["/redfish/v1", "/redfish/v1/Systems"]) + assert ec._uris_from_args(args) == ["/redfish/v1", "/redfish/v1/Systems"] + + +def test_fetch_one_calls_run_get(): + conn = MagicMock() + conn.run_get.return_value = RedfishGetResult( + path="/redfish/v1", success=True, data={}, status_code=200 + ) + out = ec._fetch_one(conn, "/redfish/v1") + conn.run_get.assert_called_once_with("/redfish/v1") + assert out.success is True + assert out.path == "/redfish/v1" + + +def test_discover_tree_single_root(): + conn = MagicMock() + conn.run_get.return_value = RedfishGetResult( + path="/redfish/v1", + success=True, + data={"@odata.id": "/redfish/v1", "Name": "Root"}, + status_code=200, + ) + paths, responses, results = ec._discover_tree( + conn, api_root="redfish/v1", max_depth=2, max_endpoints=0 + ) + assert paths == ["/redfish/v1"] + assert list(responses.keys()) == ["/redfish/v1"] + assert responses["/redfish/v1"]["Name"] == "Root" + assert len(results) == 1 + conn.run_get.assert_called_once_with("/redfish/v1") + + +def test_discover_tree_follows_links(): + conn = MagicMock() + root_data = { + "@odata.id": "/redfish/v1", + "Systems": {"@odata.id": "/redfish/v1/Systems"}, + } + systems_data = { + "@odata.id": "/redfish/v1/Systems", + "Members": [{"@odata.id": "/redfish/v1/Systems/1"}], + } + system1_data = {"@odata.id": "/redfish/v1/Systems/1", "Id": "1"} + + def run_get(path): + if path == "/redfish/v1": + return RedfishGetResult(path=path, success=True, data=root_data, status_code=200) + if path == "/redfish/v1/Systems": + return RedfishGetResult(path=path, success=True, data=systems_data, status_code=200) + if path == "/redfish/v1/Systems/1": + return RedfishGetResult(path=path, success=True, data=system1_data, status_code=200) + return RedfishGetResult(path=path, success=False, error="Not Found", status_code=404) + + conn.run_get.side_effect = run_get + paths, responses, results = ec._discover_tree( + conn, api_root="redfish/v1", max_depth=3, max_endpoints=0 + ) + assert "/redfish/v1" in paths + assert "/redfish/v1/Systems" in paths + assert "/redfish/v1/Systems/1" in paths + assert responses["/redfish/v1"]["@odata.id"] == "/redfish/v1" + assert responses["/redfish/v1/Systems"]["@odata.id"] == "/redfish/v1/Systems" + assert responses["/redfish/v1/Systems/1"]["Id"] == "1" + assert len(results) >= 3 + + +def test_discover_tree_respects_max_depth(): + conn = MagicMock() + root_data = {"@odata.id": "/redfish/v1", "Systems": {"@odata.id": "/redfish/v1/Systems"}} + systems_data = { + "@odata.id": "/redfish/v1/Systems", + "Members": [{"@odata.id": "/redfish/v1/Systems/1"}], + } + + def run_get(path): + if path == "/redfish/v1": + return RedfishGetResult(path=path, success=True, data=root_data, status_code=200) + if path == "/redfish/v1/Systems": + return RedfishGetResult(path=path, success=True, data=systems_data, status_code=200) + return RedfishGetResult(path=path, success=True, data={}, status_code=200) + + conn.run_get.side_effect = run_get + paths, responses, results = ec._discover_tree( + conn, api_root="redfish/v1", max_depth=1, max_endpoints=0 + ) + assert "/redfish/v1" in paths + assert "/redfish/v1/Systems" not in paths + assert len(responses) == 1 + + +def test_discover_tree_respects_max_endpoints(): + conn = MagicMock() + conn.run_get.return_value = RedfishGetResult( + path="/redfish/v1", + success=True, + data={"@odata.id": "/redfish/v1", "Systems": {"@odata.id": "/redfish/v1/Systems"}}, + status_code=200, + ) + paths, responses, results = ec._discover_tree( + conn, api_root="redfish/v1", max_depth=5, max_endpoints=1 + ) + assert len(paths) == 1 + assert len(responses) == 1 + conn.run_get.assert_called_once() + + +def test_collect_data_discover_tree_success(redfish_endpoint_collector, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.return_value = RedfishGetResult( + path="/redfish/v1", + success=True, + data={"@odata.id": "/redfish/v1", "Name": "Root"}, + status_code=200, + ) + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs(discover_tree=True) + ) + assert result.status == ExecutionStatus.OK + assert result.message == "Collected 1 Redfish endpoint(s) from tree" + assert data is not None + assert "/redfish/v1" in data.responses + assert data.responses["/redfish/v1"]["Name"] == "Root" + + +def test_collect_data_discover_tree_no_responses(redfish_endpoint_collector, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.return_value = RedfishGetResult( + path="/redfish/v1", success=False, error="Connection refused", status_code=None + ) + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs(discover_tree=True) + ) + assert result.status == ExecutionStatus.ERROR + assert result.message.startswith("No Redfish endpoints discovered from tree") + assert data is None + + +def test_collect_data_concurrent_two_uris(redfish_endpoint_collector, redfish_conn_mock): + redfish_conn_mock.copy.return_value = redfish_conn_mock + call_count = 0 + + def run_get(path): + nonlocal call_count + call_count += 1 + return RedfishGetResult( + path=path if path.startswith("/") else "/" + path, + success=True, + data={"path": path}, + status_code=200, + ) + + redfish_conn_mock.run_get.side_effect = run_get + result, data = redfish_endpoint_collector.collect_data( + args=RedfishEndpointCollectorArgs( + uris=["/redfish/v1", "/redfish/v1/Systems"], max_workers=2 + ) + ) + assert result.status == ExecutionStatus.OK + assert data is not None + assert len(data.responses) == 2 + assert "/redfish/v1" in data.responses or "redfish/v1" in data.responses + assert any("Systems" in k for k in data.responses) + assert redfish_conn_mock.copy.called