diff --git a/plugins/azure/fix_plugin_azure/__init__.py b/plugins/azure/fix_plugin_azure/__init__.py index 1f9e9f4025..be0770923c 100644 --- a/plugins/azure/fix_plugin_azure/__init__.py +++ b/plugins/azure/fix_plugin_azure/__init__.py @@ -60,7 +60,7 @@ def collect(self) -> None: self.task_data, ) for name, ac in account_configs.items() - for subscription in AzureSubscription.list_subscriptions(ac.credentials()) + for subscription in AzureSubscription.list_subscriptions(config, ac.credentials()) if ac.allowed(subscription.subscription_id) } args = list(args_by_subscription_id.values()) diff --git a/plugins/azure/fix_plugin_azure/azure_client.py b/plugins/azure/fix_plugin_azure/azure_client.py index 7705bf379e..f84ca420ed 100644 --- a/plugins/azure/fix_plugin_azure/azure_client.py +++ b/plugins/azure/fix_plugin_azure/azure_client.py @@ -20,7 +20,7 @@ from azure.mgmt.resource.resources._serialization import Serializer from azure.mgmt.resource.resources.models import GenericResource -from fix_plugin_azure.config import AzureCredentials +from fix_plugin_azure.config import AzureConfig, AzureCredentials from fixlib.core.actions import CoreFeedback, ErrorAccumulator from fixlib.types import Json @@ -82,6 +82,7 @@ def delete_resource_tag(self, tag_name: str, resource_id: str) -> bool: @staticmethod def __create_management_client( + config: AzureConfig, credential: AzureCredentials, subscription_id: str, core_feedback: Optional[CoreFeedback] = None, @@ -89,7 +90,7 @@ def __create_management_client( resource_group: Optional[str] = None, ) -> AzureClient: return AzureResourceManagementClient( - credential, subscription_id, resource_group, core_feedback, error_accumulator + config, credential, subscription_id, resource_group, core_feedback, error_accumulator ) create = __create_management_client @@ -98,12 +99,14 @@ def __create_management_client( class AzureResourceManagementClient(AzureClient): def __init__( self, + config: AzureConfig, credential: AzureCredentials, subscription_id: str, location: Optional[str] = None, core_feedback: Optional[CoreFeedback] = None, accumulator: Optional[ErrorAccumulator] = None, ) -> None: + self.config = config self.credential = credential self.subscription_id = subscription_id self.location = location @@ -121,10 +124,16 @@ def delete(self, resource_id: str) -> bool: try: self.client.resources.begin_delete_by_id(resource_id=resource_id, api_version="2021-04-01") except HttpResponseError as e: - if e.error and e.error.code == "ResourceNotFoundError": - return False # Resource not found to delete - else: - raise e + if error := e.error: + error_code = error.code or "Unknown" + if error_code == "ResourceNotFoundError": + return False # Resource not found to delete + else: + msg = f"An Azure API error occurred during the deletion of a resource: {e}" + self.accumulator.add_error(False, error_code, "Resource deletion", "service_resource", msg) + if self.config.discard_account_on_resource_error: + raise + return False return True @@ -160,12 +169,20 @@ def _update_or_delete_tag(self, tag_name: str, tag_value: str, resource_id: str, self.client.resources.begin_create_or_update_by_id(resource_id, "2021-04-01", updated_resource) except HttpResponseError as e: - if e.error and e.error.code == "ResourceNotFoundError": - return False # Resource not found - elif e.error and e.error.code == "ResourceExistsError": - return False # Tag for update/delete does not exist - else: - raise e + if error := e.error: + error_code = error.code or "Unknown" + if error_code == "ResourceNotFoundError": + return False # Resource not found + elif error_code == "ResourceExistsError": + return False # Tag for update/delete does not exist + else: + msg = f"An Azure API error occurred during the updating or deletion tag of a resource: {e}" + self.accumulator.add_error( + False, error_code, "Resource updating or deletion", "service_resource", msg + ) + if self.config.discard_account_on_resource_error: + raise + return False return True @@ -179,7 +196,10 @@ def _list_with_retry(self, spec: AzureApiSpec, **kwargs: Any) -> Optional[List[J try: return self._call(spec, **kwargs) except ClientAuthenticationError as e: - log.error(f"[Azure] Call to Azure API is not authorized!: {e}") + log.warning(f"[Azure] Call to Azure API is not authorized!: {e}") + if (error := e.error) and (error_code := error.code): + msg = "Call to Azure API is not authorized!" + self.accumulator.add_error(False, error_code, spec.service, spec.path, msg, self.location) return None except HttpResponseError as e: if error := e.error: @@ -195,6 +215,8 @@ def _list_with_retry(self, spec: AzureApiSpec, **kwargs: Any) -> Optional[List[J return None except Exception as e: log.warning(f"[Azure] called service={spec.service}: hit unexpected error: {e}", exc_info=e) + if self.config.discard_account_on_resource_error: + raise return None # noinspection PyProtectedMember @@ -285,4 +307,6 @@ def _make_request(self, url: str, params: MutableMapping[str, Any], headers: Mut return response def for_location(self, location: str) -> AzureClient: - return AzureClient.create(self.credential, self.subscription_id, self.core_feedback, self.accumulator, location) + return AzureClient.create( + self.config, self.credential, self.subscription_id, self.core_feedback, self.accumulator, location + ) diff --git a/plugins/azure/fix_plugin_azure/collector.py b/plugins/azure/fix_plugin_azure/collector.py index c59413b961..18d5f24800 100644 --- a/plugins/azure/fix_plugin_azure/collector.py +++ b/plugins/azure/fix_plugin_azure/collector.py @@ -74,6 +74,7 @@ def collect(self) -> None: queue = ExecutorQueue(executor, "azure_collector") error_accumulator = ErrorAccumulator() client = AzureClient.create( + self.config, self.credentials, self.subscription.subscription_id, core_feedback=self.core_feedback, diff --git a/plugins/azure/fix_plugin_azure/config.py b/plugins/azure/fix_plugin_azure/config.py index 4aa51c842b..df84d030f6 100644 --- a/plugins/azure/fix_plugin_azure/config.py +++ b/plugins/azure/fix_plugin_azure/config.py @@ -68,6 +68,14 @@ class AzureConfig: metadata={"description": "Configure accounts to collect subscriptions. You can define multiple accounts here."}, ) + discard_account_on_resource_error: bool = field( + default=False, + metadata={ + "description": "Fail the whole account if collecting a resource fails. " + "If false, the error is logged and the resource is skipped." + }, + ) + collect_usage_metrics: Optional[bool] = field( default=True, metadata={"description": "Collect resource usage metrics via Azure Metric, enabled by default"}, diff --git a/plugins/azure/fix_plugin_azure/resource/base.py b/plugins/azure/fix_plugin_azure/resource/base.py index 49fe311cce..e826ae80e4 100644 --- a/plugins/azure/fix_plugin_azure/resource/base.py +++ b/plugins/azure/fix_plugin_azure/resource/base.py @@ -32,7 +32,7 @@ def get_client(subscription_id: str) -> AzureClient: credential = account.credentials() else: credential = DefaultAzureCredential() - return AzureClient.create(credential=credential, subscription_id=subscription_id) + return AzureClient.create(config=azure_config, credential=credential, subscription_id=subscription_id) T = TypeVar("T") @@ -155,15 +155,20 @@ def collect_resources( # Default behavior: in case the class has an ApiSpec, call the api and call collect. log.debug(f"[Azure:{builder.subscription.id}] Collecting {cls.__name__} with ({kwargs})") if spec := cls.api_spec: - # TODO: add error handling - items = builder.client.list(spec, **kwargs) - collected = cls.collect(items, builder) - if builder.config.collect_usage_metrics: - try: - cls.collect_usage_metrics(builder, collected) - except Exception as e: - log.warning(f"Failed to collect usage metrics for {cls.__name__}: {e}") - return collected + try: + items = builder.client.list(spec, **kwargs) + collected = cls.collect(items, builder) + if builder.config.collect_usage_metrics: + try: + cls.collect_usage_metrics(builder, collected) + except Exception as e: + log.warning(f"Failed to collect usage metrics for {cls.__name__}: {e}") + return collected + except Exception as e: + msg = f"Error while collecting {cls.__name__} with service {spec.service} and location: {builder.location}: {e}" + builder.core_feedback.info(msg, log) + raise + return [] @classmethod @@ -406,8 +411,8 @@ class AzureSubscription(AzureResource, BaseAccount): account_name: Optional[str] = field(default=None, metadata={"description": "The account used to collect this subscription."}) # fmt: skip @classmethod - def list_subscriptions(cls, credentials: AzureCredentials) -> List[AzureSubscription]: - client = AzureClient.create(credentials, "global") + def list_subscriptions(cls, config: AzureConfig, credentials: AzureCredentials) -> List[AzureSubscription]: + client = AzureClient.create(config, credentials, "global") return [cls.from_api(js) for js in client.list(cls.api_spec)]