Skip to content
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.

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

fix(aws-elbv2): add dependency on internet gateway #1284

Merged
merged 3 commits into from
Dec 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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