From 89ffc7c48cc0b63b0bc9965a6a7e238acf3d5a62 Mon Sep 17 00:00:00 2001 From: Matthias Veit Date: Tue, 6 Aug 2024 10:14:21 +0200 Subject: [PATCH] [gcp][feat] Save parsing with feedback (#2160) --- .../azure/fix_plugin_azure/azure_client.py | 4 +- .../fix_plugin_azure/resource/postgresql.py | 6 ++- .../fix_plugin_azure/resource/sql_server.py | 6 ++- plugins/gcp/fix_plugin_gcp/resources/base.py | 51 ++++++++++++++----- .../gcp/fix_plugin_gcp/resources/compute.py | 4 +- .../gcp/fix_plugin_gcp/resources/storage.py | 12 ++--- plugins/gcp/test/random_client.py | 4 +- 7 files changed, 60 insertions(+), 27 deletions(-) diff --git a/plugins/azure/fix_plugin_azure/azure_client.py b/plugins/azure/fix_plugin_azure/azure_client.py index 064369d849..16e041854d 100644 --- a/plugins/azure/fix_plugin_azure/azure_client.py +++ b/plugins/azure/fix_plugin_azure/azure_client.py @@ -325,10 +325,10 @@ def _list_with_retry(self, spec: MicrosoftRestSpec, **kwargs: Any) -> Optional[L raise MetricRequestError from e code = error.code or "Unknown" self.accumulator.add_error(False, code, spec.service, spec.action, str(e), self.location) - log.warning(f"[Azure] Client Error: status={e.status_code}, error={e.error}, message={e}") + log.warning(f"[Azure] Client Error: status={e.status_code}, error={e.error}, message={e}, spec={spec}") return None except Exception as e: - log.warning(f"[Azure] called service={spec.service}: hit unexpected error: {e}", exc_info=e) + log.warning(f"[Azure] called service={spec.service}: hit unexpected error: {e}, spec={spec}", exc_info=e) if self.config.discard_account_on_resource_error: raise return None diff --git a/plugins/azure/fix_plugin_azure/resource/postgresql.py b/plugins/azure/fix_plugin_azure/resource/postgresql.py index ac4474a674..f6b56a4fee 100644 --- a/plugins/azure/fix_plugin_azure/resource/postgresql.py +++ b/plugins/azure/fix_plugin_azure/resource/postgresql.py @@ -550,7 +550,11 @@ def _collect_items( def post_process(self, graph_builder: GraphBuilder, source: Json) -> None: if server_id := self.id: resources_to_collect = [ - ("administrators", AzurePostgresqlServerADAdministrator, ["InternalServerError"]), + ( + "administrators", + AzurePostgresqlServerADAdministrator, + ["InternalServerError", "DatabaseDoesNotExist"], + ), ("configurations", AzurePostgresqlServerConfiguration, ["ServerStoppedError", "InternalServerError"]), ("databases", AzurePostgresqlServerDatabase, ["ServerStoppedError", "InternalServerError"]), ("firewallRules", AzurePostgresqlServerFirewallRule, None), diff --git a/plugins/azure/fix_plugin_azure/resource/sql_server.py b/plugins/azure/fix_plugin_azure/resource/sql_server.py index 45e755e431..ffeb57d5b8 100644 --- a/plugins/azure/fix_plugin_azure/resource/sql_server.py +++ b/plugins/azure/fix_plugin_azure/resource/sql_server.py @@ -268,7 +268,11 @@ def fetch_data_encryption_status(sid: str) -> None: graph_builder.submit_work(service_name, fetch_data_encryption_status, database_id) resources_to_collect = [ ("geoBackupPolicies", AzureSqlServerDatabaseGeoBackupPolicy, None), - ("advisors?$expand=recommendedAction", AzureSqlServerAdvisor, None), + ( + "advisors?$expand=recommendedAction", + AzureSqlServerAdvisor, + ["DataWarehouseNotSupported", "DatabaseDoesNotExist"], + ), ("workloadGroups", AzureSqlServerDatabaseWorkloadGroup, ["FeatureDisabledOnSelectedEdition"]), ] diff --git a/plugins/gcp/fix_plugin_gcp/resources/base.py b/plugins/gcp/fix_plugin_gcp/resources/base.py index b019d8c5a5..be00e8a026 100644 --- a/plugins/gcp/fix_plugin_gcp/resources/base.py +++ b/plugins/gcp/fix_plugin_gcp/resources/base.py @@ -47,6 +47,28 @@ def get_client(resource: BaseResource) -> GcpClient: ) +def parse_json( + json: Json, clazz: Type[T], builder: Optional[GraphBuilder] = None, mapping: Optional[Dict[str, Bender]] = None +) -> Optional[T]: + """ + Use this method to parse json into a class. If the json can not be parsed, the error is reported to the core. + Based on configuration, either the exception is raised or None is returned. + :param json: the json to parse. + :param clazz: the class to parse into. + :param builder: the graph builder. + :param mapping: the optional mapping to apply before parsing. + :return: The parsed object or None. + """ + try: + mapped = bend(mapping, json) if mapping is not None else json + return from_js(mapped, clazz) + except Exception as e: + if builder: + # report and log the error + builder.core_feedback.error(f"Failed to parse json into {clazz.__name__}: {e}. Source: {json}", log) + return None + + class GraphBuilder: def __init__( self, @@ -379,14 +401,14 @@ def collect(cls: Type[GcpResource], raw: List[Json], builder: GraphBuilder) -> L result: List[GcpResource] = [] for js in raw: # map from api - instance = cls.from_api(js) - # allow the instance to adjust itself - adjusted = instance.adjust_from_api(builder, js) - # add to graph - if (added := builder.add_node(adjusted, js)) is not None: - # post process - added.post_process(builder, js) - result.append(added) + if instance := cls.from_api(js, builder): + # allow the instance to adjust itself + adjusted = instance.adjust_from_api(builder, js) + # add to graph + if (added := builder.add_node(adjusted, js)) is not None: + # post process + added.post_process(builder, js) + result.append(added) return result @classmethod @@ -394,9 +416,10 @@ def from_json(cls: Type[GcpResourceType], json: Json) -> GcpResourceType: return from_js(json, cls) @classmethod - def from_api(cls: Type[GcpResourceType], json: Json) -> GcpResourceType: - mapped = bend(cls.mapping, json) - return cls.from_json(mapped) + def from_api( + cls: Type[GcpResourceType], json: Json, builder: Optional[GraphBuilder] = None + ) -> Optional[GcpResourceType]: + return parse_json(json, cls, builder, cls.mapping) @classmethod def called_collect_apis(cls) -> List[GcpApiSpec]: @@ -541,9 +564,9 @@ def fallback_global_region(cls: Type[GcpRegion], project: GcpProject) -> GcpRegi return cls(id="global", tags={}, name="global", account=project) def post_process(self, graph_builder: GraphBuilder, source: Json) -> None: - region_quota = GcpRegionQuota.from_api(source) - graph_builder.add_node(region_quota, source) - graph_builder.add_edge(self, node=region_quota) + if region_quota := GcpRegionQuota.from_api(source, graph_builder): + graph_builder.add_node(region_quota, source) + graph_builder.add_edge(self, node=region_quota) def connect_in_graph(self, builder: GraphBuilder, source: Json) -> None: super().connect_in_graph(builder, source) diff --git a/plugins/gcp/fix_plugin_gcp/resources/compute.py b/plugins/gcp/fix_plugin_gcp/resources/compute.py index e297e6dda9..9611662e61 100644 --- a/plugins/gcp/fix_plugin_gcp/resources/compute.py +++ b/plugins/gcp/fix_plugin_gcp/resources/compute.py @@ -4217,8 +4217,8 @@ def collect_individual(cls: Type[GcpResource], builder: GraphBuilder, zone: str, machineType=name, ) result[InternalZoneProp] = zone # `add_node()` picks this up and sets proper zone/region - machine_type_obj = GcpMachineType.from_api(result) - builder.add_node(machine_type_obj, result) + if machine_type_obj := GcpMachineType.from_api(result, builder): + builder.add_node(machine_type_obj, result) def _machine_type_matches_sku_description(self, sku_description: str) -> bool: if not self.name: diff --git a/plugins/gcp/fix_plugin_gcp/resources/storage.py b/plugins/gcp/fix_plugin_gcp/resources/storage.py index d1a7f7fa30..95764448b5 100644 --- a/plugins/gcp/fix_plugin_gcp/resources/storage.py +++ b/plugins/gcp/fix_plugin_gcp/resources/storage.py @@ -422,12 +422,12 @@ def pre_delete(self, graph: Graph) -> bool: client = get_client(self) objects = client.list(GcpObject.api_spec, bucket=self.name) for obj in objects: - object_in_bucket = GcpObject.from_api(obj) - client.delete( - object_in_bucket.api_spec.for_delete(), - bucket=self.name, - resource=object_in_bucket.name, - ) + if object_in_bucket := GcpObject.from_api(obj): + client.delete( + object_in_bucket.api_spec.for_delete(), + bucket=self.name, + resource=object_in_bucket.name, + ) return True def delete(self, graph: Graph) -> bool: diff --git a/plugins/gcp/test/random_client.py b/plugins/gcp/test/random_client.py index a05d2936d3..0f8cc5de76 100644 --- a/plugins/gcp/test/random_client.py +++ b/plugins/gcp/test/random_client.py @@ -241,7 +241,9 @@ def create_node_for( result = client.list(api_spec=spec) assert len(result) > 0 raw = adapt(result[0]) - return raw, clazz.from_api(raw) + instance = clazz.from_api(raw) + assert instance is not None + return raw, instance def create_node(clazz: Type[GcpResourceType], **kwargs: Any) -> Tuple[Json, GcpResourceType]: