From 88c25c09d09a4893813660e346f380e20253be2c Mon Sep 17 00:00:00 2001 From: drwicid Date: Tue, 10 Mar 2026 13:48:33 -0400 Subject: [PATCH] fix: handle GCP SERVICE_DISABLED responses as expected errors --- plugins/gcp/fix_plugin_gcp/resources/base.py | 9 +++- .../gcp/fix_plugin_gcp/resources/filestore.py | 4 +- plugins/gcp/test/test_base.py | 34 +++++++++++++++ plugins/gcp/test/test_filestore.py | 41 +++++++++++++++++++ 4 files changed, 85 insertions(+), 3 deletions(-) diff --git a/plugins/gcp/fix_plugin_gcp/resources/base.py b/plugins/gcp/fix_plugin_gcp/resources/base.py index e696b10007..b856362d5f 100644 --- a/plugins/gcp/fix_plugin_gcp/resources/base.py +++ b/plugins/gcp/fix_plugin_gcp/resources/base.py @@ -771,7 +771,8 @@ class GcpZone(GcpResource, BaseZone): GcpExpectedErrorCodes = { - "PERMISSION_DENIED:usageLimits:accessNotConfigured" # resource not enabled - no resources to expect + "PERMISSION_DENIED:usageLimits:accessNotConfigured", # resource not enabled - no resources to expect + "PERMISSION_DENIED:googleapis.com:SERVICE_DISABLED", # service disabled - no resources to expect } @@ -817,6 +818,12 @@ def __exit__( errors = { f'{status}:{error.get("domain", "none")}:{error.get("reason", "none")}' for error in (value_in_path(exc_content, ["error", "errors"]) or []) + if isinstance(error, dict) + } + errors |= { + f'{status}:{detail.get("domain", "none")}:{detail.get("reason", "none")}' + for detail in (value_in_path(exc_content, ["error", "details"]) or []) + if isinstance(detail, dict) and detail.get("reason") } error_details = str(exc_content.get("error", {}).get("message", exc_value)) # Check if error message matches any of the expected substrings diff --git a/plugins/gcp/fix_plugin_gcp/resources/filestore.py b/plugins/gcp/fix_plugin_gcp/resources/filestore.py index 309e037bc5..8bc2a8c731 100644 --- a/plugins/gcp/fix_plugin_gcp/resources/filestore.py +++ b/plugins/gcp/fix_plugin_gcp/resources/filestore.py @@ -5,7 +5,7 @@ from attr import define, field from fix_plugin_gcp.gcp_client import GcpApiSpec -from fix_plugin_gcp.resources.base import GraphBuilder, GcpErrorHandler, GcpResource, GcpDeprecationStatus +from fix_plugin_gcp.resources.base import GraphBuilder, GcpErrorHandler, GcpExpectedErrorCodes, GcpResource, GcpDeprecationStatus from fixlib.baseresources import BaseNetworkShare, ModelReference from fixlib.json_bender import Bender, S, Bend, ForallBend from fixlib.types import Json @@ -282,7 +282,7 @@ def collect_snapshots() -> None: graph_builder.error_accumulator, spec.service, graph_builder.region.safe_name if graph_builder.region else None, - set(), + GcpExpectedErrorCodes, f" in {graph_builder.project.id} kind {GcpFilestoreInstanceSnapshot.kind}", ): items = graph_builder.client.list(spec) diff --git a/plugins/gcp/test/test_base.py b/plugins/gcp/test/test_base.py index 5776e82b64..72b0aa5705 100644 --- a/plugins/gcp/test/test_base.py +++ b/plugins/gcp/test/test_base.py @@ -1,5 +1,9 @@ import json import os +from types import SimpleNamespace + +from googleapiclient.errors import HttpError + from fix_plugin_gcp.resources.base import * from fix_plugin_gcp.resources.compute import GcpMachineType @@ -48,3 +52,33 @@ def test_gcp_region_collects_quotas(random_builder: GraphBuilder) -> None: for predecessor in random_builder.graph.predecessors(region_quotas[0]): assert isinstance(predecessor, GcpRegion) + + +def test_gcp_error_handler_suppresses_service_disabled_error(random_builder: GraphBuilder) -> None: + error = { + "error": { + "code": 403, + "message": ( + "Cloud Filestore API has not been used in project test-project before or it is disabled. " + "Enable it by visiting https://console.developers.google.com/apis/api/file.googleapis.com/overview" + ), + "status": "PERMISSION_DENIED", + "details": [ + { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "SERVICE_DISABLED", + "domain": "googleapis.com", + "metadata": { + "consumer": "projects/test-project", + "service": "file.googleapis.com", + }, + } + ], + } + } + exc = HttpError(SimpleNamespace(status=403, reason="Forbidden"), json.dumps(error).encode("utf-8")) + + with GcpErrorHandler("list", random_builder.error_accumulator, "file", None, GcpExpectedErrorCodes): + raise exc + + assert random_builder.error_accumulator.regional_errors == {} diff --git a/plugins/gcp/test/test_filestore.py b/plugins/gcp/test/test_filestore.py index 394cab86f8..e813bc2aba 100644 --- a/plugins/gcp/test/test_filestore.py +++ b/plugins/gcp/test/test_filestore.py @@ -1,5 +1,9 @@ import json import os +from types import SimpleNamespace +from typing import Any + +from googleapiclient.errors import HttpError from fix_plugin_gcp.resources.base import GraphBuilder from fix_plugin_gcp.resources.filestore import GcpFilestoreBackup, GcpFilestoreInstance, GcpFilestoreInstanceSnapshot @@ -27,3 +31,40 @@ def test_gcp_filestore_instance_snapshot(random_builder: GraphBuilder) -> None: snapshots = random_builder.nodes(clazz=GcpFilestoreInstanceSnapshot) assert len(snapshots) == 1 + + +def test_gcp_filestore_instance_snapshot_service_disabled_is_expected( + random_builder: GraphBuilder, monkeypatch: Any +) -> None: + error = { + "error": { + "code": 403, + "message": ( + "Cloud Filestore API has not been used in project test-project before or it is disabled. " + "Enable it by visiting https://console.developers.google.com/apis/api/file.googleapis.com/overview" + ), + "status": "PERMISSION_DENIED", + "details": [ + { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "SERVICE_DISABLED", + "domain": "googleapis.com", + "metadata": { + "consumer": "projects/test-project", + "service": "file.googleapis.com", + }, + } + ], + } + } + + def raise_service_disabled(*_: Any, **__: Any) -> Any: + raise HttpError(SimpleNamespace(status=403, reason="Forbidden"), json.dumps(error).encode("utf-8")) + + monkeypatch.setattr(random_builder.client, "list", raise_service_disabled) + + instance = GcpFilestoreInstance(id="projects/test-project/locations/us-east1/instances/test-instance") + instance.post_process(random_builder, {}) + random_builder.executor.wait_for_submitted_work() + + assert random_builder.error_accumulator.regional_errors == {}