From 2d7d21a9a338f1832f18c9e569b7a555918bb7de Mon Sep 17 00:00:00 2001 From: EfratIfergan Date: Thu, 6 Aug 2026 13:09:48 +0300 Subject: [PATCH 1/2] EDM-4980: fix inventory crash on mount-only application volumes The flightctl-client SDK's pydantic model for ApplicationVolume declares image as required, but RHEM returns mount-only volumes without image. Add a fallback that catches pydantic ValidationError and retries with raw JSON endpoints, bypassing pydantic deserialization. Co-Authored-By: Claude Opus 4.6 --- plugins/inventory/flightctl.py | 73 +++++++- .../unit/plugins/inventory/test_flightctl.py | 173 ++++++++++++++++++ 2 files changed, 241 insertions(+), 5 deletions(-) diff --git a/plugins/inventory/flightctl.py b/plugins/inventory/flightctl.py index ff2b8e2..41336ea 100644 --- a/plugins/inventory/flightctl.py +++ b/plugins/inventory/flightctl.py @@ -153,6 +153,7 @@ from ansible.utils.display import Display from contextlib import contextmanager from enum import Enum +import json import re import base64 import tempfile @@ -333,7 +334,9 @@ def _populate_inventory_fleets(self, fleets: List[Any], config) -> None: if len(fleets) == 0: return - for fleet in [fleet.to_dict() for fleet in fleets]: + for raw_fleet in fleets: + fleet = raw_fleet.to_dict() if hasattr(raw_fleet, 'to_dict') else raw_fleet + fleet = _convert_enums_to_strings(fleet) fleet_id = _validate_fleet(fleet) devices = _fetch_fleet_devices(fleet_id, config, self.LIMIT_PER_PAGE) or [] self.info(f"Retrieved {len(devices)} devices from fleet {fleet_id}") @@ -482,6 +485,48 @@ def _build_auth_headers(config: Configuration) -> Dict[str, str] | None: # ---------------------- Static methods -------------------------- +def _is_pydantic_validation_error(exc: Exception) -> bool: + """Check if an exception is a pydantic ValidationError without importing pydantic.""" + exc_type = type(exc) + return exc_type.__name__ == 'ValidationError' and 'pydantic' in getattr(exc_type, '__module__', '') + + +def _get_data_raw( + list_func: Callable[..., Any], + label_list: str | None = None, + field_list: str | None = None, + limit: int | None = 1000, + headers: Dict[str, str] | None = None, + request_timeout: float | None = None, +) -> List[Dict[str, Any]]: + """Fallback pagination using a *_without_preload_content endpoint that returns raw JSON.""" + all_records: list[Dict[str, Any]] = [] + continue_token: Optional[str] = None + + while True: + try: + response = list_func( + var_continue=continue_token, + label_selector=label_list, + field_selector=field_list, + limit=limit, + _headers=headers, + _request_timeout=request_timeout, + ) + except Exception as e: + raise FlightctlApiException(f"Error retrieving data from Flight Control API: {e}") from e + data = json.loads(response.data) + records = data.get('items', []) + all_records.extend(records) + metadata = data.get('metadata', {}) + continue_token = metadata.get('continue', None) + + if not continue_token: + break + + return all_records + + def _get_data( list_func: Callable[..., Any], label_list: str | None = None, @@ -489,6 +534,7 @@ def _get_data( limit: int | None = 1000, headers: Dict[str, str] | None = None, request_timeout: float | None = None, + fallback_list_func: Callable[..., Any] | None = None, ) -> List[T]: """ Repeatedly call `list_func` until exhausted; return combined list """ all_records: list[T] = [] @@ -506,6 +552,19 @@ def _get_data( _request_timeout=request_timeout, ) except Exception as e: + if fallback_list_func is not None and _is_pydantic_validation_error(e): + Display().warning( + "Flight Control client SDK raised a pydantic validation error; " + f"falling back to raw JSON deserialization: {e}" + ) + return _get_data_raw( + fallback_list_func, + label_list=label_list, + field_list=field_list, + limit=limit, + headers=headers, + request_timeout=request_timeout, + ) raise FlightctlApiException(f"Error retrieving data from Flight Control API: {e}") from e records: Sequence[T] = response.items all_records.extend(records) @@ -717,7 +776,8 @@ def _fetch_fleet_devices(fleet_id: str, config, limit_per_page: int) -> List[Any field_list=field_list, limit=limit_per_page, headers=headers, - request_timeout=getattr(config, 'request_timeout', None) + request_timeout=getattr(config, 'request_timeout', None), + fallback_list_func=device_api.list_devices_without_preload_content, ) return devices @@ -735,13 +795,15 @@ def _get_devices_and_fleets(config, limit_per_page: int) -> Tuple[List[DeviceLis device_api.list_devices, limit=limit_per_page, headers=headers, - request_timeout=getattr(config, 'request_timeout', None) + request_timeout=getattr(config, 'request_timeout', None), + fallback_list_func=device_api.list_devices_without_preload_content, ) all_fleets = _get_data( fleet_api.list_fleets, limit=limit_per_page, headers=headers, - request_timeout=getattr(config, 'request_timeout', None) + request_timeout=getattr(config, 'request_timeout', None), + fallback_list_func=fleet_api.list_fleets_without_preload_content, ) return all_devices, all_fleets @@ -760,7 +822,8 @@ def _get_devices_by_labels_and_fields(config, label_selectors: str | None, field field_list=field_selectors, limit=limit_per_page, headers=headers, - request_timeout=getattr(config, 'request_timeout', None) + request_timeout=getattr(config, 'request_timeout', None), + fallback_list_func=device_api.list_devices_without_preload_content, ) return devices diff --git a/tests/unit/plugins/inventory/test_flightctl.py b/tests/unit/plugins/inventory/test_flightctl.py index 2719f33..9d636ce 100644 --- a/tests/unit/plugins/inventory/test_flightctl.py +++ b/tests/unit/plugins/inventory/test_flightctl.py @@ -1,3 +1,4 @@ +import json import unittest from typing import ClassVar from unittest.mock import MagicMock, patch @@ -8,10 +9,15 @@ from plugins.inventory.flightctl import ( DOCUMENTATION, InventoryModule, + _build_auth_headers, + _get_data, + _get_data_raw, + _is_pydantic_validation_error, _render_hostname_expression, _resolve_hostname, _validate_device, ) +from plugins.module_utils.exceptions import FlightctlApiException, ValidationException class TestFlightCtlInventoryModule(unittest.TestCase): @@ -602,5 +608,172 @@ def test_env_vars_match_module_utils_convention(self): f"Env var '{expected_env}' should start with FLIGHTCTL_") +class TestIsPydanticValidationError(unittest.TestCase): + """Verify _is_pydantic_validation_error detects pydantic errors without importing pydantic.""" + + def test_real_pydantic_validation_error(self): + try: + from pydantic import ValidationError, BaseModel + + class StrictModel(BaseModel): + value: int + + try: + StrictModel(value="not-an-int") # type: ignore[arg-type] + except ValidationError as exc: + self.assertTrue(_is_pydantic_validation_error(exc)) + except ImportError: + self.skipTest("pydantic not installed") + + def test_generic_exception_returns_false(self): + self.assertFalse(_is_pydantic_validation_error(ValueError("nope"))) + + def test_non_pydantic_validation_error_returns_false(self): + class ValidationError(Exception): + __module__ = "myapp.errors" + + self.assertFalse(_is_pydantic_validation_error(ValidationError("nope"))) + + +class TestGetDataRawFallback(unittest.TestCase): + """Verify _get_data_raw paginates through raw HTTP responses.""" + + def _make_raw_response(self, items, continue_token=None): + metadata = {} + if continue_token: + metadata['continue'] = continue_token + body = json.dumps({'items': items, 'metadata': metadata}).encode() + resp = MagicMock() + resp.data = body + return resp + + def test_single_page(self): + items = [{'metadata': {'name': 'dev-1'}}, {'metadata': {'name': 'dev-2'}}] + list_func = MagicMock(return_value=self._make_raw_response(items)) + + result = _get_data_raw(list_func, limit=100) + self.assertEqual(len(result), 2) + self.assertEqual(result[0]['metadata']['name'], 'dev-1') + list_func.assert_called_once() + + def test_multi_page_pagination(self): + page1 = self._make_raw_response( + [{'metadata': {'name': 'dev-1'}}], continue_token='token-abc' + ) + page2 = self._make_raw_response( + [{'metadata': {'name': 'dev-2'}}] + ) + list_func = MagicMock(side_effect=[page1, page2]) + + result = _get_data_raw(list_func, limit=1) + self.assertEqual(len(result), 2) + self.assertEqual(result[0]['metadata']['name'], 'dev-1') + self.assertEqual(result[1]['metadata']['name'], 'dev-2') + self.assertEqual(list_func.call_count, 2) + + def test_api_error_raises_flightctl_exception(self): + list_func = MagicMock(side_effect=ConnectionError("timeout")) + with self.assertRaises(FlightctlApiException): + _get_data_raw(list_func) + + +class TestGetDataPydanticFallback(unittest.TestCase): + """Verify _get_data falls back to raw JSON on pydantic ValidationError.""" + + def _make_typed_response(self, items, continue_token=None): + resp = MagicMock() + resp.items = items + metadata = {} + if continue_token: + metadata['continue'] = continue_token + resp.to_dict.return_value = {'metadata': metadata} + return resp + + def _make_raw_response(self, items, continue_token=None): + metadata = {} + if continue_token: + metadata['continue'] = continue_token + body = json.dumps({'items': items, 'metadata': metadata}).encode() + resp = MagicMock() + resp.data = body + return resp + + def _make_pydantic_error(self): + try: + from pydantic import ValidationError, BaseModel + + class StrictModel(BaseModel): + value: int + + try: + StrictModel(value="not-an-int") # type: ignore[arg-type] + except ValidationError as exc: + return exc + except ImportError: + return None + + def test_normal_path_no_fallback_needed(self): + typed_resp = self._make_typed_response([MagicMock(), MagicMock()]) + list_func = MagicMock(return_value=typed_resp) + fallback_func = MagicMock() + + result = _get_data(list_func, fallback_list_func=fallback_func) + self.assertEqual(len(result), 2) + fallback_func.assert_not_called() + + def test_pydantic_error_triggers_fallback(self): + pydantic_exc = self._make_pydantic_error() + if pydantic_exc is None: + self.skipTest("pydantic not installed") + + list_func = MagicMock(side_effect=pydantic_exc) + raw_items = [{'metadata': {'name': 'dev-1'}}] + fallback_func = MagicMock(return_value=self._make_raw_response(raw_items)) + + result = _get_data(list_func, fallback_list_func=fallback_func) + self.assertEqual(len(result), 1) + self.assertEqual(result[0]['metadata']['name'], 'dev-1') + fallback_func.assert_called_once() + + def test_non_pydantic_error_still_raises(self): + list_func = MagicMock(side_effect=ConnectionError("refused")) + fallback_func = MagicMock() + + with self.assertRaises(FlightctlApiException): + _get_data(list_func, fallback_list_func=fallback_func) + fallback_func.assert_not_called() + + def test_pydantic_error_without_fallback_raises(self): + pydantic_exc = self._make_pydantic_error() + if pydantic_exc is None: + self.skipTest("pydantic not installed") + + list_func = MagicMock(side_effect=pydantic_exc) + + with self.assertRaises(FlightctlApiException): + _get_data(list_func, fallback_list_func=None) + + +class TestPopulateInventoryFleetsWithRawDicts(unittest.TestCase): + """Verify _populate_inventory_fleets handles raw dicts (fallback path).""" + + @patch('plugins.inventory.flightctl._fetch_fleet_devices') + def test_fleet_as_raw_dict(self, mock_fetch): + mock_fetch.return_value = [] + inventory = InventoryModule() + mock_inv = MagicMock() + mock_groups = MagicMock() + mock_groups.__contains__ = MagicMock(return_value=False) + mock_inv.groups = mock_groups + inventory.inventory = mock_inv + + raw_fleet = {'metadata': {'name': 'fleet-1'}} + config = MagicMock() + + inventory._populate_inventory_fleets([raw_fleet], config) + + mock_fetch.assert_called_once_with('fleet-1', config, inventory.LIMIT_PER_PAGE) + + if __name__ == '__main__': unittest.main() From bd8f054063cdb0fa8c75771fb61e02d403a6110d Mon Sep 17 00:00:00 2001 From: EfratIfergan Date: Thu, 6 Aug 2026 13:19:34 +0300 Subject: [PATCH 2/2] fix: remove unused imports flagged by pylint sanity check Co-Authored-By: Claude Opus 4.6 --- tests/unit/plugins/inventory/test_flightctl.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/plugins/inventory/test_flightctl.py b/tests/unit/plugins/inventory/test_flightctl.py index 9d636ce..e76d28d 100644 --- a/tests/unit/plugins/inventory/test_flightctl.py +++ b/tests/unit/plugins/inventory/test_flightctl.py @@ -9,7 +9,6 @@ from plugins.inventory.flightctl import ( DOCUMENTATION, InventoryModule, - _build_auth_headers, _get_data, _get_data_raw, _is_pydantic_validation_error, @@ -17,7 +16,7 @@ _resolve_hostname, _validate_device, ) -from plugins.module_utils.exceptions import FlightctlApiException, ValidationException +from plugins.module_utils.exceptions import FlightctlApiException class TestFlightCtlInventoryModule(unittest.TestCase):