Skip to content

Commit

Permalink
Extend SubnetSelection
Browse files Browse the repository at this point in the history
BREAKING CHANGE: `SubnetSelection` now expects either a list of subnet types (`subnetTypes`) or a list of subnet names (`subnetNames`)
BREAKING CHANGE: `vpnRoutePropagation` now expects a `SubnetSelection`
BREAKING CHANGE: `vpcSubnets` in `ClusterProps` in `@aws-cdk/aws-eks` now expects a `SubnetSelection`
  • Loading branch information
jogold committed Mar 28, 2019
1 parent 3a3f896 commit c799be0
Show file tree
Hide file tree
Showing 15 changed files with 81 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
};

if (!props.vpc.isPublicSubnets(subnetIds) && props.associatePublicIpAddress) {
throw new Error("To set 'associatePublicIpAddress: true' you must select Public subnets (vpcSubnets: { subnetType: SubnetType.Public })");
throw new Error("To set 'associatePublicIpAddress: true' you must select Public subnets (vpcSubnets: { subnetTypes: [SubnetType.Public] })");
}

this.autoScalingGroup = new CfnAutoScalingGroup(this, 'ASG', asgProps);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ export = {
maxCapacity: 0,
desiredCapacity: 0,

vpcSubnets: { subnetType: ec2.SubnetType.Public },
vpcSubnets: { subnetTypes: [ec2.SubnetType.Public] },
associatePublicIpAddress: true,
});

Expand Down
17 changes: 4 additions & 13 deletions packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Connections, IConnectable } from './connections';
import { CfnVPCEndpoint } from './ec2.generated';
import { SecurityGroup } from './security-group';
import { VpcSubnet } from './vpc';
import { IVpcNetwork, IVpcSubnet, SubnetSelection } from './vpc-ref';
import { IVpcNetwork, SubnetSelection } from './vpc-ref';

/**
* A VPC endpoint.
Expand Down Expand Up @@ -142,7 +142,7 @@ export interface VpcGatewayEndpointOptions extends VpcEndpointOptions {
*
* @default private subnets
*/
readonly subnets?: SubnetSelection[]
readonly subnets?: SubnetSelection
}

/**
Expand Down Expand Up @@ -185,15 +185,7 @@ export class VpcGatewayEndpoint extends cdk.Construct implements IVpcGatewayEndp
throw new Error('The service endpoint type must be `Gateway`');
}

let subnets: IVpcSubnet[] = [];
if (props.subnets) {
for (const selection of props.subnets) {
subnets.push(...props.vpc.subnets(selection));
}
} else {
subnets = props.vpc.subnets();
}

const subnets = props.vpc.selectSubnets(props.subnets);
const routeTableIds = (subnets as VpcSubnet[]).map(subnet => subnet.routeTableId);

const endpoint = new CfnVPCEndpoint(this, 'Resource', {
Expand Down Expand Up @@ -362,8 +354,7 @@ export class VpcInterfaceEndpoint extends cdk.Construct implements IVpcInterface
this.securityGroupId = securityGroup.securityGroupId;
this.connections = new Connections({ securityGroups: [securityGroup] });

const subnets = props.vpc.subnets(props.subnets);

const subnets = props.vpc.selectSubnets(props.subnets);
const availabilityZones = new Set(subnets.map(s => s.availabilityZone));

if (availabilityZones.size !== subnets.length) {
Expand Down
65 changes: 38 additions & 27 deletions packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export interface IVpcNetwork extends IConstruct {
/**
* Return the subnets appropriate for the placement strategy
*/
subnets(selection?: SubnetSelection): IVpcSubnet[];
selectSubnets(selection?: SubnetSelection): IVpcSubnet[];

/**
* Return a dependable object representing internet connectivity for the given subnets
Expand Down Expand Up @@ -152,24 +152,24 @@ export enum SubnetType {
*/
export interface SubnetSelection {
/**
* Place the instances in the subnets of the given type
* Place the instances in the subnets of the given types
*
* At most one of `subnetType` and `subnetName` can be supplied.
*
* @default SubnetType.Private
*/
readonly subnetType?: SubnetType;
readonly subnetTypes?: SubnetType[];

/**
* Place the instances in the subnets with the given name
* Place the instances in the subnets with the given names
*
* (This is the name supplied in subnetConfiguration).
*
* At most one of `subnetType` and `subnetName` can be supplied.
*
* @default name
*/
readonly subnetName?: string;
readonly subnetNames?: string[];
}

/**
Expand Down Expand Up @@ -223,7 +223,7 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork {
public subnetIds(selection: SubnetSelection = {}): string[] {
selection = reifySelectionDefaults(selection);

const nets = this.subnets(selection);
const nets = this.selectSubnets(selection);
if (nets.length === 0) {
throw new Error(`There are no ${describeSelection(selection)} in this VPC. Use a different VPC subnet selection.`);
}
Expand All @@ -238,7 +238,7 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork {
selection = reifySelectionDefaults(selection);

const ret = new CompositeDependable();
for (const subnet of this.subnets(selection)) {
for (const subnet of this.selectSubnets(selection)) {
ret.add(subnet.internetConnectivityEstablished);
}
return ret;
Expand Down Expand Up @@ -287,27 +287,34 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork {
/**
* Return the subnets appropriate for the placement strategy
*/
public subnets(selection: SubnetSelection = {}): IVpcSubnet[] {
public selectSubnets(selection: SubnetSelection = {}): IVpcSubnet[] {
selection = reifySelectionDefaults(selection);

// Select by name
if (selection.subnetName !== undefined) {
const allSubnets = this.privateSubnets.concat(this.publicSubnets).concat(this.isolatedSubnets);
const selectedSubnets = allSubnets.filter(s => subnetName(s) === selection.subnetName);
if (selectedSubnets.length === 0) {
throw new Error(`No subnets with name: ${selection.subnetName}`);
if (selection.subnetNames !== undefined) {
const allSubnets = [...this.publicSubnets, ...this.privateSubnets, ...this.isolatedSubnets];
const names = selection.subnetNames;
const nameSubnets = allSubnets.filter(s => names.includes(subnetName(s)));
if (nameSubnets.length === 0) {
throw new Error(`No subnets with names in: ${selection.subnetNames}`);
}
return selectedSubnets;
return nameSubnets;
}

// Select by type
if (selection.subnetType === undefined) { return this.privateSubnets; }
if (selection.subnetTypes === undefined) { return this.privateSubnets; }

return {
[SubnetType.Isolated]: this.isolatedSubnets,
[SubnetType.Private]: this.privateSubnets,
[SubnetType.Public]: this.publicSubnets,
}[selection.subnetType];
let typeSubnets: IVpcSubnet[] = [];
if (selection.subnetTypes.includes(SubnetType.Public)) {
typeSubnets = [...typeSubnets, ...this.publicSubnets];
}
if (selection.subnetTypes.includes(SubnetType.Private)) {
typeSubnets = [...typeSubnets, ...this.privateSubnets];
}
if (selection.subnetTypes.includes(SubnetType.Isolated)) {
typeSubnets = [...typeSubnets, ...this.isolatedSubnets];
}
return typeSubnets;
}
}

Expand Down Expand Up @@ -392,12 +399,12 @@ export interface VpcSubnetImportProps {
* Returns "private subnets" by default.
*/
function reifySelectionDefaults(placement: SubnetSelection): SubnetSelection {
if (placement.subnetType !== undefined && placement.subnetName !== undefined) {
if (placement.subnetTypes !== undefined && placement.subnetNames !== undefined) {
throw new Error('Only one of subnetType and subnetName can be supplied');
}

if (placement.subnetType === undefined && placement.subnetName === undefined) {
return { subnetType: SubnetType.Private };
if (placement.subnetTypes === undefined && placement.subnetNames === undefined) {
return { subnetTypes: [SubnetType.Private] };
}

return placement;
Expand All @@ -407,15 +414,19 @@ function reifySelectionDefaults(placement: SubnetSelection): SubnetSelection {
* Describe the given placement strategy
*/
function describeSelection(placement: SubnetSelection): string {
if (placement.subnetType !== undefined) {
return `'${DEFAULT_SUBNET_NAME[placement.subnetType]}' subnets`;
if (placement.subnetTypes !== undefined) {
return `'${joinCommaOr(placement.subnetTypes.map(type => DEFAULT_SUBNET_NAME[type]))}' subnets`;
}
if (placement.subnetName !== undefined) {
return `subnets named '${placement.subnetName}'`;
if (placement.subnetNames !== undefined) {
return `subnets named '${joinCommaOr(placement.subnetNames)}'`;
}
return JSON.stringify(placement);
}

function joinCommaOr(array: string[]) {
return [array.slice(0, -1).join(', '), array.slice(-1)[0]].join(array.length < 2 ? '' : ' or ');
}

class CompositeDependable implements IDependable {
private readonly dependables = new Array<IDependable>();

Expand Down
20 changes: 5 additions & 15 deletions packages/@aws-cdk/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export interface VpcNetworkProps {
*
* @default on the route tables associated with private subnets
*/
readonly vpnRoutePropagation?: SubnetType[]
readonly vpnRoutePropagation?: SubnetSelection

/**
* Gateway endpoints to add to this VPC.
Expand Down Expand Up @@ -402,17 +402,7 @@ export class VpcNetwork extends VpcNetworkBase {
this.vpnGatewayId = vpnGateway.vpnGatewayName;

// Propagate routes on route tables associated with the right subnets
const vpnRoutePropagation = props.vpnRoutePropagation || [SubnetType.Private];
let subnets: IVpcSubnet[] = [];
if (vpnRoutePropagation.includes(SubnetType.Public)) {
subnets = [...subnets, ...this.publicSubnets];
}
if (vpnRoutePropagation.includes(SubnetType.Private)) {
subnets = [...subnets, ...this.privateSubnets];
}
if (vpnRoutePropagation.includes(SubnetType.Isolated)) {
subnets = [...subnets, ...this.isolatedSubnets];
}
const subnets = this.selectSubnets(props.vpnRoutePropagation);
const routePropagation = new CfnVPNGatewayRoutePropagation(this, 'RoutePropagation', {
routeTableIds: (subnets as VpcSubnet[]).map(subnet => subnet.routeTableId),
vpnGatewayId: this.vpnGatewayId
Expand Down Expand Up @@ -453,7 +443,7 @@ export class VpcNetwork extends VpcNetworkBase {
/**
* Adds a new S3 gateway endpoint to this VPC
*/
public addS3Endpoint(id: string, subnets?: SubnetSelection[]): VpcGatewayEndpoint {
public addS3Endpoint(id: string, subnets?: SubnetSelection): VpcGatewayEndpoint {
return new VpcGatewayEndpoint(this, id, {
service: VpcEndpointAwsService.S3,
vpc: this,
Expand All @@ -464,7 +454,7 @@ export class VpcNetwork extends VpcNetworkBase {
/**
* Adds a new DynamoDB gateway endpoint to this VPC
*/
public addDynamoDbEndpoint(id: string, subnets?: SubnetSelection[]): VpcGatewayEndpoint {
public addDynamoDbEndpoint(id: string, subnets?: SubnetSelection): VpcGatewayEndpoint {
return new VpcGatewayEndpoint(this, id, {
service: VpcEndpointAwsService.DynamoDb,
vpc: this,
Expand Down Expand Up @@ -502,7 +492,7 @@ export class VpcNetwork extends VpcNetworkBase {

let natSubnets: VpcPublicSubnet[];
if (placement) {
const subnets = this.subnets(placement);
const subnets = this.selectSubnets(placement);
for (const sub of subnets) {
if (this.publicSubnets.indexOf(sub) === -1) {
throw new Error(`natGatewayPlacement ${placement} contains non public subnet ${sub}`);
Expand Down
7 changes: 3 additions & 4 deletions packages/@aws-cdk/aws-ec2/test/test.vpc-endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,9 @@ export = {
gatewayEndpoints: {
S3: {
service: VpcEndpointAwsService.S3,
subnets: [
{ subnetType: SubnetType.Public },
{ subnetType: SubnetType.Private }
]
subnets: {
subnetTypes: [SubnetType.Public, SubnetType.Private]
}
}
}
});
Expand Down
29 changes: 15 additions & 14 deletions packages/@aws-cdk/aws-ec2/test/test.vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export = {
test.done();
},

"with custom subents, the VPC should have the right number of subnets, an IGW, and a NAT Gateway per AZ"(test: Test) {
"with custom subnets, the VPC should have the right number of subnets, an IGW, and a NAT Gateway per AZ"(test: Test) {
const stack = getTestStack();
const zones = new AvailabilityZoneProvider(stack).availabilityZones.length;
new VpcNetwork(stack, 'TheVPC', {
Expand Down Expand Up @@ -154,7 +154,7 @@ export = {
}
test.done();
},
"with custom subents and natGateways = 2 there should be only two NATGW"(test: Test) {
"with custom subnets and natGateways = 2 there should be only two NATGW"(test: Test) {
const stack = getTestStack();
new VpcNetwork(stack, 'TheVPC', {
cidr: '10.0.0.0/21',
Expand Down Expand Up @@ -290,7 +290,7 @@ export = {
},
],
natGatewaySubnets: {
subnetName: 'egress'
subnetNames: ['egress']
},
});
expect(stack).to(countResources("AWS::EC2::NatGateway", 3));
Expand Down Expand Up @@ -318,7 +318,7 @@ export = {
},
],
natGatewaySubnets: {
subnetName: 'notthere',
subnetNames: ['notthere'],
},
}));
test.done();
Expand Down Expand Up @@ -371,7 +371,9 @@ export = {
{ subnetType: SubnetType.Isolated, name: 'Isolated' },
],
vpnGateway: true,
vpnRoutePropagation: [SubnetType.Isolated]
vpnRoutePropagation: {
subnetTypes: [SubnetType.Isolated]
}
});

expect(stack).to(haveResource('AWS::EC2::VPNGatewayRoutePropagation', {
Expand Down Expand Up @@ -401,10 +403,9 @@ export = {
{ subnetType: SubnetType.Isolated, name: 'Isolated' },
],
vpnGateway: true,
vpnRoutePropagation: [
SubnetType.Private,
SubnetType.Isolated
]
vpnRoutePropagation: {
subnetTypes: [SubnetType.Private, SubnetType.Isolated]
}
});

expect(stack).to(haveResource('AWS::EC2::VPNGatewayRoutePropagation', {
Expand Down Expand Up @@ -546,7 +547,7 @@ export = {
const vpc = new VpcNetwork(stack, 'VPC');

// WHEN
const nets = vpc.subnetIds({ subnetType: SubnetType.Public });
const nets = vpc.subnetIds({ subnetTypes: [SubnetType.Public] });

// THEN
test.deepEqual(nets, vpc.publicSubnets.map(s => s.subnetId));
Expand All @@ -565,7 +566,7 @@ export = {
});

// WHEN
const nets = vpc.subnetIds({ subnetType: SubnetType.Isolated });
const nets = vpc.subnetIds({ subnetTypes: [SubnetType.Isolated] });

// THEN
test.deepEqual(nets, vpc.isolatedSubnets.map(s => s.subnetId));
Expand All @@ -584,7 +585,7 @@ export = {
});

// WHEN
const nets = vpc.subnetIds({ subnetName: 'DontTalkToMe' });
const nets = vpc.subnetIds({ subnetNames: ['DontTalkToMe'] });

// THEN
test.deepEqual(nets, vpc.privateSubnets.map(s => s.subnetId));
Expand Down Expand Up @@ -665,7 +666,7 @@ export = {
});

// WHEN
const nets = importedVpc.subnetIds({ subnetType: SubnetType.Isolated });
const nets = importedVpc.subnetIds({ subnetTypes: [SubnetType.Isolated] });

// THEN
test.equal(3, importedVpc.isolatedSubnets.length);
Expand All @@ -688,7 +689,7 @@ export = {
});

// WHEN
const nets = importedVpc.subnetIds({ subnetName: isolatedName });
const nets = importedVpc.subnetIds({ subnetNames: [isolatedName] });

// THEN
test.equal(3, importedVpc.isolatedSubnets.length);
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ export abstract class BaseService extends cdk.Construct
// tslint:disable-next-line:max-line-length
protected configureAwsVpcNetworking(vpc: ec2.IVpcNetwork, assignPublicIp?: boolean, vpcSubnets?: ec2.SubnetSelection, securityGroup?: ec2.ISecurityGroup) {
if (vpcSubnets === undefined) {
vpcSubnets = { subnetType: assignPublicIp ? ec2.SubnetType.Public : ec2.SubnetType.Private };
vpcSubnets = { subnetTypes: assignPublicIp ? [ec2.SubnetType.Public] : [ec2.SubnetType.Private] };
}
if (securityGroup === undefined) {
securityGroup = new ec2.SecurityGroup(this, 'SecurityGroup', { vpc });
Expand Down
Loading

0 comments on commit c799be0

Please sign in to comment.