From 4fb14bbb21cc595fb19d308570889135a8a0d044 Mon Sep 17 00:00:00 2001 From: StylusFrost Date: Thu, 7 May 2026 10:00:07 +0200 Subject: [PATCH] perf(sdk): cache misses in Provider._load_ep_provider Repeated lookups for unknown provider names (built-ins, typos, names with no registered entry point) re-iterated entry_points() on every call because only hits were cached. importlib.metadata.entry_points() walks the metadata of every installed Python package, so the cost is proportional to the size of the venv, not just to Prowler. Caches None on miss so subsequent lookups hit the existing `if name in Provider._ep_providers` short-circuit and return immediately. Aligns Provider._ep_providers with the symmetric cache in tool_wrapper._ep_class_cache, which already had this behavior. Includes a regression test that mocks importlib.metadata.entry_points, calls _load_ep_provider("nonexistent") twice, and asserts entry_points is invoked exactly once. Also underscore-prefix the remaining unused parameters on the abstract Provider stubs (get_output_options, get_stdout_detail, generate_compliance_output, display_compliance_table) so vulture stops flagging them now that the file is in the diff. Same pattern applied in bbe3a7dbf for the previous batch. --- prowler/providers/common/provider.py | 24 ++++++++++++------- .../external/test_dynamic_provider_loading.py | 15 ++++++++++++ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/prowler/providers/common/provider.py b/prowler/providers/common/provider.py index 3f8d2d7c65..cea9f09112 100644 --- a/prowler/providers/common/provider.py +++ b/prowler/providers/common/provider.py @@ -153,13 +153,13 @@ class Provider(ABC): """ raise NotImplementedError(f"{cls.__name__} has not implemented from_cli_args()") - def get_output_options(self, arguments, bulk_checks_metadata): + def get_output_options(self, arguments, _bulk_checks_metadata): """Create the provider-specific OutputOptions.""" raise NotImplementedError( f"{self.__class__.__name__} has not implemented get_output_options()" ) - def get_stdout_detail(self, finding) -> str: + def get_stdout_detail(self, _finding) -> str: """Return the detail string for stdout reporting (region, location, etc.).""" raise NotImplementedError( f"{self.__class__.__name__} has not implemented get_stdout_detail()" @@ -189,10 +189,10 @@ class Provider(ABC): def generate_compliance_output( self, - findings, - bulk_compliance_frameworks, + _findings, + _bulk_compliance_frameworks, _input_compliance_frameworks, - output_options, + _output_options, _generated_outputs, ) -> None: """Generate compliance CSV output for this provider's frameworks.""" @@ -213,9 +213,9 @@ class Provider(ABC): def display_compliance_table( self, - findings: list, - bulk_checks_metadata: dict, - compliance_framework: str, + _findings: list, + _bulk_checks_metadata: dict, + _compliance_framework: str, _output_filename: str, _output_directory: str, _compliance_overview: bool, @@ -626,7 +626,12 @@ class Provider(ABC): @staticmethod def _load_ep_provider(name: str): - """Load an external provider class from entry points, with cache.""" + """Load an external provider class from entry points, with cache. + + Caches both hits and misses so repeated lookups for unknown names do + not re-iterate entry_points(). Symmetric with + tool_wrapper._ep_class_cache. + """ if name in Provider._ep_providers: return Provider._ep_providers[name] for ep in importlib.metadata.entry_points(group="prowler.providers"): @@ -639,6 +644,7 @@ class Provider(ABC): logger.warning( f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) + Provider._ep_providers[name] = None return None @staticmethod diff --git a/tests/providers/external/test_dynamic_provider_loading.py b/tests/providers/external/test_dynamic_provider_loading.py index bb15dad839..9f45648c94 100644 --- a/tests/providers/external/test_dynamic_provider_loading.py +++ b/tests/providers/external/test_dynamic_provider_loading.py @@ -302,6 +302,21 @@ class TestProviderDiscovery: # load() should only be called once due to caching mock_ep.return_value[0].load.assert_called_once() + @patch("prowler.providers.common.provider.importlib.metadata.entry_points") + def test_load_ep_provider_caches_misses(self, mock_ep): + """A miss (unknown provider) must also be cached so repeated lookups + do not re-iterate entry_points(). Aligns with tool_wrapper._ep_class_cache, + which already caches None on miss.""" + mock_ep.return_value = [] + + first = Provider._load_ep_provider("nonexistent") + second = Provider._load_ep_provider("nonexistent") + + assert first is None + assert second is None + # entry_points() should only be called once across the two lookups + mock_ep.assert_called_once() + @patch("prowler.providers.common.provider.Provider._load_ep_provider") @patch("prowler.providers.common.provider.Provider.get_available_providers") def test_get_providers_help_text_reads_cli_help_text(