From c532fd893697d0e5e6ef66fdbcc5a0136b15f690 Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Tue, 26 Mar 2019 11:22:47 -0700 Subject: [PATCH 01/10] feat: update bin/sam-translate.py * Add additional commands * Update parameters to be consistent with SAM CLI * Add additional parameters * Improve logging * Fixes --- bin/sam-translate.py | 95 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 84 insertions(+), 11 deletions(-) diff --git a/bin/sam-translate.py b/bin/sam-translate.py index 4b0df0973..eda75b030 100755 --- a/bin/sam-translate.py +++ b/bin/sam-translate.py @@ -5,15 +5,25 @@ Known limitations: cannot transform CodeUri pointing at local directory. Usage: - sam-translate.py --input-file=sam-template.yaml [--output-file=] + sam-translate.py --template-file=sam-template.yaml [--verbose] [--output-template=] + sam-translate.py package --template-file=sam-template.yaml --s3-bucket=my-bucket [--verbose] [--output-template=] + sam-translate.py deploy --template-file=sam-template.yaml --s3-bucket=my-bucket --capabilities=CAPABILITY_NAMED_IAM --stack-name=my-stack [--verbose] [--output-template=] Options: - --input-file= Location of SAM template to transform. - --output-file= Location to store resulting CloudFormation template [default: cfn-template.json]. + --template-file= Location of SAM template to transform [default: template.yaml]. + --output-template= Location to store resulting CloudFormation template [default: transformed-template.json]. + --s3-bucket= S3 bucket to use for SAM artifacts when using the `package` command + --capabilities= Capabilities + --stack-name= Unique name for your CloudFormation Stack + --verbose Enables verbose logging """ import json +import logging import os +import platform +import subprocess +import sys import boto3 from docopt import docopt @@ -23,24 +33,60 @@ from samtranslator.yaml_helper import yaml_parse from samtranslator.model.exceptions import InvalidDocumentException - +LOG = logging.getLogger(__name__) cli_options = docopt(__doc__) iam_client = boto3.client('iam') cwd = os.getcwd() +if cli_options.get('--verbose'): + logging.basicConfig(level=logging.DEBUG) +else: + logging.basicConfig() + +def execute_command(command, args): + try: + aws_cmd = 'aws' if platform.system().lower() != 'windows' else 'aws.cmd' + command_with_args = [aws_cmd, 'cloudformation', command] + list(args) + + LOG.debug("Executing command: %s", command_with_args) + + subprocess.check_call(command_with_args) + + LOG.debug("Command successful") + except subprocess.CalledProcessError as e: + # Underlying aws command will print the exception to the user + LOG.debug("Exception: %s", e) + sys.exit(e.returncode) + def get_input_output_file_paths(): - input_file_option = cli_options.get('--input-file') - output_file_option = cli_options.get('--output-file') + input_file_option = cli_options.get('--template-file') + output_file_option = cli_options.get('--output-template') input_file_path = os.path.join(cwd, input_file_option) output_file_path = os.path.join(cwd, output_file_option) return input_file_path, output_file_path -def main(): - input_file_path, output_file_path = get_input_output_file_paths() +def package(input_file_path, output_file_path): + template_file = input_file_path + package_output_template_file = input_file_path + '._sam_packaged_.yaml' + s3_bucket = cli_options.get('--s3-bucket') + args = [ + '--template-file', + template_file, + '--output-template-file', + package_output_template_file, + '--s3-bucket', + s3_bucket + ] + + execute_command('package', args) + return package_output_template_file + + +def transform_template(input_file_path, output_file_path): with open(input_file_path, 'r') as f: sam_template = yaml_parse(f) @@ -56,10 +102,37 @@ def main(): print('Wrote transformed CloudFormation template to: ' + output_file_path) except InvalidDocumentException as e: errorMessage = reduce(lambda message, error: message + ' ' + error.message, e.causes, e.message) - print(errorMessage) + LOG.error(errorMessage) errors = map(lambda cause: cause.message, e.causes) - print(errors) + LOG.error(errors) + + +def deploy(template_file): + capabilities = cli_options.get('--capabilities') + stack_name = cli_options.get('--stack-name') + args = [ + '--template-file', + template_file, + '--capabilities', + capabilities, + '--stack-name', + stack_name + ] + + execute_command('deploy', args) + + return package_output_template_file if __name__ == '__main__': - main() + input_file_path, output_file_path = get_input_output_file_paths() + + if cli_options.get('package'): + package_output_template_file = package(input_file_path, output_file_path) + transform_template(package_output_template_file, output_file_path) + elif cli_options.get('deploy'): + package_output_template_file = package(input_file_path, output_file_path) + transform_template(package_output_template_file, output_file_path) + deploy(output_file_path) + else: + transform_template(input_file_path, output_file_path) From ec54e7952acccce49253591fdca3314760c9b643 Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Wed, 27 Mar 2019 09:31:26 -0700 Subject: [PATCH 02/10] fix: add validation for SAR Plugin Location Properties --- .../application/serverless_app_plugin.py | 22 ++++++++++++++++++- .../input/error_application_properties.yaml | 11 +++++++++- .../output/error_application_properties.json | 4 ++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/samtranslator/plugins/application/serverless_app_plugin.py b/samtranslator/plugins/application/serverless_app_plugin.py index b61730c9b..ac419d9ba 100644 --- a/samtranslator/plugins/application/serverless_app_plugin.py +++ b/samtranslator/plugins/application/serverless_app_plugin.py @@ -1,4 +1,5 @@ import boto3 +import json from botocore.exceptions import ClientError, EndpointConnectionError import logging from time import sleep, time @@ -86,10 +87,17 @@ def on_before_transform_template(self, template_dict): app_id = self._replace_value(app.properties[self.LOCATION_KEY], self.APPLICATION_ID_KEY, intrinsic_resolvers) + semver = self._replace_value(app.properties[self.LOCATION_KEY], self.SEMANTIC_VERSION_KEY, intrinsic_resolvers) + 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: # Lazy initialization of the client- create it when it is needed @@ -211,11 +219,23 @@ def on_before_transform_resource(self, logical_id, resource_type, resource_prope [self.APPLICATION_ID_KEY, self.SEMANTIC_VERSION_KEY]) app_id = resource_properties[self.LOCATION_KEY].get(self.APPLICATION_ID_KEY) + if not app_id: raise InvalidResourceException(logical_id, "Property 'ApplicationId' cannot be blank.") + + if isinstance(app_id, dict): + raise InvalidResourceException(logical_id, "Property 'ApplicationId' cannot be resolved. Only FindInMap " + "and Ref intrinsic functions are supported.") + semver = resource_properties[self.LOCATION_KEY].get(self.SEMANTIC_VERSION_KEY) + if not semver: - raise InvalidResourceException(logical_id, "Property 'SemanticVersion cannot be blank.") + raise InvalidResourceException(logical_id, "Property 'SemanticVersion' cannot be blank.") + + if isinstance(semver, dict): + raise InvalidResourceException(logical_id, "Property 'SemanticVersion' cannot be resolved. Only FindInMap " + "and Ref intrinsic functions are supported.") + key = (app_id, semver) # Throw any resource exceptions saved from the before_transform_template event diff --git a/tests/translator/input/error_application_properties.yaml b/tests/translator/input/error_application_properties.yaml index 8914e099d..2d2d51ab7 100644 --- a/tests/translator/input/error_application_properties.yaml +++ b/tests/translator/input/error_application_properties.yaml @@ -33,4 +33,13 @@ Resources: Properties: Location: ApplicationId: - SemanticVersion: \ No newline at end of file + SemanticVersion: + + IntrinsicProperties: + Type: 'AWS::Serverless::Application' + Properties: + Location: + ApplicationId: + Sub: foobar + SemanticVersion: + Sub: foobar \ No newline at end of file diff --git a/tests/translator/output/error_application_properties.json b/tests/translator/output/error_application_properties.json index 4bd94227b..1bf610981 100644 --- a/tests/translator/output/error_application_properties.json +++ b/tests/translator/output/error_application_properties.json @@ -1,8 +1,8 @@ { "errors": [ { - "errorMessage": "Resource with id [BlankProperties] is invalid. Property 'ApplicationId' cannot be blank. 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": "Resource with id [BlankProperties] is invalid. Property 'ApplicationId' cannot be blank. 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: 6. Resource with id [BlankProperties] is invalid. Property 'ApplicationId' cannot be blank. 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: 7. Resource with id [BlankProperties] is invalid. Property 'ApplicationId' cannot be blank. 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." } \ No newline at end of file From 26e37a19d1bf3ff8d3287a449701b7f58e7b4bb2 Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Wed, 27 Mar 2019 11:37:41 -0700 Subject: [PATCH 03/10] fix: add validation to ensure Resource Properties is a dict --- samtranslator/parser/parser.py | 18 ++++++++++++++++-- .../error_resource_properties_not_dict.yaml | 4 ++++ .../error_resource_properties_not_dict.json | 8 ++++++++ tests/translator/test_translator.py | 1 + 4 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 tests/translator/input/error_resource_properties_not_dict.yaml create mode 100644 tests/translator/output/error_resource_properties_not_dict.json diff --git a/samtranslator/parser/parser.py b/samtranslator/parser/parser.py index a8eebfadd..2672ae645 100644 --- a/samtranslator/parser/parser.py +++ b/samtranslator/parser/parser.py @@ -1,6 +1,7 @@ -from samtranslator.model.exceptions import InvalidDocumentException, InvalidTemplateException +from samtranslator.model.exceptions import InvalidDocumentException, InvalidTemplateException, InvalidResourceException from samtranslator.validator.validator import SamTemplateValidator from samtranslator.plugins import LifeCycleEvents +from samtranslator.public.sdk.template import SamTemplate class Parser: @@ -26,11 +27,24 @@ def _validate(self, sam_template, parameter_values): raise InvalidDocumentException( [InvalidTemplateException("'Resources' section is required")]) - if (not all(isinstance(value, dict) for value in sam_template["Resources"].values())): + if (not all(isinstance(sam_resource, dict) for sam_resource in sam_template["Resources"].values())): raise InvalidDocumentException( [InvalidTemplateException( "All 'Resources' must be Objects. If you're using YAML, this may be an " "indentation issue." )]) + sam_template_instance = SamTemplate(sam_template) + + for resource_logical_id, sam_resource in sam_template_instance.iterate(): + # NOTE: Properties isn't required for SimpleTable, so we can't check + # `not isinstance(sam_resources.get("Properties"), dict)` as this would be a breaking change. + # sam_resource.properties defaults to {} in SamTemplate init + if (not isinstance(sam_resource.properties, dict)): + raise InvalidDocumentException( + [InvalidResourceException(resource_logical_id, + "All 'Resources' must be Objects and have a 'Properties' Object. If " + "you're using YAML, this may be an indentation issue." + )]) + SamTemplateValidator.validate(sam_template) diff --git a/tests/translator/input/error_resource_properties_not_dict.yaml b/tests/translator/input/error_resource_properties_not_dict.yaml new file mode 100644 index 000000000..af9248898 --- /dev/null +++ b/tests/translator/input/error_resource_properties_not_dict.yaml @@ -0,0 +1,4 @@ +Resources: + Function: + Type: AWS::Serverless::Function + Properties: diff --git a/tests/translator/output/error_resource_properties_not_dict.json b/tests/translator/output/error_resource_properties_not_dict.json new file mode 100644 index 000000000..a7929ef22 --- /dev/null +++ b/tests/translator/output/error_resource_properties_not_dict.json @@ -0,0 +1,8 @@ +{ + "errors": [ + { + "errorMessage": "Resource with id [Function] is invalid. All 'Resources' must be Objects and have a 'Properties' Object. If you're using YAML, this may be an indentation issue." + } + ], + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [Function] is invalid. All 'Resources' must be Objects and have a 'Properties' Object. If you're using YAML, this may be an indentation issue." +} \ No newline at end of file diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py index 62a8bd0ea..43a80c29c 100644 --- a/tests/translator/test_translator.py +++ b/tests/translator/test_translator.py @@ -388,6 +388,7 @@ def _generate_new_deployment_hash(self, logical_id, dict_to_hash, rest_api_to_sw 'existing_role_logical_id', 'error_invalid_template', 'error_resource_not_dict', + 'error_resource_properties_not_dict', 'error_globals_is_not_dict', 'error_globals_unsupported_type', 'error_globals_unsupported_property', From 036b6630b397f587d449ff8fa0800999224ba178 Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Thu, 28 Mar 2019 12:29:45 -0700 Subject: [PATCH 04/10] fix: add validation for invalid FindInMap and GetAtt --- samtranslator/intrinsics/actions.py | 12 ++++-- tests/intrinsics/test_actions.py | 6 +-- .../input/error_invalid_findinmap.yaml | 39 +++++++++++++++++++ .../input/error_invalid_getatt.yaml | 17 ++++++++ .../output/error_invalid_findinmap.json | 8 ++++ .../output/error_invalid_getatt.json | 8 ++++ tests/translator/test_translator.py | 2 + 7 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 tests/translator/input/error_invalid_findinmap.yaml create mode 100644 tests/translator/input/error_invalid_getatt.yaml create mode 100644 tests/translator/output/error_invalid_findinmap.json create mode 100644 tests/translator/output/error_invalid_getatt.json diff --git a/samtranslator/intrinsics/actions.py b/samtranslator/intrinsics/actions.py index a70745502..d535305ed 100644 --- a/samtranslator/intrinsics/actions.py +++ b/samtranslator/intrinsics/actions.py @@ -1,7 +1,7 @@ import re from six import string_types -from samtranslator.model.exceptions import InvalidTemplateException +from samtranslator.model.exceptions import InvalidTemplateException, InvalidDocumentException class Action(object): @@ -427,6 +427,11 @@ def resolve_resource_refs(self, input_dict, supported_resource_refs): if not isinstance(value, list) or len(value) < 2: return input_dict + if (not all(isinstance(entry, string_types) for entry in value)): + raise InvalidDocumentException( + [InvalidTemplateException('Invalid GetAtt value {}. GetAtt expects an array with 2 strings.' + .format(value))]) + # Value of GetAtt is an array. It can contain any number of elements, with first being the LogicalId of # resource and rest being the attributes. In a SAM template, a reference to a resource can be used in the # first parameter. However tools like AWS CLI might break them down as well. So let's just concatenate @@ -529,8 +534,9 @@ def resolve_parameter_refs(self, input_dict, parameters): # FindInMap expects an array with 3 values if not isinstance(value, list) or len(value) != 3: - raise InvalidTemplateException('Invalid FindInMap value {}. FindInMap expects an array with 3 values.' - .format(value)) + raise InvalidDocumentException( + [InvalidTemplateException('Invalid FindInMap value {}. FindInMap expects an array with 3 values.' + .format(value))]) map_name = self.resolve_parameter_refs(value[0], parameters) top_level_key = self.resolve_parameter_refs(value[1], parameters) diff --git a/tests/intrinsics/test_actions.py b/tests/intrinsics/test_actions.py index 05e9bb11b..81dad2abe 100644 --- a/tests/intrinsics/test_actions.py +++ b/tests/intrinsics/test_actions.py @@ -2,7 +2,7 @@ from mock import patch, Mock from samtranslator.intrinsics.actions import Action, RefAction, SubAction, GetAttAction, FindInMapAction from samtranslator.intrinsics.resource_refs import SupportedResourceReferences -from samtranslator.model.exceptions import InvalidTemplateException +from samtranslator.model.exceptions import InvalidTemplateException, InvalidDocumentException class TestAction(TestCase): @@ -954,7 +954,7 @@ def test_value_not_list(self): input = { "Fn::FindInMap": "a string" } - with self.assertRaises(InvalidTemplateException): + with self.assertRaises(InvalidDocumentException): self.ref.resolve_parameter_refs(input, {}) def test_value_not_list_of_length_three(self): @@ -962,7 +962,7 @@ def test_value_not_list_of_length_three(self): "Fn::FindInMap": ["a string"] } - with self.assertRaises(InvalidTemplateException): + with self.assertRaises(InvalidDocumentException): self.ref.resolve_parameter_refs(input, {}) def test_mapping_not_string(self): diff --git a/tests/translator/input/error_invalid_findinmap.yaml b/tests/translator/input/error_invalid_findinmap.yaml new file mode 100644 index 000000000..22bcda161 --- /dev/null +++ b/tests/translator/input/error_invalid_findinmap.yaml @@ -0,0 +1,39 @@ +Parameters: + ApplicationIdParam: + Type: String + Default: arn:aws:serverlessrepo:us-east-1:123456789012:applications/hello-world + VersionParam: + Type: String + Default: 1.0.0 + +Mappings: + ApplicationLocations: + ap-southeast-1: + ApplicationId: arn:aws:serverlessrepo:ap-southeast-1:123456789012:applications/hello-world + Version: 1.0.1 + cn-north-1: + ApplicationId: arn:aws-cn:serverlessrepo:cn-north-1:123456789012:applications/hello-world + Version: 1.0.2 + us-gov-west-1: + ApplicationId: arn:aws-gov:serverlessrepo:us-gov-west-1:123456789012:applications/hello-world + Version: 1.0.3 + +Resources: + ApplicationRefParameter: + Type: 'AWS::Serverless::Application' + Properties: + Location: + ApplicationId: !Ref ApplicationIdParam + SemanticVersion: !Ref VersionParam + + ApplicationFindInMap: + Type: 'AWS::Serverless::Application' + Properties: + Location: + ApplicationId: !FindInMap + - ApplicationLocations + - !Ref 'AWS::Region' + SemanticVersion: !FindInMap + - ApplicationLocations + - !Ref 'AWS::Region' + - Version \ No newline at end of file diff --git a/tests/translator/input/error_invalid_getatt.yaml b/tests/translator/input/error_invalid_getatt.yaml new file mode 100644 index 000000000..87e122688 --- /dev/null +++ b/tests/translator/input/error_invalid_getatt.yaml @@ -0,0 +1,17 @@ +Resources: + FunctionWithInvalidGetAtt: + Type: AWS::Serverless::Function + Properties: + CodeUri: s3://sam-demo-bucket/hello.zip + Handler: hello.handler + Runtime: python2.7 + Role: + Fn::GetAtt: + - Fn::Sub: Foo + - Arn + Events: + GET: + Type: Api + Properties: + Path: / + Method: GET diff --git a/tests/translator/output/error_invalid_findinmap.json b/tests/translator/output/error_invalid_findinmap.json new file mode 100644 index 000000000..6332622b8 --- /dev/null +++ b/tests/translator/output/error_invalid_findinmap.json @@ -0,0 +1,8 @@ +{ + "errors": [ + { + "errorMessage": "Structure of the SAM template is invalid. Invalid FindInMap value ['ApplicationLocations', 'ap-southeast-1']. FindInMap expects an array with 3 values." + } + ], + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Structure of the SAM template is invalid. Invalid FindInMap value ['ApplicationLocations', 'ap-southeast-1']. FindInMap expects an array with 3 values." +} \ No newline at end of file diff --git a/tests/translator/output/error_invalid_getatt.json b/tests/translator/output/error_invalid_getatt.json new file mode 100644 index 000000000..ff6893d52 --- /dev/null +++ b/tests/translator/output/error_invalid_getatt.json @@ -0,0 +1,8 @@ +{ + "errors": [ + { + "errorMessage": "Structure of the SAM template is invalid. Invalid GetAtt value [{'Fn::Sub': 'Foo'}, 'Arn']. GetAtt expects an array with 2 strings." + } + ], + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Structure of the SAM template is invalid. Invalid GetAtt value [{'Fn::Sub': 'Foo'}, 'Arn']. GetAtt expects an array with 2 strings." +} \ No newline at end of file diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py index 43a80c29c..55753ef5f 100644 --- a/tests/translator/test_translator.py +++ b/tests/translator/test_translator.py @@ -362,6 +362,8 @@ def _generate_new_deployment_hash(self, logical_id, dict_to_hash, rest_api_to_sw 'error_application_preparing_timeout', 'error_cors_on_external_swagger', 'error_invalid_cors_dict', + 'error_invalid_findinmap', + 'error_invalid_getatt', 'error_cors_credentials_true_with_wildcard_origin', 'error_cors_credentials_true_without_explicit_origin', 'error_function_invalid_codeuri', From 1f2971ab11b6456da027a33db18015c625c9fa23 Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Thu, 28 Mar 2019 16:09:58 -0700 Subject: [PATCH 05/10] fix: add validation for empty StageName --- samtranslator/model/eventsources/push.py | 4 +++- tests/swagger/test_swagger.py | 4 ++-- .../input/error_api_invalid_stagename.yaml | 20 +++++++++++++++++++ .../output/error_api_invalid_stagename.json | 8 ++++++++ tests/translator/test_translator.py | 1 + 5 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 tests/translator/input/error_api_invalid_stagename.yaml create mode 100644 tests/translator/output/error_api_invalid_stagename.json diff --git a/samtranslator/model/eventsources/push.py b/samtranslator/model/eventsources/push.py index 51fa80fbd..d07cf3c78 100644 --- a/samtranslator/model/eventsources/push.py +++ b/samtranslator/model/eventsources/push.py @@ -12,7 +12,7 @@ from samtranslator.model.events import EventsRule from samtranslator.model.iot import IotTopicRule from samtranslator.translator.arn_generator import ArnGenerator -from samtranslator.model.exceptions import InvalidEventException +from samtranslator.model.exceptions import InvalidEventException, InvalidResourceException from samtranslator.swagger.swagger import SwaggerEditor CONDITION = 'Condition' @@ -419,6 +419,8 @@ def resources_to_link(self, resources): # Stage could be a intrinsic, in which case leave the suffix to default value if isinstance(permitted_stage, string_types): + if not permitted_stage: + raise InvalidResourceException(rest_api_id, 'StageName cannot be empty.') stage_suffix = permitted_stage else: stage_suffix = "Stage" diff --git a/tests/swagger/test_swagger.py b/tests/swagger/test_swagger.py index b484380d8..3de6bc5b4 100644 --- a/tests/swagger/test_swagger.py +++ b/tests/swagger/test_swagger.py @@ -339,7 +339,7 @@ def test_must_add_credentials_to_the_integration(self): self.editor.add_lambda_integration(path, method, integration_uri, None, api_auth_config) actual = self.editor.swagger["paths"][path][method][_X_INTEGRATION]['credentials'] - self.assertEquals(expected, actual) + self.assertEqual(expected, actual) def test_must_add_credentials_to_the_integration_overrides(self): path = "/newpath" @@ -356,7 +356,7 @@ def test_must_add_credentials_to_the_integration_overrides(self): self.editor.add_lambda_integration(path, method, integration_uri, method_auth_config, api_auth_config) actual = self.editor.swagger["paths"][path][method][_X_INTEGRATION]['credentials'] - self.assertEquals(expected, actual) + self.assertEqual(expected, actual) class TestSwaggerEditor_iter_on_path(TestCase): diff --git a/tests/translator/input/error_api_invalid_stagename.yaml b/tests/translator/input/error_api_invalid_stagename.yaml new file mode 100644 index 000000000..a3c178759 --- /dev/null +++ b/tests/translator/input/error_api_invalid_stagename.yaml @@ -0,0 +1,20 @@ +Resources: + ApiWithEmptyStageName: + Type: AWS::Serverless::Api + Properties: + DefinitionBody: {} + StageName: '' + Function: + Type: AWS::Serverless::Function + Properties: + Handler: lambda.handler + CodeUri: s3://bucket/api + Runtime: nodejs8.10 + Events: + ProxyApiRoot: + Type: Api + Properties: + RestApiId: + Ref: ApiWithEmptyStageName + Path: "/" + Method: ANY diff --git a/tests/translator/output/error_api_invalid_stagename.json b/tests/translator/output/error_api_invalid_stagename.json new file mode 100644 index 000000000..1076d671f --- /dev/null +++ b/tests/translator/output/error_api_invalid_stagename.json @@ -0,0 +1,8 @@ +{ + "errors": [ + { + "errorMessage": "Resource with id [ApiWithEmptyStageName] is invalid. StageName cannot be empty." + } + ], + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [ApiWithEmptyStageName] is invalid. StageName cannot be empty." +} \ No newline at end of file diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py index 55753ef5f..141fc76b8 100644 --- a/tests/translator/test_translator.py +++ b/tests/translator/test_translator.py @@ -355,6 +355,7 @@ def _generate_new_deployment_hash(self, logical_id, dict_to_hash, rest_api_to_sw 'error_api_invalid_auth', 'error_api_invalid_definitionuri', 'error_api_invalid_definitionbody', + 'error_api_invalid_stagename', 'error_api_invalid_restapiid', 'error_application_properties', 'error_application_does_not_exist', From 7eeb01a8f5cb3e3e7cad7cbe621ae1b5ba98eb98 Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Thu, 28 Mar 2019 17:03:19 -0700 Subject: [PATCH 06/10] fix: raise InvalidDocumentException for Swagger invalid path Previously this was raising ValueError --- samtranslator/swagger/swagger.py | 5 ++++- tests/swagger/test_swagger.py | 5 +++-- tests/translator/input/error_api_invalid_.yaml | 10 ++++++++++ tests/translator/output/error_api_invalid_.json | 3 +++ tests/translator/test_translator.py | 1 + 5 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 tests/translator/input/error_api_invalid_.yaml create mode 100644 tests/translator/output/error_api_invalid_.json diff --git a/samtranslator/swagger/swagger.py b/samtranslator/swagger/swagger.py index d8b99e5b3..f1803f33d 100644 --- a/samtranslator/swagger/swagger.py +++ b/samtranslator/swagger/swagger.py @@ -3,6 +3,7 @@ from samtranslator.model.intrinsics import ref from samtranslator.model.intrinsics import make_conditional +from samtranslator.model.exceptions import InvalidDocumentException, InvalidTemplateException class SwaggerEditor(object): @@ -124,7 +125,9 @@ def add_path(self, path, method=None): if not isinstance(path_dict, dict): # Either customers has provided us an invalid Swagger, or this class has messed it somehow - raise ValueError("Value of '{}' path must be a dictionary according to Swagger spec".format(path)) + raise InvalidDocumentException( + [InvalidTemplateException("Value of '{}' path must be a dictionary according to Swagger spec." + .format(path))]) if self._CONDITIONAL_IF in path_dict: path_dict = path_dict[self._CONDITIONAL_IF][1] diff --git a/tests/swagger/test_swagger.py b/tests/swagger/test_swagger.py index 3de6bc5b4..388209213 100644 --- a/tests/swagger/test_swagger.py +++ b/tests/swagger/test_swagger.py @@ -5,6 +5,7 @@ from parameterized import parameterized, param from samtranslator.swagger.swagger import SwaggerEditor +from samtranslator.model.exceptions import InvalidDocumentException _X_INTEGRATION = "x-amazon-apigateway-integration" _X_ANY_METHOD = 'x-amazon-apigateway-any-method' @@ -193,7 +194,7 @@ def test_must_raise_non_dict_path_values(self): path = "/badpath" method = "get" - with self.assertRaises(ValueError): + with self.assertRaises(InvalidDocumentException): self.editor.add_path(path, method) def test_must_skip_existing_path(self): @@ -430,7 +431,7 @@ def test_must_skip_existing_path(self): def test_must_fail_with_bad_values_for_path(self): path = "/bad" - with self.assertRaises(ValueError): + with self.assertRaises(InvalidDocumentException): self.editor.add_cors(path, "origins", "headers", "methods") def test_must_fail_for_invalid_allowed_origin(self): diff --git a/tests/translator/input/error_api_invalid_.yaml b/tests/translator/input/error_api_invalid_.yaml new file mode 100644 index 000000000..09764a024 --- /dev/null +++ b/tests/translator/input/error_api_invalid_.yaml @@ -0,0 +1,10 @@ +Resources: + ApiWithInvalidBodyType: + Type: AWS::Serverless::Api + Properties: + StageName: Prod + Cors: "'*'" + DefinitionBody: + swagger: 2.0 + paths: + /foo: diff --git a/tests/translator/output/error_api_invalid_.json b/tests/translator/output/error_api_invalid_.json new file mode 100644 index 000000000..2aedc0684 --- /dev/null +++ b/tests/translator/output/error_api_invalid_.json @@ -0,0 +1,3 @@ +{ + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Structure of the SAM template is invalid. Value of '/foo' path must be a dictionary according to Swagger spec." +} \ No newline at end of file diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py index 141fc76b8..13823c7c3 100644 --- a/tests/translator/test_translator.py +++ b/tests/translator/test_translator.py @@ -353,6 +353,7 @@ def _generate_new_deployment_hash(self, logical_id, dict_to_hash, rest_api_to_sw 'error_api_gateway_responses_unknown_responseparameter', 'error_api_gateway_responses_unknown_responseparameter_property', 'error_api_invalid_auth', + 'error_api_invalid_', 'error_api_invalid_definitionuri', 'error_api_invalid_definitionbody', 'error_api_invalid_stagename', From 70b4d5b1b2729ae21bb37f80f5cf3a8e54ad404a Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Fri, 29 Mar 2019 13:51:21 -0700 Subject: [PATCH 07/10] fix: change ValueError to InvalidResourceException for empty Alias name --- samtranslator/model/sam_resources.py | 2 +- ...ror_function_invalid_autopublishalias.yaml | 115 ++++++++++++++++++ ...ror_function_invalid_autopublishalias.json | 8 ++ tests/translator/test_function_resources.py | 3 +- tests/translator/test_translator.py | 1 + 5 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 tests/translator/input/error_function_invalid_autopublishalias.yaml create mode 100644 tests/translator/output/error_function_invalid_autopublishalias.json diff --git a/samtranslator/model/sam_resources.py b/samtranslator/model/sam_resources.py index 07c050b31..3e6ae61c2 100644 --- a/samtranslator/model/sam_resources.py +++ b/samtranslator/model/sam_resources.py @@ -381,7 +381,7 @@ def _construct_alias(self, name, function, version): """ if not name: - raise ValueError("Alias name is required to create an alias") + raise InvalidResourceException(self.logical_id, "Alias name is required to create an alias") logical_id = "{id}Alias{suffix}".format(id=function.logical_id, suffix=name) alias = LambdaAlias(logical_id=logical_id, attributes=self.get_passthrough_resource_attributes()) diff --git a/tests/translator/input/error_function_invalid_autopublishalias.yaml b/tests/translator/input/error_function_invalid_autopublishalias.yaml new file mode 100644 index 000000000..f5bf938bf --- /dev/null +++ b/tests/translator/input/error_function_invalid_autopublishalias.yaml @@ -0,0 +1,115 @@ +Parameters: + MyAlias: + Type: String + Default: "" +Resources: + InvalidAutoPublishAliasFunction: + Type: AWS::Serverless::Function + Properties: + Handler: index.lambda_handler + CodeUri: s3://bucket/object + Runtime: python3.7 + AutoPublishAlias: + Ref: MyAlias + Events: + Get: + Type: Api + Properties: + Path: "/path" + Method: GET +# --- +# Parameters: +# Branch: +# Type: String +# Default: "DeploymentBucket" +# Globals: +# Function: +# AutoPublishAlias: +# Ref: Branch +# Description: __HIDDEN__ +# Resources: +# # AdminAPICustomDomainRoute53: +# # Type: AWS::Route53::RecordSet +# # Properties: +# # HostedZoneName: +# # Ref: HostedZoneName +# # ResourceRecords: +# # - Fn::GetAtt: +# # - AdminAPICustomDomain +# # - DistributionDomainName +# # Type: CNAME +# # Name: +# # Fn::Sub: "${Branch}-${DomainName}" +# # TTL: 300 +# TestFunction: +# Type: AWS::Serverless::Function +# Properties: +# CodeUri: s3://bucket/object +# Handler: lambda_test.lambda_handler +# Role: +# Fn::GetAtt: +# - LambdaExecutionRole +# - Arn +# Events: +# GetEvent: +# Type: Api +# Properties: +# Path: "/dar/test" +# Method: GET +# Runtime: python3.7 +# AdminAPICustomDomain: +# Type: AWS::ApiGateway::DomainName +# Properties: +# CertificateArn: +# Ref: DomainCertificateArn +# DomainName: +# Fn::Sub: "${Branch}-${DomainName}" +# AdminAPICustomDomainBasePath: +# Type: AWS::ApiGateway::BasePathMapping +# Properties: +# DomainName: +# Ref: AdminAPICustomDomain +# RestApiId: +# Ref: AdminApi +# Stage: +# Ref: Branch +# ControlFileFunction: +# Type: AWS::Serverless::Function +# Properties: +# Handler: mdlcontrolfile.lambda_handler +# Role: +# Fn::GetAtt: +# - LambdaExecutionRole +# - Arn +# Timeout: 60 +# CodeUri: s3://bucket/object +# Runtime: python3.7 +# Events: +# GetEvent: +# Type: Api +# Properties: +# Path: "/dar/controlFile/create" +# Method: GET +# LambdaExecutionRole: +# Type: AWS::IAM::Role +# Properties: +# Policies: +# - PolicyName: AllowS3AssumeRole +# PolicyDocument: +# Version: '2012-10-17' +# Statement: +# - Action: s3:* +# Resource: +# - arn:aws:s3:::darmdl-test1 +# - arn:aws:s3:::darmdl-test1/* +# Effect: Allow +# - Action: sts:AssumeRole +# Resource: arn:aws:iam::176494265958:role/EMR_EC2_DefaultRole +# Effect: Allow +# AssumeRolePolicyDocument: +# Version: '2012-10-17' +# Statement: +# - Action: sts:AssumeRole +# Effect: Allow +# Principal: +# Service: lambda.amazonaws.com diff --git a/tests/translator/output/error_function_invalid_autopublishalias.json b/tests/translator/output/error_function_invalid_autopublishalias.json new file mode 100644 index 000000000..820df472f --- /dev/null +++ b/tests/translator/output/error_function_invalid_autopublishalias.json @@ -0,0 +1,8 @@ +{ + "errors": [ + { + "errorMessage": "Resource with id [InvalidAutoPublishAliasFunction] is invalid. Alias name is required to create an alias" + } + ], + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [InvalidAutoPublishAliasFunction] is invalid. Alias name is required to create an alias" +} \ No newline at end of file diff --git a/tests/translator/test_function_resources.py b/tests/translator/test_function_resources.py index 3cdf7fe8f..a34917415 100644 --- a/tests/translator/test_function_resources.py +++ b/tests/translator/test_function_resources.py @@ -470,7 +470,7 @@ def test_alias_creation(self): def test_alias_creation_error(self): - with self.assertRaises(ValueError): + with self.assertRaises(InvalidResourceException): self.sam_func._construct_alias(None, self.lambda_func, self.lambda_version) def test_get_resolved_alias_name_must_work(self): @@ -515,7 +515,6 @@ def test_get_resolved_alias_name_must_error_if_intrinsics_are_not_resolved_with_ ex = raises_assert.exception self.assertEqual(expected_exception_msg, ex.message) - def _make_lambda_function(self, logical_id): func = LambdaFunction(logical_id) func.Code = { diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py index 13823c7c3..51c308439 100644 --- a/tests/translator/test_translator.py +++ b/tests/translator/test_translator.py @@ -370,6 +370,7 @@ def _generate_new_deployment_hash(self, logical_id, dict_to_hash, rest_api_to_sw 'error_cors_credentials_true_without_explicit_origin', 'error_function_invalid_codeuri', 'error_function_invalid_api_event', + 'error_function_invalid_autopublishalias', 'error_function_invalid_event_type', 'error_function_invalid_layer', 'error_function_no_codeuri', From 4b0281c3eac166faa67cd240c3cd581717735c66 Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Fri, 29 Mar 2019 14:27:10 -0700 Subject: [PATCH 08/10] refactor: remove commented out config --- ...ror_function_invalid_autopublishalias.yaml | 96 ------------------- 1 file changed, 96 deletions(-) diff --git a/tests/translator/input/error_function_invalid_autopublishalias.yaml b/tests/translator/input/error_function_invalid_autopublishalias.yaml index f5bf938bf..4883b3421 100644 --- a/tests/translator/input/error_function_invalid_autopublishalias.yaml +++ b/tests/translator/input/error_function_invalid_autopublishalias.yaml @@ -17,99 +17,3 @@ Resources: Properties: Path: "/path" Method: GET -# --- -# Parameters: -# Branch: -# Type: String -# Default: "DeploymentBucket" -# Globals: -# Function: -# AutoPublishAlias: -# Ref: Branch -# Description: __HIDDEN__ -# Resources: -# # AdminAPICustomDomainRoute53: -# # Type: AWS::Route53::RecordSet -# # Properties: -# # HostedZoneName: -# # Ref: HostedZoneName -# # ResourceRecords: -# # - Fn::GetAtt: -# # - AdminAPICustomDomain -# # - DistributionDomainName -# # Type: CNAME -# # Name: -# # Fn::Sub: "${Branch}-${DomainName}" -# # TTL: 300 -# TestFunction: -# Type: AWS::Serverless::Function -# Properties: -# CodeUri: s3://bucket/object -# Handler: lambda_test.lambda_handler -# Role: -# Fn::GetAtt: -# - LambdaExecutionRole -# - Arn -# Events: -# GetEvent: -# Type: Api -# Properties: -# Path: "/dar/test" -# Method: GET -# Runtime: python3.7 -# AdminAPICustomDomain: -# Type: AWS::ApiGateway::DomainName -# Properties: -# CertificateArn: -# Ref: DomainCertificateArn -# DomainName: -# Fn::Sub: "${Branch}-${DomainName}" -# AdminAPICustomDomainBasePath: -# Type: AWS::ApiGateway::BasePathMapping -# Properties: -# DomainName: -# Ref: AdminAPICustomDomain -# RestApiId: -# Ref: AdminApi -# Stage: -# Ref: Branch -# ControlFileFunction: -# Type: AWS::Serverless::Function -# Properties: -# Handler: mdlcontrolfile.lambda_handler -# Role: -# Fn::GetAtt: -# - LambdaExecutionRole -# - Arn -# Timeout: 60 -# CodeUri: s3://bucket/object -# Runtime: python3.7 -# Events: -# GetEvent: -# Type: Api -# Properties: -# Path: "/dar/controlFile/create" -# Method: GET -# LambdaExecutionRole: -# Type: AWS::IAM::Role -# Properties: -# Policies: -# - PolicyName: AllowS3AssumeRole -# PolicyDocument: -# Version: '2012-10-17' -# Statement: -# - Action: s3:* -# Resource: -# - arn:aws:s3:::darmdl-test1 -# - arn:aws:s3:::darmdl-test1/* -# Effect: Allow -# - Action: sts:AssumeRole -# Resource: arn:aws:iam::176494265958:role/EMR_EC2_DefaultRole -# Effect: Allow -# AssumeRolePolicyDocument: -# Version: '2012-10-17' -# Statement: -# - Action: sts:AssumeRole -# Effect: Allow -# Principal: -# Service: lambda.amazonaws.com From 7a3336afc6b0f43640a4970bb03db28fcc17ee64 Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Fri, 29 Mar 2019 15:46:21 -0700 Subject: [PATCH 09/10] refactor: rename test file --- .../{error_api_invalid_.yaml => error_api_invalid_path.yaml} | 2 +- .../{error_api_invalid_.json => error_api_invalid_path.json} | 0 tests/translator/test_translator.py | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename tests/translator/input/{error_api_invalid_.yaml => error_api_invalid_path.yaml} (86%) rename tests/translator/output/{error_api_invalid_.json => error_api_invalid_path.json} (100%) diff --git a/tests/translator/input/error_api_invalid_.yaml b/tests/translator/input/error_api_invalid_path.yaml similarity index 86% rename from tests/translator/input/error_api_invalid_.yaml rename to tests/translator/input/error_api_invalid_path.yaml index 09764a024..67d5925b6 100644 --- a/tests/translator/input/error_api_invalid_.yaml +++ b/tests/translator/input/error_api_invalid_path.yaml @@ -1,5 +1,5 @@ Resources: - ApiWithInvalidBodyType: + ApiWithInvalidPath: Type: AWS::Serverless::Api Properties: StageName: Prod diff --git a/tests/translator/output/error_api_invalid_.json b/tests/translator/output/error_api_invalid_path.json similarity index 100% rename from tests/translator/output/error_api_invalid_.json rename to tests/translator/output/error_api_invalid_path.json diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py index 51c308439..88657b15b 100644 --- a/tests/translator/test_translator.py +++ b/tests/translator/test_translator.py @@ -353,7 +353,7 @@ def _generate_new_deployment_hash(self, logical_id, dict_to_hash, rest_api_to_sw 'error_api_gateway_responses_unknown_responseparameter', 'error_api_gateway_responses_unknown_responseparameter_property', 'error_api_invalid_auth', - 'error_api_invalid_', + 'error_api_invalid_path', 'error_api_invalid_definitionuri', 'error_api_invalid_definitionbody', 'error_api_invalid_stagename', From eda4c2f64efc2a6c2bda965eb8d9912266a2f6aa Mon Sep 17 00:00:00 2001 From: Brett Andrews Date: Fri, 29 Mar 2019 16:34:56 -0700 Subject: [PATCH 10/10] test: update error_application_properties to include a Fn:Sub --- tests/translator/input/error_application_properties.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/translator/input/error_application_properties.yaml b/tests/translator/input/error_application_properties.yaml index 2d2d51ab7..2b069f90f 100644 --- a/tests/translator/input/error_application_properties.yaml +++ b/tests/translator/input/error_application_properties.yaml @@ -42,4 +42,4 @@ Resources: ApplicationId: Sub: foobar SemanticVersion: - Sub: foobar \ No newline at end of file + Fn::Sub: foobar \ No newline at end of file