From 8458acbb5f5b0dc68d5b4d435211ac32ce91b90e Mon Sep 17 00:00:00 2001 From: Sam Liu Date: Mon, 21 Nov 2022 13:33:47 -0800 Subject: [PATCH] fix: Raise correct exception when httpapi event RouteSettings is not of valid type --- samtranslator/open_api/open_api.py | 2 +- .../plugins/api/implicit_http_api_plugin.py | 21 +++++++++----- .../plugins/api/implicit_rest_api_plugin.py | 2 +- samtranslator/sdk/resource.py | 2 +- samtranslator/sdk/template.py | 2 +- samtranslator/validator/value_validator.py | 27 ++++++++++++++---- .../error_http_api_event_invalid_api.yaml | 28 +++++++++++++++++++ .../error_http_api_event_invalid_api.json | 7 +---- 8 files changed, 68 insertions(+), 23 deletions(-) diff --git a/samtranslator/open_api/open_api.py b/samtranslator/open_api/open_api.py index 54e85ed35..f82167e31 100644 --- a/samtranslator/open_api/open_api.py +++ b/samtranslator/open_api/open_api.py @@ -626,7 +626,7 @@ def has_api_gateway_cors(self): # type: ignore[no-untyped-def] return False @property - def openapi(self): # type: ignore[no-untyped-def] + def openapi(self) -> Dict[str, Any]: """ Returns a **copy** of the OpenApi specification as a dictionary. diff --git a/samtranslator/plugins/api/implicit_http_api_plugin.py b/samtranslator/plugins/api/implicit_http_api_plugin.py index 8b30cafe3..7a95fa576 100644 --- a/samtranslator/plugins/api/implicit_http_api_plugin.py +++ b/samtranslator/plugins/api/implicit_http_api_plugin.py @@ -1,9 +1,13 @@ +from typing import Any, Dict, Optional, cast + from samtranslator.model.intrinsics import make_conditional from samtranslator.model.naming import GeneratedLogicalId from samtranslator.plugins.api.implicit_api_plugin import ImplicitApiPlugin from samtranslator.public.open_api import OpenApiEditor from samtranslator.public.exceptions import InvalidEventException from samtranslator.public.sdk.resource import SamResourceType, SamResource +from samtranslator.sdk.template import SamTemplate +from samtranslator.validator.value_validator import sam_expect class ImplicitHttpApiPlugin(ImplicitApiPlugin): @@ -103,7 +107,7 @@ def _process_api_events( # type: ignore[no-untyped-def] self._add_api_to_swagger(logicalId, event_properties, template) # type: ignore[no-untyped-call] if "RouteSettings" in event_properties: - self._add_route_settings_to_api(logicalId, event_properties, template, condition) # type: ignore[no-untyped-call] + self._add_route_settings_to_api(logicalId, event_properties, template, condition) api_events[logicalId] = event # We could have made changes to the Events structure. Write it back to function @@ -120,25 +124,27 @@ def _add_implicit_api_id_if_necessary(self, event_properties): # type: ignore[n if "ApiId" not in event_properties: event_properties["ApiId"] = {"Ref": self.implicit_api_logical_id} - def _generate_implicit_api_resource(self): # type: ignore[no-untyped-def] + def _generate_implicit_api_resource(self) -> Dict[str, Any]: """ Uses the implicit API in this file to generate an Implicit API resource """ - return ImplicitHttpApiResource().to_dict() # type: ignore[no-untyped-call] + return ImplicitHttpApiResource().to_dict() - def _get_api_definition_from_editor(self, editor): # type: ignore[no-untyped-def] + def _get_api_definition_from_editor(self, editor: OpenApiEditor) -> Dict[str, Any]: """ Helper function to return the OAS definition from the editor """ return editor.openapi - def _get_api_resource_type_name(self): # type: ignore[no-untyped-def] + def _get_api_resource_type_name(self) -> str: """ Returns the type of API resource """ return "AWS::Serverless::HttpApi" - def _add_route_settings_to_api(self, event_id, event_properties, template, condition): # type: ignore[no-untyped-def] + def _add_route_settings_to_api( + self, event_id: str, event_properties: Dict[str, Any], template: SamTemplate, condition: Optional[str] + ) -> None: """ Adds the RouteSettings for this path/method from the given event to the RouteSettings configuration on the AWS::Serverless::HttpApi that this refers to. @@ -150,7 +156,7 @@ def _add_route_settings_to_api(self, event_id, event_properties, template, condi """ api_id = self._get_api_id(event_properties) # type: ignore[no-untyped-call] - resource = template.get(api_id) + resource = cast(SamResource, template.get(api_id)) # TODO: make this not an assumption path = event_properties["Path"] method = event_properties["Method"] @@ -165,6 +171,7 @@ def _add_route_settings_to_api(self, event_id, event_properties, template, condi event_route_settings = event_properties.get("RouteSettings", {}) if condition: event_route_settings = make_conditional(condition, event_properties.get("RouteSettings", {})) + sam_expect(event_route_settings, event_id, "RouteSettings", is_sam_event=True).to_be_a_map() # Merge event-level and api-level RouteSettings properties api_route_settings.setdefault(route, {}) diff --git a/samtranslator/plugins/api/implicit_rest_api_plugin.py b/samtranslator/plugins/api/implicit_rest_api_plugin.py index e8cd5fac1..3f9f77c61 100644 --- a/samtranslator/plugins/api/implicit_rest_api_plugin.py +++ b/samtranslator/plugins/api/implicit_rest_api_plugin.py @@ -121,7 +121,7 @@ def _generate_implicit_api_resource(self): # type: ignore[no-untyped-def] """ Uses the implicit API in this file to generate an Implicit API resource """ - return ImplicitApiResource().to_dict() # type: ignore[no-untyped-call] + return ImplicitApiResource().to_dict() def _get_api_definition_from_editor(self, editor): # type: ignore[no-untyped-def] """ diff --git a/samtranslator/sdk/resource.py b/samtranslator/sdk/resource.py index 325a36709..550f5f766 100644 --- a/samtranslator/sdk/resource.py +++ b/samtranslator/sdk/resource.py @@ -61,7 +61,7 @@ def valid(self): # type: ignore[no-untyped-def] return SamResourceType.has_value(self.type) # type: ignore[no-untyped-call] - def to_dict(self): # type: ignore[no-untyped-def] + def to_dict(self) -> Dict[str, Any]: if self.valid(): # type: ignore[no-untyped-call] # Touch a resource dictionary ONLY if it is valid diff --git a/samtranslator/sdk/template.py b/samtranslator/sdk/template.py index 62a984d43..0db0d8ded 100644 --- a/samtranslator/sdk/template.py +++ b/samtranslator/sdk/template.py @@ -50,7 +50,7 @@ def set(self, logical_id: str, resource: Union[SamResource, Dict[str, Any]]) -> resource_dict = resource if isinstance(resource, SamResource): - resource_dict = resource.to_dict() # type: ignore[no-untyped-call] + resource_dict = resource.to_dict() self.resources[logical_id] = resource_dict diff --git a/samtranslator/validator/value_validator.py b/samtranslator/validator/value_validator.py index 7e121f95c..503adeb5c 100644 --- a/samtranslator/validator/value_validator.py +++ b/samtranslator/validator/value_validator.py @@ -2,7 +2,7 @@ from enum import Enum from typing import Generic, Optional, TypeVar -from samtranslator.model.exceptions import InvalidResourceException +from samtranslator.model.exceptions import InvalidEventException, InvalidResourceException class ExpectedType(Enum): @@ -17,13 +17,20 @@ class ExpectedType(Enum): class _ResourcePropertyValueValidator(Generic[T]): value: Optional[T] - resource_logical_id: str + resource_logical_id: Optional[str] + event_id: Optional[str] property_identifier: str - def __init__(self, value: Optional[T], resource_logical_id: str, property_identifier: str) -> None: + def __init__( + self, value: Optional[T], resource_id: str, property_identifier: str, is_sam_event: bool = False + ) -> None: self.value = value - self.resource_logical_id = resource_logical_id self.property_identifier = property_identifier + self.resource_logical_id, self.event_id = (None, None) + if is_sam_event: + self.event_id = resource_id + else: + self.resource_logical_id = resource_id def to_be_a(self, expected_type: ExpectedType, message: Optional[str] = "") -> Optional[T]: """ @@ -35,7 +42,11 @@ def to_be_a(self, expected_type: ExpectedType, message: Optional[str] = "") -> O if not isinstance(self.value, type_class): if not message: message = f"Property '{self.property_identifier}' should be a {type_description}." - raise InvalidResourceException(self.resource_logical_id, message) + if self.event_id: + raise InvalidEventException(self.event_id, message) + if self.resource_logical_id: + raise InvalidResourceException(self.resource_logical_id, message) + raise RuntimeError("event_id and resource_logical_id are both None") # mypy is not smart to derive class from expected_type.value[1], ignore types: return self.value # type: ignore @@ -48,7 +59,11 @@ def to_not_be_none(self, message: Optional[str] = "") -> T: if self.value is None: if not message: message = f"Property '{self.property_identifier}' is required." - raise InvalidResourceException(self.resource_logical_id, message) + if self.event_id: + raise InvalidEventException(self.event_id, message) + if self.resource_logical_id: + raise InvalidResourceException(self.resource_logical_id, message) + raise RuntimeError("event_id and resource_logical_id are both None") return self.value # diff --git a/tests/translator/input/error_http_api_event_invalid_api.yaml b/tests/translator/input/error_http_api_event_invalid_api.yaml index 426236c4c..adbb41569 100644 --- a/tests/translator/input/error_http_api_event_invalid_api.yaml +++ b/tests/translator/input/error_http_api_event_invalid_api.yaml @@ -10,3 +10,31 @@ Resources: Type: HttpApi Properties: ApiId: !Ref SomeApi + + HttpApiFunctionInvalidRouteSettings: + Type: AWS::Serverless::Function + Properties: + CodeUri: s3://bucket/key + Handler: index.handler + Runtime: python3.7 + Events: + ApiNullRouteSettings: + Type: HttpApi + Properties: + RouteSettings: + Path: /path + Method: POST + + HttpApiFunctionInvalidRouteSettings2: + Type: AWS::Serverless::Function + Properties: + CodeUri: s3://bucket/key + Handler: index.handler + Runtime: python3.7 + Events: + ApiRouteSettingsNotMap: + Type: HttpApi + Properties: + RouteSettings: this should be a map + Path: /path2 + Method: POST diff --git a/tests/translator/output/error_http_api_event_invalid_api.json b/tests/translator/output/error_http_api_event_invalid_api.json index a7ac1365e..0649422c1 100644 --- a/tests/translator/output/error_http_api_event_invalid_api.json +++ b/tests/translator/output/error_http_api_event_invalid_api.json @@ -1,8 +1,3 @@ { - "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [HttpApiFunction] is invalid. Event with id [Api] is invalid. ApiId must be a valid reference to an 'AWS::Serverless::HttpApi' resource in same template.", - "errors": [ - { - "errorMessage": "Resource with id [HttpApiFunction] is invalid. Event with id [Api] is invalid. ApiId must be a valid reference to an 'AWS::Serverless::HttpApi' resource in same template." - } - ] + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 3. Resource with id [HttpApiFunction] is invalid. Event with id [Api] is invalid. ApiId must be a valid reference to an 'AWS::Serverless::HttpApi' resource in same template. Resource with id [HttpApiFunctionInvalidRouteSettings] is invalid. Event with id [ApiNullRouteSettings] is invalid. Property 'RouteSettings' should be a map. Resource with id [HttpApiFunctionInvalidRouteSettings2] is invalid. Event with id [ApiRouteSettingsNotMap] is invalid. Property 'RouteSettings' should be a map." }