diff --git a/samtranslator/model/__init__.py b/samtranslator/model/__init__.py index 6d4ad47ab..eb1279975 100644 --- a/samtranslator/model/__init__.py +++ b/samtranslator/model/__init__.py @@ -9,7 +9,11 @@ from pydantic.error_wrappers import ValidationError from samtranslator.intrinsics.resolver import IntrinsicsResolver -from samtranslator.model.exceptions import ExpectedType, InvalidResourceException, InvalidResourcePropertyTypeException +from samtranslator.model.exceptions import ( + ExpectedType, + InvalidResourceException, + InvalidResourcePropertyTypeException, +) from samtranslator.model.tags.resource_tagging import get_tag_list from samtranslator.model.types import IS_DICT, IS_STR, Validator, any_type, is_type from samtranslator.plugins import LifeCycleEvents @@ -121,7 +125,7 @@ class Resource(ABC): def __init__( self, - logical_id: str, + logical_id: Optional[Any], relative_id: Optional[str] = None, depends_on: Optional[List[str]] = None, attributes: Optional[Dict[str, Any]] = None, @@ -134,8 +138,7 @@ def __init__( :param depends_on Value of DependsOn resource attribute :param attributes Dictionary of resource attributes and their values """ - self._validate_logical_id(logical_id) # type: ignore[no-untyped-call] - self.logical_id = logical_id + self.logical_id = self._validate_logical_id(logical_id) self.relative_id = relative_id self.depends_on = depends_on @@ -214,8 +217,8 @@ def from_dict(cls, logical_id: str, resource_dict: Dict[str, Any], relative_id: resource.validate_properties() return resource - @classmethod - def _validate_logical_id(cls, logical_id): # type: ignore[no-untyped-def] + @staticmethod + def _validate_logical_id(logical_id: Optional[Any]) -> str: """Validates that the provided logical id is an alphanumeric string. :param str logical_id: the logical id to validate @@ -224,9 +227,12 @@ def _validate_logical_id(cls, logical_id): # type: ignore[no-untyped-def] :raises TypeError: if the logical id is invalid """ pattern = re.compile(r"^[A-Za-z0-9]+$") - if logical_id is not None and pattern.match(logical_id): - return True - raise InvalidResourceException(logical_id, "Logical ids must be alphanumeric.") + if isinstance(logical_id, str) and pattern.match(logical_id): + return logical_id + # TODO: Doing validation in this class is kind of off, + # we need to surface this validation to where the template is loaded + # or the logical IDs are generated. + raise InvalidResourceException(str(logical_id), "Logical ids must be alphanumeric.") @classmethod def _validate_resource_dict(cls, logical_id, resource_dict): # type: ignore[no-untyped-def] diff --git a/samtranslator/model/eventsources/push.py b/samtranslator/model/eventsources/push.py index e6949580f..899bf8ac2 100644 --- a/samtranslator/model/eventsources/push.py +++ b/samtranslator/model/eventsources/push.py @@ -634,47 +634,49 @@ class Api(PushEventSource): RequestModel: Optional[Dict[str, Any]] RequestParameters: Optional[List[Any]] - def resources_to_link(self, resources): # type: ignore[no-untyped-def] + def resources_to_link(self, resources: Dict[str, Any]) -> Dict[str, Any]: """ If this API Event Source refers to an explicit API resource, resolve the reference and grab necessary data from the explicit API """ + return self.resources_to_link_for_rest_api(resources, self.relative_id, self.RestApiId) + @staticmethod + def resources_to_link_for_rest_api( + resources: Dict[str, Any], relative_id: str, raw_rest_api_id: Optional[Any] + ) -> Dict[str, Any]: # If RestApiId is a resource in the same template, then we try find the StageName by following the reference # Otherwise we default to a wildcard. This stage name is solely used to construct the permission to # allow this stage to invoke the Lambda function. If we are unable to resolve the stage name, we will # simply permit all stages to invoke this Lambda function # This hack is necessary because customers could use !ImportValue, !Ref or other intrinsic functions which # can be sometimes impossible to resolve (ie. when it has cross-stack references) - permitted_stage = "*" stage_suffix = "AllStages" - explicit_api = None - rest_api_id = self.get_rest_api_id_string(self.RestApiId) + explicit_api_resource_properties = None + rest_api_id = Api.get_rest_api_id_string(raw_rest_api_id) if isinstance(rest_api_id, str): - if ( - rest_api_id in resources - and "Properties" in resources[rest_api_id] - and "StageName" in resources[rest_api_id]["Properties"] - ): - explicit_api = resources[rest_api_id]["Properties"] - permitted_stage = explicit_api["StageName"] - - # Stage could be a intrinsic, in which case leave the suffix to default value - if isinstance(permitted_stage, str): - if not permitted_stage: - raise InvalidResourceException(rest_api_id, "StageName cannot be empty.") - stage_suffix = permitted_stage - else: - stage_suffix = "Stage" # type: ignore[unreachable] - + rest_api_resource = sam_expect( + resources.get(rest_api_id), relative_id, "RestApiId", is_sam_event=True + ).to_be_a_map("RestApiId property of Api event must reference a valid resource in the same template.") + + explicit_api_resource_properties = sam_expect( + rest_api_resource.get("Properties", {}), rest_api_id, "Properties", is_resource_attribute=True + ).to_be_a_map() + permitted_stage = explicit_api_resource_properties.get("StageName") + + # Stage could be an intrinsic, in which case leave the suffix to default value + if isinstance(permitted_stage, str): + if not permitted_stage: + raise InvalidResourceException(rest_api_id, "StageName cannot be empty.") + stage_suffix = permitted_stage else: - # RestApiId is a string, not an intrinsic, but we did not find a valid API resource for this ID - raise InvalidEventException( - self.relative_id, - "RestApiId property of Api event must reference a valid resource in the same template.", - ) + stage_suffix = "Stage" - return {"explicit_api": explicit_api, "api_id": rest_api_id, "explicit_api_stage": {"suffix": stage_suffix}} + return { + "explicit_api": explicit_api_resource_properties, + "api_id": rest_api_id, + "explicit_api_stage": {"suffix": stage_suffix}, + } @cw_timer(prefix=FUNCTION_EVETSOURCE_METRIC_PREFIX) def to_cloudformation(self, **kwargs): # type: ignore[no-untyped-def] diff --git a/samtranslator/model/stepfunctions/events.py b/samtranslator/model/stepfunctions/events.py index b77693ae6..6b47ece1d 100644 --- a/samtranslator/model/stepfunctions/events.py +++ b/samtranslator/model/stepfunctions/events.py @@ -299,42 +299,12 @@ class Api(EventSource): Auth: Optional[Dict[str, Any]] UnescapeMappingTemplate: Optional[bool] - def resources_to_link(self, resources): # type: ignore[no-untyped-def] + def resources_to_link(self, resources: Dict[str, Any]) -> Dict[str, Any]: """ If this API Event Source refers to an explicit API resource, resolve the reference and grab necessary data from the explicit API """ - - # If RestApiId is a resource in the same template, then we try find the StageName by following the reference - # Otherwise we default to a wildcard. This stage name is solely used to construct the permission to - # allow this stage to invoke the State Machine. If we are unable to resolve the stage name, we will - # simply permit all stages to invoke this State Machine - # This hack is necessary because customers could use !ImportValue, !Ref or other intrinsic functions which - # can be sometimes impossible to resolve (ie. when it has cross-stack references) - permitted_stage = "*" - stage_suffix = "AllStages" - explicit_api = None - rest_api_id = PushApi.get_rest_api_id_string(self.RestApiId) - if isinstance(rest_api_id, str): - if ( - rest_api_id in resources - and "Properties" in resources[rest_api_id] - and "StageName" in resources[rest_api_id]["Properties"] - ): - explicit_api = resources[rest_api_id]["Properties"] - permitted_stage = explicit_api["StageName"] - - # Stage could be a intrinsic, in which case leave the suffix to default value - stage_suffix = permitted_stage if isinstance(permitted_stage, str) else "Stage" - - else: - # RestApiId is a string, not an intrinsic, but we did not find a valid API resource for this ID - raise InvalidEventException( - self.relative_id, - "RestApiId property of Api event must reference a valid resource in the same template.", - ) - - return {"explicit_api": explicit_api, "api_id": rest_api_id, "explicit_api_stage": {"suffix": stage_suffix}} + return PushApi.resources_to_link_for_rest_api(resources, self.relative_id, self.RestApiId) @cw_timer(prefix=SFN_EVETSOURCE_METRIC_PREFIX) def to_cloudformation(self, resource, **kwargs): # type: ignore[no-untyped-def] diff --git a/samtranslator/plugins/api/implicit_api_plugin.py b/samtranslator/plugins/api/implicit_api_plugin.py index d3a77618d..36e5ebf07 100644 --- a/samtranslator/plugins/api/implicit_api_plugin.py +++ b/samtranslator/plugins/api/implicit_api_plugin.py @@ -207,7 +207,7 @@ def _add_api_to_swagger(self, event_id, event_properties, template): # type: ig and template.get(api_id).type != self.SERVERLESS_API_RESOURCE_TYPE ) - # RestApiId is not pointing to a valid API resource + # RestApiId is not pointing to a valid API resource if isinstance(api_id, dict) or is_referencing_http_from_api_event: raise InvalidEventException( event_id, diff --git a/tests/model/stepfunctions/test_api_event.py b/tests/model/stepfunctions/test_api_event.py index a7b9e0674..25b458d95 100644 --- a/tests/model/stepfunctions/test_api_event.py +++ b/tests/model/stepfunctions/test_api_event.py @@ -75,6 +75,7 @@ def test_resources_to_link_with_explicit_api(self): def test_resources_to_link_with_undefined_explicit_api(self): resources = {} + self.api_event_source.relative_id = "event_id" self.api_event_source.RestApiId = {"Ref": "MyExplicitApi"} with self.assertRaises(InvalidEventException): self.api_event_source.resources_to_link(resources) diff --git a/tests/translator/input/error_api_event_ref_invalid_resource.yaml b/tests/translator/input/error_api_event_ref_invalid_resource.yaml new file mode 100644 index 000000000..2d9041c59 --- /dev/null +++ b/tests/translator/input/error_api_event_ref_invalid_resource.yaml @@ -0,0 +1,18 @@ +AWSTemplateFormatVersion: '2010-09-09' + +Resources: + MyFunction: + Type: AWS::Serverless::Function + Properties: + Runtime: python3.9 + Events: + Event1: + Type: Api + Properties: + Path: /channel/limit-validate + RestApiId: RestApi + Method: OPTIONS + + RestApi: + Type: AWS::Serverless::Api + Properties: 123 diff --git a/tests/translator/input/error_api_event_ref_nothing.yaml b/tests/translator/input/error_api_event_ref_nothing.yaml new file mode 100644 index 000000000..d019c27a0 --- /dev/null +++ b/tests/translator/input/error_api_event_ref_nothing.yaml @@ -0,0 +1,14 @@ +AWSTemplateFormatVersion: '2010-09-09' + +Resources: + MyFunction: + Type: AWS::Serverless::Function + Properties: + Runtime: python3.9 + Events: + Event1: + Type: Api + Properties: + Path: /channel/limit-validate + RestApiId: RestApi + Method: OPTIONS diff --git a/tests/translator/input/error_invalid_logical_id.yaml b/tests/translator/input/error_invalid_logical_id.yaml index a94d94348..59f39420f 100644 --- a/tests/translator/input/error_invalid_logical_id.yaml +++ b/tests/translator/input/error_invalid_logical_id.yaml @@ -5,3 +5,9 @@ Resources: Runtime: python2.7 Handler: hello.handler CodeUri: s3://bucket/code.zip + 0: + Type: AWS::Serverless::Function + Properties: + Runtime: python2.7 + Handler: hello.handler + CodeUri: s3://bucket/code.zip diff --git a/tests/translator/output/error_api_event_ref_invalid_resource.json b/tests/translator/output/error_api_event_ref_invalid_resource.json new file mode 100644 index 000000000..fb38771a3 --- /dev/null +++ b/tests/translator/output/error_api_event_ref_invalid_resource.json @@ -0,0 +1,3 @@ +{ + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [RestApi] is invalid. Attribute 'Properties' should be a map." +} diff --git a/tests/translator/output/error_api_event_ref_nothing.json b/tests/translator/output/error_api_event_ref_nothing.json new file mode 100644 index 000000000..1b455d998 --- /dev/null +++ b/tests/translator/output/error_api_event_ref_nothing.json @@ -0,0 +1,3 @@ +{ + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyFunction] is invalid. Event with id [Event1] is invalid. RestApiId must be a valid reference to an 'AWS::Serverless::Api' resource in same template." +} diff --git a/tests/translator/output/error_invalid_logical_id.json b/tests/translator/output/error_invalid_logical_id.json index cf07b8801..4f10ff694 100644 --- a/tests/translator/output/error_invalid_logical_id.json +++ b/tests/translator/output/error_invalid_logical_id.json @@ -1,8 +1,3 @@ { - "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [Bad-Function-Name-@#$%^&] is invalid. Logical ids must be alphanumeric.", - "errors": [ - { - "errorMessage": "Resource with id [Bad-Function-Name-@#$%^&] is invalid. Logical ids must be alphanumeric." - } - ] + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 2. Resource with id [0] is invalid. Logical ids must be alphanumeric. Resource with id [Bad-Function-Name-@#$%^&] is invalid. Logical ids must be alphanumeric." }