Skip to content

Commit

Permalink
fix: Raise correct exception when Fn::If value is not a list (aws#2731)
Browse files Browse the repository at this point in the history
  • Loading branch information
aahung committed Dec 16, 2022
1 parent 71c9794 commit 91b1e8b
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 6 deletions.
18 changes: 14 additions & 4 deletions samtranslator/plugins/application/serverless_app_plugin.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Any, Tuple

import boto3
import json
from botocore.config import Config
Expand Down Expand Up @@ -73,6 +75,11 @@ def __init__(self, sar_client=None, wait_for_template_active_status=False, valid
message = "Cannot set both validate_only and wait_for_template_active_status flags to True."
raise InvalidPluginException(ServerlessAppPlugin.__name__, message) # type: ignore[no-untyped-call]

@staticmethod
def _make_app_key(app_id: Any, semver: Any) -> Tuple[str, str]:
"""Generate a key that is always hashable."""
return json.dumps(app_id, default=str), json.dumps(semver, default=str)

@cw_timer(prefix=PLUGIN_METRICS_PREFIX)
def on_before_transform_template(self, template_dict): # type: ignore[no-untyped-def]
"""
Expand Down Expand Up @@ -106,15 +113,18 @@ def on_before_transform_template(self, template_dict): # type: ignore[no-untype
app.properties[self.LOCATION_KEY], self.SEMANTIC_VERSION_KEY, intrinsic_resolvers
)

key = self._make_app_key(app_id, semver)

if isinstance(app_id, dict) or isinstance(semver, dict):
key = (json.dumps(app_id), json.dumps(semver))
self._applications[key] = False
continue

key = (app_id, semver)

if key not in self._applications:
try:
# Examine the type of ApplicationId and SemanticVersion
# before calling SAR API.
sam_expect(app_id, logical_id, "Location.ApplicationId").to_be_a_string()
sam_expect(semver, logical_id, "Location.SemanticVersion").to_be_a_string()
if not RegionConfiguration.is_service_supported("serverlessrepo"): # type: ignore[no-untyped-call]
raise InvalidResourceException(
logical_id, "Serverless Application Repository is not available in this region."
Expand Down Expand Up @@ -286,7 +296,7 @@ def on_before_transform_resource(self, logical_id, resource_type, resource_prope
"and Ref intrinsic functions are supported.",
)

key = (app_id, semver)
key = self._make_app_key(app_id, semver)

# Throw any resource exceptions saved from the before_transform_template event
if isinstance(self._applications[key], InvalidResourceException):
Expand Down
2 changes: 1 addition & 1 deletion tests/plugins/application/test_serverless_app_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def test_sar_throttling_doesnt_stop_processing(self, SamTemplateMock):

self.plugin.on_before_transform_template({})
self.assertEqual(
self.plugin._applications.get(("id1", "1.0.0")).message,
self.plugin._applications.get(ServerlessAppPlugin._make_app_key("id1", "1.0.0")).message,
"Resource with id [id1] is invalid. Failed to call SAR, timeout limit exceeded.",
)
# confirm we had at least two attempts to call SAR and that we executed a sleep
Expand Down
16 changes: 16 additions & 0 deletions tests/translator/input/error_application_properties.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,19 @@ Resources:
Sub: foobar
SemanticVersion:
Fn::Sub: foobar

WrongTypeApplicationId:
Type: AWS::Serverless::Application
Properties:
Location:
ApplicationId:
- this should not be a list
SemanticVersion: 2.0.0

WrongTypeSemanticVersion:
Type: AWS::Serverless::Application
Properties:
Location:
ApplicationId: some-id
SemanticVersion:
- this should not be a list
2 changes: 1 addition & 1 deletion tests/translator/output/error_application_properties.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 7. Resource with id [BlankProperties] is invalid. Property 'ApplicationId' is required. Resource with id [IntrinsicProperties] is invalid. Property 'ApplicationId' cannot be resolved. Only FindInMap and Ref intrinsic functions are supported. Resource with id [MissingApplicationId] is invalid. Resource is missing the required [ApplicationId] property. Resource with id [MissingLocation] is invalid. Resource is missing the required [Location] property. Resource with id [MissingSemanticVersion] is invalid. Resource is missing the required [SemanticVersion] property. Resource with id [NormalApplication] is invalid. Type of property 'ApplicationId' is invalid. Resource with id [UnsupportedProperty] is invalid. Resource is missing the required [Location] property."
"errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 9. Resource with id [BlankProperties] is invalid. Property 'ApplicationId' is required. Resource with id [IntrinsicProperties] is invalid. Property 'ApplicationId' cannot be resolved. Only FindInMap and Ref intrinsic functions are supported. Resource with id [MissingApplicationId] is invalid. Resource is missing the required [ApplicationId] property. Resource with id [MissingLocation] is invalid. Resource is missing the required [Location] property. Resource with id [MissingSemanticVersion] is invalid. Resource is missing the required [SemanticVersion] property. Resource with id [NormalApplication] is invalid. Property 'Location.ApplicationId' should be a string. Resource with id [UnsupportedProperty] is invalid. Resource is missing the required [Location] property. Resource with id [WrongTypeApplicationId] is invalid. Property 'Location.ApplicationId' should be a string. Resource with id [WrongTypeSemanticVersion] is invalid. Property 'Location.SemanticVersion' should be a string."
}

0 comments on commit 91b1e8b

Please sign in to comment.