feat: Added new handling implementation for azure errors (#2038)

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())

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,14 +82,15 @@ 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,
         error_accumulator: Optional[ErrorAccumulator] = None,
         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
+        )

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,

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"},

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)]