From 319e0cc752714e8da3710fe886f31ac4a174b360 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 20 Aug 2018 17:33:35 +0200 Subject: [PATCH 1/7] fix(aws-ec2): VPC properly uses complex subnet config It is now possible to pick Isolated subnets for the VPC placement strategy. Also, Isolated subnets are properly exported and imported using the VPC export()/import() functions, and it's now possible not have all three of public/private/isolated subnets in the export/import. Fixes #597. --- packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts | 126 ++++++++++++++++--- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 52 +------- packages/@aws-cdk/aws-ec2/test/test.fleet.ts | 1 + packages/@aws-cdk/aws-ec2/test/test.vpc.ts | 55 ++++++++ 4 files changed, 168 insertions(+), 66 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts index 7a7500fe4b538..a6dc66fdba02b 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts @@ -1,5 +1,41 @@ import { Construct, IDependable, Output, StringListOutput, Token } from "@aws-cdk/cdk"; +/** + * The type of Subnet + */ +export enum SubnetType { + /** + * Isolated Subnets do not route Outbound traffic + * + * This can be good for subnets with RDS or + * Elasticache endpoints + */ + Isolated = 1, + + /** + * Subnet that routes to the internet, but not vice versa. + * + * Instances in a private subnet can connect to the Internet, but will not + * allow connections to be initiated from the Internet. + * + * Outbound traffic will be routed via a NAT Gateway. Preference being in + * the same AZ, but if not available will use another AZ. This is common for + * experimental cost conscious accounts or accounts where HA outbound + * traffic is not needed. + */ + Private = 2, + + /** + * Subnet connected to the Internet + * + * Instances in a Public subnet can connect to the Internet and can be + * connected to from the Internet as long as they are launched with public IPs. + * + * Public subnets route outbound traffic via an Internet Gateway. + */ + Public = 3 +} + /** * Customize how instances are placed inside a VPC * @@ -8,13 +44,13 @@ import { Construct, IDependable, Output, StringListOutput, Token } from "@aws-cd */ export interface VpcPlacementStrategy { /** - * Whether to use the VPC's public subnets to start instances + * What subnet type to place the instances in * - * If false, the instances are started in the private subnets. + * By default, the instances are placed in the private subnets. * - * @default false + * @default SubnetType.Private */ - usePublicSubnets?: boolean; + subnetsToUse?: SubnetType; } /** @@ -43,6 +79,16 @@ export abstract class VpcNetworkRef extends Construct implements IDependable { */ public abstract readonly privateSubnets: VpcSubnetRef[]; + /** + * List of isolated subnets in this VPC + */ + public abstract readonly isolatedSubnets: VpcSubnetRef[]; + + /** + * AZs for this VPC + */ + public abstract readonly availabilityZones: string[]; + /** * Parts of the VPC that constitute full construction */ @@ -51,9 +97,13 @@ export abstract class VpcNetworkRef extends Construct implements IDependable { /** * Return the subnets appropriate for the placement strategy */ - public subnets(placement?: VpcPlacementStrategy): VpcSubnetRef[] { - if (!placement) { return this.privateSubnets; } - return placement.usePublicSubnets ? this.publicSubnets : this.privateSubnets; + public subnets(placement: VpcPlacementStrategy = {}): VpcSubnetRef[] { + if (placement.subnetsToUse === undefined) { return this.privateSubnets; } + return { + [SubnetType.Isolated]: this.isolatedSubnets, + [SubnetType.Private]: this.privateSubnets, + [SubnetType.Public]: this.publicSubnets, + }[placement.subnetsToUse]; } /** @@ -62,11 +112,17 @@ export abstract class VpcNetworkRef extends Construct implements IDependable { public export(): VpcNetworkRefProps { return { vpcId: new Output(this, 'VpcId', { value: this.vpcId }).makeImportValue(), - availabilityZones: this.publicSubnets.map(s => s.availabilityZone), - publicSubnetIds: new StringListOutput(this, 'PublicSubnetIDs', { values: this.publicSubnets.map(s => s.subnetId) }).makeImportValues(), - privateSubnetIds: new StringListOutput(this, 'PrivateSubnetIDs', { values: this.privateSubnets.map(s => s.subnetId) }).makeImportValues(), + availabilityZones: this.availabilityZones, + publicSubnetIds: this.exportSubnetIds('PublicSubnetIDs', this.publicSubnets), + privateSubnetIds: this.exportSubnetIds('PrivateSubnetIDs', this.privateSubnets), + isolatedSubnetIds: this.exportSubnetIds('IsolatedSubnetIDs', this.isolatedSubnets), }; } + + private exportSubnetIds(name: string, subnets: VpcSubnetRef[]): Token[] | undefined { + if (subnets.length === 0) { return undefined; } + return new StringListOutput(this, name, { values: subnets.map(s => s.subnetId) }).makeImportValues(); + } } /** @@ -88,27 +144,50 @@ class ImportedVpcNetwork extends VpcNetworkRef { */ public readonly privateSubnets: VpcSubnetRef[]; + /** + * List of isolated subnets in this VPC + */ + public readonly isolatedSubnets: VpcSubnetRef[]; + + /** + * AZs for this VPC + */ + public readonly availabilityZones: string[]; + constructor(parent: Construct, name: string, props: VpcNetworkRefProps) { super(parent, name); this.vpcId = props.vpcId; + this.availabilityZones = props.availabilityZones; + + const privateSubnetIds = props.privateSubnetIds || []; + const publicSubnetIds = props.publicSubnetIds || []; + const isolatedSubnetIds = props.isolatedSubnetIds || []; - if (props.availabilityZones.length !== props.publicSubnetIds.length) { - throw new Error('Availability zone and public subnet ID arrays must be same length'); + if (publicSubnetIds.length > 0 && this.availabilityZones.length !== publicSubnetIds.length) { + throw new Error('Must have Public subnet for every AZ'); } - if (props.availabilityZones.length !== props.privateSubnetIds.length) { - throw new Error('Availability zone and private subnet ID arrays must be same length'); + if (privateSubnetIds.length > 0 && this.availabilityZones.length !== privateSubnetIds.length) { + throw new Error('Must have Private subnet for every AZ'); + } + + if (isolatedSubnetIds.length > 0 && this.availabilityZones.length !== isolatedSubnetIds.length) { + throw new Error('Must have Isolated subnet for every AZ'); } const n = props.availabilityZones.length; this.publicSubnets = range(n).map(i => VpcSubnetRef.import(this, `PublicSubnet${i}`, { - availabilityZone: props.availabilityZones[i], - subnetId: props.publicSubnetIds[i] + availabilityZone: this.availabilityZones[i], + subnetId: publicSubnetIds[i] })); this.privateSubnets = range(n).map(i => VpcSubnetRef.import(this, `PrivateSubnet${i}`, { - availabilityZone: props.availabilityZones[i], - subnetId: props.privateSubnetIds[i] + availabilityZone: this.availabilityZones[i], + subnetId: privateSubnetIds[i] + })); + this.isolatedSubnets = range(n).map(i => VpcSubnetRef.import(this, `IsolatedSubnet${i}`, { + availabilityZone: this.availabilityZones[i], + subnetId: isolatedSubnetIds[i] })); } } @@ -135,14 +214,21 @@ export interface VpcNetworkRefProps { * * Must match the availability zones and private subnet ids in length and order. */ - publicSubnetIds: VpcSubnetId[]; + publicSubnetIds?: VpcSubnetId[]; /** * List of private subnet IDs, one for every subnet * * Must match the availability zones and public subnet ids in length and order. */ - privateSubnetIds: VpcSubnetId[]; + privateSubnetIds?: VpcSubnetId[]; + + /** + * List of isolated subnet IDs, one for every subnet + * + * Must match the availability zones and public subnet ids in length and order. + */ + isolatedSubnetIds?: VpcSubnetId[]; } /** diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index 17c2c33a368a1..03a0b9db630c3 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -2,7 +2,7 @@ import cdk = require('@aws-cdk/cdk'); import { Obj } from '@aws-cdk/util'; import { cloudformation } from './ec2.generated'; import { NetworkBuilder } from './network-util'; -import { VpcNetworkId, VpcNetworkRef, VpcSubnetId, VpcSubnetRef } from './vpc-ref'; +import { SubnetType, VpcNetworkId, VpcNetworkRef, VpcSubnetId, VpcSubnetRef } from './vpc-ref'; /** * VpcNetworkProps allows you to specify configuration options for a VPC */ @@ -117,44 +117,6 @@ export enum DefaultInstanceTenancy { Dedicated = 'dedicated' } -/** - * The type of Subnet - */ -export enum SubnetType { - - /** - * Isolated Subnets do not route Outbound traffic - * - * This can be good for subnets with RDS or - * Elasticache endpoints - */ - Isolated = 1, - - /** - * Subnet that routes to the internet, but not vice versa. - * - * Instances in a private subnet can connect to the Internet, but will not - * allow connections to be initiated from the Internet. - * - * Outbound traffic will be routed via a NAT Gateway. Preference being in - * the same AZ, but if not available will use another AZ. This is common for - * experimental cost conscious accounts or accounts where HA outbound - * traffic is not needed. - */ - Private = 2, - - /** - * Subnet connected to the Internet - * - * Instances in a Public subnet can connect to the Internet and can be - * connected to from the Internet as long as they are launched with public IPs. - * - * Public subnets route outbound traffic via an Internet Gateway. - */ - Public = 3 - -} - /** * Specify configuration parameters for a VPC to be built */ @@ -248,6 +210,11 @@ export class VpcNetwork extends VpcNetworkRef { */ public readonly isolatedSubnets: VpcSubnetRef[] = []; + /** + * AZs for this VPC + */ + public readonly availabilityZones: string[]; + /** * Maximum Number of NAT Gateways used to control cost * @@ -275,13 +242,6 @@ export class VpcNetwork extends VpcNetworkRef { */ private subnetConfiguration: SubnetConfiguration[] = []; - /** - * Maximum AZs to Uses for this VPC - * - * @default All - */ - private availabilityZones: string[]; - /** * VpcNetwork creates a VPC that spans a whole region. * It will automatically divide the provided VPC CIDR range, and create public and private subnets per Availability Zone. diff --git a/packages/@aws-cdk/aws-ec2/test/test.fleet.ts b/packages/@aws-cdk/aws-ec2/test/test.fleet.ts index 71314266b7f01..68f24ffa6b7d1 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.fleet.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.fleet.ts @@ -241,5 +241,6 @@ function mockVpc(stack: Stack) { availabilityZones: [ 'az1' ], publicSubnetIds: [ new VpcSubnetId('pub1') ], privateSubnetIds: [ new VpcSubnetId('pri1') ], + isolatedSubnetIds: [], }); } diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts index 81c51eeba65c9..5161552747032 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts @@ -255,7 +255,62 @@ export = { })); test.done(); } + }, + + 'can select public subnets'(test: Test) { + // GIVEN + const stack = getTestStack(); + const vpc = new VpcNetwork(stack, 'VPC'); + + // WHEN + const nets = vpc.subnets({ subnetsToUse: SubnetType.Public }); + + // THEN + test.deepEqual(nets, vpc.publicSubnets); + + test.done(); + }, + + 'can select isolated subnets'(test: Test) { + // GIVEN + const stack = getTestStack(); + const vpc = new VpcNetwork(stack, 'VPC', { + subnetConfiguration: [ + { subnetType: SubnetType.Private, name: 'Private' }, + { subnetType: SubnetType.Isolated, name: 'Isolated' }, + ] + }); + + // WHEN + const nets = vpc.subnets({ subnetsToUse: SubnetType.Isolated }); + + // THEN + test.deepEqual(nets, vpc.isolatedSubnets); + + test.done(); + }, + + 'can select isolated subnets after exporting and importing'(test: Test) { + // GIVEN + const stack1 = getTestStack(); + const stack2 = getTestStack(); + const vpc1 = new VpcNetwork(stack1, 'VPC', { + subnetConfiguration: [ + { subnetType: SubnetType.Private, name: 'Private' }, + { subnetType: SubnetType.Isolated, name: 'Isolated' }, + ] + }); + + const importedVpc = VpcNetwork.import(stack2, 'VPC', vpc1.export()); + + // WHEN + const nets = importedVpc.subnets({ subnetsToUse: SubnetType.Isolated }); + + // THEN + test.equal(3, importedVpc.isolatedSubnets.length); + test.deepEqual(nets, importedVpc.isolatedSubnets); + test.done(); }, }; From 5b8bc687e0e47bcb260ffdb6f156b995fb1200f6 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 20 Aug 2018 18:54:56 +0200 Subject: [PATCH 2/7] Fix references in aws-rds --- packages/@aws-cdk/aws-rds/README.md | 2 +- packages/@aws-cdk/aws-rds/lib/cluster.ts | 2 +- packages/@aws-cdk/aws-rds/test/integ.cluster.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-rds/README.md b/packages/@aws-cdk/aws-rds/README.md index 2d309e45f079e..3d303f44a72cf 100644 --- a/packages/@aws-cdk/aws-rds/README.md +++ b/packages/@aws-cdk/aws-rds/README.md @@ -28,7 +28,7 @@ const cluster = new DatabaseCluster(stack, 'Database', { instanceProps: { instanceType: new InstanceTypePair(InstanceClass.Burstable2, InstanceSize.Small), vpcPlacement: { - usePublicSubnets: true + subnetsToUse: ec2.SubnetType.Public, }, vpc } diff --git a/packages/@aws-cdk/aws-rds/lib/cluster.ts b/packages/@aws-cdk/aws-rds/lib/cluster.ts index c85c3238df141..fd6f8059f0f76 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster.ts @@ -183,7 +183,7 @@ export class DatabaseCluster extends DatabaseClusterRef { props.clusterIdentifier != null ? `${props.clusterIdentifier}instance${instanceIndex}` : undefined; - const publiclyAccessible = props.instanceProps.vpcPlacement && props.instanceProps.vpcPlacement.usePublicSubnets; + const publiclyAccessible = props.instanceProps.vpcPlacement && props.instanceProps.vpcPlacement.subnetsToUse === ec2.SubnetType.Public; const instance = new cloudformation.DBInstanceResource(this, `Instance${instanceIndex}`, { // Link to cluster diff --git a/packages/@aws-cdk/aws-rds/test/integ.cluster.ts b/packages/@aws-cdk/aws-rds/test/integ.cluster.ts index 07bc809e428e1..2458f833319fb 100644 --- a/packages/@aws-cdk/aws-rds/test/integ.cluster.ts +++ b/packages/@aws-cdk/aws-rds/test/integ.cluster.ts @@ -15,7 +15,7 @@ const cluster = new DatabaseCluster(stack, 'Database', { }, instanceProps: { instanceType: new ec2.InstanceTypePair(ec2.InstanceClass.Burstable2, ec2.InstanceSize.Small), - vpcPlacement: { usePublicSubnets: true }, + vpcPlacement: { subnetsToUse: ec2.SubnetType.Public }, vpc } }); From 4e2d510084820fc6afcb36813286e6c35db4d15e Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 21 Aug 2018 13:05:29 +0200 Subject: [PATCH 3/7] Fix eample build --- examples/cdk-examples-typescript/neptune-demo/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/cdk-examples-typescript/neptune-demo/index.ts b/examples/cdk-examples-typescript/neptune-demo/index.ts index 423754c9f71ca..b6bd3136ea517 100644 --- a/examples/cdk-examples-typescript/neptune-demo/index.ts +++ b/examples/cdk-examples-typescript/neptune-demo/index.ts @@ -14,7 +14,7 @@ class NeptuneDemoStack extends cdk.Stack { instanceProps: { instanceType: new ec2.InstanceTypePair(ec2.InstanceClass.Burstable2, ec2.InstanceSize.Small), vpc, - vpcPlacement: { usePublicSubnets: true }, + vpcPlacement: { subnetsToUse: ec2.SubnetType.Public }, }, masterUser: { // This would normally be imported from SSM parmeter store encrypted string, From 60bcbb98ef7b2e8dac44cea7974749c1e2dfd744 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 21 Aug 2018 14:57:52 +0200 Subject: [PATCH 4/7] Also allow selecting subnets by configured name --- packages/@aws-cdk/aws-ec2/lib/util.ts | 155 +++++++++++++++++++++ packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts | 139 ++++++++++-------- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 7 +- packages/@aws-cdk/aws-ec2/test/test.vpc.ts | 134 ++++++++++++++---- 4 files changed, 345 insertions(+), 90 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/util.ts b/packages/@aws-cdk/aws-ec2/lib/util.ts index f3d3edd216fd1..3a8a52adfcc98 100644 --- a/packages/@aws-cdk/aws-ec2/lib/util.ts +++ b/packages/@aws-cdk/aws-ec2/lib/util.ts @@ -1,3 +1,6 @@ +import cdk = require('@aws-cdk/cdk'); +import { SubnetType, VpcSubnetRef } from "./vpc-ref"; + /** * Turn an arbitrary string into one that can be used as a CloudFormation identifier by stripping special characters * @@ -6,3 +9,155 @@ export function slugify(x: string): string { return x.replace(/[^a-zA-Z0-9]/g, ''); } + +/** + * The default names for every subnet type + */ +export const DEFAULT_SUBNET_NAME = { + [SubnetType.Public]: 'Public', + [SubnetType.Private]: 'Private', + [SubnetType.Isolated]: 'Isolated', +}; + +/** + * Return a subnet name from its construct ID + * + * All subnet names look like NAME <> "Subnet" <> INDEX + */ +export function subnetName(subnet: VpcSubnetRef) { + return subnet.id.replace(/Subnet\d+$/, ''); +} + +/** + * Make the subnet construct ID from a name and number + */ +export function subnetId(name: string, i: number) { + return `${name}Subnet${i + 1}`; +} + +/** + * Helper class to export/import groups of subnets + */ +export class ExportSubnetGroup { + public readonly ids?: cdk.Token[]; + public readonly names?: string[]; + + private readonly groups: number; + + constructor( + parent: cdk.Construct, + exportName: string, + private readonly subnets: VpcSubnetRef[], + private readonly type: SubnetType, + private readonly azs: number) { + + this.groups = subnets.length / azs; + + // ASSERTION + if (Math.floor(this.groups) !== this.groups) { + throw new Error(`Number of subnets (${subnets.length}) must be a multiple of number of availability zones (${azs})`); + } + + this.ids = this.exportIds(parent, exportName); + this.names = this.exportNames(); + } + + private exportIds(parent: cdk.Construct, name: string): cdk.Token[] | undefined { + if (this.subnets.length === 0) { return undefined; } + return new cdk.StringListOutput(parent, name, { values: this.subnets.map(s => s.subnetId) }).makeImportValues(); + } + + /** + * Return the list of subnet names if they're not equal to the default + */ + private exportNames(): string[] | undefined { + if (this.subnets.length === 0) { return undefined; } + const netNames = this.subnets.map(subnetName); + + // Do some assertion that the 'netNames' array is laid out like this: + // + // [ INGRESS, INGRESS, INGRESS, EGRESS, EGRESS, EGRESS, ... ] + for (let i = 0; i < netNames.length; i++) { + const k = Math.floor(i / this.azs); + if (netNames[i] !== netNames[k * this.azs]) { + throw new Error(`Subnets must be grouped by name, got: ${JSON.stringify(netNames)}`); + } + } + + // Splat down to [ INGRESS, EGRESS, ... ] + const groupNames = range(this.groups).map(i => netNames[i * this.azs]); + if (groupNames.length === 1 && groupNames[0] === DEFAULT_SUBNET_NAME[this.type]) { return undefined; } + + return groupNames; + } +} + +export class ImportSubnetGroup { + private readonly subnetIds: cdk.Token[]; + private readonly names: string[]; + private readonly groups: number; + + constructor( + subnetIds: cdk.Token[] | undefined, + names: string[] | undefined, + type: SubnetType, + private readonly availabilityZones: string[], + idField: string, + nameField: string) { + + this.subnetIds = subnetIds || []; + this.groups = this.subnetIds.length / this.availabilityZones.length; + + if (Math.floor(this.groups) !== this.groups) { + // tslint:disable-next-line:max-line-length + throw new Error(`Amount of ${idField} (${this.subnetIds.length}) must be a multiple of availability zones (${this.availabilityZones.length}).`); + } + + this.names = this.normalizeNames(names, DEFAULT_SUBNET_NAME[type], nameField); + } + + public import(parent: cdk.Construct): VpcSubnetRef[] { + return range(this.subnetIds.length).map(i => { + const k = Math.floor(i / this.availabilityZones.length); + return VpcSubnetRef.import(parent, subnetId(this.names[k], i), { + availabilityZone: this.pickAZ(i), + subnetId: this.subnetIds[i] + }); + }); + } + + /** + * Return a list with a name for every subnet + */ + private normalizeNames(names: string[] | undefined, defaultName: string, fieldName: string) { + // If not given, return default + if (names === undefined || names.length === 0) { + return [defaultName]; + } + + // If given, must match given subnets + if (names.length !== this.groups) { + throw new Error(`${fieldName} must have an entry for every corresponding subnet group, got: ${JSON.stringify(names)}`); + } + + return names; + } + + /** + * Return the i'th AZ + */ + private pickAZ(i: number) { + return this.availabilityZones[i % this.availabilityZones.length]; + } +} + +/** + * Generate the list of numbers of [0..n) + */ +export function range(n: number): number[] { + const ret: number[] = []; + for (let i = 0; i < n; i++) { + ret.push(i); + } + return ret; +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts index a6dc66fdba02b..e8cd157bbfd28 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts @@ -1,4 +1,5 @@ -import { Construct, IDependable, Output, StringListOutput, Token } from "@aws-cdk/cdk"; +import { Construct, IDependable, Output, Token } from "@aws-cdk/cdk"; +import { ExportSubnetGroup, ImportSubnetGroup, subnetName } from './util'; /** * The type of Subnet @@ -41,16 +42,29 @@ export enum SubnetType { * * 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 { /** - * What subnet type to place the instances in + * Place the instances in the subnets of the given type * - * By default, the instances are placed in the private subnets. + * At most one of `subnetsToUse` and `subnetName` can be supplied. * * @default SubnetType.Private */ subnetsToUse?: 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. + * + * @default name + */ + subnetName?: string; } /** @@ -98,7 +112,23 @@ export abstract class VpcNetworkRef extends Construct implements IDependable { * Return the subnets appropriate for the placement strategy */ public subnets(placement: VpcPlacementStrategy = {}): VpcSubnetRef[] { + if (placement.subnetsToUse !== undefined && placement.subnetName !== undefined) { + throw new Error('At most one of subnetsToUse and subnetName can be supplied'); + } + + // 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; + } + + // Select by type if (placement.subnetsToUse === undefined) { return this.privateSubnets; } + return { [SubnetType.Isolated]: this.isolatedSubnets, [SubnetType.Private]: this.privateSubnets, @@ -110,19 +140,21 @@ export abstract class VpcNetworkRef extends Construct implements IDependable { * Export this VPC from the stack */ public export(): VpcNetworkRefProps { + const pub = new ExportSubnetGroup(this, 'PublicSubnetIDs', this.publicSubnets, SubnetType.Public, this.availabilityZones.length); + const priv = new ExportSubnetGroup(this, 'PrivateSubnetIDs', this.privateSubnets, SubnetType.Private, this.availabilityZones.length); + const iso = new ExportSubnetGroup(this, 'IsolatedSubnetIDs', this.isolatedSubnets, SubnetType.Isolated, this.availabilityZones.length); + return { vpcId: new Output(this, 'VpcId', { value: this.vpcId }).makeImportValue(), availabilityZones: this.availabilityZones, - publicSubnetIds: this.exportSubnetIds('PublicSubnetIDs', this.publicSubnets), - privateSubnetIds: this.exportSubnetIds('PrivateSubnetIDs', this.privateSubnets), - isolatedSubnetIds: this.exportSubnetIds('IsolatedSubnetIDs', this.isolatedSubnets), + publicSubnetIds: pub.ids, + publicSubnetNames: pub.names, + privateSubnetIds: priv.ids, + privateSubnetNames: priv.names, + isolatedSubnetIds: iso.ids, + isolatedSubnetNames: iso.names, }; } - - private exportSubnetIds(name: string, subnets: VpcSubnetRef[]): Token[] | undefined { - if (subnets.length === 0) { return undefined; } - return new StringListOutput(this, name, { values: subnets.map(s => s.subnetId) }).makeImportValues(); - } } /** @@ -160,35 +192,15 @@ class ImportedVpcNetwork extends VpcNetworkRef { this.vpcId = props.vpcId; this.availabilityZones = props.availabilityZones; - const privateSubnetIds = props.privateSubnetIds || []; - const publicSubnetIds = props.publicSubnetIds || []; - const isolatedSubnetIds = props.isolatedSubnetIds || []; + // tslint:disable:max-line-length + const pub = new ImportSubnetGroup(props.publicSubnetIds, props.publicSubnetNames, SubnetType.Public, this.availabilityZones, 'publicSubnetIds', 'publicSubnetNames'); + const priv = new ImportSubnetGroup(props.privateSubnetIds, props.privateSubnetNames, SubnetType.Private, this.availabilityZones, 'privateSubnetIds', 'privateSubnetNames'); + const iso = new ImportSubnetGroup(props.isolatedSubnetIds, props.isolatedSubnetNames, SubnetType.Isolated, this.availabilityZones, 'isolatedSubnetIds', 'isolatedSubnetNames'); + // tslint:enable:max-line-length - if (publicSubnetIds.length > 0 && this.availabilityZones.length !== publicSubnetIds.length) { - throw new Error('Must have Public subnet for every AZ'); - } - - if (privateSubnetIds.length > 0 && this.availabilityZones.length !== privateSubnetIds.length) { - throw new Error('Must have Private subnet for every AZ'); - } - - if (isolatedSubnetIds.length > 0 && this.availabilityZones.length !== isolatedSubnetIds.length) { - throw new Error('Must have Isolated subnet for every AZ'); - } - - const n = props.availabilityZones.length; - this.publicSubnets = range(n).map(i => VpcSubnetRef.import(this, `PublicSubnet${i}`, { - availabilityZone: this.availabilityZones[i], - subnetId: publicSubnetIds[i] - })); - this.privateSubnets = range(n).map(i => VpcSubnetRef.import(this, `PrivateSubnet${i}`, { - availabilityZone: this.availabilityZones[i], - subnetId: privateSubnetIds[i] - })); - this.isolatedSubnets = range(n).map(i => VpcSubnetRef.import(this, `IsolatedSubnet${i}`, { - availabilityZone: this.availabilityZones[i], - subnetId: isolatedSubnetIds[i] - })); + this.publicSubnets = pub.import(this); + this.privateSubnets = priv.import(this); + this.isolatedSubnets = iso.import(this); } } @@ -202,33 +214,51 @@ export interface VpcNetworkRefProps { vpcId: VpcNetworkId; /** - * List of a availability zones, one for every subnet. - * - * The first half are for the public subnets, the second half are for - * the private subnets. + * List of availability zones for the subnets in this VPC. */ availabilityZones: string[]; /** - * List of public subnet IDs, one for every subnet + * List of public subnet IDs * - * Must match the availability zones and private subnet ids in length and order. + * Must be undefined or match the availability zones in length and order. */ publicSubnetIds?: VpcSubnetId[]; /** - * List of private subnet IDs, one for every subnet + * List of names for the public subnets + * + * Must be undefined or have a name for every public subnet group. + */ + publicSubnetNames?: string[]; + + /** + * List of private subnet IDs * - * Must match the availability zones and public subnet ids in length and order. + * Must be undefined or match the availability zones in length and order. */ privateSubnetIds?: VpcSubnetId[]; /** - * List of isolated subnet IDs, one for every subnet + * List of names for the private subnets + * + * Must be undefined or have a name for every private subnet group. + */ + privateSubnetNames?: string[]; + + /** + * List of isolated subnet IDs * - * Must match the availability zones and public subnet ids in length and order. + * Must be undefined or match the availability zones in length and order. */ isolatedSubnetIds?: VpcSubnetId[]; + + /** + * List of names for the isolated subnets + * + * Must be undefined or have a name for every isolated subnet group. + */ + isolatedSubnetNames?: string[]; } /** @@ -300,14 +330,3 @@ export interface VpcSubnetRefProps { */ export class VpcSubnetId extends Token { } - -/** - * Generate the list of numbers of [0..n) - */ -function range(n: number): number[] { - const ret: number[] = []; - for (let i = 0; i < n; i++) { - ret.push(i); - } - return ret; -} diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index 03a0b9db630c3..ce8d678955fa8 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -2,6 +2,7 @@ import cdk = require('@aws-cdk/cdk'); import { Obj } from '@aws-cdk/util'; import { cloudformation } from './ec2.generated'; import { NetworkBuilder } from './network-util'; +import { DEFAULT_SUBNET_NAME, subnetId } from './util'; import { SubnetType, VpcNetworkId, VpcNetworkRef, VpcSubnetId, VpcSubnetRef } from './vpc-ref'; /** * VpcNetworkProps allows you to specify configuration options for a VPC @@ -182,11 +183,11 @@ export class VpcNetwork extends VpcNetworkRef { public static readonly DEFAULT_SUBNETS: SubnetConfiguration[] = [ { subnetType: SubnetType.Public, - name: 'Public', + name: DEFAULT_SUBNET_NAME[SubnetType.Public], }, { subnetType: SubnetType.Private, - name: 'Private', + name: DEFAULT_SUBNET_NAME[SubnetType.Private], } ]; @@ -354,7 +355,7 @@ export class VpcNetwork extends VpcNetworkRef { private createSubnetResources(subnetConfig: SubnetConfiguration, cidrMask: number) { this.availabilityZones.forEach((zone, index) => { - const name: string = `${subnetConfig.name}Subnet${index + 1}`; + const name = subnetId(subnetConfig.name, index); const subnetProps = { availabilityZone: zone, vpcId: this.vpcId, diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts index c14cd0bceaa68..5efb578e99fd5 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts @@ -1,7 +1,7 @@ import { countResources, expect, haveResource } from '@aws-cdk/assert'; -import { AvailabilityZoneProvider, resolve, Stack } from '@aws-cdk/cdk'; +import { AvailabilityZoneProvider, Construct, resolve, Stack } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; -import { DefaultInstanceTenancy, SubnetType, VpcNetwork } from '../lib'; +import { DefaultInstanceTenancy, SubnetType, VpcNetwork, VpcNetworkRef } from '../lib'; export = { @@ -290,48 +290,128 @@ export = { test.done(); }, - 'can select isolated subnets after exporting and importing'(test: Test) { + 'can select subnets by name'(test: Test) { // GIVEN - const stack1 = getTestStack(); - const stack2 = getTestStack(); - const vpc1 = new VpcNetwork(stack1, 'VPC', { + const stack = getTestStack(); + const vpc = new VpcNetwork(stack, 'VPC', { subnetConfiguration: [ - { subnetType: SubnetType.Private, name: 'Private' }, - { subnetType: SubnetType.Isolated, name: 'Isolated' }, + { subnetType: SubnetType.Private, name: 'DontTalkToMe' }, + { subnetType: SubnetType.Isolated, name: 'DontTalkAtAll' }, ] }); - const importedVpc = VpcNetwork.import(stack2, 'VPC', vpc1.export()); - // WHEN - const nets = importedVpc.subnets({ subnetsToUse: SubnetType.Isolated }); + const nets = vpc.subnets({ subnetName: 'DontTalkToMe' }); // THEN - test.equal(3, importedVpc.isolatedSubnets.length); - test.deepEqual(nets, importedVpc.isolatedSubnets); - + test.deepEqual(nets, vpc.privateSubnets); test.done(); }, - 'export/import'(test: Test) { - // GIVEN - const stack1 = getTestStack(); - const stack2 = getTestStack(); + 'export/import': { + 'simple VPC'(test: Test) { + // WHEN + const vpc2 = doImportExportTest(stack => { + return new VpcNetwork(stack, 'TheVPC'); + }); - const vpc1 = new VpcNetwork(stack1, 'TheVPC', { cidr: '192.168.0.0/16' }); + // THEN + test.deepEqual(resolve(vpc2.vpcId), { + 'Fn::ImportValue': 'TestStack:TheVPCVpcIdD346CDBA' + }); - // WHEN - const vpc2 = VpcNetwork.import(stack2, 'VPC2', vpc1.export()); + test.done(); + }, - // THEN - test.deepEqual(resolve(vpc2.vpcId), { - 'Fn::ImportValue': 'TestStack:TheVPCVpcIdD346CDBA' - }); + 'multiple subnets of the same type'(test: Test) { + // WHEN + const imported = doImportExportTest(stack => { + return new VpcNetwork(stack, 'TheVPC', { + subnetConfiguration: [ + { name: 'Ingress', subnetType: SubnetType.Public }, + { name: 'Egress', subnetType: SubnetType.Public }, + ] + }); + }); - test.done(); - } + // THEN + test.deepEqual(resolve(imported.vpcId), { + 'Fn::ImportValue': 'TestStack:TheVPCVpcIdD346CDBA' + }); + + test.equal(6, imported.publicSubnets.length); + + for (let i = 0; i < 3; i++) { + test.equal(true, imported.publicSubnets[i].id.startsWith('Ingress'), `${imported.publicSubnets[i].id} does not start with "Ingress"`); + } + for (let i = 3; i < 6; i++) { + test.equal(true, imported.publicSubnets[i].id.startsWith('Egress'), `${imported.publicSubnets[i].id} does not start with "Egress"`); + } + + test.done(); + }, + + 'can select isolated subnets by type'(test: Test) { + // GIVEN + const importedVpc = doImportExportTest(stack => { + return new VpcNetwork(stack, 'TheVPC', { + subnetConfiguration: [ + { subnetType: SubnetType.Private, name: 'Private' }, + { subnetType: SubnetType.Isolated, name: 'Isolated' }, + ] + }); + }); + + // WHEN + const nets = importedVpc.subnets({ subnetsToUse: SubnetType.Isolated }); + + // THEN + test.equal(3, importedVpc.isolatedSubnets.length); + test.deepEqual(nets, importedVpc.isolatedSubnets); + + test.done(); + }, + + 'can select isolated subnets by name'(test: Test) { + // Do the test with both default name and custom name + for (const isolatedName of ['Isolated', 'LeaveMeAlone']) { + // GIVEN + const importedVpc = doImportExportTest(stack => { + return new VpcNetwork(stack, 'TheVPC', { + subnetConfiguration: [ + { subnetType: SubnetType.Private, name: 'Private' }, + { subnetType: SubnetType.Isolated, name: isolatedName }, + ] + }); + }); + + // WHEN + const nets = importedVpc.subnets({ subnetName: isolatedName }); + + // THEN + test.equal(3, importedVpc.isolatedSubnets.length); + test.deepEqual(nets, importedVpc.isolatedSubnets); + } + + test.done(); + }, + }, }; function getTestStack(): Stack { return new Stack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } }); } + +/** + * Do a complete import/export test, return the imported VPC + */ +function doImportExportTest(constructFn: (parent: Construct) => VpcNetwork): VpcNetworkRef { + // GIVEN + const stack1 = getTestStack(); + const stack2 = getTestStack(); + + const vpc1 = constructFn(stack1); + + // WHEN + return VpcNetwork.import(stack2, 'VPC2', vpc1.export()); +} \ No newline at end of file From de44e5acebc1b3d96ec0fb9ab0cd7869f059d54c Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 7 Sep 2018 17:01:50 +0200 Subject: [PATCH 5/7] Update SubnetType documentation some --- packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts index e8cd157bbfd28..7219f097f33c4 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts @@ -20,7 +20,8 @@ export enum SubnetType { * allow connections to be initiated from the Internet. * * Outbound traffic will be routed via a NAT Gateway. Preference being in - * the same AZ, but if not available will use another AZ. This is common for + * the same AZ, but if not available will use another AZ (control by + * specifing `maxGateways` on VpcNetwork). This might be used for * experimental cost conscious accounts or accounts where HA outbound * traffic is not needed. */ @@ -30,7 +31,9 @@ export enum SubnetType { * Subnet connected to the Internet * * Instances in a Public subnet can connect to the Internet and can be - * connected to from the Internet as long as they are launched with public IPs. + * connected to from the Internet as long as they are launched with public + * IPs (controlled on the AutoScalingGroup or other constructs that launch + * instances). * * Public subnets route outbound traffic via an Internet Gateway. */ From 02aa2116b2488c2453d0fc5928e0979d6bd199d1 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Sun, 9 Sep 2018 13:40:08 +0200 Subject: [PATCH 6/7] Remove newline, add private members to classes to disable structural similarity --- packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts | 1 - tools/cfn2ts/lib/codegen.ts | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts index 04ddef275f8ab..2cfc6c36dd97f 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts @@ -265,7 +265,6 @@ export interface VpcNetworkRefProps { isolatedSubnetNames?: string[]; } - /** * A new or imported VPC Subnet */ diff --git a/tools/cfn2ts/lib/codegen.ts b/tools/cfn2ts/lib/codegen.ts index 4e62651eb3b87..ae1e2701ccd39 100644 --- a/tools/cfn2ts/lib/codegen.ts +++ b/tools/cfn2ts/lib/codegen.ts @@ -477,6 +477,12 @@ export default class CodeGenerator { */ private emitAttributeType(attr: genspec.ClassDeclaration) { this.openClass(attr.typeName, attr.docLink, attr.baseClassName.fqn); + // Add a private member that will make the class structurally + // different in TypeScript, which prevents assigning returning + // incorrectly-typed Tokens. Those will cause ClassCastExceptions + // in strictly-typed languages. + this.code.line(`private readonly thisIsA = '${attr.typeName}';`); + this.closeClass(attr.typeName); } From ccd22fbc2ddba88f7a4ce550f1edc1d224335af7 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Sun, 9 Sep 2018 14:20:35 +0200 Subject: [PATCH 7/7] Fix remaining structural typing mishaps by adding private field to generated strong-typed gclasses. --- packages/@aws-cdk/assets/lib/asset.ts | 2 +- packages/@aws-cdk/aws-codebuild/lib/project.ts | 6 +++--- packages/@aws-cdk/aws-codecommit/lib/repository.ts | 4 ++-- packages/@aws-cdk/aws-codedeploy/lib/application.ts | 2 +- .../@aws-cdk/aws-codedeploy/lib/deployment-group.ts | 4 ++-- packages/@aws-cdk/aws-ec2/lib/util.ts | 13 +++++++------ packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts | 2 +- packages/@aws-cdk/aws-lambda/lib/alias.ts | 6 ++++-- packages/@aws-cdk/aws-lambda/lib/lambda-ref.ts | 4 ++-- packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts | 2 +- packages/@aws-cdk/aws-rds/lib/cluster.ts | 8 +++++--- packages/@aws-cdk/aws-s3/lib/bucket.ts | 7 ++++++- packages/@aws-cdk/aws-s3/lib/util.ts | 9 +++++---- tools/cfn2ts/lib/codegen.ts | 3 ++- 14 files changed, 42 insertions(+), 30 deletions(-) diff --git a/packages/@aws-cdk/assets/lib/asset.ts b/packages/@aws-cdk/assets/lib/asset.ts index f8e53a0a0a36a..35fc91fbfe643 100644 --- a/packages/@aws-cdk/assets/lib/asset.ts +++ b/packages/@aws-cdk/assets/lib/asset.ts @@ -94,7 +94,7 @@ export class Asset extends cdk.Construct { description: `S3 key for asset version "${this.path}"` }); - this.s3BucketName = bucketParam.value; + this.s3BucketName = new s3.BucketName(bucketParam.value); this.s3Prefix = new cdk.FnSelect(0, new cdk.FnSplit(cxapi.ASSET_PREFIX_SEPARATOR, keyParam.value)); const s3Filename = new cdk.FnSelect(1, new cdk.FnSplit(cxapi.ASSET_PREFIX_SEPARATOR, keyParam.value)); this.s3ObjectKey = new s3.ObjectKey(new cdk.FnConcat(this.s3Prefix, s3Filename)); diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts index 99fca662b8866..39f97d530ad4d 100644 --- a/packages/@aws-cdk/aws-codebuild/lib/project.ts +++ b/packages/@aws-cdk/aws-codebuild/lib/project.ts @@ -300,11 +300,11 @@ class ImportedProjectRef extends ProjectRef { constructor(parent: cdk.Construct, name: string, props: ProjectRefProps) { super(parent, name); - this.projectArn = cdk.Arn.fromComponents({ + this.projectArn = new ProjectArn(cdk.Arn.fromComponents({ service: 'codebuild', resource: 'project', resourceName: props.projectName, - }); + })); this.projectName = props.projectName; } } @@ -723,4 +723,4 @@ export enum BuildEnvironmentVariableType { * An environment variable stored in Systems Manager Parameter Store. */ ParameterStore = 'PARAMETER_STORE' -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-codecommit/lib/repository.ts b/packages/@aws-cdk/aws-codecommit/lib/repository.ts index 5171cfa755289..a92da69b20bbf 100644 --- a/packages/@aws-cdk/aws-codecommit/lib/repository.ts +++ b/packages/@aws-cdk/aws-codecommit/lib/repository.ts @@ -176,10 +176,10 @@ class ImportedRepositoryRef extends RepositoryRef { constructor(parent: cdk.Construct, name: string, props: RepositoryRefProps) { super(parent, name); - this.repositoryArn = cdk.Arn.fromComponents({ + this.repositoryArn = new RepositoryArn(cdk.Arn.fromComponents({ service: 'codecommit', resource: props.repositoryName, - }); + })); this.repositoryName = props.repositoryName; } } diff --git a/packages/@aws-cdk/aws-codedeploy/lib/application.ts b/packages/@aws-cdk/aws-codedeploy/lib/application.ts index 21a84ae618505..e64a5bff80b2c 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/application.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/application.ts @@ -47,7 +47,7 @@ export abstract class ServerApplicationRef extends cdk.Construct { public export(): ServerApplicationRefProps { return { - applicationName: new cdk.Output(this, 'ApplicationName', { value: this.applicationName }).makeImportValue(), + applicationName: new ApplicationName(new cdk.Output(this, 'ApplicationName', { value: this.applicationName }).makeImportValue()), }; } } diff --git a/packages/@aws-cdk/aws-codedeploy/lib/deployment-group.ts b/packages/@aws-cdk/aws-codedeploy/lib/deployment-group.ts index 593a6819aecf7..e4cb8404cd3dc 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/deployment-group.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/deployment-group.ts @@ -56,9 +56,9 @@ export abstract class ServerDeploymentGroupRef extends cdk.Construct { public export(): ServerDeploymentGroupRefProps { return { application: this.application, - deploymentGroupName: new cdk.Output(this, 'DeploymentGroupName', { + deploymentGroupName: new DeploymentGroupName(new cdk.Output(this, 'DeploymentGroupName', { value: this.deploymentGroupName - }).makeImportValue(), + }).makeImportValue()), }; } } diff --git a/packages/@aws-cdk/aws-ec2/lib/util.ts b/packages/@aws-cdk/aws-ec2/lib/util.ts index 3a8a52adfcc98..e577665bc1141 100644 --- a/packages/@aws-cdk/aws-ec2/lib/util.ts +++ b/packages/@aws-cdk/aws-ec2/lib/util.ts @@ -1,4 +1,5 @@ import cdk = require('@aws-cdk/cdk'); +import { SubnetId } from './ec2.generated'; import { SubnetType, VpcSubnetRef } from "./vpc-ref"; /** @@ -39,7 +40,7 @@ export function subnetId(name: string, i: number) { * Helper class to export/import groups of subnets */ export class ExportSubnetGroup { - public readonly ids?: cdk.Token[]; + public readonly ids?: SubnetId[]; public readonly names?: string[]; private readonly groups: number; @@ -62,9 +63,9 @@ export class ExportSubnetGroup { this.names = this.exportNames(); } - private exportIds(parent: cdk.Construct, name: string): cdk.Token[] | undefined { + private exportIds(parent: cdk.Construct, name: string): SubnetId[] | undefined { if (this.subnets.length === 0) { return undefined; } - return new cdk.StringListOutput(parent, name, { values: this.subnets.map(s => s.subnetId) }).makeImportValues(); + return new cdk.StringListOutput(parent, name, { values: this.subnets.map(s => s.subnetId) }).makeImportValues().map(x => new SubnetId(x)); } /** @@ -93,12 +94,12 @@ export class ExportSubnetGroup { } export class ImportSubnetGroup { - private readonly subnetIds: cdk.Token[]; + private readonly subnetIds: SubnetId[]; private readonly names: string[]; private readonly groups: number; constructor( - subnetIds: cdk.Token[] | undefined, + subnetIds: SubnetId[] | undefined, names: string[] | undefined, type: SubnetType, private readonly availabilityZones: string[], @@ -160,4 +161,4 @@ export function range(n: number): number[] { ret.push(i); } return ret; -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts index 2cfc6c36dd97f..868eacfdd771d 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts @@ -149,7 +149,7 @@ export abstract class VpcNetworkRef extends Construct implements IDependable { const iso = new ExportSubnetGroup(this, 'IsolatedSubnetIDs', this.isolatedSubnets, SubnetType.Isolated, this.availabilityZones.length); return { - vpcId: new Output(this, 'VpcId', { value: this.vpcId }).makeImportValue(), + vpcId: new VPCId(new Output(this, 'VpcId', { value: this.vpcId }).makeImportValue()), availabilityZones: this.availabilityZones, publicSubnetIds: pub.ids, publicSubnetNames: pub.names, diff --git a/packages/@aws-cdk/aws-lambda/lib/alias.ts b/packages/@aws-cdk/aws-lambda/lib/alias.ts index 034f3e4ad4489..6ae1d888bcda2 100644 --- a/packages/@aws-cdk/aws-lambda/lib/alias.ts +++ b/packages/@aws-cdk/aws-lambda/lib/alias.ts @@ -93,8 +93,10 @@ export class Alias extends FunctionRef { routingConfig: this.determineRoutingConfig(props) }); - this.functionName = alias.ref; - this.functionArn = alias.ref; + // Not actually the name, but an ARN can be used in all places + // where the name is expected, and an ARN can refer to an Alias. + this.functionName = new FunctionName(alias.ref); + this.functionArn = new FunctionArn(alias.ref); } public addPermission(name: string, permission: Permission) { diff --git a/packages/@aws-cdk/aws-lambda/lib/lambda-ref.ts b/packages/@aws-cdk/aws-lambda/lib/lambda-ref.ts index 559794eb6112d..3c152effd802c 100644 --- a/packages/@aws-cdk/aws-lambda/lib/lambda-ref.ts +++ b/packages/@aws-cdk/aws-lambda/lib/lambda-ref.ts @@ -320,7 +320,7 @@ class LambdaRefImport extends FunctionRef { super(parent, name); this.functionArn = props.functionArn; - this.functionName = this.extractNameFromArn(props.functionArn); + this.functionName = new FunctionName(this.extractNameFromArn(props.functionArn)); this.role = props.role; } @@ -341,4 +341,4 @@ class LambdaRefImport extends FunctionRef { return new cdk.FnSelect(6, new cdk.FnSplit(':', arn)); } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts b/packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts index d7bca3737dbe1..55770bdecd224 100644 --- a/packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts +++ b/packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts @@ -48,7 +48,7 @@ export class RemoteDesktopGateway extends cdk.Construct implements ec2.IConnecta }); const securityGroup = ec2.SecurityGroupRef.import(this, 'SecurityGroup', { - securityGroupId: nestedStack.getAtt('Outputs.RemoteDesktopGatewaySGID') + securityGroupId: new ec2.SecurityGroupId(nestedStack.getAtt('Outputs.RemoteDesktopGatewaySGID')) }); const defaultPortRange = new ec2.TcpPort(RemoteDesktopGateway.PORT); diff --git a/packages/@aws-cdk/aws-rds/lib/cluster.ts b/packages/@aws-cdk/aws-rds/lib/cluster.ts index 833c9995a0960..8dbd844ca1432 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster.ts @@ -3,7 +3,7 @@ import kms = require('@aws-cdk/aws-kms'); import cdk = require('@aws-cdk/cdk'); import { DatabaseClusterRef, Endpoint } from './cluster-ref'; import { BackupProps, DatabaseClusterEngine, InstanceProps, Login, Parameters } from './props'; -import { cloudformation, DBClusterName, DBInstanceId } from './rds.generated'; +import { cloudformation, DBClusterEndpointAddress, DBClusterEndpointPort, DBClusterName, DBInstanceId } from './rds.generated'; /** * Properties for a new database cluster @@ -169,7 +169,7 @@ export class DatabaseCluster extends DatabaseClusterRef { this.clusterIdentifier = cluster.ref; this.clusterEndpoint = new Endpoint(cluster.dbClusterEndpointAddress, cluster.dbClusterEndpointPort); - this.readerEndpoint = new Endpoint(cluster.dbClusterReadEndpointAddress, cluster.dbClusterEndpointPort); + this.readerEndpoint = new Endpoint(new DBClusterEndpointAddress(cluster.dbClusterReadEndpointAddress), cluster.dbClusterEndpointPort); const instanceCount = props.instances != null ? props.instances : 2; if (instanceCount < 1) { @@ -206,7 +206,9 @@ export class DatabaseCluster extends DatabaseClusterRef { } this.instanceIdentifiers.push(instance.ref); - this.instanceEndpoints.push(new Endpoint(instance.dbInstanceEndpointAddress, instance.dbInstanceEndpointPort)); + this.instanceEndpoints.push(new Endpoint( + new DBClusterEndpointAddress(instance.dbInstanceEndpointAddress), + new DBClusterEndpointPort(instance.dbInstanceEndpointPort))); } const defaultPortRange = new ec2.TcpPortFromAttribute(this.clusterEndpoint.port); diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index e4c94dab2f489..48bb6651ff19a 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -732,8 +732,13 @@ class ImportedBucketRef extends BucketRef { constructor(parent: cdk.Construct, name: string, props: BucketRefProps) { super(parent, name); + const bucketName = parseBucketName(props); + if (!bucketName) { + throw new Error('Bucket name is required'); + } + this.bucketArn = parseBucketArn(props); - this.bucketName = parseBucketName(props); + this.bucketName = bucketName; this.autoCreatePolicy = false; this.policy = undefined; } diff --git a/packages/@aws-cdk/aws-s3/lib/util.ts b/packages/@aws-cdk/aws-s3/lib/util.ts index 830f51a349fc7..a733a7ce7b5ab 100644 --- a/packages/@aws-cdk/aws-s3/lib/util.ts +++ b/packages/@aws-cdk/aws-s3/lib/util.ts @@ -1,7 +1,8 @@ import { Arn } from '@aws-cdk/cdk'; import { BucketRefProps } from './bucket'; +import { BucketArn, BucketName } from './s3.generated'; -export function parseBucketArn(props: BucketRefProps) { +export function parseBucketArn(props: BucketRefProps): BucketArn { // if we have an explicit bucket ARN, use it. if (props.bucketArn) { @@ -9,20 +10,20 @@ export function parseBucketArn(props: BucketRefProps) { } if (props.bucketName) { - return Arn.fromComponents({ + return new BucketArn(Arn.fromComponents({ // S3 Bucket names are globally unique in a partition, // and so their ARNs have empty region and account components region: '', account: '', service: 's3', resource: props.bucketName - }); + })); } throw new Error('Cannot determine bucket ARN. At least `bucketArn` or `bucketName` is needed'); } -export function parseBucketName(props: BucketRefProps) { +export function parseBucketName(props: BucketRefProps): BucketName | undefined { // if we have an explicit bucket name, use it. if (props.bucketName) { diff --git a/tools/cfn2ts/lib/codegen.ts b/tools/cfn2ts/lib/codegen.ts index ae1e2701ccd39..b3ee81f601c5a 100644 --- a/tools/cfn2ts/lib/codegen.ts +++ b/tools/cfn2ts/lib/codegen.ts @@ -481,7 +481,8 @@ export default class CodeGenerator { // different in TypeScript, which prevents assigning returning // incorrectly-typed Tokens. Those will cause ClassCastExceptions // in strictly-typed languages. - this.code.line(`private readonly thisIsA = '${attr.typeName}';`); + this.code.line('// @ts-ignore: private but unused on purpose.'); + this.code.line(`private readonly thisIsA${attr.typeName.className} = true;`); this.closeClass(attr.typeName); }