From bf4ea2c6c45c50156f2309e95e8bdf89cc848471 Mon Sep 17 00:00:00 2001 From: Niall Creech Date: Thu, 30 Jul 2015 10:39:10 +0100 Subject: [PATCH] ELB Automatic resource naming This change removes settings LoadBalancerName manually and uses the AWS auto naming instead. * Do not set LoadBalancerName manually, use AWS auto naming * Add elb dns names as outputs * Pass troposphere template to elb function so that resources and outputs can be set up internal to the function * Update tests for elb method requiring template and resultant need to seperate out resource componenents for comparison * PEP8 fixes for test.py and test_iam.py (Closes ministryofjustice/bootstrap-cfn#136) --- CHANGELOG.md | 6 ++ bootstrap_cfn/config.py | 55 +++++++++---- tests/tests.py | 166 ++++++++++++++++++++++++++++++++++------ 3 files changed, 185 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c68419..afd903a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ This is used by bootstrap-salt to remove a file that it places and manages in an S3 bucket so that the stack can be cleanly deleted. +* Let Cloudformation name the ELBs automatically to make creating multiple + stacks easier. + + The 'name' parameter in each load balancer config is now only used to + generate the DNS entries. + ## Version 0.5.6 * Automaticly generate the RDS identifier diff --git a/bootstrap_cfn/config.py b/bootstrap_cfn/config.py index 796dea6..9d49c43 100644 --- a/bootstrap_cfn/config.py +++ b/bootstrap_cfn/config.py @@ -67,10 +67,7 @@ def process(self): map(template.add_resource, ec2) if 'elb' in self.data: - elbs, sgs = self.elb() - map(template.add_resource, elbs) - map(template.add_resource, sgs) - template = self._attach_elbs(template) + self.elb(template) if 'rds' in self.data: self.rds(template) @@ -257,7 +254,7 @@ def s3(self, template): Args: template: - The cloudformation template file + The troposphere.Template object """ # As there are no required fields, although we may not have any # subkeys we still need to be able to have a parent key 's3:' to @@ -311,12 +308,12 @@ def ssl(self): def rds(self, template): """ Create an RDS resource configuration from the config file data - and add it to the troposphere template. Outputs for the RDS name, + and add it to the troposphere.Template. Outputs for the RDS name, host and port are created. Args: template: - The cloudformation template file + The troposphere.Template object """ # REQUIRED FIELDS MAPPING required_fields = { @@ -400,12 +397,23 @@ def rds(self, template): Value=GetAtt(rds_instance, "Endpoint.Port") )) - def elb(self): + def elb(self, template): + """ + Create an ELB resource configuration from the config file data + and add them to the troposphere template. Outputs for each ELB's + DNSName are created. + + Args: + template: + The cloudformation template file + """ # REQUIRED FIELDS AND MAPPING + # Note, 'name' field is used internally to help label + # logical ids, and as part of the DNS record name. required_fields = { 'listeners': 'Listeners', 'scheme': 'Scheme', - 'name': 'LoadBalancerName', + 'name': None, 'hosted_zone': 'HostedZoneName' } @@ -425,7 +433,6 @@ def elb(self): Subnets=[Ref("SubnetA"), Ref("SubnetB"), Ref("SubnetC")], Listeners=elb['listeners'], Scheme=elb['scheme'], - LoadBalancerName=self._get_elb_canonical_name(elb['name']), ConnectionDrainingPolicy=ConnectionDrainingPolicy( Enabled=True, Timeout=120, @@ -502,7 +509,8 @@ def elb(self): Ref("AWS::Region"), ":", Ref("AWS::AccountId"), - ':loadbalancer/%s' % load_balancer.LoadBalancerName + ':loadbalancer/', + Ref(load_balancer) ]) ], "Effect": "Allow"} @@ -544,7 +552,20 @@ def elb(self): ) load_balancer.SecurityGroups = [Ref(sg)] elb_sgs.append(sg) - return elb_list, elb_sgs + + # Add outputs + output_name = "ELB" + safe_name + logging.debug("config:elb:Adding output to ELB '%s'" % (output_name)) + template.add_output(Output( + output_name, + Description="ELB DNSName", + Value=GetAtt(load_balancer, "DNSName") + )) + + # Update template with ELB resources + map(template.add_resource, elb_list) + map(template.add_resource, elb_sgs) + template = self._attach_elbs(template) def _convert_ref_dict_to_objects(self, o): """ @@ -725,10 +746,10 @@ def _attach_elbs(self, template): return template asgs = self._find_resources(template, 'AWS::AutoScaling::AutoScalingGroup') - elbs = self._find_resources(template, - 'AWS::ElasticLoadBalancing::LoadBalancer') - - asgs[0].LoadBalancerNames = [x.LoadBalancerName for x in elbs] - template.resources[asgs[0].title] = asgs[0] + if len(asgs) > 0: + elbs = self._find_resources(template, + 'AWS::ElasticLoadBalancing::LoadBalancer') + asgs[0].LoadBalancerNames = [Ref(x) for x in elbs] + template.resources[asgs[0].title] = asgs[0] return template diff --git a/tests/tests.py b/tests/tests.py index ec70b60..967fc91 100755 --- a/tests/tests.py +++ b/tests/tests.py @@ -343,7 +343,6 @@ def test_elb(self): } ], SecurityGroups=[Ref("DefaultSGtestdevinternal")], - LoadBalancerName="ELB-test-dev-internal", Scheme="internal", Policies=[ Policy( @@ -368,7 +367,8 @@ def test_elb(self): "", ["arn:aws:elasticloadbalancing:", Ref("AWS::Region"), ":", Ref("AWS::AccountId"), - ":loadbalancer/ELB-test-dev-external"] + ":loadbalancer/", + {"Ref": "ELBtestdevexternal"}] )], "Effect": "Allow"} ] @@ -390,7 +390,8 @@ def test_elb(self): Join("", ["arn:aws:elasticloadbalancing:", Ref("AWS::Region"), ":", Ref("AWS::AccountId"), - ":loadbalancer/ELB-test-dev-internal"] + ":loadbalancer/", + {"Ref": "ELBtestdevinternal"}] ) ], "Effect": "Allow" @@ -450,7 +451,6 @@ def test_elb(self): "Protocol": "TCP"} ], SecurityGroups=[Ref("DefaultSGtestdevexternal")], - LoadBalancerName="ELB-test-dev-external", Scheme="internet-facing", Policies=[ Policy( @@ -460,7 +460,10 @@ def test_elb(self): ) ], ) - known = [lb, lb2, pt1, pt2, rs, rsg] + known_load_balancer_resources = [lb, lb2] + known_policy_type_resources = [pt1, pt2] + known_record_set_resources = [rs, rsg] + expected_sgs = [ SecurityGroup( "DefaultSGtestdevexternal", @@ -502,13 +505,46 @@ def test_elb(self): ProjectConfig( 'tests/sample-project.yaml', 'dev').config, 'my-stack-name') - elb_cfg, elb_sgs = config.elb() - compare(self._resources_to_dict(known), - self._resources_to_dict(elb_cfg)) + # Create elb resources with template + template = Template() + config.elb(template) + elb_cfg = config._find_resources(template, "AWS::ElasticLoadBalancing::LoadBalancer") + elb_pt = config._find_resources(template, "AWS::IAM::Policy") + elb_rsg = config._find_resources(template, "AWS::Route53::RecordSetGroup") + elb_sgs = config._find_resources(template, "AWS::EC2::SecurityGroup") + compare(self._resources_to_dict(known_load_balancer_resources), + self._resources_to_dict(elb_cfg)) + compare(self._resources_to_dict(known_policy_type_resources), + self._resources_to_dict(elb_pt)) + compare(self._resources_to_dict(known_record_set_resources), + self._resources_to_dict(elb_rsg)) compare(self._resources_to_dict(expected_sgs), self._resources_to_dict(elb_sgs)) + # Test for outputs + expected_outputs = { + "ELBtestdevexternal": { + "Description": "ELB DNSName", + "Value": { + "Fn::GetAtt": [ + "ELBtestdevexternal", + "DNSName" + ] + } + }, + "ELBtestdevinternal": { + "Description": "ELB DNSName", + "Value": { + "Fn::GetAtt": [ + "ELBtestdevinternal", + "DNSName" + ] + } + } + } + actual_outputs = self._resources_to_dict(template.outputs.values()) + compare(expected_outputs, actual_outputs) def test_elb_custom_sg(self): @@ -551,7 +587,13 @@ def test_elb_custom_sg(self): }] config = ConfigParser(project_config.config, 'my-stack-name') - elb_cfg, elb_sgs = config.elb() + + # Create elb resources with template + template = Template() + config.elb(template) + elb_cfg = config._find_resources(template, "AWS::ElasticLoadBalancing::LoadBalancer") + elb_sgs = config._find_resources(template, "AWS::EC2::SecurityGroup") + elb_dict = self._resources_to_dict(elb_cfg) sgs_dict = self._resources_to_dict(elb_sgs) compare(expected_sgs, sgs_dict) @@ -581,6 +623,14 @@ def test_cf_includes(self): "StaticBucketName": { "Description": "S3 bucket name", "Value": {"Ref": "StaticBucket"} + }, + "ELBtestdevexternal": { + "Description": "ELB DNSName", + "Value": {"Fn::GetAtt": ["ELBtestdevexternal", "DNSName"]} + }, + "ELBtestdevinternal": { + "Description": "ELB DNSName", + "Value": {"Fn::GetAtt": ["ELBtestdevinternal", "DNSName"]} } } config = ConfigParser(project_config.config, 'my-stack-name') @@ -619,7 +669,11 @@ def test_process(self): resource_names.sort() compare(resource_names, wanted) - wanted = ["StaticBucketName", "dbhost", "dbport"] + wanted = ["ELBtestdevexternal", + "ELBtestdevinternal", + "StaticBucketName", + "dbhost", + "dbport"] output_names = cfn_template['Outputs'].keys() output_names.sort() compare(wanted, output_names) @@ -675,7 +729,11 @@ def test_process_with_vpc_config(self): resource_names.sort() compare(resource_names, wanted) - wanted = ["StaticBucketName", "dbhost", "dbport"] + wanted = ["ELBtestdevexternal", + "ELBtestdevinternal", + "StaticBucketName", + "dbhost", + "dbport"] output_names = cfn_template['Outputs'].keys() output_names.sort() compare(wanted, output_names) @@ -726,7 +784,7 @@ def test_elb_missing_cert(self): }] config = ConfigParser(project_config.config, 'my-stack-name') with self.assertRaises(errors.CfnConfigError): - config.elb() + config.elb(Template()) def test_elb_missing_cert_name(self): @@ -750,7 +808,7 @@ def test_elb_missing_cert_name(self): }] config = ConfigParser(project_config.config, 'my-stack-name') with self.assertRaises(errors.CfnConfigError): - config.elb() + config.elb(Template()) def test_elb_with_ssl(self): @@ -786,7 +844,6 @@ def test_elb_with_ssl(self): "LoadBalancerPort": 443, "Protocol": "HTTPS", "PolicyNames": ["PinDownSSLNegotiationPolicy201505"]}], SecurityGroups=[Ref("DefaultSGdockerregistryservice")], - LoadBalancerName="ELB-docker-registryservice", Scheme="internet-facing", Policies=[ Policy( @@ -811,7 +868,8 @@ def test_elb_with_ssl(self): Join("", ["arn:aws:elasticloadbalancing:", Ref("AWS::Region"), ":", Ref("AWS::AccountId"), - ":loadbalancer/ELB-docker-registryservice" + ":loadbalancer/", + {"Ref": "ELBdockerregistryservice"} ] ) ], @@ -821,8 +879,22 @@ def test_elb_with_ssl(self): }, Roles=[Ref("BaseHostRole")], ) + DefaultSGdockerregistryservice = SecurityGroup( + "DefaultSGdockerregistryservice", + GroupDescription="DefaultELBSecurityGroup", + SecurityGroupIngress=[{"CidrIp": "0.0.0.0/0", + "FromPort": 443, + "IpProtocol": "tcp", + "ToPort": 443}, + {"CidrIp": "0.0.0.0/0", + "FromPort": 80, + "IpProtocol": "tcp", + "ToPort": 80} + ], + VpcId=Ref("VPC") + ) known = [DNSdockerregistryservice, ELBdockerregistryservice, - Policydockerregistryservice] + Policydockerregistryservice, DefaultSGdockerregistryservice] project_config = ProjectConfig('tests/sample-project.yaml', 'dev') # Ugh. Fixtures please? @@ -843,7 +915,12 @@ def test_elb_with_ssl(self): ], }] config = ConfigParser(project_config.config, 'my-stack-name') - elb_cfg, _ = config.elb() + + # Create elb resources with template + template = Template() + config.elb(template) + elb_cfg = template.resources.values() + # elb_dict = json.loads(json.dumps([{r.title: r} for r in elb_cfg ], # cls=awsencode)) compare(self._resources_to_dict(known), @@ -891,7 +968,6 @@ def test_elb_with_healthcheck(self): "Protocol": "TCP"} ], SecurityGroups=[Ref("DefaultSGdockerregistryservice")], - LoadBalancerName="ELB-docker-registryservice", Scheme="internet-facing", Policies=[ Policy( @@ -915,7 +991,8 @@ def test_elb_with_healthcheck(self): Join("", ["arn:aws:elasticloadbalancing:", Ref("AWS::Region"), ":", Ref("AWS::AccountId"), - ":loadbalancer/ELB-docker-registryservice"] + ":loadbalancer/", + {"Ref": "ELBdockerregistryservice"}] )], "Effect": "Allow" } @@ -924,6 +1001,21 @@ def test_elb_with_healthcheck(self): }, Roles=[Ref("BaseHostRole")], ) + DefaultSGdockerregistryservice = SecurityGroup( + "DefaultSGdockerregistryservice", + GroupDescription="DefaultELBSecurityGroup", + SecurityGroupIngress=[{"CidrIp": "0.0.0.0/0", + "FromPort": 443, + "IpProtocol": "tcp", + "ToPort": 443}, + {"CidrIp": "0.0.0.0/0", + "FromPort": 80, + "IpProtocol": "tcp", + "ToPort": 80} + ], + VpcId=Ref("VPC") + ) + project_config = ProjectConfig('tests/sample-project.yaml', 'dev') project_config.config['elb'] = [ { @@ -950,9 +1042,14 @@ def test_elb_with_healthcheck(self): } ] config = ConfigParser(project_config.config, 'my-stack-name') - elb_cfg, _ = config.elb() + + # Create elb resources with template + template = Template() + config.elb(template) + elb_cfg = template.resources.values() + known = [DNSdockerregistryservice, ELBdockerregistryservice, - Policydockerregistryservice] + Policydockerregistryservice, DefaultSGdockerregistryservice] compare(self._resources_to_dict(known), self._resources_to_dict(elb_cfg)) @@ -974,7 +1071,6 @@ def test_elb_with_reserved_chars(self): } ], SecurityGroups=[Ref("DefaultSGdevdockerregistryservice")], - LoadBalancerName="ELB-dev_docker-registryservice", Scheme="internet-facing", Policies=[ Policy( @@ -1018,7 +1114,8 @@ def test_elb_with_reserved_chars(self): ["arn:aws:elasticloadbalancing:", Ref("AWS::Region"), ":", Ref("AWS::AccountId"), - ":loadbalancer/ELB-dev_docker-registryservice"] + ":loadbalancer/", + {"Ref": "ELBdevdockerregistryservice"}] ) ], "Effect": "Allow" @@ -1027,6 +1124,20 @@ def test_elb_with_reserved_chars(self): }, Roles=[Ref("BaseHostRole")], ) + DefaultSGdevdockerregistryservice = SecurityGroup( + "DefaultSGdevdockerregistryservice", + GroupDescription="DefaultELBSecurityGroup", + SecurityGroupIngress=[{"CidrIp": "0.0.0.0/0", + "FromPort": 443, + "IpProtocol": "tcp", + "ToPort": 443}, + {"CidrIp": "0.0.0.0/0", + "FromPort": 80, + "IpProtocol": "tcp", + "ToPort": 80} + ], + VpcId=Ref("VPC") + ) project_config = ProjectConfig('tests/sample-project.yaml', 'dev') # Ugh. Fixtures please? @@ -1046,9 +1157,14 @@ def test_elb_with_reserved_chars(self): ], }] config = ConfigParser(project_config.config, 'my-stack-name') - elb_cfg, _ = config.elb() + + # Create elb resources with template + template = Template() + config.elb(template) + elb_cfg = template.resources.values() + known = [DNSdevdockerregistryservice, ELBdevdockerregistryservice, - Policydevdockerregistryservice] + Policydevdockerregistryservice, DefaultSGdevdockerregistryservice] compare(self._resources_to_dict(known), self._resources_to_dict(elb_cfg))