Skip to content

Commit

Permalink
bugfix/integrate_intrinsics (#1343)
Browse files Browse the repository at this point in the history
* Fix and Update Template Resolution Tests

* Remove Test with ignoring Refs with globals

* Handle Top level resource case

* Update Style

* Handle Layer Integ Test Failure on KeyboardExit
When running local layer integration tests, there was a cleanup error that
occured if there was a KeyboardExit occurred on the previous run.
This causes problems with cached values on consecutive tests.

* Handle Layer Integ Test Failure on KeyboardExit

When running local layer integration tests, there was a cleanup error that
occured if there was a KeyboardExit occurred on the previous run.
This causes problems with cached values on consecutive tests.

* Update Intrinsic Resolver to handle Layers

Lambda Layers were added as a special case where Ref's to unresolved resources remained as {"Ref": logical_id}.
This was chosen to be the default since SamTranslator handles the translation of some resource and converts some types such as
{"Ref":"AWS::Region"} -> {"Ref": "us-east-1"} and replacing Globals.

* Remove old layer ref code
  • Loading branch information
viksrivat authored and sriram-mv committed Aug 16, 2019
1 parent 40468a7 commit 9c0cfd3
Show file tree
Hide file tree
Showing 15 changed files with 1,385 additions and 65 deletions.
3 changes: 0 additions & 3 deletions samcli/commands/local/lib/cfn_api_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,9 @@ def _extract_cloud_formation_stage(resources, stage_resource, collector):
stage_name = properties.get("StageName")
stage_variables = properties.get("Variables")

# Currently, we aren't resolving any Refs or other intrinsic properties that come with it
# A separate pr will need to fully resolve intrinsics
logical_id = properties.get("RestApiId")
if not logical_id:
raise InvalidSamTemplateException("The AWS::ApiGateway::Stage must have a RestApiId property")

rest_api_resource_type = resources.get(logical_id, {}).get("Type")
if rest_api_resource_type != CfnApiProvider.APIGATEWAY_RESTAPI:
raise InvalidSamTemplateException(
Expand Down
3 changes: 1 addition & 2 deletions samcli/commands/local/lib/sam_base_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,21 @@ def get_template(template_dict, parameter_overrides=None):
dict
Processed SAM template
"""

template_dict = template_dict or {}
if template_dict:
template_dict = SamTranslatorWrapper(template_dict).run_plugins()
ResourceMetadataNormalizer.normalize(template_dict)
logical_id_translator = SamBaseProvider._get_parameter_values(
template_dict, parameter_overrides
)

resolver = IntrinsicResolver(
template=template_dict,
symbol_resolver=IntrinsicsSymbolTable(
logical_id_translator=logical_id_translator, template=template_dict
),
)
template_dict = resolver.resolve_template(ignore_errors=True)

return template_dict

@staticmethod
Expand Down
29 changes: 12 additions & 17 deletions samcli/lib/intrinsic_resolver/intrinsic_property_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,17 +194,11 @@ def intrinsic_property_resolver(self, intrinsic, parent_function="template"):
The simplified version of the intrinsic function. This could be a list,str,dict depending on the format required
"""
if intrinsic is None:
raise InvalidIntrinsicException(
"Missing Intrinsic property in {}".format(parent_function)
)

if (
any(
isinstance(intrinsic, object_type)
for object_type in [string_types, list, bool, int]
) or intrinsic == {}
):
raise InvalidIntrinsicException("Missing Intrinsic property in {}".format(parent_function))
if any(isinstance(intrinsic, object_type) for object_type in [string_types, bool, int]) or intrinsic == {}:
return intrinsic
if isinstance(intrinsic, list):
return [self.intrinsic_property_resolver(item) for item in intrinsic]

keys = list(intrinsic.keys())
key = keys[0]
Expand Down Expand Up @@ -238,12 +232,13 @@ def resolve_template(self, ignore_errors=False):
-------
Return a processed template
"""
processed_template = OrderedDict()
processed_template["Resources"] = self.resolve_attribute(self._resources, ignore_errors)
processed_template["Outputs"] = self.resolve_attribute(self._outputs, ignore_errors)
processed_template["Mappings"] = self.resolve_attribute(self._resources, ignore_errors)
processed_template["Parameters"] = self.resolve_attribute(self._resources, ignore_errors)
processed_template["Conditions"] = self.resolve_attribute(self._resources, ignore_errors)
processed_template = self._template

if self._resources:
processed_template["Resources"] = self.resolve_attribute(self._resources, ignore_errors)
if self._outputs:
processed_template["Outputs"] = self.resolve_attribute(self._outputs, ignore_errors)

return processed_template

def resolve_attribute(self, cloud_formation_property, ignore_errors=False):
Expand All @@ -265,7 +260,7 @@ def resolve_attribute(self, cloud_formation_property, ignore_errors=False):
for key, val in cloud_formation_property.items():
processed_key = self._symbol_resolver.get_translation(key) or key
try:
processed_resource = self.intrinsic_property_resolver(val)
processed_resource = self.intrinsic_property_resolver(val, parent_function=processed_key)
processed_dict[processed_key] = processed_resource
except (InvalidIntrinsicException, InvalidSymbolException) as e:
resource_type = val.get("Type", "")
Expand Down
17 changes: 15 additions & 2 deletions samcli/lib/intrinsic_resolver/intrinsics_symbol_table.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
The symbol table that is used in IntrinsicResolver in order to resolve runtime attributes
"""
import logging
import os

from six import string_types
Expand All @@ -10,6 +11,8 @@
InvalidSymbolException,
)

LOG = logging.getLogger(__name__)


class IntrinsicsSymbolTable(object):
AWS_ACCOUNT_ID = "AWS::AccountId"
Expand Down Expand Up @@ -174,6 +177,12 @@ def get_default_type_resolver():
return {
"AWS::ApiGateway::RestApi": {
"RootResourceId": "/" # It usually used as a reference to the parent id of the RestApi,
},
"AWS::Lambda::LayerVersion": {
IntrinsicResolver.REF: lambda logical_id: {IntrinsicResolver.REF: logical_id}
},
"AWS::Serverless::LayerVersion": {
IntrinsicResolver.REF: lambda logical_id: {IntrinsicResolver.REF: logical_id}
}
}

Expand Down Expand Up @@ -230,7 +239,7 @@ def resolve_symbols(self, logical_id, resource_attribute, ignore_errors=False):
)
if resolver:
if callable(resolver):
return resolver(logical_id, resource_attribute)
return resolver(logical_id)
return resolver

# Handle Attribute Type Resolution
Expand Down Expand Up @@ -301,12 +310,16 @@ def get_translation(self, logical_id, resource_attributes=IntrinsicResolver.REF)
"""
logical_id_item = self.logical_id_translator.get(logical_id, {})
if any(isinstance(logical_id_item, object_type) for object_type in [string_types, list, bool, int]):
if any(
isinstance(logical_id_item, object_type)
for object_type in [string_types, list, bool, int]
):
if (
resource_attributes != IntrinsicResolver.REF and resource_attributes != ""
):
return None
return logical_id_item

return logical_id_item.get(resource_attributes)

@staticmethod
Expand Down
9 changes: 9 additions & 0 deletions tests/integration/local/invoke/test_integrations_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,15 @@ def setUpClass(cls):
@classmethod
def tearDownClass(cls):
cls.layer_utils.delete_layers()
# Added to handle the case where ^C failed the test due to invalid cleanup of layers
docker_client = docker.from_env()
samcli_images = docker_client.images.list(name='samcli/lambda')
for image in samcli_images:
docker_client.images.remove(image.id)
integ_layer_cache_dir = Path().home().joinpath("integ_layer_cache")
if integ_layer_cache_dir.exists():
shutil.rmtree(str(integ_layer_cache_dir))

super(TestLayerVersion, cls).tearDownClass()

@parameterized.expand([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ Resources:
Type: AWS::ApiGateway::Stage
Properties:
StageName: Dev
RestApiId: TestApi
RestApiId: !Ref TestApi
Variables:
Stack: Dev
Stack: Dev

MyNonServerlessLambdaFunction:
Properties:
Expand Down Expand Up @@ -144,4 +144,4 @@ Resources:
Uri:
Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${MyNonServerlessLambdaFunction.Arn}/invocations
ResourceId: "NoServerlessFunctionResource"
RestApiId: "TestApi"
RestApiId: "TestApi"
2 changes: 1 addition & 1 deletion tests/unit/commands/local/lib/test_sam_api_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ def test_provider_must_support_binary_media_types(self):
"image~1gif",
"image~1png",
"image~1png", # Duplicates must be ignored
{"Ref": "SomeParameter"}, # Refs are ignored as well
]
}
},
Expand Down Expand Up @@ -288,6 +287,7 @@ def test_provider_must_support_binary_media_types(self):
list(provider.routes)[0],
Route(path="/path", methods=["GET"], function_name="SamFunc1"),
)

assertCountEqual(
self, provider.api.binary_media_types, ["image/gif", "image/png"]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@
},
true,
{
"Fn::If": [
"TestCondition",
true,
false
]
"Fn::If": ["TestCondition", true, false]
}
]
},
Expand All @@ -49,11 +45,24 @@
}
]
},
"InvalidCondition": [
"random items"
]
"InvalidCondition": ["random items"]
},
"Resources": {
"ReferenceLambdaLayerVersionLambdaFunction": {
"Properties": {
"Handler": "layer-main.custom_layer_handler",
"Runtime": "python3.6",
"CodeUri": ".",
"Layers": [{ "Ref": "MyCustomLambdaLayer" }]
},
"Type": "AWS::Serverless::Function"
},
"MyCustomLambdaLayer": {
"Type": "AWS::Lambda::LayerVersion",
"Properties": {
"Content": "custom_layer/"
}
},
"RestApi.Deployment": {
"Type": "AWS::ApiGateway::RestApi",
"Properties": {
Expand All @@ -65,37 +74,22 @@
"Fn::Split": [
",",
{
"Fn::Join": [
",",
[
"a",
"e",
"f",
"d"
]
]
"Fn::Join": [",", ["a", "e", "f", "d"]]
}
]
}
]
}
},
"BodyS3Location": {
"Fn::FindInMap": [
"TopLevel",
"SecondLevelKey",
"key"
]
"Fn::FindInMap": ["TopLevel", "SecondLevelKey", "key"]
}
}
},
"RestApiResource": {
"Properties": {
"parentId": {
"Fn::GetAtt": [
"RestApi.Deployment",
"RootResourceId"
]
"Fn::GetAtt": ["RestApi.Deployment", "RootResourceId"]
},
"PathPart": "{proxy+}",
"RestApiId": {
Expand Down Expand Up @@ -133,10 +127,7 @@
},
":lambda:path/2015-03-31/functions/",
{
"Fn::GetAtt": [
"HelloHandler2E4FBA4D",
"Arn"
]
"Fn::GetAtt": ["HelloHandler2E4FBA4D", "Arn"]
},
"/invocations"
]
Expand All @@ -145,4 +136,4 @@
}
}
}
}
}
Loading

0 comments on commit 9c0cfd3

Please sign in to comment.