From 927a1395d8b2b940cb2ae0331cdffd53fd3bd685 Mon Sep 17 00:00:00 2001 From: Luke Andrews Date: Wed, 18 Jun 2025 17:16:43 -0400 Subject: [PATCH 1/4] dissallow extra input for args --- nodescraper/plugins/inband/bios/analyzer_args.py | 2 ++ nodescraper/plugins/inband/cmdline/analyzer_args.py | 2 ++ nodescraper/plugins/inband/dkms/analyzer_args.py | 8 ++++++++ nodescraper/plugins/inband/dkms/dkms_analyzer.py | 2 +- nodescraper/plugins/inband/dmesg/analyzer_args.py | 2 ++ nodescraper/plugins/inband/kernel/analyzer_args.py | 2 ++ nodescraper/plugins/inband/memory/analyzer_args.py | 2 ++ nodescraper/plugins/inband/package/analyzer_args.py | 2 ++ nodescraper/plugins/inband/process/analyzer_args.py | 2 ++ nodescraper/plugins/inband/process/collector_args.py | 2 ++ nodescraper/plugins/inband/rocm/analyzer_args.py | 2 ++ nodescraper/plugins/inband/storage/analyzer_args.py | 2 ++ 12 files changed, 29 insertions(+), 1 deletion(-) diff --git a/nodescraper/plugins/inband/bios/analyzer_args.py b/nodescraper/plugins/inband/bios/analyzer_args.py index edb7cb2a..ac6ecbca 100644 --- a/nodescraper/plugins/inband/bios/analyzer_args.py +++ b/nodescraper/plugins/inband/bios/analyzer_args.py @@ -30,6 +30,8 @@ class BiosAnalyzerArgs(BaseModel): exp_bios_version: list[str] = Field(default_factory=list) regex_match: bool = False + model_config = {"extra": "forbid"} + @field_validator("exp_bios_version", mode="before") @classmethod def validate_exp_bios_version(cls, exp_bios_version: str | list) -> list: diff --git a/nodescraper/plugins/inband/cmdline/analyzer_args.py b/nodescraper/plugins/inband/cmdline/analyzer_args.py index 1c97e52a..be47f4fd 100644 --- a/nodescraper/plugins/inband/cmdline/analyzer_args.py +++ b/nodescraper/plugins/inband/cmdline/analyzer_args.py @@ -30,6 +30,8 @@ class CmdlineAnalyzerArgs(BaseModel): required_cmdline: str | list = Field(default_factory=list) banned_cmdline: str | list = Field(default_factory=list) + model_config = {"extra": "forbid"} + @field_validator("required_cmdline", mode="before") @classmethod def validate_required_cmdline(cls, required_cmdline: str | list) -> list: diff --git a/nodescraper/plugins/inband/dkms/analyzer_args.py b/nodescraper/plugins/inband/dkms/analyzer_args.py index 4810a48b..e4fa7193 100644 --- a/nodescraper/plugins/inband/dkms/analyzer_args.py +++ b/nodescraper/plugins/inband/dkms/analyzer_args.py @@ -23,6 +23,8 @@ # SOFTWARE. # ############################################################################### +from typing import Any + from pydantic import BaseModel, Field, field_validator @@ -31,6 +33,12 @@ class DkmsAnalyzerArgs(BaseModel): dkms_version: str | list = Field(default_factory=list) regex_match: bool = False + model_config = {"extra": "forbid"} + + def model_post_init(self, __context: Any) -> None: + if not self.dkms_status and not self.dkms_version: + raise ValueError("At least one of dkms_status or dkms_version must be provided") + @field_validator("dkms_status", mode="before") @classmethod def validate_dkms_status(cls, dkms_status: str | list) -> list: diff --git a/nodescraper/plugins/inband/dkms/dkms_analyzer.py b/nodescraper/plugins/inband/dkms/dkms_analyzer.py index aa040b7d..6ccdc01e 100644 --- a/nodescraper/plugins/inband/dkms/dkms_analyzer.py +++ b/nodescraper/plugins/inband/dkms/dkms_analyzer.py @@ -67,8 +67,8 @@ def analyze_data( error_state = False for check, accepted_values in check_map.items(): + actual_value = getattr(data, check) for accepted_value in accepted_values: - actual_value = getattr(data, check) if args.regex_match: try: regex_data = re.compile(accepted_value) diff --git a/nodescraper/plugins/inband/dmesg/analyzer_args.py b/nodescraper/plugins/inband/dmesg/analyzer_args.py index 8b4e8e04..aa0eb2f7 100644 --- a/nodescraper/plugins/inband/dmesg/analyzer_args.py +++ b/nodescraper/plugins/inband/dmesg/analyzer_args.py @@ -31,3 +31,5 @@ class DmesgAnalyzerArgs(TimeRangeAnalyisArgs): check_unknown_dmesg_errors: Optional[bool] = True exclude_category: Optional[set[str]] = None + + model_config = {"extra": "forbid"} diff --git a/nodescraper/plugins/inband/kernel/analyzer_args.py b/nodescraper/plugins/inband/kernel/analyzer_args.py index 920903b1..b6212987 100644 --- a/nodescraper/plugins/inband/kernel/analyzer_args.py +++ b/nodescraper/plugins/inband/kernel/analyzer_args.py @@ -30,6 +30,8 @@ class KernelAnalyzerArgs(BaseModel): exp_kernel: str | list = Field(default_factory=list) regex_match: bool = False + model_config = {"extra": "forbid"} + @field_validator("exp_kernel", mode="before") @classmethod def validate_exp_kernel(cls, exp_kernel: str | list) -> list: diff --git a/nodescraper/plugins/inband/memory/analyzer_args.py b/nodescraper/plugins/inband/memory/analyzer_args.py index cc5f0ef4..1f58840f 100644 --- a/nodescraper/plugins/inband/memory/analyzer_args.py +++ b/nodescraper/plugins/inband/memory/analyzer_args.py @@ -29,3 +29,5 @@ class MemoryAnalyzerArgs(BaseModel): ratio: float = 0.66 memory_threshold: str = "30Gi" + + model_config = {"extra": "forbid"} diff --git a/nodescraper/plugins/inband/package/analyzer_args.py b/nodescraper/plugins/inband/package/analyzer_args.py index d8eca70a..2ea9d3de 100644 --- a/nodescraper/plugins/inband/package/analyzer_args.py +++ b/nodescraper/plugins/inband/package/analyzer_args.py @@ -29,3 +29,5 @@ class PackageAnalyzerArgs(BaseModel): exp_package_ver: dict[str, str | None] = Field(default_factory=dict) regex_match: bool = True + + model_config = {"extra": "forbid"} diff --git a/nodescraper/plugins/inband/process/analyzer_args.py b/nodescraper/plugins/inband/process/analyzer_args.py index 1a8ba83f..5159288d 100644 --- a/nodescraper/plugins/inband/process/analyzer_args.py +++ b/nodescraper/plugins/inband/process/analyzer_args.py @@ -29,3 +29,5 @@ class ProcessAnalyzerArgs(BaseModel): max_kfd_processes: int = 0 max_cpu_usage: int = 20 + + model_config = {"extra": "forbid"} diff --git a/nodescraper/plugins/inband/process/collector_args.py b/nodescraper/plugins/inband/process/collector_args.py index ea5d2367..e0d54652 100644 --- a/nodescraper/plugins/inband/process/collector_args.py +++ b/nodescraper/plugins/inband/process/collector_args.py @@ -28,3 +28,5 @@ class ProcessCollectorArgs(BaseModel): top_n_process: int = 10 + + model_config = {"extra": "forbid"} diff --git a/nodescraper/plugins/inband/rocm/analyzer_args.py b/nodescraper/plugins/inband/rocm/analyzer_args.py index 3ece3c1c..e7e7023c 100644 --- a/nodescraper/plugins/inband/rocm/analyzer_args.py +++ b/nodescraper/plugins/inband/rocm/analyzer_args.py @@ -29,6 +29,8 @@ class RocmAnalyzerArgs(BaseModel): exp_rocm: str | list = Field(default_factory=list) + model_config = {"extra": "forbid"} + @field_validator("exp_rocm", mode="before") @classmethod def validate_exp_rocm(cls, exp_rocm: str | list) -> list: diff --git a/nodescraper/plugins/inband/storage/analyzer_args.py b/nodescraper/plugins/inband/storage/analyzer_args.py index 1f44d1d3..5c061e41 100644 --- a/nodescraper/plugins/inband/storage/analyzer_args.py +++ b/nodescraper/plugins/inband/storage/analyzer_args.py @@ -34,3 +34,5 @@ class StorageAnalyzerArgs(BaseModel): ignore_devices: Optional[list[str]] = Field(default_factory=list) check_devices: Optional[list[str]] = Field(default_factory=list) regex_match: bool = False + + model_config = {"extra": "forbid"} From 4a17ea7ca555217182925fa20955142c75d55b90 Mon Sep 17 00:00:00 2001 From: Luke Andrews Date: Wed, 18 Jun 2025 17:17:08 -0400 Subject: [PATCH 2/4] task result: ensure that not ran status is logged --- nodescraper/models/taskresult.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/nodescraper/models/taskresult.py b/nodescraper/models/taskresult.py index d9039321..8f22d4dc 100644 --- a/nodescraper/models/taskresult.py +++ b/nodescraper/models/taskresult.py @@ -33,6 +33,15 @@ from .event import Event +STATUS_LOG_LEVEL_MAP = { + ExecutionStatus.UNSET: logging.INFO, + ExecutionStatus.NOT_RAN: logging.INFO, + ExecutionStatus.OK: logging.INFO, + ExecutionStatus.WARNING: logging.WARNING, + ExecutionStatus.ERROR: logging.ERROR, + ExecutionStatus.EXECUTION_FAILURE: logging.CRITICAL, +} + class TaskResult(BaseModel): """Object for result of a task""" @@ -133,4 +142,9 @@ def finalize(self, logger: Optional[logging.Logger] = None) -> None: self.message += f" ({event_summary})" if logger: - logger.log(self.status.value, "(%s) %s", self.__class__.__name__, self.message) + logger.log( + STATUS_LOG_LEVEL_MAP.get(self.status, logging.INFO), + "(%s) %s", + self.parent, + self.message, + ) From aaba09644fb0792913a963fd0b2131ece5e19728 Mon Sep 17 00:00:00 2001 From: Luke Andrews Date: Wed, 18 Jun 2025 17:17:45 -0400 Subject: [PATCH 3/4] improve handling of validation errors --- nodescraper/cli/inputargtypes.py | 2 +- nodescraper/interfaces/dataanalyzertask.py | 3 +-- nodescraper/interfaces/datacollectortask.py | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/nodescraper/cli/inputargtypes.py b/nodescraper/cli/inputargtypes.py index 2ef061d9..ac0ba17f 100644 --- a/nodescraper/cli/inputargtypes.py +++ b/nodescraper/cli/inputargtypes.py @@ -104,7 +104,7 @@ def process_file_arg(self, file_path: str) -> TModelType: return self.model(**data) except ValidationError as e: raise argparse.ArgumentTypeError( - f"Validation errors when processing {file_path}: {e.errors()}" + f"Validation errors when processing {file_path}: {e.errors(include_url=False)}" ) from e diff --git a/nodescraper/interfaces/dataanalyzertask.py b/nodescraper/interfaces/dataanalyzertask.py index 18da1ef3..116ae029 100644 --- a/nodescraper/interfaces/dataanalyzertask.py +++ b/nodescraper/interfaces/dataanalyzertask.py @@ -76,13 +76,12 @@ def wrapper( if not analyze_arg_model: raise ValueError("No model defined for analysis args") args = analyze_arg_model(**args) # type: ignore - func(analyzer, data, args) except ValidationError as exception: analyzer._log_event( category=EventCategory.RUNTIME, description="Validation error during analysis", - data=get_exception_traceback(exception), + data={"errors": exception.errors(include_url=False)}, priority=EventPriority.CRITICAL, console_log=True, ) diff --git a/nodescraper/interfaces/datacollectortask.py b/nodescraper/interfaces/datacollectortask.py index d9c926cc..7a778ed7 100644 --- a/nodescraper/interfaces/datacollectortask.py +++ b/nodescraper/interfaces/datacollectortask.py @@ -41,7 +41,7 @@ from nodescraper.interfaces.task import SystemCompatibilityError, Task from nodescraper.models import DataModel, SystemInfo, TaskResult from nodescraper.typeutils import TypeUtils -from nodescraper.utils import get_exception_details, get_exception_traceback +from nodescraper.utils import get_exception_traceback from .connectionmanager import TConnection from .taskresulthook import TaskResultHook @@ -76,7 +76,7 @@ def wrapper( collector._log_event( category=EventCategory.RUNTIME, description="Pydantic validation error", - data=get_exception_details(exception), + data={"errors": exception.errors(include_url=False)}, priority=EventPriority.CRITICAL, console_log=True, ) From ccdfeb0184bbea3130ab36e20bce12ad4ad373e7 Mon Sep 17 00:00:00 2001 From: Luke Andrews Date: Wed, 18 Jun 2025 17:18:00 -0400 Subject: [PATCH 4/4] unit test fixes --- test/unit/plugin/test_dkms_analyzer.py | 2 +- test/unit/plugin/test_storage_analyzer.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/plugin/test_dkms_analyzer.py b/test/unit/plugin/test_dkms_analyzer.py index 01145c0b..1e182b15 100644 --- a/test/unit/plugin/test_dkms_analyzer.py +++ b/test/unit/plugin/test_dkms_analyzer.py @@ -95,7 +95,7 @@ def test_missing_data(system_info, model_obj, config): def test_invalid_data(system_info, model_obj, config): analyzer = DkmsAnalyzer(system_info=system_info) - args = DkmsAnalyzerArgs(dkms_status=config["status"], invalid=config["invalid"]) + args = DkmsAnalyzerArgs(dkms_status=config["status"], dkms_version=config["invalid"]) result = analyzer.analyze_data(data=model_obj, args=args) assert result.status == ExecutionStatus.ERROR diff --git a/test/unit/plugin/test_storage_analyzer.py b/test/unit/plugin/test_storage_analyzer.py index 21a16a8c..bf2b4ef6 100644 --- a/test/unit/plugin/test_storage_analyzer.py +++ b/test/unit/plugin/test_storage_analyzer.py @@ -146,7 +146,7 @@ def test_both_abs_and_prct_fail(system_info): assert any(e.category == EventCategory.STORAGE.value for e in result.events) assert any(e.priority == EventPriority.CRITICAL for e in result.events) - args2 = StorageAnalyzerArgs(min_required_free_space_prct=40, min_required_dree_space_abs="1GB") + args2 = StorageAnalyzerArgs(min_required_free_space_prct=40, min_required_free_space_abs="1GB") result2 = analyzer.analyze_data(model, args2) assert result2.status == ExecutionStatus.OK