Skip to content

Commit

Permalink
Deprecate subnetIds
Browse files Browse the repository at this point in the history
  • Loading branch information
jogold committed Apr 3, 2019
1 parent af46e05 commit a3978aa
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
throw new Error(`Should have minCapacity (${minCapacity}) <= desiredCapacity (${desiredCapacity}) <= maxCapacity (${maxCapacity})`);
}

const subnetIds = props.vpc.subnetIds(props.vpcSubnets);
const subnetIds = props.vpc.selectSubnets(props.vpcSubnets).map(s => s.subnetId);
const asgProps: CfnAutoScalingGroupProps = {
cooldown: props.cooldownSeconds !== undefined ? `${props.cooldownSeconds}` : undefined,
minSize: minCapacity.toString(),
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ export class Project extends ProjectBase {
.addAction('ec2:CreateNetworkInterfacePermission'));
return {
vpcId: props.vpc.vpcId,
subnets: props.vpc.subnetIds(props.subnetSelection),
subnets: props.vpc.selectSubnets(props.subnetSelection).map(s => s.subnetId),
securityGroupIds: this._securityGroups.map(s => s.securityGroupId)
};
}
Expand Down
47 changes: 25 additions & 22 deletions packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,18 @@ export interface IVpcNetwork extends IConstruct {
/**
* Return IDs of the subnets appropriate for the given selection strategy
*
* Requires that at least once subnet is matched, throws a descriptive
* Requires that at least one subnet is matched, throws a descriptive
* error message otherwise.
*
* Prefer to use this method over {@link subnets} if you need to pass subnet
* IDs to a CloudFormation Resource.
* @deprecated map over the results of `selectSubnets`
*/
subnetIds(selection?: SubnetSelection): string[];

/**
* Return the subnets appropriate for the placement strategy
*
* Requires that at least one subnet is matched, throws a descriptive
* error message otherwise.
*/
selectSubnets(selection?: SubnetSelection): IVpcSubnet[];

Expand Down Expand Up @@ -219,6 +221,8 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork {

/**
* Returns IDs of selected subnets
*
* @deprecated map over the results of `selectSubnets`
*/
public subnetIds(selection: SubnetSelection = {}): string[] {
selection = reifySelectionDefaults(selection);
Expand Down Expand Up @@ -289,32 +293,31 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork {
*/
public selectSubnets(selection: SubnetSelection = {}): IVpcSubnet[] {
selection = reifySelectionDefaults(selection);
let subnets: IVpcSubnet[] = [];

// Select by name
if (selection.subnetNames !== undefined) {
if (selection.subnetNames !== undefined) { // Select by name
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}`);
subnets = allSubnets.filter(s => names.includes(subnetName(s)));
} else if (selection.subnetTypes === undefined) { // No type
subnets = this.privateSubnets;
} else { // Select by type
if (selection.subnetTypes.includes(SubnetType.Public)) {
subnets = [...subnets, ...this.publicSubnets];
}
if (selection.subnetTypes.includes(SubnetType.Private)) {
subnets = [...subnets, ...this.privateSubnets];
}
if (selection.subnetTypes.includes(SubnetType.Isolated)) {
subnets = [...subnets, ...this.isolatedSubnets];
}
return nameSubnets;
}

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

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];
if (subnets.length === 0) {
throw new Error(`There are no ${describeSelection(selection)} in this VPC. Use a different VPC subnet selection.`);
}
return typeSubnets;

return subnets;
}
}

Expand Down
26 changes: 13 additions & 13 deletions packages/@aws-cdk/aws-ec2/test/test.vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -602,10 +602,10 @@ export = {
const vpc = new VpcNetwork(stack, 'VPC');

// WHEN
const nets = vpc.subnetIds();
const nets = vpc.selectSubnets();

// THEN
test.deepEqual(nets, vpc.privateSubnets.map(s => s.subnetId));
test.deepEqual(nets, vpc.privateSubnets);
test.done();
},

Expand All @@ -615,10 +615,10 @@ export = {
const vpc = new VpcNetwork(stack, 'VPC');

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

// THEN
test.deepEqual(nets, vpc.publicSubnets.map(s => s.subnetId));
test.deepEqual(nets, vpc.publicSubnets);

test.done();
},
Expand All @@ -634,10 +634,10 @@ export = {
});

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

// THEN
test.deepEqual(nets, vpc.isolatedSubnets.map(s => s.subnetId));
test.deepEqual(nets, vpc.isolatedSubnets);

test.done();
},
Expand All @@ -653,10 +653,10 @@ export = {
});

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

// THEN
test.deepEqual(nets, vpc.privateSubnets.map(s => s.subnetId));
test.deepEqual(nets, vpc.privateSubnets);
test.done();
},

Expand All @@ -670,7 +670,7 @@ export = {
});

test.throws(() => {
vpc.subnetIds();
vpc.selectSubnets();
}, /There are no 'Private' subnets in this VPC/);

test.done();
Expand Down Expand Up @@ -734,11 +734,11 @@ export = {
});

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

// THEN
test.equal(3, importedVpc.isolatedSubnets.length);
test.deepEqual(nets, importedVpc.isolatedSubnets.map(s => s.subnetId));
test.deepEqual(nets, importedVpc.isolatedSubnets);

test.done();
},
Expand All @@ -757,11 +757,11 @@ export = {
});

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

// THEN
test.equal(3, importedVpc.isolatedSubnets.length);
test.deepEqual(nets, importedVpc.isolatedSubnets.map(s => s.subnetId));
test.deepEqual(nets, importedVpc.isolatedSubnets);
}

test.done();
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 @@ -189,7 +189,7 @@ export abstract class BaseService extends cdk.Construct
this.networkConfiguration = {
awsvpcConfiguration: {
assignPublicIp: assignPublicIp ? 'ENABLED' : 'DISABLED',
subnets: vpc.subnetIds(vpcSubnets),
subnets: vpc.selectSubnets(vpcSubnets).map(s => s.subnetId),
securityGroups: new cdk.Token(() => [securityGroup!.securityGroupId]).toList(),
}
};
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export class Cluster extends ClusterBase {

// Get subnetIds for all selected subnets
const placements = props.vpcSubnets || { subnetTypes: [ec2.SubnetType.Public, ec2.SubnetType.Private] };
const subnetIds = this.vpc.subnetIds(placements);
const subnetIds = this.vpc.selectSubnets(placements).map(s => s.subnetId);

const resource = new CfnCluster(this, 'Resource', {
name: props.clusterName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export abstract class BaseLoadBalancer extends cdk.Construct implements route53.
const vpcSubnets = ifUndefined(baseProps.vpcSubnets,
{ subnetTypes: internetFacing ? [ec2.SubnetType.Public] : [ec2.SubnetType.Private] });

const subnets = baseProps.vpc.subnetIds(vpcSubnets);
const subnets = baseProps.vpc.selectSubnets(vpcSubnets).map(s => s.subnetId);

this.vpc = baseProps.vpc;

Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ export class Function extends FunctionBase {
// won't work because the ENIs don't get a Public IP.
// Why are we not simply forcing vpcSubnets? Because you might still be choosing
// Isolated networks or selecting among 2 sets of Private subnets by name.
const subnetIds = props.vpc.subnetIds(props.vpcSubnets);
const subnetIds = props.vpc.selectSubnets(props.vpcSubnets).map(s => s.subnetId);
const publicSubnetIds = new Set(props.vpc.publicSubnets.map(s => s.subnetId));
for (const subnetId of subnetIds) {
if (publicSubnetIds.has(subnetId)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-rds/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export class DatabaseCluster extends DatabaseClusterBase implements IDatabaseClu
this.vpc = props.instanceProps.vpc;
this.vpcSubnets = props.instanceProps.vpcSubnets;

const subnetIds = props.instanceProps.vpc.subnetIds(props.instanceProps.vpcSubnets);
const subnetIds = props.instanceProps.vpc.selectSubnets(props.instanceProps.vpcSubnets).map(s => s.subnetId);

// Cannot test whether the subnets are in different AZs, but at least we can test the amount.
if (subnetIds.length < 2) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-rds/lib/rotation-single-user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export class RotationSingleUser extends cdk.Construct {
vpc: props.vpc
});

const vpcSubnetIds = props.vpc.subnetIds(props.vpcSubnets);
const vpcSubnetIds = props.vpc.selectSubnets(props.vpcSubnets).map(s => s.subnetId);

props.target.connections.allowDefaultPortFrom(securityGroup);

Expand Down

0 comments on commit a3978aa

Please sign in to comment.