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

(BREAKING) Enhancement - Custom VPC Subnets #250

Merged
merged 37 commits into from
Jul 24, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
28aea13
initial network builder design
Jul 6, 2018
2beb4a7
fixing tabs == 4 spaces in network-util.ts
Jul 10, 2018
2a172ed
moving to `get` methods for subnets, adding additional comment detail
Jul 10, 2018
9da7a2f
moving network-util vaildIp func to filter instead of reduce for read…
moofish32 Jul 10, 2018
55e3666
renaming of VpcSubnetBuilderProps to SubnetConfiguration; doc updates…
moofish32 Jul 10, 2018
a2b527d
removing custom error classes and asserting on regex message
moofish32 Jul 10, 2018
c9d4f39
removing more bad uses of reduce
moofish32 Jul 10, 2018
af75c41
network-utils doc improvements
moofish32 Jul 10, 2018
88a6d94
refactor complete snapping a line to debug
moofish32 Jul 11, 2018
9ac38f4
adding error for too many AZs requested
moofish32 Jul 11, 2018
60ef32d
additional comments for CIDR math and odd behaviors
moofish32 Jul 11, 2018
0f0079a
integ test can run but we are breaking existing here
moofish32 Jul 11, 2018
abd5086
making default vpc backwards compatible
moofish32 Jul 11, 2018
6dabebe
fixing linter errors
moofish32 Jul 11, 2018
7be9cc5
refactoring to keep backwards compatibility with vpc subnet generatio…
moofish32 Jul 12, 2018
2d1bb07
Merge remote-tracking branch 'origin/master' into f-improve-vpc-network
moofish32 Jul 12, 2018
1ef7db6
fixing imports removed incorrectly on conflict resolution
moofish32 Jul 12, 2018
06350b4
removed numazs
moofish32 Jul 12, 2018
455d9ef
fixing private subnet routes in default
moofish32 Jul 13, 2018
701858c
refactor to remove SubnetConfigFinalized
moofish32 Jul 15, 2018
5ecb64a
updating to support maxNatGateways
moofish32 Jul 16, 2018
a5ce5d9
updates for integ tests for rds and vpc
moofish32 Jul 18, 2018
2fbb213
fixing route53
moofish32 Jul 19, 2018
1777874
updating Changelog, README, and comments; reordered SubnetType to be …
moofish32 Jul 19, 2018
9d8067f
minor README in ec2 update
moofish32 Jul 19, 2018
d639de4
changes from the readme (impact to vpc.ts because the rename of the s…
moofish32 Jul 19, 2018
f2d4571
updating to fix `||` idiom; updating Changelog to explain breaking
moofish32 Jul 19, 2018
5c1bc5d
addressing code comments and tests for natGateway default behavior
moofish32 Jul 20, 2018
0864bb8
addressing comments for maxNatGateway -> natGateways, removing mapPub…
moofish32 Jul 23, 2018
776f1e0
refactoring cidrblock and network builder for number centric ip manag…
moofish32 Jul 23, 2018
82246ae
refactoring CidrBlock for constructor overloading, renaming nextIp ot…
moofish32 Jul 23, 2018
864a341
a couple of comment clarifications and making the CidrBlock construct…
moofish32 Jul 23, 2018
9722cdc
missed one README typo
moofish32 Jul 23, 2018
bba5e67
final README (ec2) correction to create Advanced Subnet Configuration
moofish32 Jul 23, 2018
c2baa77
Merge branch 'master' into f-improve-vpc-network
moofish32 Jul 24, 2018
31057b2
minor tweaks for renames from master merge
moofish32 Jul 24, 2018
f3f5428
README modification to match `import ec2 = require('@aws-cdk/aws-ec2')`
moofish32 Jul 24, 2018
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
300 changes: 234 additions & 66 deletions packages/@aws-cdk/ec2/lib/network-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,87 +3,255 @@
* range that is either not valid, or outside of the VPC size limits.
*/
export class InvalidCidrRangeError extends Error {
constructor(cidr: string) {
super(cidr + ' is not a valid VPC CIDR range (must be between /16 and /28)');
// The following line is required for type checking of custom errors, and must be called right after super()
// https://stackoverflow.com/questions/31626231/custom-error-class-in-typescript
Object.setPrototypeOf(this, InvalidCidrRangeError.prototype);
}
constructor(cidr: string) {
super(cidr + ' is not a valid VPC CIDR range (must be between /16 and /28)');
// The following line is required for type checking of custom errors, and must be called right after super()
// https://stackoverflow.com/questions/31626231/custom-error-class-in-typescript
Object.setPrototypeOf(this, InvalidCidrRangeError.prototype);
}
}

/**
* InvalidSubnetCountError is thrown when attempting to split a CIDR range into more
* subnets than it has IP space for.
*/
export class InvalidSubnetCountError extends Error {
constructor(cidr: string, count: number) {
super('VPC range (' + cidr + ') does not have enough IP space to be split into ' + count + ' subnets');
// The following line is required for type checking of custom errors, and must be called right after super()
// https://stackoverflow.com/questions/31626231/custom-error-class-in-typescript
Object.setPrototypeOf(this, InvalidSubnetCountError.prototype);
}
constructor(cidr: string, count: number) {
super('VPC range (' + cidr + ') does not have enough IP space to be split into ' + count + ' subnets');
// The following line is required for type checking of custom errors, and must be called right after super()
// https://stackoverflow.com/questions/31626231/custom-error-class-in-typescript
Object.setPrototypeOf(this, InvalidSubnetCountError.prototype);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix indent to 4 spaces


/**
* InvalidSubnetCidrError is thrown when attempting to split a CIDR range into more
* subnets than it has IP space for.
*/
export class InvalidSubnetCidrError extends Error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an issue with our cross-language binding tech (jsii) that doesn't allow you to subclass Error. I would just use the message content for test assertions:

assert.throws(() => doSomething(), /matchError/);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eladb should I still create custom Error classes and then only test for the matching text? Or are you saying don't even create subclasses of Error?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use Error. I know, it's not pretty.

constructor(cidr: string, subnet: string) {
super('VPC range (' + cidr + ') does not contain ' + subnet);
// The following line is required for type checking of custom errors, and must be called right after super()
// https://stackoverflow.com/questions/31626231/custom-error-class-in-typescript
Object.setPrototypeOf(this, InvalidSubnetCidrError.prototype);
}
}

/**
* InvalidIpAddressError is thrown when attempting to split a CIDR range into more
* subnets than it has IP space for.
*/
export class InvalidIpAddressError extends Error {
constructor(ip: string) {
super(ip + ' is not a valid IP must be (0-255).(0-255).(0-255).(0-255).');
// The following line is required for type checking of custom errors, and must be called right after super()
// https://stackoverflow.com/questions/31626231/custom-error-class-in-typescript
Object.setPrototypeOf(this, InvalidIpAddressError.prototype);
}
}

/**
* NetworkUtils contains helpers to work with network constructs (subnets/ranges)
*/
export class NetworkUtils {

/**
*
* splitCIDR takes a CIDR range (e.g. 10.0.0.0/16) and splits it into
* the provided number of smaller subnets (eg 2 of 10.0.0.0/17).
*
* @param {string} cidr The CIDR range to split (e.g. 10.0.0.0/16)
* @param {number} subnetCount How many subnets to create (min:2 max:30)
* @returns Array An array of CIDR strings (e.g. [ '10.0.0.0/17', '10.0.128.0/17' ])
*/
public static splitCIDR(cidr: string, subnetCount: number): string[] {

const parts = cidr.toString().split(/([0-9]+)\.([0-9]+)\.([0-9]+)\.([0-9]+)\/([0-9]+)/);

if (parts.length !== 7) {
throw new InvalidCidrRangeError(cidr);
}

const range = parseInt(parts[5], 10);
const rangeHosts = Math.pow(2, (32 - range));
const subnetSize = range + Math.round((subnetCount / 2));
const subnetHosts = Math.pow(2, (32 - subnetSize));

// Ensure the VPC cidr range fits within the EC2 VPC parameter ranges
if (range < 16 || range > 28) {
throw new InvalidCidrRangeError(cidr);
}

// Ensure the resulting subnet size is within the EC2 VPC parameter ranges
if (subnetSize < 16 || subnetSize > 28) {
throw new InvalidSubnetCountError(cidr, subnetCount);
}

// Check that the requested number of subnets fits into the provided CIDR range
if (subnetHosts === 0 || subnetHosts * subnetCount > rangeHosts) {
throw new InvalidSubnetCountError(cidr, subnetCount);
}

// Convert the initial CIDR to decimal format
const rangeDec = ((((((+parts[1]) * 256) + (+parts[2])) * 256) + (+parts[3])) * 256) + (+parts[4]);

// Generate each of the subnets required
const subnets: string[] = [];
for (let i = 0; i < subnetCount; i++) {
const subnetDec = rangeDec + (i * subnetHosts);
// tslint:disable:no-bitwise
const p1 = subnetDec & 255;
const p2 = ((subnetDec >> 8) & 255);
const p3 = ((subnetDec >> 16) & 255);
const p4 = ((subnetDec >> 24) & 255);
// tslint:enable:no-bitwise
subnets.push(p4 + '.' + p3 + '.' + p2 + '.' + p1 + '/' + subnetSize);
}

return subnets;
/**
*
* splitCIDR takes a CIDR range (e.g. 10.0.0.0/16) and splits it into
* the provided number of smaller subnets (eg 2 of 10.0.0.0/17).
*
* @param {string} cidr The CIDR range to split (e.g. 10.0.0.0/16)
* @param {number} subnetCount How many subnets to create (min:2 max:30)
* @returns Array An array of CIDR strings (e.g. [ '10.0.0.0/17', '10.0.128.0/17' ])
*/
public static splitCIDR(cidr: string, subnetCount: number): string[] {

const parts = cidr.toString().split(/([0-9]+)\.([0-9]+)\.([0-9]+)\.([0-9]+)\/([0-9]+)/);

if (parts.length !== 7) {
throw new InvalidCidrRangeError(cidr);
}

const range = parseInt(parts[5], 10);
const rangeHosts = Math.pow(2, (32 - range));
const subnetSize = range + Math.round((subnetCount / 2));
const subnetHosts = Math.pow(2, (32 - subnetSize));

// Ensure the VPC cidr range fits within the EC2 VPC parameter ranges
if (range < 16 || range > 28) {
throw new InvalidCidrRangeError(cidr);
}

// Ensure the resulting subnet size is within the EC2 VPC parameter ranges
if (subnetSize < 16 || subnetSize > 28) {
throw new InvalidSubnetCountError(cidr, subnetCount);
}

// Check that the requested number of subnets fits into the provided CIDR range
if (subnetHosts === 0 || subnetHosts * subnetCount > rangeHosts) {
throw new InvalidSubnetCountError(cidr, subnetCount);
}

// Convert the initial CIDR to decimal format
const rangeDec = ((((((+parts[1]) * 256) + (+parts[2])) * 256) + (+parts[3])) * 256) + (+parts[4]);

// Generate each of the subnets required
const subnets: string[] = [];
for (let i = 0; i < subnetCount; i++) {
const subnetDec = rangeDec + (i * subnetHosts);
// tslint:disable:no-bitwise
const p1 = subnetDec & 255;
const p2 = ((subnetDec >> 8) & 255);
const p3 = ((subnetDec >> 16) & 255);
const p4 = ((subnetDec >> 24) & 255);
// tslint:enable:no-bitwise
subnets.push(p4 + '.' + p3 + '.' + p2 + '.' + p1 + '/' + subnetSize);
}

return subnets;

}

public static validIp(ipAddress: string): boolean {
return ipAddress.split('.').map(
(octet: string) => (parseInt(octet, 10) >= 0 && parseInt(octet, 10) <= 255)).
reduce((valid: boolean, value: boolean) => (valid && value));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think "find" might be more readable than map/reduce, and you should also verify the address includes exactly 4 octets:

return ipAddress.split('.').map(parseInt).find(o >= 0 && o <= 255).length === 4

Alternatively, find a well-tested regex...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll move to your find solution. The regex googling had mixed reviews.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved to filter because find seemed to return only the first match. Still better than reduce since it's not really standard sum type of behavior?

}
/**
*
* takes an IP Address (e.g. 174.66.173.168) and converts to a number
* (e.g 2923605416); currently only supports IPv4
*
* Uses the formula:
* (first octet * 256³) + (second octet * 256²) + (third octet * 256) +
* (fourth octet)
*
* @param {string} the IP address (e.g. 174.66.173.168)
* @returns {number} the integer value of the IP address (e.g 2923605416)
*/

public static ipToNum(ipAddress: string): number {
if (!this.validIp(ipAddress)) {
throw new InvalidIpAddressError(ipAddress);
}

return ipAddress
.split('.')
.reduce(
(p: number, c: string, i: number) => p + parseInt(c, 10) * 256 ** (3 - i),
0
);
}

public static numToIp(ipNum: number): string {
// this all because bitwise math is signed
let remaining = ipNum;
const address = [];
for (let i = 0; i < 4; i++) {
if (remaining !== 0) {
address.push(Math.floor(remaining / 256 ** (3 - i)));
remaining = remaining % 256 ** (3 - i);
} else {
address.push(0);
}
}
const ipAddress: string = address.join('.');
if ( !this.validIp(ipAddress) ) {
throw new InvalidIpAddressError(ipAddress);
}
return ipAddress;
}

}

export class NetworkBuilder {
public readonly networkCidr: CidrBlock;
protected subnets: CidrBlock[];
protected maxIpConsumed: string;

constructor(cidr: string) {
this.networkCidr = new CidrBlock(cidr);
this.subnets = [];
this.maxIpConsumed = NetworkUtils.numToIp(NetworkUtils.
ipToNum(this.networkCidr.minIp()) - 1);
}

public addSubnet(mask: number): string {
return this.addSubnets(mask, 1)[0];
}

public addSubnets(mask: number, count?: number): string[] {
if (mask < 16 || mask > 28) {
throw new InvalidCidrRangeError(`x.x.x.x/${mask}`);
}
const subnets: CidrBlock[] = [];
if (!count) {
count = 1;
}
for (let i = 0; i < count; i ++) {
const subnet: CidrBlock = CidrBlock.fromOffsetIp(this.maxIpConsumed, mask);
this.maxIpConsumed = subnet.maxIp();
this.subnets.push(subnet);
if (!this.validate()) {
throw new InvalidSubnetCidrError(this.networkCidr.cidr, subnet.cidr);
}
subnets.push(subnet);
}
return subnets.map((subnet) => (subnet.cidr));
}

public getSubnets(): string[] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use getter: public get subnets(): string[]

return this.subnets.map((subnet) => (subnet.cidr));
}

public validate(): boolean {
return this.subnets.map(
(cidrBlock) => this.networkCidr.containsCidr(cidrBlock)).reduce(
(containsAll: boolean, contains: boolean) => (containsAll && contains));
}

}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually make sure to add jsdocs to all exported entities

export class CidrBlock {

public static calculateNetmask(mask: number): string {
return NetworkUtils.numToIp(2 ** 32 - 2 ** (32 - mask));
}

public static fromOffsetIp(ipAddress: string, mask: number): CidrBlock {
const initialCidr = new CidrBlock(`${ipAddress}/${mask}`);
const baseIpNum = NetworkUtils.ipToNum(initialCidr.maxIp()) + 1;
return new this(`${NetworkUtils.numToIp(baseIpNum)}/${mask}`);
}

public readonly cidr: string;
public readonly netmask: string;
public readonly mask: number;

constructor(cidr: string) {
this.cidr = cidr;
this.mask = parseInt(cidr.split('/')[1], 10);
this.netmask = CidrBlock.calculateNetmask(this.mask);
}

public maxIp(): string {
const minIpNum = NetworkUtils.ipToNum(this.minIp());
return NetworkUtils.numToIp(minIpNum + 2 ** (32 - this.mask) - 1);
}

public minIp(): string {
const netmaskOct = this.netmask.split('.');
const ipOct = this.cidr.split('/')[0].split('.');
// tslint:disable:no-bitwise
return netmaskOct.map(
(maskOct, index) => parseInt(maskOct, 10) & parseInt(ipOct[index], 10)).join('.');
// tslint:enable:no-bitwise
}

public containsCidr(cidr: CidrBlock): boolean {
return [
NetworkUtils.ipToNum(this.minIp()) <= NetworkUtils.ipToNum(cidr.minIp()),
NetworkUtils.ipToNum(this.maxIp()) >= NetworkUtils.ipToNum(cidr.maxIp()),
].reduce((contained: boolean, value: boolean) => (contained && value), true);
}
}
Loading