Skip to content

Commit

Permalink
fix(aws-elbv2): add dependency on internet gateway (#1284)
Browse files Browse the repository at this point in the history
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).
* Add a script to run integration tests in parallel.
  • Loading branch information
rix0rrr authored Dec 5, 2018
1 parent 3a89e21 commit c298a7c
Show file tree
Hide file tree
Showing 22 changed files with 407 additions and 57 deletions.
23 changes: 22 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,27 @@ $ nodeunit test/test.*.js
<BOOM>
```

### 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
Expand Down Expand Up @@ -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:

Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-autoscaling/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ export class LazyDependable implements cdk.IDependable {
public get dependencyElements(): cdk.IDependable[] {
return this.dependableSource.dependencyElements;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@
},
"VPCPublicSubnet1DefaultRoute91CEF279": {
"Type": "AWS::EC2::Route",
"DependsOn": [
"VPCVPCGW99B986DC"
],
"Properties": {
"RouteTableId": {
"Ref": "VPCPublicSubnet1RouteTableFEE4B781"
Expand All @@ -78,7 +75,10 @@
"GatewayId": {
"Ref": "VPCIGWB7E252D3"
}
}
},
"DependsOn": [
"VPCVPCGW99B986DC"
]
},
"VPCPublicSubnet1EIP6AD938E8": {
"Type": "AWS::EC2::EIP",
Expand Down Expand Up @@ -158,9 +158,6 @@
},
"VPCPublicSubnet2DefaultRouteB7481BBA": {
"Type": "AWS::EC2::Route",
"DependsOn": [
"VPCVPCGW99B986DC"
],
"Properties": {
"RouteTableId": {
"Ref": "VPCPublicSubnet2RouteTable6F1A15F1"
Expand All @@ -169,7 +166,10 @@
"GatewayId": {
"Ref": "VPCIGWB7E252D3"
}
}
},
"DependsOn": [
"VPCVPCGW99B986DC"
]
},
"VPCPublicSubnet2EIP4947BC00": {
"Type": "AWS::EC2::EIP",
Expand Down Expand Up @@ -449,4 +449,4 @@
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,10 @@
"Ref": "VPCPublicSubnet3Subnet631C5E25"
}
]
}
},
"DependsOn": [
"VPCIGWB7E252D3"
]
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,10 @@
}
],
"Type": "application"
}
},
"DependsOn": [
"VPCIGWB7E252D3"
]
},
"LBSecurityGroup8A41EA2B": {
"Type": "AWS::EC2::SecurityGroup",
Expand Down
30 changes: 30 additions & 0 deletions packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<IDependable>();

/**
* Return the subnets appropriate for the placement strategy
*/
Expand Down Expand Up @@ -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);
}
}

/**
Expand Down Expand Up @@ -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;
}
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,10 @@
}
],
"Type": "application"
}
},
"DependsOn": [
"VpcIGWD7BA715C"
]
},
"LBSecurityGroup8A41EA2B": {
"Type": "AWS::EC2::SecurityGroup",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,10 @@
}
],
"Type": "application"
}
},
"DependsOn": [
"VpcIGWD7BA715C"
]
},
"LBSecurityGroup8A41EA2B": {
"Type": "AWS::EC2::SecurityGroup",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,10 @@
}
],
"Type": "application"
}
},
"DependsOn": [
"VpcIGWD7BA715C"
]
},
"FargateServiceLBSecurityGroup5F444C78": {
"Type": "AWS::EC2::SecurityGroup",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@
},
"VpcPublicSubnet1DefaultRoute3DA9E72A": {
"Type": "AWS::EC2::Route",
"DependsOn": [
"VpcVPCGWBF912B6E"
],
"Properties": {
"RouteTableId": {
"Ref": "VpcPublicSubnet1RouteTable6C95E38E"
Expand All @@ -78,7 +75,10 @@
"GatewayId": {
"Ref": "VpcIGWD7BA715C"
}
}
},
"DependsOn": [
"VpcVPCGWBF912B6E"
]
},
"VpcPublicSubnet1EIPD7E02669": {
"Type": "AWS::EC2::EIP",
Expand Down Expand Up @@ -158,9 +158,6 @@
},
"VpcPublicSubnet2DefaultRoute97F91067": {
"Type": "AWS::EC2::Route",
"DependsOn": [
"VpcVPCGWBF912B6E"
],
"Properties": {
"RouteTableId": {
"Ref": "VpcPublicSubnet2RouteTable94F7E489"
Expand All @@ -169,7 +166,10 @@
"GatewayId": {
"Ref": "VpcIGWD7BA715C"
}
}
},
"DependsOn": [
"VpcVPCGWBF912B6E"
]
},
"VpcPublicSubnet2EIP3C605A87": {
"Type": "AWS::EC2::EIP",
Expand Down Expand Up @@ -581,7 +581,10 @@
}
],
"Type": "application"
}
},
"DependsOn": [
"VpcIGWD7BA715C"
]
},
"LBSecurityGroup8A41EA2B": {
"Type": "AWS::EC2::SecurityGroup",
Expand Down Expand Up @@ -669,4 +672,4 @@
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@
},
"VPCPublicSubnet1DefaultRoute91CEF279": {
"Type": "AWS::EC2::Route",
"DependsOn": [
"VPCVPCGW99B986DC"
],
"Properties": {
"RouteTableId": {
"Ref": "VPCPublicSubnet1RouteTableFEE4B781"
Expand All @@ -78,7 +75,10 @@
"GatewayId": {
"Ref": "VPCIGWB7E252D3"
}
}
},
"DependsOn": [
"VPCVPCGW99B986DC"
]
},
"VPCPublicSubnet1EIP6AD938E8": {
"Type": "AWS::EC2::EIP",
Expand Down Expand Up @@ -249,7 +249,10 @@
"Ref": "VPCPublicSubnet1SubnetB4246D30"
}
]
}
},
"DependsOn": [
"VPCIGWB7E252D3"
]
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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'); }

Expand Down
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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();
Expand Down
Loading

0 comments on commit c298a7c

Please sign in to comment.