From 81381cd035ea58eb5a058f1e2754c1fcd6112ad2 Mon Sep 17 00:00:00 2001 From: William Richard Date: Wed, 11 Apr 2018 18:16:56 -0400 Subject: [PATCH 1/2] Correctly generate resource name for target groups when using cloudformation They need to have less than 32 character names, so when you don't specify a name cloudformation generates a name that is less than 32 characters. And make sure that flake8 passes --- moto/cloudformation/parsing.py | 24 +++++-- moto/elbv2/models.py | 7 +- tests/test_elbv2/test_elbv2.py | 115 ++++++++++++++++++++++++++++++++- 3 files changed, 133 insertions(+), 13 deletions(-) diff --git a/moto/cloudformation/parsing.py b/moto/cloudformation/parsing.py index 19018158d..6b655983d 100644 --- a/moto/cloudformation/parsing.py +++ b/moto/cloudformation/parsing.py @@ -96,6 +96,7 @@ NAME_TYPE_MAP = { "AWS::ElasticBeanstalk::Application": "ApplicationName", "AWS::ElasticBeanstalk::Environment": "EnvironmentName", "AWS::ElasticLoadBalancing::LoadBalancer": "LoadBalancerName", + "AWS::ElasticLoadBalancingV2::TargetGroup": "Name", "AWS::RDS::DBInstance": "DBInstanceIdentifier", "AWS::S3::Bucket": "BucketName", "AWS::SNS::Topic": "TopicName", @@ -244,6 +245,18 @@ def resource_name_property_from_type(resource_type): return NAME_TYPE_MAP.get(resource_type) +def generate_resource_name(resource_type, stack_name, logical_id): + if resource_type == "AWS::ElasticLoadBalancingV2::TargetGroup": + # Target group names need to be less than 32 characters, so when cloudformation creates a name for you + # it makes sure to stay under that limit + name_prefix = '{0}-{1}'.format(stack_name, logical_id) + my_random_suffix = random_suffix() + truncated_name_prefix = name_prefix[0:32 - (len(my_random_suffix) + 1)] + return '{0}-{1}'.format(truncated_name_prefix, my_random_suffix) + else: + return '{0}-{1}-{2}'.format(stack_name, logical_id, random_suffix()) + + def parse_resource(logical_id, resource_json, resources_map): resource_type = resource_json['Type'] resource_class = resource_class_from_type(resource_type) @@ -258,15 +271,12 @@ def parse_resource(logical_id, resource_json, resources_map): if 'Properties' not in resource_json: resource_json['Properties'] = dict() if resource_name_property not in resource_json['Properties']: - resource_json['Properties'][resource_name_property] = '{0}-{1}-{2}'.format( - resources_map.get('AWS::StackName'), - logical_id, - random_suffix()) + resource_json['Properties'][resource_name_property] = generate_resource_name( + resource_type, resources_map.get('AWS::StackName'), logical_id) resource_name = resource_json['Properties'][resource_name_property] else: - resource_name = '{0}-{1}-{2}'.format(resources_map.get('AWS::StackName'), - logical_id, - random_suffix()) + resource_name = generate_resource_name(resource_type, resources_map.get('AWS::StackName'), logical_id) + return resource_class, resource_json, resource_name diff --git a/moto/elbv2/models.py b/moto/elbv2/models.py index 8921581d3..3925fa95d 100644 --- a/moto/elbv2/models.py +++ b/moto/elbv2/models.py @@ -124,10 +124,7 @@ class FakeTargetGroup(BaseModel): elbv2_backend = elbv2_backends[region_name] - # per cloudformation docs: - # The target group name should be shorter than 22 characters because - # AWS CloudFormation uses the target group name to create the name of the load balancer. - name = properties.get('Name', resource_name[:22]) + name = properties.get('Name') vpc_id = properties.get("VpcId") protocol = properties.get('Protocol') port = properties.get("Port") @@ -437,7 +434,7 @@ class ELBv2Backend(BaseBackend): def create_target_group(self, name, **kwargs): if len(name) > 32: raise InvalidTargetGroupNameError( - "Target group name '%s' cannot be longer than '22' characters" % name + "Target group name '%s' cannot be longer than '32' characters" % name ) if not re.match('^[a-zA-Z0-9\-]+$', name): raise InvalidTargetGroupNameError( diff --git a/tests/test_elbv2/test_elbv2.py b/tests/test_elbv2/test_elbv2.py index ce092976a..c5c425382 100644 --- a/tests/test_elbv2/test_elbv2.py +++ b/tests/test_elbv2/test_elbv2.py @@ -1,4 +1,6 @@ from __future__ import unicode_literals + +import json import os import boto3 import botocore @@ -6,7 +8,7 @@ from botocore.exceptions import ClientError from nose.tools import assert_raises import sure # noqa -from moto import mock_elbv2, mock_ec2, mock_acm +from moto import mock_elbv2, mock_ec2, mock_acm, mock_cloudformation from moto.elbv2 import elbv2_backends @@ -416,6 +418,7 @@ def test_create_target_group_and_listeners(): response = conn.describe_target_groups() response.get('TargetGroups').should.have.length_of(0) + @mock_elbv2 @mock_ec2 def test_create_target_group_without_non_required_parameters(): @@ -454,6 +457,7 @@ def test_create_target_group_without_non_required_parameters(): target_group = response.get('TargetGroups')[0] target_group.should_not.be.none + @mock_elbv2 @mock_ec2 def test_create_invalid_target_group(): @@ -1105,6 +1109,50 @@ def test_describe_invalid_target_group(): conn.describe_target_groups(Names=['invalid']) +@mock_elbv2 +@mock_ec2 +def test_describe_target_groups_no_arguments(): + conn = boto3.client('elbv2', region_name='us-east-1') + ec2 = boto3.resource('ec2', region_name='us-east-1') + + security_group = ec2.create_security_group( + GroupName='a-security-group', Description='First One') + vpc = ec2.create_vpc(CidrBlock='172.28.7.0/24', InstanceTenancy='default') + subnet1 = ec2.create_subnet( + VpcId=vpc.id, + CidrBlock='172.28.7.192/26', + AvailabilityZone='us-east-1a') + subnet2 = ec2.create_subnet( + VpcId=vpc.id, + CidrBlock='172.28.7.192/26', + AvailabilityZone='us-east-1b') + + response = conn.create_load_balancer( + Name='my-lb', + Subnets=[subnet1.id, subnet2.id], + SecurityGroups=[security_group.id], + Scheme='internal', + Tags=[{'Key': 'key_name', 'Value': 'a_value'}]) + + response.get('LoadBalancers')[0].get('LoadBalancerArn') + + conn.create_target_group( + Name='a-target', + Protocol='HTTP', + Port=8080, + VpcId=vpc.id, + HealthCheckProtocol='HTTP', + HealthCheckPort='8080', + HealthCheckPath='/', + HealthCheckIntervalSeconds=5, + HealthCheckTimeoutSeconds=5, + HealthyThresholdCount=5, + UnhealthyThresholdCount=2, + Matcher={'HttpCode': '200'}) + + assert len(conn.describe_target_groups()['TargetGroups']) == 1 + + @mock_elbv2 def test_describe_account_limits(): client = boto3.client('elbv2', region_name='eu-central-1') @@ -1473,3 +1521,68 @@ def test_modify_listener_http_to_https(): {'Type': 'forward', 'TargetGroupArn': target_group_arn} ] ) + + +@mock_ec2 +@mock_elbv2 +@mock_cloudformation +def test_create_target_groups_through_cloudformation(): + cfn_conn = boto3.client('cloudformation', region_name='us-east-1') + elbv2_client = boto3.client('elbv2', region_name='us-east-1') + + # test that setting a name manually as well as letting cloudformation create a name both work + # this is a special case because test groups have a name length limit of 22 characters, and must be unique + # https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-targetgroup.html#cfn-elasticloadbalancingv2-targetgroup-name + template = { + "AWSTemplateFormatVersion": "2010-09-09", + "Description": "ECS Cluster Test CloudFormation", + "Resources": { + "testVPC": { + "Type": "AWS::EC2::VPC", + "Properties": { + "CidrBlock": "10.0.0.0/16", + }, + }, + "testGroup1": { + "Type": "AWS::ElasticLoadBalancingV2::TargetGroup", + "Properties": { + "Port": 80, + "Protocol": "HTTP", + "VpcId": {"Ref": "testVPC"}, + }, + }, + "testGroup2": { + "Type": "AWS::ElasticLoadBalancingV2::TargetGroup", + "Properties": { + "Port": 90, + "Protocol": "HTTP", + "VpcId": {"Ref": "testVPC"}, + }, + }, + "testGroup3": { + "Type": "AWS::ElasticLoadBalancingV2::TargetGroup", + "Properties": { + "Name": "MyTargetGroup", + "Port": 70, + "Protocol": "HTTPS", + "VpcId": {"Ref": "testVPC"}, + }, + }, + } + } + template_json = json.dumps(template) + cfn_conn.create_stack( + StackName="test-stack", + TemplateBody=template_json, + ) + + describe_target_groups_response = elbv2_client.describe_target_groups() + target_group_dicts = describe_target_groups_response['TargetGroups'] + assert len(target_group_dicts) == 3 + + # there should be 2 target groups with the same prefix of 10 characters (since the random suffix is 12) + # and one named MyTargetGroup + assert len([tg for tg in target_group_dicts if tg['TargetGroupName'] == 'MyTargetGroup']) == 1 + assert len( + [tg for tg in target_group_dicts if tg['TargetGroupName'].startswith('test-stack-test')] + ) == 2 From eb018c01a529bc92080abae55936302766b6ae30 Mon Sep 17 00:00:00 2001 From: William Richard Date: Thu, 12 Apr 2018 14:48:37 -0400 Subject: [PATCH 2/2] Handle edge case where you can end up with double dashes in target group names --- moto/cloudformation/parsing.py | 4 ++++ tests/test_elbv2/test_elbv2.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/moto/cloudformation/parsing.py b/moto/cloudformation/parsing.py index 6b655983d..c4059a06b 100644 --- a/moto/cloudformation/parsing.py +++ b/moto/cloudformation/parsing.py @@ -252,6 +252,10 @@ def generate_resource_name(resource_type, stack_name, logical_id): name_prefix = '{0}-{1}'.format(stack_name, logical_id) my_random_suffix = random_suffix() truncated_name_prefix = name_prefix[0:32 - (len(my_random_suffix) + 1)] + # if the truncated name ends in a dash, we'll end up with a double dash in the final name, which is + # not allowed + if truncated_name_prefix.endswith('-'): + truncated_name_prefix = truncated_name_prefix[:-1] return '{0}-{1}'.format(truncated_name_prefix, my_random_suffix) else: return '{0}-{1}-{2}'.format(stack_name, logical_id, random_suffix()) diff --git a/tests/test_elbv2/test_elbv2.py b/tests/test_elbv2/test_elbv2.py index c5c425382..b58345fdb 100644 --- a/tests/test_elbv2/test_elbv2.py +++ b/tests/test_elbv2/test_elbv2.py @@ -1584,5 +1584,5 @@ def test_create_target_groups_through_cloudformation(): # and one named MyTargetGroup assert len([tg for tg in target_group_dicts if tg['TargetGroupName'] == 'MyTargetGroup']) == 1 assert len( - [tg for tg in target_group_dicts if tg['TargetGroupName'].startswith('test-stack-test')] + [tg for tg in target_group_dicts if tg['TargetGroupName'].startswith('test-stack')] ) == 2