From 526da099d5c818237af59c5787b6a174d6a7d6d6 Mon Sep 17 00:00:00 2001 From: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Date: Tue, 29 Jun 2021 17:39:06 -0700 Subject: [PATCH 1/4] fix: fail with invalid resource, when RetentionPolicy has unresolved intrinsic value --- samtranslator/model/sam_resources.py | 10 ++++++++++ tests/translator/input/basic_layer.yaml | 8 +++++++- .../input/error_layer_invalid_properties.yaml | 11 ++++++++++- tests/translator/output/aws-cn/basic_layer.json | 11 +++++++++++ tests/translator/output/aws-us-gov/basic_layer.json | 11 +++++++++++ tests/translator/output/basic_layer.json | 11 +++++++++++ .../output/error_layer_invalid_properties.json | 2 +- 7 files changed, 61 insertions(+), 3 deletions(-) diff --git a/samtranslator/model/sam_resources.py b/samtranslator/model/sam_resources.py index e8ddd291e5..66dcd78e15 100644 --- a/samtranslator/model/sam_resources.py +++ b/samtranslator/model/sam_resources.py @@ -37,6 +37,7 @@ from samtranslator.translator import logical_id_generator from samtranslator.translator.arn_generator import ArnGenerator from samtranslator.model.intrinsics import ( + is_intrinsic, is_intrinsic_if, is_intrinsic_no_value, ref, @@ -1224,6 +1225,15 @@ def _get_retention_policy_value(self): :return: value for the DeletionPolicy attribute. """ + if is_intrinsic(self.RetentionPolicy): + # RetentionPolicy attribute of AWS::Serverless::LayerVersion does set the DeletionPolicy + # attribute. And DeletionPolicy attribute does not support intrinsic values. + raise InvalidResourceException( + self.logical_id, + "'{}' does not accept intrinsic functions, please use one of the following options: {}" + .format("RetentionPolicy", [self.RETAIN, self.DELETE]) + ) + if self.RetentionPolicy is None: return None elif self.RetentionPolicy.lower() == self.RETAIN.lower(): diff --git a/tests/translator/input/basic_layer.yaml b/tests/translator/input/basic_layer.yaml index 170af09b1a..35302582e8 100644 --- a/tests/translator/input/basic_layer.yaml +++ b/tests/translator/input/basic_layer.yaml @@ -35,4 +35,10 @@ Resources: Type: 'AWS::Serverless::LayerVersion' Condition: TestCondition Properties: - ContentUri: s3://sam-demo-bucket/layer.zip + ContentUri: s3://sam-demo-bucket/layer.zip + + LayerWithCaseInsensitiveRetentionPolicy: + Type: 'AWS::Serverless::LayerVersion' + Properties: + ContentUri: s3://sam-demo-bucket/layer.zip + RetentionPolicy: DeleTe diff --git a/tests/translator/input/error_layer_invalid_properties.yaml b/tests/translator/input/error_layer_invalid_properties.yaml index 606530d6ad..c6b9c531d2 100644 --- a/tests/translator/input/error_layer_invalid_properties.yaml +++ b/tests/translator/input/error_layer_invalid_properties.yaml @@ -6,6 +6,9 @@ Parameters: Default: - Retain Type: List + DeletePolicyIf: + Default: False + Type: Boolean Resources: LayerWithRuntimesString: @@ -43,4 +46,10 @@ Resources: Type: 'AWS::Serverless::LayerVersion' Properties: ContentUri: s3://sam-demo-bucket/layer.zip - RetentionPolicy: !Ref DeleteList \ No newline at end of file + RetentionPolicy: !Ref DeleteList + + LayerWithRetentionPolicyAsIntrinsic: + Type: 'AWS::Serverless::LayerVersion' + Properties: + ContentUri: s3://sam-demo-bucket/layer.zip + RetentionPolicy: !If [DeletePolicyIf, Retain, Delete] \ No newline at end of file diff --git a/tests/translator/output/aws-cn/basic_layer.json b/tests/translator/output/aws-cn/basic_layer.json index d373bd19c9..88bc3ea71f 100644 --- a/tests/translator/output/aws-cn/basic_layer.json +++ b/tests/translator/output/aws-cn/basic_layer.json @@ -59,6 +59,17 @@ }, "LayerName": "LayerWithContentUriObject" } + }, + "LayerWithCaseInsensitiveRetentionPolicyead52a491d": { + "DeletionPolicy": "Delete", + "Type": "AWS::Lambda::LayerVersion", + "Properties": { + "Content": { + "S3Bucket": "sam-demo-bucket", + "S3Key": "layer.zip" + }, + "LayerName": "LayerWithCaseInsensitiveRetentionPolicy" + } } } } \ No newline at end of file diff --git a/tests/translator/output/aws-us-gov/basic_layer.json b/tests/translator/output/aws-us-gov/basic_layer.json index d373bd19c9..88bc3ea71f 100644 --- a/tests/translator/output/aws-us-gov/basic_layer.json +++ b/tests/translator/output/aws-us-gov/basic_layer.json @@ -59,6 +59,17 @@ }, "LayerName": "LayerWithContentUriObject" } + }, + "LayerWithCaseInsensitiveRetentionPolicyead52a491d": { + "DeletionPolicy": "Delete", + "Type": "AWS::Lambda::LayerVersion", + "Properties": { + "Content": { + "S3Bucket": "sam-demo-bucket", + "S3Key": "layer.zip" + }, + "LayerName": "LayerWithCaseInsensitiveRetentionPolicy" + } } } } \ No newline at end of file diff --git a/tests/translator/output/basic_layer.json b/tests/translator/output/basic_layer.json index d373bd19c9..88bc3ea71f 100644 --- a/tests/translator/output/basic_layer.json +++ b/tests/translator/output/basic_layer.json @@ -59,6 +59,17 @@ }, "LayerName": "LayerWithContentUriObject" } + }, + "LayerWithCaseInsensitiveRetentionPolicyead52a491d": { + "DeletionPolicy": "Delete", + "Type": "AWS::Lambda::LayerVersion", + "Properties": { + "Content": { + "S3Bucket": "sam-demo-bucket", + "S3Key": "layer.zip" + }, + "LayerName": "LayerWithCaseInsensitiveRetentionPolicy" + } } } } \ No newline at end of file diff --git a/tests/translator/output/error_layer_invalid_properties.json b/tests/translator/output/error_layer_invalid_properties.json index 696ca48b0d..f4eaa83f8c 100644 --- a/tests/translator/output/error_layer_invalid_properties.json +++ b/tests/translator/output/error_layer_invalid_properties.json @@ -4,5 +4,5 @@ "errorMessage": "Resource with id [LayerWithLicenseInfoList] is invalid. Type of property 'LicenseInfo' is invalid. Resource with id [LayerWithNoContentUri] is invalid. Missing required property 'ContentUri'. Resource with id [LayerWithRetentionPolicy] is invalid. 'RetentionPolicy' must be one of the following options: ['Retain', 'Delete']. Resource with id [LayerWithRetentionPolicyListParam] is invalid. Could not resolve parameter for 'RetentionPolicy' or parameter is not a String. Resource with id [LayerWithRetentionPolicyParam] is invalid. 'RetentionPolicy' must be one of the following options: ['Retain', 'Delete']. Resource with id [LayerWithRuntimesString] is invalid. Type of property 'CompatibleRuntimes' is invalid." } ], - "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 6. Resource with id [LayerWithLicenseInfoList] is invalid. Type of property 'LicenseInfo' is invalid. Resource with id [LayerWithNoContentUri] is invalid. Missing required property 'ContentUri'. Resource with id [LayerWithRetentionPolicy] is invalid. 'RetentionPolicy' must be one of the following options: ['Retain', 'Delete']. Resource with id [LayerWithRetentionPolicyListParam] is invalid. Could not resolve parameter for 'RetentionPolicy' or parameter is not a String. Resource with id [LayerWithRetentionPolicyParam] is invalid. 'RetentionPolicy' must be one of the following options: ['Retain', 'Delete']. Resource with id [LayerWithRuntimesString] is invalid. Type of property 'CompatibleRuntimes' is invalid." + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 7. Resource with id [LayerWithLicenseInfoList] is invalid. Type of property 'LicenseInfo' is invalid. Resource with id [LayerWithNoContentUri] is invalid. Missing required property 'ContentUri'. Resource with id [LayerWithRetentionPolicy] is invalid. 'RetentionPolicy' must be one of the following options: ['Retain', 'Delete']. Resource with id [LayerWithRetentionPolicyAsIntrinsic] is invalid. 'RetentionPolicy' does not accept intrinsic functions, please use one of the following options: ['Retain', 'Delete'] Resource with id [LayerWithRetentionPolicyListParam] is invalid. Could not resolve parameter for 'RetentionPolicy' or parameter is not a String. Resource with id [LayerWithRetentionPolicyParam] is invalid. 'RetentionPolicy' must be one of the following options: ['Retain', 'Delete']. Resource with id [LayerWithRuntimesString] is invalid. Type of property 'CompatibleRuntimes' is invalid." } \ No newline at end of file From 0080071580a5a56a8ab89d1da7865940de279b39 Mon Sep 17 00:00:00 2001 From: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Date: Tue, 29 Jun 2021 17:53:44 -0700 Subject: [PATCH 2/4] make black --- samtranslator/model/sam_resources.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/samtranslator/model/sam_resources.py b/samtranslator/model/sam_resources.py index 66dcd78e15..a8665b4365 100644 --- a/samtranslator/model/sam_resources.py +++ b/samtranslator/model/sam_resources.py @@ -1230,8 +1230,9 @@ def _get_retention_policy_value(self): # attribute. And DeletionPolicy attribute does not support intrinsic values. raise InvalidResourceException( self.logical_id, - "'{}' does not accept intrinsic functions, please use one of the following options: {}" - .format("RetentionPolicy", [self.RETAIN, self.DELETE]) + "'{}' does not accept intrinsic functions, please use one of the following options: {}".format( + "RetentionPolicy", [self.RETAIN, self.DELETE] + ), ) if self.RetentionPolicy is None: From b44d3672850c2221c789fbb825131d4970d74c0c Mon Sep 17 00:00:00 2001 From: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Date: Tue, 29 Jun 2021 19:30:35 -0700 Subject: [PATCH 3/4] remove extra formatting --- samtranslator/model/sam_resources.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/samtranslator/model/sam_resources.py b/samtranslator/model/sam_resources.py index a8665b4365..473f54d72e 100644 --- a/samtranslator/model/sam_resources.py +++ b/samtranslator/model/sam_resources.py @@ -1230,9 +1230,8 @@ def _get_retention_policy_value(self): # attribute. And DeletionPolicy attribute does not support intrinsic values. raise InvalidResourceException( self.logical_id, - "'{}' does not accept intrinsic functions, please use one of the following options: {}".format( - "RetentionPolicy", [self.RETAIN, self.DELETE] - ), + "'RetentionPolicy' does not accept intrinsic functions, " + "please use one of the following options: {}".format([self.RETAIN, self.DELETE]), ) if self.RetentionPolicy is None: @@ -1244,7 +1243,7 @@ def _get_retention_policy_value(self): elif self.RetentionPolicy.lower() not in self.retention_policy_options: raise InvalidResourceException( self.logical_id, - "'{}' must be one of the following options: {}.".format("RetentionPolicy", [self.RETAIN, self.DELETE]), + "'RetentionPolicy' must be one of the following options: {}.".format([self.RETAIN, self.DELETE]), ) From 61e377dc6739f8c041d1d9b4cb78345c3d315478 Mon Sep 17 00:00:00 2001 From: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Date: Wed, 30 Jun 2021 13:31:10 -0700 Subject: [PATCH 4/4] add test case for !ref --- tests/translator/input/basic_layer.yaml | 11 ++++++++++ .../translator/output/aws-cn/basic_layer.json | 19 ++++++++++++++++- .../output/aws-us-gov/basic_layer.json | 21 +++++++++++++++++-- tests/translator/output/basic_layer.json | 19 ++++++++++++++++- 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/tests/translator/input/basic_layer.yaml b/tests/translator/input/basic_layer.yaml index 35302582e8..6bed1762ae 100644 --- a/tests/translator/input/basic_layer.yaml +++ b/tests/translator/input/basic_layer.yaml @@ -4,6 +4,11 @@ Conditions: - beta - beta +Parameters: + DeletePolicy: + Default: Retain + Type: String + Resources: MinimalLayer: Type: 'AWS::Serverless::LayerVersion' @@ -42,3 +47,9 @@ Resources: Properties: ContentUri: s3://sam-demo-bucket/layer.zip RetentionPolicy: DeleTe + + LayerWithRetentionPolicyParam: + Type: 'AWS::Serverless::LayerVersion' + Properties: + ContentUri: s3://sam-demo-bucket/layer.zip + RetentionPolicy: !Ref DeletePolicy diff --git a/tests/translator/output/aws-cn/basic_layer.json b/tests/translator/output/aws-cn/basic_layer.json index 88bc3ea71f..33816f0ad7 100644 --- a/tests/translator/output/aws-cn/basic_layer.json +++ b/tests/translator/output/aws-cn/basic_layer.json @@ -6,7 +6,13 @@ "beta" ] } - }, + }, + "Parameters": { + "DeletePolicy": { + "Default": "Retain", + "Type": "String" + } + }, "Resources": { "LayerWithCondition7c655e10ea": { "DeletionPolicy": "Retain", @@ -70,6 +76,17 @@ }, "LayerName": "LayerWithCaseInsensitiveRetentionPolicy" } + }, + "LayerWithRetentionPolicyParamcc64815342": { + "DeletionPolicy": "Retain", + "Type": "AWS::Lambda::LayerVersion", + "Properties": { + "Content": { + "S3Bucket": "sam-demo-bucket", + "S3Key": "layer.zip" + }, + "LayerName": "LayerWithRetentionPolicyParam" + } } } } \ No newline at end of file diff --git a/tests/translator/output/aws-us-gov/basic_layer.json b/tests/translator/output/aws-us-gov/basic_layer.json index 88bc3ea71f..707f6a57c3 100644 --- a/tests/translator/output/aws-us-gov/basic_layer.json +++ b/tests/translator/output/aws-us-gov/basic_layer.json @@ -6,7 +6,13 @@ "beta" ] } - }, + }, + "Parameters": { + "DeletePolicy": { + "Default": "Retain", + "Type": "String" + } + }, "Resources": { "LayerWithCondition7c655e10ea": { "DeletionPolicy": "Retain", @@ -47,7 +53,7 @@ "python2.7" ] } - }, + }, "LayerWithContentUriObjectbdbf1b82ac": { "DeletionPolicy": "Delete", "Type": "AWS::Lambda::LayerVersion", @@ -70,6 +76,17 @@ }, "LayerName": "LayerWithCaseInsensitiveRetentionPolicy" } + }, + "LayerWithRetentionPolicyParamcc64815342": { + "DeletionPolicy": "Retain", + "Type": "AWS::Lambda::LayerVersion", + "Properties": { + "Content": { + "S3Bucket": "sam-demo-bucket", + "S3Key": "layer.zip" + }, + "LayerName": "LayerWithRetentionPolicyParam" + } } } } \ No newline at end of file diff --git a/tests/translator/output/basic_layer.json b/tests/translator/output/basic_layer.json index 88bc3ea71f..33816f0ad7 100644 --- a/tests/translator/output/basic_layer.json +++ b/tests/translator/output/basic_layer.json @@ -6,7 +6,13 @@ "beta" ] } - }, + }, + "Parameters": { + "DeletePolicy": { + "Default": "Retain", + "Type": "String" + } + }, "Resources": { "LayerWithCondition7c655e10ea": { "DeletionPolicy": "Retain", @@ -70,6 +76,17 @@ }, "LayerName": "LayerWithCaseInsensitiveRetentionPolicy" } + }, + "LayerWithRetentionPolicyParamcc64815342": { + "DeletionPolicy": "Retain", + "Type": "AWS::Lambda::LayerVersion", + "Properties": { + "Content": { + "S3Bucket": "sam-demo-bucket", + "S3Key": "layer.zip" + }, + "LayerName": "LayerWithRetentionPolicyParam" + } } } } \ No newline at end of file