Skip to content

Commit

Permalink
[gcp][feat] Save parsing with feedback (#2160)
Browse files Browse the repository at this point in the history
  • Loading branch information
aquamatthias authored Aug 6, 2024
1 parent 34d50ba commit 89ffc7c
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 27 deletions.
4 changes: 2 additions & 2 deletions plugins/azure/fix_plugin_azure/azure_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion plugins/azure/fix_plugin_azure/resource/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 5 additions & 1 deletion plugins/azure/fix_plugin_azure/resource/sql_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]),
]

Expand Down
51 changes: 37 additions & 14 deletions plugins/gcp/fix_plugin_gcp/resources/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -379,24 +401,25 @@ 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
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]:
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions plugins/gcp/fix_plugin_gcp/resources/compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
12 changes: 6 additions & 6 deletions plugins/gcp/fix_plugin_gcp/resources/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion plugins/gcp/test/random_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down

0 comments on commit 89ffc7c

Please sign in to comment.