Skip to content

fix: fix bug in custom DeploymentPreference feature #966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 11, 2019
Merged

fix: fix bug in custom DeploymentPreference feature #966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
491b8d9
Added new testcase covering error cases in custom deployment preference
keetonian Jun 10, 2019
35b7357
Fix custom codedeploy deployment preference bug
keetonian Jun 11, 2019
8b7cc61
remove print statement... shame on me
keetonian Jun 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions samtranslator/model/preferences/deployment_preference_collection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,19 @@ def deployment_group(self, function_logical_id):

return deployment_group

def _replace_deployment_types(self, value):
def _replace_deployment_types(self, value, key=None):
if isinstance(value, list):
for i in range(len(value)):
value[i] = self._replace_deployment_types(value[i])
return value
elif is_instrinsic(value):
for (k, v) in value.items():
value[k] = self._replace_deployment_types(v)
value[k] = self._replace_deployment_types(v, k)
return value
else:
if value in CODEDEPLOY_PREDEFINED_CONFIGURATIONS_LIST:
if key == "Fn::Sub": # Don't nest a "Sub" in a "Sub"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this means. Could you add a clearer comment? Also, what about !Sub? Or has it been transformed into Fn::Sub at this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been transformed to Fn::Sub at this point.

CfnLint error:
[E1019: Sub validation of parameters] (Sub should be a string or array of 2 items for Resources/CustomWithCondition2DeploymentGroup/Properties/DeploymentConfigName/Fn::If/1/Fn::Sub)

It was doing the following:

            "Fn::Sub": {
              "Fn::Sub": [
                "CodeDeployDefault.Lambda${ConfigName}", 
                {
                  "ConfigName": "AllAtOnce"
                }
              ]
            }

return ["CodeDeployDefault.Lambda${ConfigName}", {"ConfigName": value}]
return fnSub("CodeDeployDefault.Lambda${ConfigName}", {"ConfigName": value})
return value

Expand Down
13 changes: 11 additions & 2 deletions samtranslator/model/sam_resources.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ def to_cloudformation(self, **kwargs):
"""
resources = []
intrinsics_resolver = kwargs["intrinsics_resolver"]
mappings_resolver = kwargs.get("mappings_resolver", None)

if self.DeadLetterQueue:
self._validate_dlq()
Expand All @@ -105,7 +106,8 @@ def to_cloudformation(self, **kwargs):
if self.DeploymentPreference:
self._validate_deployment_preference_and_add_update_policy(kwargs.get('deployment_preference_collection',
None),
lambda_alias, intrinsics_resolver)
lambda_alias, intrinsics_resolver,
mappings_resolver)

managed_policy_map = kwargs.get('managed_policy_map', {})
if not managed_policy_map:
Expand Down Expand Up @@ -392,13 +394,20 @@ def _construct_alias(self, name, function, version):
return alias

def _validate_deployment_preference_and_add_update_policy(self, deployment_preference_collection, lambda_alias,
intrinsics_resolver):
intrinsics_resolver, mappings_resolver):
if 'Enabled' in self.DeploymentPreference:
self.DeploymentPreference['Enabled'] = intrinsics_resolver.resolve_parameter_refs(
self.DeploymentPreference['Enabled'])
if isinstance(self.DeploymentPreference['Enabled'], dict):
raise InvalidResourceException(self.logical_id, "'Enabled' must be a boolean value")

if 'Type' in self.DeploymentPreference:
# resolve intrinsics and mappings for Type
preference_type = self.DeploymentPreference['Type']
preference_type = intrinsics_resolver.resolve_parameter_refs(preference_type)
preference_type = mappings_resolver.resolve_parameter_refs(preference_type)
self.DeploymentPreference['Type'] = preference_type

if deployment_preference_collection is None:
raise ValueError('deployment_preference_collection required for parsing the deployment preference')

Expand Down
4 changes: 4 additions & 0 deletions samtranslator/translator/translator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from samtranslator.model.exceptions import (InvalidDocumentException, InvalidResourceException,
DuplicateLogicalIdException, InvalidEventException)
from samtranslator.intrinsics.resolver import IntrinsicsResolver
from samtranslator.intrinsics.actions import FindInMapAction
from samtranslator.intrinsics.resource_refs import SupportedResourceReferences
from samtranslator.plugins.api.default_definition_body_plugin import DefaultDefinitionBodyPlugin
from samtranslator.plugins.application.serverless_app_plugin import ServerlessAppPlugin
Expand Down Expand Up @@ -62,6 +63,8 @@ def translate(self, sam_template, parameter_values):
template = copy.deepcopy(sam_template)
macro_resolver = ResourceTypeResolver(sam_resources)
intrinsics_resolver = IntrinsicsResolver(parameter_values)
mappings_resolver = IntrinsicsResolver(template.get('Mappings', {}),
{FindInMapAction.intrinsic_name: FindInMapAction()})
deployment_preference_collection = DeploymentPreferenceCollection()
supported_resource_refs = SupportedResourceReferences()
document_errors = []
Expand All @@ -76,6 +79,7 @@ def translate(self, sam_template, parameter_values):
kwargs = macro.resources_to_link(sam_template['Resources'])
kwargs['managed_policy_map'] = self.managed_policy_map
kwargs['intrinsics_resolver'] = intrinsics_resolver
kwargs['mappings_resolver'] = mappings_resolver
kwargs['deployment_preference_collection'] = deployment_preference_collection
translated = macro.to_cloudformation(**kwargs)

Expand Down
88 changes: 87 additions & 1 deletion tests/translator/input/function_with_custom_codedeploy_deployment_preference.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,29 @@
Mappings:
DeploymentPreferenceMap:
prod:
DeploymentPreference:
AllAtOnce
beta:
DeploymentPreference:
CustomDeployment

Parameters:
Stage:
Type: String
Default: 'beta'
Deployment:
Type: String
Default: 'AllAtOnce'
Custom:
Type: String
Default: 'CustomDeployment'

Conditions:
MyCondition:
Fn::Equals:
- true
- false

Resources:
MinimalFunction:
Type: 'AWS::Serverless::Function'
Expand All @@ -7,4 +33,64 @@ Resources:
Runtime: python2.7
AutoPublishAlias: live
DeploymentPreference:
Type: TestDeploymentConfiguration
Type: TestDeploymentConfiguration

CustomWithFindInMap: # Doesn't work
Type: 'AWS::Serverless::Function'
Properties:
CodeUri: s3://sam-demo-bucket/hello.zip
Handler: hello.handler
Runtime: python2.7
AutoPublishAlias: live
DeploymentPreference:
Type: !FindInMap [DeploymentPreferenceMap, !Ref Stage, DeploymentPreference]

CustomWithCondition: # Works
Type: 'AWS::Serverless::Function'
Properties:
CodeUri: s3://sam-demo-bucket/hello.zip
Handler: hello.handler
Runtime: python2.7
AutoPublishAlias: live
DeploymentPreference:
Type: !If [MyCondition, TestDeploymentConfiguration, AllAtOnce]

CustomWithCondition2:
Type: 'AWS::Serverless::Function'
Properties:
CodeUri: s3://sam-demo-bucket/hello.zip
Handler: hello.handler
Runtime: python2.7
AutoPublishAlias: live
DeploymentPreference:
Type: !If [MyCondition, !Sub "${Deployment}", !Ref Custom]

NormalWithSub:
Type: 'AWS::Serverless::Function'
Properties:
CodeUri: s3://sam-demo-bucket/hello.zip
Handler: hello.handler
Runtime: python2.7
AutoPublishAlias: live
DeploymentPreference:
Type: !Sub ${Deployment}

CustomWithSub:
Type: 'AWS::Serverless::Function'
Properties:
CodeUri: s3://sam-demo-bucket/hello.zip
Handler: hello.handler
Runtime: python2.7
AutoPublishAlias: live
DeploymentPreference:
Type: !Sub ${Custom}

NormalWithRef:
Type: 'AWS::Serverless::Function'
Properties:
CodeUri: s3://sam-demo-bucket/hello.zip
Handler: hello.handler
Runtime: python2.7
AutoPublishAlias: live
DeploymentPreference:
Type: !Ref Deployment
Loading