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(ec2): descriptive error message when selecting 0 subnets #2025

Merged
merged 4 commits into from
Mar 21, 2019
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
5 changes: 2 additions & 3 deletions packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export interface CommonAutoScalingGroupProps {
/**
* Where to place instances within the VPC
*/
vpcPlacement?: ec2.VpcPlacementStrategy;
vpcSubnets?: ec2.SubnetSelection;

/**
* SNS topic to send notifications about fleet changes
Expand Down Expand Up @@ -291,8 +291,7 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
});
}

const subnets = props.vpc.subnets(props.vpcPlacement);
asgProps.vpcZoneIdentifier = subnets.map(n => n.subnetId);
asgProps.vpcZoneIdentifier = props.vpc.subnetIds(props.vpcSubnets);

this.autoScalingGroup = new CfnAutoScalingGroup(this, 'ASG', asgProps);
this.osType = machineImage.os.type;
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ec2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ instances for your project.

Our default `VpcNetwork` class creates a private and public subnet for every
availability zone. Classes that use the VPC will generally launch instances
into all private subnets, and provide a parameter called `vpcPlacement` to
into all private subnets, and provide a parameter called `vpcSubnets` to
allow you to override the placement. [Read more about
subnets](https://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_Subnets.html).

Expand Down
144 changes: 116 additions & 28 deletions packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Construct, IConstruct, IDependable } from "@aws-cdk/cdk";
import { subnetName } from './util';
import { DEFAULT_SUBNET_NAME, subnetName } from './util';
import { VpnConnection, VpnConnectionOptions } from './vpn';

export interface IVpcSubnet extends IConstruct {
Expand Down Expand Up @@ -61,9 +61,20 @@ export interface IVpcNetwork extends IConstruct {
readonly vpnGatewayId?: string;

/**
* Return the subnets appropriate for the placement strategy
* Return IDs of the subnets appropriate for the given selection strategy
*
* Requires that at least once subnet is matched, throws a descriptive
* error message otherwise.
*
* Prefer to use this method over {@link subnets} if you need to pass subnet
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
* IDs to a CloudFormation Resource.
*/
subnets(placement?: VpcPlacementStrategy): IVpcSubnet[];
subnetIds(selection?: SubnetSelection): string[];

/**
* Return a dependable object representing internet connectivity for the given subnets
*/
subnetInternetDependencies(selection?: SubnetSelection): IDependable;

/**
* Return whether the given subnet is one of this VPC's public subnets.
Expand Down Expand Up @@ -125,29 +136,29 @@ export enum SubnetType {
}

/**
* Customize how instances are placed inside a VPC
* Customize subnets that are selected for placement of ENIs
*
* Constructs that allow customization of VPC placement use parameters of this
* type to provide placement settings.
*
* By default, the instances are placed in the private subnets.
*/
export interface VpcPlacementStrategy {
export interface SubnetSelection {
/**
* Place the instances in the subnets of the given type
*
* At most one of `subnetsToUse` and `subnetName` can be supplied.
* At most one of `subnetType` and `subnetName` can be supplied.
*
* @default SubnetType.Private
*/
subnetsToUse?: SubnetType;
subnetType?: SubnetType;

/**
* Place the instances in the subnets with the given name
*
* (This is the name supplied in subnetConfiguration).
*
* At most one of `subnetsToUse` and `subnetName` can be supplied.
* At most one of `subnetType` and `subnetName` can be supplied.
*
* @default name
*/
Expand Down Expand Up @@ -200,31 +211,30 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork {
public readonly natDependencies = new Array<IConstruct>();

/**
* Return the subnets appropriate for the placement strategy
* Returns IDs of selected subnets
*/
public subnets(placement: VpcPlacementStrategy = {}): IVpcSubnet[] {
if (placement.subnetsToUse !== undefined && placement.subnetName !== undefined) {
throw new Error('At most one of subnetsToUse and subnetName can be supplied');
}
public subnetIds(selection: SubnetSelection = {}): string[] {
selection = reifySelectionDefaults(selection);

// Select by name
if (placement.subnetName !== undefined) {
const allSubnets = this.privateSubnets.concat(this.publicSubnets).concat(this.isolatedSubnets);
const selectedSubnets = allSubnets.filter(s => subnetName(s) === placement.subnetName);
if (selectedSubnets.length === 0) {
throw new Error(`No subnets with name: ${placement.subnetName}`);
}
return selectedSubnets;
const nets = this.subnets(selection);
if (nets.length === 0) {
throw new Error(`There are no ${describeSelection(selection)} in this VPC. Use a different VPC subnet selection.`);
}

// Select by type
if (placement.subnetsToUse === undefined) { return this.privateSubnets; }
return nets.map(n => n.subnetId);
}

return {
[SubnetType.Isolated]: this.isolatedSubnets,
[SubnetType.Private]: this.privateSubnets,
[SubnetType.Public]: this.publicSubnets,
}[placement.subnetsToUse];
/**
* Return a dependable object representing internet connectivity for the given subnets
*/
public subnetInternetDependencies(selection: SubnetSelection = {}): IDependable {
selection = reifySelectionDefaults(selection);

const ret = new CompositeDependable();
for (const subnet of this.subnets(selection)) {
ret.add(subnet.internetConnectivityEstablished);
}
return ret;
}

/**
Expand Down Expand Up @@ -260,6 +270,31 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork {
return this.node.stack.region;
}

/**
* Return the subnets appropriate for the placement strategy
*/
protected subnets(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}`);
}
return selectedSubnets;
}

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

return {
[SubnetType.Isolated]: this.isolatedSubnets,
[SubnetType.Private]: this.privateSubnets,
[SubnetType.Public]: this.publicSubnets,
}[selection.subnetType];
}
}

/**
Expand Down Expand Up @@ -335,3 +370,56 @@ export interface VpcSubnetImportProps {
*/
subnetId: string;
}

/**
* If the placement strategy is completely "default", reify the defaults so
* consuming code doesn't have to reimplement the same analysis every time.
*
* Returns "private subnets" by default.
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
*/
function reifySelectionDefaults(placement: SubnetSelection): SubnetSelection {
if (placement.subnetType !== undefined && placement.subnetName !== undefined) {
throw new Error('Only one of subnetType and subnetName can be supplied');
}

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

return placement;
}

/**
* Describe the given placement strategy
*/
function describeSelection(placement: SubnetSelection): string {
if (placement.subnetType !== undefined) {
return `'${DEFAULT_SUBNET_NAME[placement.subnetType]}' subnets`;
}
if (placement.subnetName !== undefined) {
return `subnets named '${placement.subnetName}'`;
}
return JSON.stringify(placement);
}

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

/**
* Add a construct to the dependency roots
*/
public add(dep: IDependable) {
this.dependables.push(dep);
}

/**
* Retrieve the current set of dependency roots
*/
public get dependencyRoots(): IConstruct[] {
const ret = [];
for (const dep of this.dependables) {
ret.push(...dep.dependencyRoots);
}
return ret;
}
}
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { CfnRouteTable, CfnSubnet, CfnSubnetRouteTableAssociation, CfnVPC, CfnVP
import { NetworkBuilder } from './network-util';
import { DEFAULT_SUBNET_NAME, ExportSubnetGroup, ImportSubnetGroup, subnetId } from './util';
import { VpcNetworkProvider, VpcNetworkProviderProps } from './vpc-network-provider';
import { IVpcNetwork, IVpcSubnet, SubnetType, VpcNetworkBase, VpcNetworkImportProps, VpcPlacementStrategy, VpcSubnetImportProps } from './vpc-ref';
import { IVpcNetwork, IVpcSubnet, SubnetSelection, SubnetType, VpcNetworkBase, VpcNetworkImportProps, VpcSubnetImportProps } from './vpc-ref';
import { VpnConnectionOptions, VpnConnectionType } from './vpn';

/**
Expand Down Expand Up @@ -80,7 +80,7 @@ export interface VpcNetworkProps {
*
* @default All public subnets
*/
natGatewayPlacement?: VpcPlacementStrategy;
natGatewaySubnets?: SubnetSelection;

/**
* Configure the subnets to build for each AZ
Expand Down Expand Up @@ -365,7 +365,7 @@ export class VpcNetwork extends VpcNetworkBase {
});

// if gateways are needed create them
this.createNatGateways(props.natGateways, props.natGatewayPlacement);
this.createNatGateways(props.natGateways, props.natGatewaySubnets);

(this.privateSubnets as VpcPrivateSubnet[]).forEach((privateSubnet, i) => {
let ngwId = this.natGatewayByAZ[privateSubnet.availabilityZone];
Expand Down Expand Up @@ -445,7 +445,7 @@ export class VpcNetwork extends VpcNetworkBase {
};
}

private createNatGateways(gateways?: number, placement?: VpcPlacementStrategy): void {
private createNatGateways(gateways?: number, placement?: SubnetSelection): void {
const useNatGateway = this.subnetConfiguration.filter(
subnet => (subnet.subnetType === SubnetType.Private)).length > 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ new ec2.SecurityGroup(stack, 'SecurityGroup', {
});

// Try subnet selection
new cdk.CfnOutput(stack, 'PublicSubnets', { value: 'ids:' + vpc.subnets({ subnetsToUse: ec2.SubnetType.Public }).map(s => s.subnetId).join(',') });
new cdk.CfnOutput(stack, 'PrivateSubnets', { value: 'ids:' + vpc.subnets({ subnetsToUse: ec2.SubnetType.Private }).map(s => s.subnetId).join(',') });
new cdk.CfnOutput(stack, 'PublicSubnets', { value: 'ids:' + vpc.publicSubnets.map(s => s.subnetId).join(',') });
new cdk.CfnOutput(stack, 'PrivateSubnets', { value: 'ids:' + vpc.privateSubnets.map(s => s.subnetId).join(',') });

app.run();
Loading