Skip to content

Commit

Permalink
feat(ec2): subnet ipv4 cidr blocks on imported vpc (#23317)
Browse files Browse the repository at this point in the history 
When using `Vpc.fromVpcAttributes()` to import a VPC, it is possible to specify all subnet properties except for the IPv4 CIDR block. This means that any attempt to read the `ipv4CidrBlock` property of a subnet on such a VPC will throw with the message:

> You cannot reference an imported Subnet's IPv4 CIDR if it was not supplied. Add the ipv4CidrBlock when importing using Subnet.fromSubnetAttributes()

This PR extends the `VpcAttributes` interface to allow for subnet IPv4 CIDR blocks to be passed in alongside the IDs, names and route table IDs for each subnet group (public, private, isolated).

I have added a unit test showing how the values passed into `Vpc.fromVpcAttributes()` end up in the correct positions across the `ImportedSubnet` instances. As far as I can tell, there are no existing integration tests for this type of import, leading me to assume an integration test is not needed for this PR either.

After checking the current README in the `@aws-cdk/aws-ec2` module, I am of the impression that nothing needs to be added here. Let me know if you believe otherwise.

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@schourode
schourode authored Jan 4, 2023
1 parent 4e98c63 commit e0885db
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 9 deletions.
26 changes: 26 additions & 0 deletions packages/@aws-cdk/aws-ec2/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,32 @@ const vpc = ec2.Vpc.fromVpcAttributes(this, 'VPC', {
});
```

For each subnet group the import function accepts optional parameters for subnet
names, route table ids and IPv4 CIDR blocks. When supplied, the length of these
lists are required to match the length of the list of subnet ids, allowing the
lists to be zipped together to form `ISubnet` instances.

Public subnet group example (for private or isolated subnet groups, use the properties with the respective prefix):

```ts
const vpc = ec2.Vpc.fromVpcAttributes(this, 'VPC', {
vpcId: 'vpc-1234',
availabilityZones: ['us-east-1a', 'us-east-1b', 'us-east-1c'],
publicSubnetIds: ['s-12345', 's-34567', 's-56789'],
publicSubnetNames: ['Subnet A', 'Subnet B', 'Subnet C'],
publicSubnetRouteTableIds: ['rt-12345', 'rt-34567', 'rt-56789'],
publicSubnetIpv4CidrBlocks: ['10.0.0.0/24', '10.0.1.0/24', '10.0.2.0/24'],
});
```

The above example will create an `IVpc` instance with three public subnets:

| Subnet id | Availability zone | Subnet name | Route table id | IPv4 CIDR |
| --------- | ----------------- | ----------- | -------------- | ----------- |
| s-12345 | us-east-1a | Subnet A | rt-12345 | 10.0.0.0/24 |
| s-34567 | us-east-1b | Subnet B | rt-34567 | 10.0.1.0/24 |
| s-56789 | us-east-1c | Subnet B | rt-56789 | 10.0.2.0/24 |

## Allowing Connections

In AWS, all network traffic in and out of **Elastic Network Interfaces** (ENIs)
Expand Down
12 changes: 11 additions & 1 deletion packages/@aws-cdk/aws-ec2/lib/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,24 @@ export class ImportSubnetGroup {
private readonly subnetIds: string[];
private readonly names: string[];
private readonly routeTableIds: string[];
private readonly ipv4CidrBlocks: string[];
private readonly groups: number;

constructor(
subnetIds: string[] | undefined,
names: string[] | undefined,
routeTableIds: string[] | undefined,
ipv4CidrBlocks: string[] | undefined,
type: SubnetType,
private readonly availabilityZones: string[],
idField: string,
nameField: string,
routeTableIdField: string) {
routeTableIdField: string,
ipv4CidrBlockField: string) {

this.subnetIds = subnetIds || [];
this.routeTableIds = routeTableIds || [];
this.ipv4CidrBlocks = ipv4CidrBlocks || [];
this.groups = this.subnetIds.length / this.availabilityZones.length;

if (Math.floor(this.groups) !== this.groups) {
Expand All @@ -71,6 +75,11 @@ export class ImportSubnetGroup {
/* eslint-disable max-len */
throw new Error(`Number of ${routeTableIdField} (${this.routeTableIds.length}) must be equal to the amount of ${idField} (${this.subnetIds.length}).`);
}
if (this.ipv4CidrBlocks.length !== this.subnetIds.length && ipv4CidrBlocks != null) {
// We don't err if no ipv4CidrBlocks were provided to maintain backwards-compatibility.
/* eslint-disable max-len */
throw new Error(`Number of ${ipv4CidrBlockField} (${this.ipv4CidrBlocks.length}) must be equal to the amount of ${idField} (${this.subnetIds.length}).`);
}

this.names = this.normalizeNames(names, defaultSubnetName(type), nameField);
}
Expand All @@ -82,6 +91,7 @@ export class ImportSubnetGroup {
availabilityZone: this.pickAZ(i),
subnetId: this.subnetIds[i],
routeTableId: this.routeTableIds[i],
ipv4CidrBlock: this.ipv4CidrBlocks[i],
});
});
}
Expand Down
57 changes: 51 additions & 6 deletions packages/@aws-cdk/aws-ec2/lib/vpc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,65 +701,110 @@ export interface VpcAttributes {
* List of public subnet IDs
*
* Must be undefined or match the availability zones in length and order.
*
* @default - The VPC does not have any public subnets
*/
readonly publicSubnetIds?: string[];

/**
* List of names for the public subnets
*
* Must be undefined or have a name for every public subnet group.
*
* @default - All public subnets will have the name `Public`
*/
readonly publicSubnetNames?: string[];

/**
* List of IDs of routing tables for the public subnets.
* List of IDs of route tables for the public subnets.
*
* Must be undefined or have a name for every public subnet group.
*
* @default - Retrieving the route table ID of any public subnet will fail
*/
readonly publicSubnetRouteTableIds?: string[];

/**
* List of IPv4 CIDR blocks for the public subnets.
*
* Must be undefined or have an entry for every public subnet group.
*
* @default - Retrieving the IPv4 CIDR block of any public subnet will fail
*/
readonly publicSubnetIpv4CidrBlocks?: string[];

/**
* List of private subnet IDs
*
* Must be undefined or match the availability zones in length and order.
*
* @default - The VPC does not have any private subnets
*/
readonly privateSubnetIds?: string[];

/**
* List of names for the private subnets
*
* Must be undefined or have a name for every private subnet group.
*
* @default - All private subnets will have the name `Private`
*/
readonly privateSubnetNames?: string[];

/**
* List of IDs of routing tables for the private subnets.
* List of IDs of route tables for the private subnets.
*
* Must be undefined or have a name for every private subnet group.
*
* @default - Retrieving the route table ID of any private subnet will fail
*/
readonly privateSubnetRouteTableIds?: string[];

/**
* List of IPv4 CIDR blocks for the private subnets.
*
* Must be undefined or have an entry for every private subnet group.
*
* @default - Retrieving the IPv4 CIDR block of any private subnet will fail
*/
readonly privateSubnetIpv4CidrBlocks?: string[];

/**
* List of isolated subnet IDs
*
* Must be undefined or match the availability zones in length and order.
*
* @default - The VPC does not have any isolated subnets
*/
readonly isolatedSubnetIds?: string[];

/**
* List of names for the isolated subnets
*
* Must be undefined or have a name for every isolated subnet group.
*
* @default - All isolated subnets will have the name `Isolated`
*/
readonly isolatedSubnetNames?: string[];

/**
* List of IDs of routing tables for the isolated subnets.
* List of IDs of route tables for the isolated subnets.
*
* Must be undefined or have a name for every isolated subnet group.
*
* @default - Retrieving the route table ID of any isolated subnet will fail
*/
readonly isolatedSubnetRouteTableIds?: string[];

/**
* List of IPv4 CIDR blocks for the isolated subnets.
*
* Must be undefined or have an entry for every isolated subnet group.
*
* @default - Retrieving the IPv4 CIDR block of any isolated subnet will fail
*/
readonly isolatedSubnetIpv4CidrBlocks?: string[];

/**
* VPN gateway's identifier
*/
Expand Down Expand Up @@ -2084,9 +2129,9 @@ class ImportedVpc extends VpcBase {
}

/* eslint-disable max-len */
const pub = new ImportSubnetGroup(props.publicSubnetIds, props.publicSubnetNames, props.publicSubnetRouteTableIds, SubnetType.PUBLIC, this.availabilityZones, 'publicSubnetIds', 'publicSubnetNames', 'publicSubnetRouteTableIds');
const priv = new ImportSubnetGroup(props.privateSubnetIds, props.privateSubnetNames, props.privateSubnetRouteTableIds, SubnetType.PRIVATE_WITH_EGRESS, this.availabilityZones, 'privateSubnetIds', 'privateSubnetNames', 'privateSubnetRouteTableIds');
const iso = new ImportSubnetGroup(props.isolatedSubnetIds, props.isolatedSubnetNames, props.isolatedSubnetRouteTableIds, SubnetType.PRIVATE_ISOLATED, this.availabilityZones, 'isolatedSubnetIds', 'isolatedSubnetNames', 'isolatedSubnetRouteTableIds');
const pub = new ImportSubnetGroup(props.publicSubnetIds, props.publicSubnetNames, props.publicSubnetRouteTableIds, props.publicSubnetIpv4CidrBlocks, SubnetType.PUBLIC, this.availabilityZones, 'publicSubnetIds', 'publicSubnetNames', 'publicSubnetRouteTableIds', 'publicSubnetIpv4CidrBlocks');
const priv = new ImportSubnetGroup(props.privateSubnetIds, props.privateSubnetNames, props.privateSubnetRouteTableIds, props.privateSubnetIpv4CidrBlocks, SubnetType.PRIVATE_WITH_EGRESS, this.availabilityZones, 'privateSubnetIds', 'privateSubnetNames', 'privateSubnetRouteTableIds', 'privateSubnetIpv4CidrBlocks');
const iso = new ImportSubnetGroup(props.isolatedSubnetIds, props.isolatedSubnetNames, props.isolatedSubnetRouteTableIds, props.isolatedSubnetIpv4CidrBlocks, SubnetType.PRIVATE_ISOLATED, this.availabilityZones, 'isolatedSubnetIds', 'isolatedSubnetNames', 'isolatedSubnetRouteTableIds', 'isolatedSubnetIpv4CidrBlocks');
/* eslint-enable max-len */

this.publicSubnets = pub.import(this);
Expand Down
82 changes: 80 additions & 2 deletions packages/@aws-cdk/aws-ec2/test/vpc.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1222,10 +1222,11 @@ describe('vpc', () => {
NetworkInterfaceId: 'router-1',
});


});
});

test('fromVpcAttributes passes region correctly', () => {
describe('fromVpcAttributes', () => {
test('passes region correctly', () => {
// GIVEN
const stack = getTestStack();

Expand All @@ -1241,6 +1242,83 @@ describe('vpc', () => {
// THEN
expect(vpc.env.region).toEqual('region-12345');
});

test('passes subnet IPv4 CIDR blocks correctly', () => {
// GIVEN
const stack = new Stack();
const vpc = Vpc.fromVpcAttributes(stack, 'VPC', {
vpcId: 'vpc-1234',
availabilityZones: ['dummy1a', 'dummy1b', 'dummy1c'],
publicSubnetIds: ['pub-1', 'pub-2', 'pub-3'],
publicSubnetIpv4CidrBlocks: ['10.0.0.0/18', '10.0.64.0/18', '10.0.128.0/18'],
privateSubnetIds: ['pri-1', 'pri-2', 'pri-3'],
privateSubnetIpv4CidrBlocks: ['10.10.0.0/18', '10.10.64.0/18', '10.10.128.0/18'],
isolatedSubnetIds: ['iso-1', 'iso-2', 'iso-3'],
isolatedSubnetIpv4CidrBlocks: ['10.20.0.0/18', '10.20.64.0/18', '10.20.128.0/18'],
});

// WHEN
const public1 = vpc.publicSubnets.find(({ subnetId }) => subnetId === 'pub-1');
const public2 = vpc.publicSubnets.find(({ subnetId }) => subnetId === 'pub-2');
const public3 = vpc.publicSubnets.find(({ subnetId }) => subnetId === 'pub-3');
const private1 = vpc.privateSubnets.find(({ subnetId }) => subnetId === 'pri-1');
const private2 = vpc.privateSubnets.find(({ subnetId }) => subnetId === 'pri-2');
const private3 = vpc.privateSubnets.find(({ subnetId }) => subnetId === 'pri-3');
const isolated1 = vpc.isolatedSubnets.find(({ subnetId }) => subnetId === 'iso-1');
const isolated2 = vpc.isolatedSubnets.find(({ subnetId }) => subnetId === 'iso-2');
const isolated3 = vpc.isolatedSubnets.find(({ subnetId }) => subnetId === 'iso-3');

// THEN
expect(public1?.ipv4CidrBlock).toEqual('10.0.0.0/18');
expect(public2?.ipv4CidrBlock).toEqual('10.0.64.0/18');
expect(public3?.ipv4CidrBlock).toEqual('10.0.128.0/18');
expect(private1?.ipv4CidrBlock).toEqual('10.10.0.0/18');
expect(private2?.ipv4CidrBlock).toEqual('10.10.64.0/18');
expect(private3?.ipv4CidrBlock).toEqual('10.10.128.0/18');
expect(isolated1?.ipv4CidrBlock).toEqual('10.20.0.0/18');
expect(isolated2?.ipv4CidrBlock).toEqual('10.20.64.0/18');
expect(isolated3?.ipv4CidrBlock).toEqual('10.20.128.0/18');

});

test('throws on incorrect number of subnet names', () => {
const stack = new Stack();

expect(() =>
Vpc.fromVpcAttributes(stack, 'VPC', {
vpcId: 'vpc-1234',
availabilityZones: ['us-east-1a', 'us-east-1b', 'us-east-1c'],
publicSubnetIds: ['s-12345', 's-34567', 's-56789'],
publicSubnetNames: ['Public 1', 'Public 2'],
}),
).toThrow(/publicSubnetNames must have an entry for every corresponding subnet group/);
});

test('throws on incorrect number of route table ids', () => {
const stack = new Stack();

expect(() =>
Vpc.fromVpcAttributes(stack, 'VPC', {
vpcId: 'vpc-1234',
availabilityZones: ['us-east-1a', 'us-east-1b', 'us-east-1c'],
publicSubnetIds: ['s-12345', 's-34567', 's-56789'],
publicSubnetRouteTableIds: ['rt-12345'],
}),
).toThrow('Number of publicSubnetRouteTableIds (1) must be equal to the amount of publicSubnetIds (3).');
});

test('throws on incorrect number of subnet IPv4 CIDR blocks', () => {
const stack = new Stack();

expect(() =>
Vpc.fromVpcAttributes(stack, 'VPC', {
vpcId: 'vpc-1234',
availabilityZones: ['us-east-1a', 'us-east-1b', 'us-east-1c'],
publicSubnetIds: ['s-12345', 's-34567', 's-56789'],
publicSubnetIpv4CidrBlocks: ['10.0.0.0/18', '10.0.64.0/18'],
}),
).toThrow('Number of publicSubnetIpv4CidrBlocks (2) must be equal to the amount of publicSubnetIds (3).');
});
});

describe('NAT instances', () => {
Expand Down

0 comments on commit e0885db

Please sign in to comment.