From 9945113fa7cb54804aa8e6b1a70c3eda4df8872c Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 3 Dec 2018 16:39:28 +0100 Subject: [PATCH 1/3] fix(aws-elbv2): add dependency on internet gateway Internet-facing load balancers can only be created once the VPC's Internet Gateway has been created, otherwise creation is an error. Usually the gateway is speedy enough to create so that this lack of a dependency doesn't show, but to be correct we still need to add it. Fixes #1129. ALSO IN THIS CHANGE - Add a script to visualize dependencies in a CloudFormation template (by converting to .dot). --- packages/@aws-cdk/aws-autoscaling/lib/util.ts | 2 +- .../test/integ.amazonlinux2.expected.json | 18 +- ...g.asg-w-classic-loadbalancer.expected.json | 5 +- .../test/integ.asg-w-elbv2.expected.json | 5 +- packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts | 30 ++++ packages/@aws-cdk/aws-ec2/lib/vpc.ts | 1 + .../test/ec2/integ.lb-awsvpc-nw.expected.json | 5 +- .../test/ec2/integ.lb-bridge-nw.expected.json | 5 +- .../fargate/integ.asset-image.expected.json | 5 +- .../fargate/integ.lb-awsvpc-nw.expected.json | 23 +-- .../lib/load-balancer.ts | 3 + .../test/integ.elb.expected.json | 13 +- .../lib/shared/base-load-balancer.ts | 3 + .../test/alb/test.load-balancer.ts | 21 ++- .../test/integ.alb-alias-target.expected.json | 21 ++- .../test/integ.alb.expected.json | 5 +- .../test/integ.nlb.expected.json | 23 +-- scripts/template-deps-to-dot | 164 ++++++++++++++++++ 18 files changed, 301 insertions(+), 51 deletions(-) create mode 100755 scripts/template-deps-to-dot diff --git a/packages/@aws-cdk/aws-autoscaling/lib/util.ts b/packages/@aws-cdk/aws-autoscaling/lib/util.ts index 01301981713da..fc9f504660464 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/util.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/util.ts @@ -9,4 +9,4 @@ export class LazyDependable implements cdk.IDependable { public get dependencyElements(): cdk.IDependable[] { return this.dependableSource.dependencyElements; } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.amazonlinux2.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.amazonlinux2.expected.json index 45d5e9f6bac02..e81d3315737a5 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/integ.amazonlinux2.expected.json +++ b/packages/@aws-cdk/aws-autoscaling/test/integ.amazonlinux2.expected.json @@ -67,9 +67,6 @@ }, "VPCPublicSubnet1DefaultRoute91CEF279": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VPCVPCGW99B986DC" - ], "Properties": { "RouteTableId": { "Ref": "VPCPublicSubnet1RouteTableFEE4B781" @@ -78,7 +75,10 @@ "GatewayId": { "Ref": "VPCIGWB7E252D3" } - } + }, + "DependsOn": [ + "VPCVPCGW99B986DC" + ] }, "VPCPublicSubnet1EIP6AD938E8": { "Type": "AWS::EC2::EIP", @@ -158,9 +158,6 @@ }, "VPCPublicSubnet2DefaultRouteB7481BBA": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VPCVPCGW99B986DC" - ], "Properties": { "RouteTableId": { "Ref": "VPCPublicSubnet2RouteTable6F1A15F1" @@ -169,7 +166,10 @@ "GatewayId": { "Ref": "VPCIGWB7E252D3" } - } + }, + "DependsOn": [ + "VPCVPCGW99B986DC" + ] }, "VPCPublicSubnet2EIP4947BC00": { "Type": "AWS::EC2::EIP", @@ -449,4 +449,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json index 8a7a47474a4a1..7979814d99c10 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json +++ b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json @@ -708,7 +708,10 @@ "Ref": "VPCPublicSubnet3Subnet631C5E25" } ] - } + }, + "DependsOn": [ + "VPCIGWB7E252D3" + ] } } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json index a2f3320e35c4e..5088ec82a34c3 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json +++ b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json @@ -569,7 +569,10 @@ } ], "Type": "application" - } + }, + "DependsOn": [ + "VPCIGWB7E252D3" + ] }, "LBSecurityGroup8A41EA2B": { "Type": "AWS::EC2::SecurityGroup", diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts index 9833283d2c7a4..233754b08d879 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts @@ -119,6 +119,11 @@ export abstract class VpcNetworkRef extends Construct implements IDependable { */ public readonly dependencyElements: IDependable[] = []; + /** + * Dependencies for internet connectivity + */ + protected readonly internetDependencies = new Array(); + /** * Return the subnets appropriate for the placement strategy */ @@ -177,6 +182,19 @@ export abstract class VpcNetworkRef extends Construct implements IDependable { public isPublicSubnet(subnet: VpcSubnetRef) { return this.publicSubnets.indexOf(subnet) > -1; } + + /** + * Take a dependency on internet connectivity having been added to this VPC + * + * Take a dependency on this if your constructs need an Internet Gateway + * added to the VPC before they can be constructed. + * + * This method is for construct authors; application builders should not + * need to call this. + */ + public internetDependency(): IDependable { + return new DependencyList(this.internetDependencies); + } } /** @@ -340,3 +358,15 @@ export interface VpcSubnetRefProps { */ subnetId: string; } + +/** + * Allows using an array as a list of dependables. + */ +class DependencyList implements IDependable { + constructor(private readonly dependenclyElements: IDependable[]) { + } + + public get dependencyElements(): IDependable[] { + return this.dependenclyElements; + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index 0d3d236c8ccff..20bb8407210b8 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -312,6 +312,7 @@ export class VpcNetwork extends VpcNetworkRef implements cdk.ITaggable { const igw = new cloudformation.InternetGatewayResource(this, 'IGW', { tags: new cdk.TagManager(this), }); + this.internetDependencies.push(igw); const att = new cloudformation.VPCGatewayAttachmentResource(this, 'VPCGW', { internetGatewayId: igw.ref, vpcId: this.resource.ref diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json index 6db17a600e7e2..fc5ddba7d30ee 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json @@ -866,7 +866,10 @@ } ], "Type": "application" - } + }, + "DependsOn": [ + "VpcIGWD7BA715C" + ] }, "LBSecurityGroup8A41EA2B": { "Type": "AWS::EC2::SecurityGroup", diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json index 7b6ed2f8fae52..4e80a7b3cdb0f 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json @@ -829,7 +829,10 @@ } ], "Type": "application" - } + }, + "DependsOn": [ + "VpcIGWD7BA715C" + ] }, "LBSecurityGroup8A41EA2B": { "Type": "AWS::EC2::SecurityGroup", diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/integ.asset-image.expected.json b/packages/@aws-cdk/aws-ecs/test/fargate/integ.asset-image.expected.json index 20d71e997cd3d..dccd63ec329bb 100644 --- a/packages/@aws-cdk/aws-ecs/test/fargate/integ.asset-image.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/fargate/integ.asset-image.expected.json @@ -888,7 +888,10 @@ } ], "Type": "application" - } + }, + "DependsOn": [ + "VpcIGWD7BA715C" + ] }, "FargateServiceLBSecurityGroup5F444C78": { "Type": "AWS::EC2::SecurityGroup", diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/integ.lb-awsvpc-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/fargate/integ.lb-awsvpc-nw.expected.json index 690bee1da284b..5cdc9fd00b4bf 100644 --- a/packages/@aws-cdk/aws-ecs/test/fargate/integ.lb-awsvpc-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/fargate/integ.lb-awsvpc-nw.expected.json @@ -67,9 +67,6 @@ }, "VpcPublicSubnet1DefaultRoute3DA9E72A": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VpcVPCGWBF912B6E" - ], "Properties": { "RouteTableId": { "Ref": "VpcPublicSubnet1RouteTable6C95E38E" @@ -78,7 +75,10 @@ "GatewayId": { "Ref": "VpcIGWD7BA715C" } - } + }, + "DependsOn": [ + "VpcVPCGWBF912B6E" + ] }, "VpcPublicSubnet1EIPD7E02669": { "Type": "AWS::EC2::EIP", @@ -158,9 +158,6 @@ }, "VpcPublicSubnet2DefaultRoute97F91067": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VpcVPCGWBF912B6E" - ], "Properties": { "RouteTableId": { "Ref": "VpcPublicSubnet2RouteTable94F7E489" @@ -169,7 +166,10 @@ "GatewayId": { "Ref": "VpcIGWD7BA715C" } - } + }, + "DependsOn": [ + "VpcVPCGWBF912B6E" + ] }, "VpcPublicSubnet2EIP3C605A87": { "Type": "AWS::EC2::EIP", @@ -581,7 +581,10 @@ } ], "Type": "application" - } + }, + "DependsOn": [ + "VpcIGWD7BA715C" + ] }, "LBSecurityGroup8A41EA2B": { "Type": "AWS::EC2::SecurityGroup", @@ -669,4 +672,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts index 3fc0f35b46c70..75ba6921676e9 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts @@ -220,6 +220,9 @@ export class LoadBalancer extends cdk.Construct implements IConnectable, codedep scheme: props.internetFacing ? 'internet-facing' : 'internal', healthCheck: props.healthCheck && healthCheckToJSON(props.healthCheck), }); + if (props.internetFacing) { + this.elb.addDependency(props.vpc.internetDependency()); + } ifUndefined(props.listeners, []).forEach(b => this.addListener(b)); ifUndefined(props.targets, []).forEach(t => this.addTarget(t)); diff --git a/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json b/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json index c61fcd74337c6..38baffc69786a 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json @@ -67,9 +67,6 @@ }, "VPCPublicSubnet1DefaultRoute91CEF279": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VPCVPCGW99B986DC" - ], "Properties": { "RouteTableId": { "Ref": "VPCPublicSubnet1RouteTableFEE4B781" @@ -78,7 +75,10 @@ "GatewayId": { "Ref": "VPCIGWB7E252D3" } - } + }, + "DependsOn": [ + "VPCVPCGW99B986DC" + ] }, "VPCPublicSubnet1EIP6AD938E8": { "Type": "AWS::EC2::EIP", @@ -249,7 +249,10 @@ "Ref": "VPCPublicSubnet1SubnetB4246D30" } ] - } + }, + "DependsOn": [ + "VPCIGWB7E252D3" + ] } } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts index 71263d80b4813..1a43a4cc7fd95 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts @@ -110,6 +110,9 @@ export abstract class BaseLoadBalancer extends cdk.Construct implements route53. loadBalancerAttributes: new cdk.Token(() => renderAttributes(this.attributes)), ...additionalProps }); + if (internetFacing) { + resource.addDependency(this.vpc.internetDependency()); + } if (baseProps.deletionProtection) { this.setAttribute('deletion_protection.enabled', 'true'); } diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.load-balancer.ts index cbad96ac9e4af..7ee8070cbf444 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.load-balancer.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.load-balancer.ts @@ -1,4 +1,4 @@ -import { expect, haveResource } from '@aws-cdk/assert'; +import { expect, haveResource, ResourcePart } from '@aws-cdk/assert'; import ec2 = require('@aws-cdk/aws-ec2'); import s3 = require('@aws-cdk/aws-s3'); import cdk = require('@aws-cdk/cdk'); @@ -31,6 +31,25 @@ export = { test.done(); }, + 'internet facing load balancer has dependency on IGW'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.VpcNetwork(stack, 'Stack'); + + // WHEN + new elbv2.ApplicationLoadBalancer(stack, 'LB', { + vpc, + internetFacing: true, + }); + + // THEN + expect(stack).to(haveResource('AWS::ElasticLoadBalancingV2::LoadBalancer', { + DependsOn: ['StackIGW2F0A1126'] + }, ResourcePart.CompleteDefinition)); + + test.done(); + }, + 'Trivial construction: internal'(test: Test) { // GIVEN const stack = new cdk.Stack(); diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb-alias-target.expected.json b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb-alias-target.expected.json index 2a2d938388c3f..9e233a1989206 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb-alias-target.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb-alias-target.expected.json @@ -67,9 +67,6 @@ }, "VPCPublicSubnet1DefaultRoute91CEF279": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VPCVPCGW99B986DC" - ], "Properties": { "RouteTableId": { "Ref": "VPCPublicSubnet1RouteTableFEE4B781" @@ -78,7 +75,10 @@ "GatewayId": { "Ref": "VPCIGWB7E252D3" } - } + }, + "DependsOn": [ + "VPCVPCGW99B986DC" + ] }, "VPCPublicSubnet1EIP6AD938E8": { "Type": "AWS::EC2::EIP", @@ -158,9 +158,6 @@ }, "VPCPublicSubnet2DefaultRouteB7481BBA": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VPCVPCGW99B986DC" - ], "Properties": { "RouteTableId": { "Ref": "VPCPublicSubnet2RouteTable6F1A15F1" @@ -169,7 +166,10 @@ "GatewayId": { "Ref": "VPCIGWB7E252D3" } - } + }, + "DependsOn": [ + "VPCVPCGW99B986DC" + ] }, "VPCPublicSubnet2EIP4947BC00": { "Type": "AWS::EC2::EIP", @@ -365,7 +365,10 @@ } ], "Type": "application" - } + }, + "DependsOn": [ + "VPCIGWB7E252D3" + ] }, "LBSecurityGroup8A41EA2B": { "Type": "AWS::EC2::SecurityGroup", diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json index 6aa99e1919368..daad87f5eee6f 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json @@ -365,7 +365,10 @@ } ], "Type": "application" - } + }, + "DependsOn": [ + "VPCIGWB7E252D3" + ] }, "LBSecurityGroup8A41EA2B": { "Type": "AWS::EC2::SecurityGroup", diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.nlb.expected.json b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.nlb.expected.json index 38a08693cf2fc..0826aa107b84d 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.nlb.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.nlb.expected.json @@ -67,9 +67,6 @@ }, "VPCPublicSubnet1DefaultRoute91CEF279": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VPCVPCGW99B986DC" - ], "Properties": { "RouteTableId": { "Ref": "VPCPublicSubnet1RouteTableFEE4B781" @@ -78,7 +75,10 @@ "GatewayId": { "Ref": "VPCIGWB7E252D3" } - } + }, + "DependsOn": [ + "VPCVPCGW99B986DC" + ] }, "VPCPublicSubnet1EIP6AD938E8": { "Type": "AWS::EC2::EIP", @@ -158,9 +158,6 @@ }, "VPCPublicSubnet2DefaultRouteB7481BBA": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VPCVPCGW99B986DC" - ], "Properties": { "RouteTableId": { "Ref": "VPCPublicSubnet2RouteTable6F1A15F1" @@ -169,7 +166,10 @@ "GatewayId": { "Ref": "VPCIGWB7E252D3" } - } + }, + "DependsOn": [ + "VPCVPCGW99B986DC" + ] }, "VPCPublicSubnet2EIP4947BC00": { "Type": "AWS::EC2::EIP", @@ -357,7 +357,10 @@ } ], "Type": "network" - } + }, + "DependsOn": [ + "VPCIGWB7E252D3" + ] }, "LBListener49E825B4": { "Type": "AWS::ElasticLoadBalancingV2::Listener", @@ -400,4 +403,4 @@ ] } } -} +} \ No newline at end of file diff --git a/scripts/template-deps-to-dot b/scripts/template-deps-to-dot new file mode 100755 index 0000000000000..49a350fe2042e --- /dev/null +++ b/scripts/template-deps-to-dot @@ -0,0 +1,164 @@ +#!/usr/bin/env python3 +"""A script to analyze the dependency relations in a CloudFormation template. + +Use like so: + + cdk -a integ.something.js synth | ./template-deps-to-dot | dot -Tpng > deps.png +""" +import collections +import fileinput +import sys +try: + import yaml +except ImportError: + print("Please run 'pip3 install pyyaml'") + sys.exit(1) + + +def main(): + args = sys.argv[1:] + if args: + files = [open(filename) for filename in args] + else: + files = [sys.stdin] + + templates = [yaml.safe_load(f) for f in files] + + graph = DepGraph() + + for template in templates: + if not template: + sys.stderr.write('Input does not look like a CloudFormation template.\n') + continue + parse_template(template, graph) + + graph.render_dot(sys.stdout) + + +def parse_template(template, graph): + """Parse template and add all encountered dependencies to the graph.""" + + for logical_id, resource_spec in template.get('Resources', {}).items(): + resource_type = resource_spec.get('Type', 'AWS::???::???') + path = resource_spec.get('Metadata', {}).get('aws:cdk:path', None) + if path: + path = resource_name_from_path(path) + + source = '%s\n(%s)' % (path or logical_id, resource_type) + + graph.annotate(logical_id, source) + + for dep in find_property_references(resource_spec.get('Properties', {})): + if not dep.target.startswith('AWS::'): + graph.add(logical_id, dep) + + for depends_on in resource_spec.get('DependsOn', []): + graph.add(logical_id, Dep(depends_on, 'DependsOn')) + + +def resource_name_from_path(path): + return '/'.join([p for p in path.split('/') if p != 'Resource'][1:]) + + +def find_property_references(properties): + """Find references in a resource's Properties. + + Returns: + list of Dep objects + """ + ret = [] + + def recurse(prop_name, obj): + if isinstance(obj, list): + for x in obj: + recurse(prop_name, x) + + if isinstance(obj, dict): + ref = parse_reference(obj) + if ref: + ret.append(Dep(ref[0], prop_name)) + return + + for key, value in obj.items(): + recurse(prop_name, value) + + for prop_name, prop in properties.items(): + recurse(prop_name, prop) + return ret + + +class DepGraph: + def __init__(self): + # { source -> [ { dependency, label } ] + self.graph = collections.defaultdict(set) + self.annotations = {} + self.has_incoming = set([]) + + def annotate(self, node, annotation): + self.annotations[node] = annotation + + def add(self, source, dep): + self.graph[source].add(dep) + self.has_incoming.add(dep.target) + + def render_dot(self, f): + """Render a dot version of this graph to the given stream.""" + f.write('digraph G {\n') + f.write(' rankdir=LR;\n') + f.write(' node [shape=box];\n') + + for node, annotation in self.annotations.items(): + if node in self.graph or node in self.has_incoming: + f.write(' %s [label=%s];\n'% (dot_escape(node), fancy_label(annotation))) + + for source, deps in self.graph.items(): + for dep in deps: + f.write(' %s -> %s [label=%s];\n' % (dot_escape(source), dot_escape(dep.target), dot_escape(dep.label))) + + f.write('}\n') + + +def fancy_label(s): + lines = s.split('\n') + return ('<' + lines[0] + '' + + ''.join('
' + line + '' for line in lines[1:]) + + ' >') + + +def dot_escape(s): + return '"' + s.replace('\n', '\\n') + '"' + + +def parse_reference(obj): + """If this object is an intrinsic reference, return info about it. + + Returns: (logicalId, reference) if reference, None otherwise + + """ + keys = list(obj.keys()) + if keys == ['Ref']: + return (obj[keys[0]], 'Ref') + if keys == ['Fn::GetAtt']: + return (obj[keys[0]][0], obj[keys[0]][1]) + return None + + + +class Dep: + def __init__(self, target, label): + self.target = target + self.label = label + + def __eq__(self, rhs): + return isinstance(rhs, Dep) and self.target == rhs.target and self.label == rhs.label + + def __ne__(self, rhs): + return not (self == rhs) + + def __hash__(self): + return hash((self.target, self.label)) + + + +if __name__ == '__main__': + main() From 89e28066b024a865fb1d58794649c3aeb96a6e05 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 19 Nov 2018 13:16:45 +0100 Subject: [PATCH 2/3] build: add a script to run integration tests in parallel --- scripts/run-integ-parallel | 65 ++++++++++++++++++++++ tools/cdk-integ-tools/bin/cdk-integ.ts | 8 ++- tools/cdk-integ-tools/lib/integ-helpers.ts | 16 ++++-- 3 files changed, 84 insertions(+), 5 deletions(-) create mode 100755 scripts/run-integ-parallel diff --git a/scripts/run-integ-parallel b/scripts/run-integ-parallel new file mode 100755 index 0000000000000..dd72c4ecc0277 --- /dev/null +++ b/scripts/run-integ-parallel @@ -0,0 +1,65 @@ +#!/bin/bash +# Run unit tests in parallel in different regions +set -euo pipefail +lerna=${LERNA:-node_modules/.bin/lerna} + +# Only regions that support most services +regions=(eu-west-1 eu-west-2 us-east-1 us-east-2 us-west-1 us-west-2 eu-central-1 ca-central-1) + +if ! which parallel > /dev/null; then + echo "Please install GNU Parallel." >&2 + exit 1 +fi + +parallel --help > /dev/null || if [[ $? -ne 255 ]]; then + echo "You do have a program called 'parallel' on your machine, but it's not GNU Parallel." >&2 + echo "It might come from 'moreutils'. Please install the proper GNU Parallel tool." >&2 + exit 1 +fi + +if [[ "${1:-}" == "" ]]; then + echo "Usage: run-integ-parallel PACKAGE [PACKAGE [..]]" >&2 + exit 1 +fi + +echo "Discovering tests..." >&2 +# Zipped arrays +packages=() +tests=() +while [[ "${1:-}" != "" ]]; do + # Yes, -- (end lerna args) -- (end npm args) --list (argument to cdk-integ) + for tst in $(lerna --loglevel=silent --scope $1 run integ -- --silent -- --list); do + packages+=("$1") + tests+=("$tst") + done + + shift +done + + +# CRAZY CRAZY CRAZY gnu parallel command line +# +# Here's the main shape: +# +# parallel --xapply ::: ::: ::: +# +# Runs the command with pairs of " as arguments, +# cycling through regions as necessary (packages and tests will have the same length) +# (--xapply is also sometimes called --link but --xapply is supported on more GNUP versions) +# +# The command that is run for each pair of arguments is: +# +# parallel --semaphore --fg -j1 --id +# +# To make sure that there is only -j1 job for every --id semaphore at +# a time, --fg to see the command to completion. We use the ({3}) as a semaphore, +# to make sure only one test is ever running on a single region. +# +# Mix in some -u to avoid buffering output, --delay to avoid tests trashing each other's cdk.json. +parallel --xapply -j0 -u --delay 3 \ + parallel \ + --fg --semaphore -j1 --id {3} -u \ + "echo Running {1}:{2} in {3}; env AWS_REGION={3} ${lerna} --loglevel=silent --scope {1} run integ -- --silent -- {2} || { echo Running {1}:{2} in {3} failed; exit 1; }" \ + ::: "${packages[@]}" \ + ::: "${tests[@]}" \ + ::: "${regions[@]}" diff --git a/tools/cdk-integ-tools/bin/cdk-integ.ts b/tools/cdk-integ-tools/bin/cdk-integ.ts index 1398c0e6f2bce..c0189c8e5804a 100644 --- a/tools/cdk-integ-tools/bin/cdk-integ.ts +++ b/tools/cdk-integ-tools/bin/cdk-integ.ts @@ -8,12 +8,18 @@ import { IntegrationTests, STATIC_TEST_CONTEXT } from '../lib/integ-helpers'; async function main() { const argv = yargs .usage('Usage: cdk-integ [TEST...]') - .option('clean', { type: 'boolean', default: true, desc: 'Skipps stack clean up after test is completed (use --no-clean to negate)' }) + .option('list', { type: 'boolean', default: false, desc: 'List tests instead of running them' }) + .option('clean', { type: 'boolean', default: true, desc: 'Skips stack clean up after test is completed (use --no-clean to negate)' }) .option('verbose', { type: 'boolean', default: false, alias: 'v', desc: 'Verbose logs' }) .argv; const tests = await new IntegrationTests('test').fromCliArgs(argv._); + if (argv.list) { + process.stdout.write(tests.map(t => t.name).join(' ') + '\n'); + return; + } + for (const test of tests) { console.error(`Trying to deploy ${test.name}`); diff --git a/tools/cdk-integ-tools/lib/integ-helpers.ts b/tools/cdk-integ-tools/lib/integ-helpers.ts index cb20c87655d0d..741d5cc253985 100644 --- a/tools/cdk-integ-tools/lib/integ-helpers.ts +++ b/tools/cdk-integ-tools/lib/integ-helpers.ts @@ -12,12 +12,20 @@ export class IntegrationTests { constructor(private readonly directory: string) { } - public fromCliArgs(tests?: string[]): Promise { + public async fromCliArgs(tests?: string[]): Promise { + let allTests = await this.discover(); + if (tests && tests.length > 0) { - return this.request(tests); - } else { - return this.discover(); + // Pare down found tests to filter + allTests = allTests.filter(t => tests.includes(t.name)); + + const selectedNames = allTests.map(t => t.name); + for (const unmatched of tests.filter(t => !selectedNames.includes(t))) { + process.stderr.write(`No such integ test: ${unmatched}\n`); + } } + + return allTests; } public async discover(): Promise { From bea04f763aa3527d1662830cd0f641ec4362ce32 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 5 Dec 2018 10:09:13 +0100 Subject: [PATCH 3/3] Mention new tools in CONTRIBUTING guide --- CONTRIBUTING.md | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 489c81e4e2611..5561bf3a76157 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -169,6 +169,27 @@ $ nodeunit test/test.*.js ``` +### Running integration tests in parallel + +Integration tests may take a long time to complete. We can speed this up by running them in parallel +in different regions. + +```shell +# Install GNU parallel (may require uninstall 'moreutils' if you have it) +$ apt-get install parallel +$ brew install parallel + +$ scripts/run-integ-parallel @aws-cdk/aws-ec2 @aws-cdk/aws-autoscaling ... +``` + +### Visualizing dependencies in a CloudFormation Template + +Use GraphViz with `template-deps-to-dot`: + +```shell +$ cdk -a some.app.js synth | $awscdk/scripts/template-deps-to-dot | dot -Tpng > deps.png +``` + ### Build Documentation Only The CDK documentation source is hosted under [`./docs/src`](./docs/src). Module reference documentation is gathered @@ -200,7 +221,7 @@ Guidelines: updates to automatically be picked up. * Make sure `package-lock.json` files are included in your commit. -### Finding Dependency Cycles +### Finding dependency cycles between packages You can use `find-cycles` to print a list of internal dependency cycles: