Skip to content

Commit

Permalink
fix(ec2-alpha): readme updates, new unit tests, logic update (#33086)
Browse files Browse the repository at this point in the history
### Issue # (if applicable)

Closes #30762 .

### Reason for this change

Adding more unit tests to meet the global coverage before module graduation to `developer-preview`.

### Description of changes

- Add more unit test case to cover all `if branches` test cases.

- As per the discussion with service team, added optional field under IGW to allow users to choose the subnets for gateway routing, as there can be Public Subnet without an IGW attached( eg. using VPNGW to access internet).

- Update IPAM README to higlight the problem of IPAM pool deletion as discussed with service team.

- Update SubnetV2 README to higlight that a custom route table is being created through CDK.

### Describe any new or updated permissions being added

No changes to IAM permissions.

### Description of how you validated changes

`yarn build`
`yarn test`

```
yarn run v1.22.21
$ cdk-test
PASS test/ipam.test.ts
PASS test/subnet-v2.test.ts
PASS test/vpc-tagging.test.ts
PASS test/util.test.ts
PASS test/route.test.ts
PASS test/vpc-add-method.test.ts
PASS test/vpcv2-import.test.ts
PASS test/vpc-v2.test.ts

=============================== Coverage summary ===============================
Statements   : 89.88% ( 640/712 )
Branches     : 81.68% ( 223/273 )
Functions    : 82.6% ( 133/161 )
Lines        : 89.89% ( 614/683 )
================================================================================

Test Suites: 8 passed, 8 total
Tests:       126 passed, 126 total
Snapshots:   0 total
Time:        2.244 s
Ran all test suites.

Verifying integration test snapshots...

  UNCHANGED  integ.byoip-ipv6 0.888s
  UNCHANGED  integ.ipam 0.928s
  UNCHANGED  integ.subnet-v2 1.036s
  UNCHANGED  integ.vpc-v2-alpha 1.047s
  UNCHANGED  integ.test-import 1.053s
  UNCHANGED  integ.peering-cross-account 1.101s
  UNCHANGED  integ.vpc-v2-tagging 1.264s
  UNCHANGED  integ.route-v2 1.29s

Snapshot Results: 

Tests:    8 passed, 8 total
Tests successful. Total time (4.5s) | /Users/shikagg/vpc_peering/aws-cdk/node_modules/jest/bin/jest.js (2.7s) | integ-runner (1.8s)
✨  Done in 4.87s.
```

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

BREAKING CHANGE: `operatingRegion` property under IPAM class is now renamed to `operatingRegions`.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
shikha372 authored Jan 27, 2025
1 parent 1b666db commit bcb7f9b
Show file tree
Hide file tree
Showing 16 changed files with 675 additions and 44 deletions.
40 changes: 39 additions & 1 deletion packages/@aws-cdk/aws-ec2-alpha/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ new VpcV2(this, 'Vpc', {

`SubnetV2` is a re-write of the [`ec2.Subnet`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.Subnet.html) construct.
This new construct can be used to add subnets to a `VpcV2` instance:
Note: When defining a subnet with `SubnetV2`, CDK automatically creates a new route table, unless a route table is explicitly provided as an input to the construct.

```ts

Expand All @@ -66,11 +67,19 @@ By default `VpcV2` uses `10.0.0.0/16` as the primary CIDR if none is defined.
Additional CIDRs can be adding to the VPC via the `secondaryAddressBlocks` prop.
The following example illustrates the different options of defining the address blocks:

Note: There’s currently an issue with IPAM pool deletion that may affect the `cdk --destroy` command. This is because IPAM takes time to detect when the IP address pool has been deallocated after the VPC is deleted. The current workaround is to wait until the IP address is fully deallocated from the pool before retrying the deletion. Below command can be used to check allocations for a pool using CLI

```shell
aws ec2 get-ipam-pool-allocations --ipam-pool-id <ipam-pool-id>
```

Ref: https://docs.aws.amazon.com/cli/latest/reference/ec2/get-ipam-pool-allocations.html

```ts

const stack = new Stack();
const ipam = new Ipam(this, 'Ipam', {
operatingRegion: ['us-west-1']
operatingRegions: ['us-west-1']
});
const ipamPublicPool = ipam.publicScope.addPool('PublicPoolA', {
addressFamily: AddressFamily.IP_V6,
Expand Down Expand Up @@ -527,6 +536,7 @@ For more information, see [Enable VPC internet access using internet gateways](h

You can add an internet gateway to a VPC using `addInternetGateway` method. By default, this method creates a route in all Public Subnets with outbound destination set to `0.0.0.0` for IPv4 and `::0` for IPv6 enabled VPC.
Instead of using the default settings, you can configure a custom destination range by providing an optional input `destination` to the method.
In addition to the custom IP range, you can also choose to filter subnets where default routes should be created.

The code example below shows how to add an internet gateway with a custom outbound destination IP range:

Expand All @@ -546,6 +556,34 @@ myVpc.addInternetGateway({
});
```

The following code examples demonstrates how to add an internet gateway with a custom outbound destination IP range for specific subnets:

```ts
const stack = new Stack();
const myVpc = new VpcV2(this, 'Vpc');

const mySubnet = new SubnetV2(this, 'Subnet', {
vpc: myVpc,
availabilityZone: 'eu-west-2a',
ipv4CidrBlock: new IpCidr('10.0.0.0/24'),
subnetType: SubnetType.PUBLIC });

myVpc.addInternetGateway({
ipv4Destination: '192.168.0.0/16',
subnets: [mySubnet],
});
```

```ts
const stack = new Stack();
const myVpc = new VpcV2(this, 'Vpc');

myVpc.addInternetGateway({
ipv4Destination: '192.168.0.0/16',
subnets: [{subnetType: SubnetType.PRIVATE_WITH_EGRESS}],
});
```

## Importing an existing VPC

You can import an existing VPC and its subnets using the `VpcV2.fromVpcV2Attributes()` method or an individual subnet using `SubnetV2.fromSubnetV2Attributes()` method.
Expand Down
6 changes: 0 additions & 6 deletions packages/@aws-cdk/aws-ec2-alpha/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
const baseConfig = require('@aws-cdk/cdk-build-tools/config/jest.config');
module.exports = {
...baseConfig,
coverageThreshold: {
global: {
statements: 75,
branches: 63,
},
},
};;
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-ec2-alpha/lib/ipam.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export interface IpamProps {
*
* @default - Stack.region if defined in the stack
*/
readonly operatingRegion?: string[];
readonly operatingRegions?: string[];

/**
* Name of IPAM that can be used for tagging resource
Expand Down Expand Up @@ -511,11 +511,11 @@ export class Ipam extends Resource {
if (props?.ipamName) {
Tags.of(this).add(NAME_TAG, props.ipamName);
}
if (!props?.operatingRegion && !Stack.of(this).region) {
if (props?.operatingRegions && (props.operatingRegions.length === 0)) {
throw new Error('Please provide at least one operating region');
}

this.operatingRegions = props?.operatingRegion ?? [Stack.of(this).region];
this.operatingRegions = props?.operatingRegions ?? [Stack.of(this).region];
this.ipamName = props?.ipamName;

this._ipam = new CfnIPAM(this, 'Ipam', {
Expand Down
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-ec2-alpha/lib/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -605,12 +605,8 @@ export class RouteTargetType {
readonly endpoint?: IVpcEndpoint;

constructor(props: RouteTargetProps) {
if ((props.gateway && props.endpoint) || (!props.gateway && !props.endpoint)) {
throw new Error('Exactly one of `gateway` or `endpoint` must be specified.');
} else {
this.gateway = props.gateway;
this.endpoint = props.endpoint;
}
this.gateway = props.gateway;
this.endpoint = props.endpoint;
}
}

Expand Down Expand Up @@ -729,6 +725,10 @@ export class Route extends Resource implements IRouteV2 {
if (this.target.gateway?.routerType === RouterType.EGRESS_ONLY_INTERNET_GATEWAY && isDestinationIpv4) {
throw new Error('Egress only internet gateway does not support IPv4 routing');
}

if ((props.target.gateway && props.target.endpoint) || (!props.target.gateway && !props.target.endpoint)) {
throw new Error('Exactly one of `gateway` or `endpoint` must be specified.');
}
this.targetRouterType = this.target.gateway ? this.target.gateway.routerType : RouterType.VPC_ENDPOINT;
// Gateway generates route automatically via its RouteTable, thus we don't need to generate the resource for it
if (!(this.target.endpoint instanceof GatewayVpcEndpoint)) {
Expand Down
38 changes: 30 additions & 8 deletions packages/@aws-cdk/aws-ec2-alpha/lib/vpc-v2-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ export interface InternetGatewayOptions{
* @default - provisioned without a resource name
*/
readonly internetGatewayName?: string;

/**
* List of subnets where route to IGW will be added
*
* @default - route created for all subnets with Type `SubnetType.Public`
*/
readonly subnets?: SubnetSelection[];
}

/**
Expand Down Expand Up @@ -437,9 +444,14 @@ export abstract class VpcV2Base extends Resource implements IVpcV2 {
}

if (options?.subnets) {
// Use Set to ensure unique subnets
const processedSubnets = new Set<string>();
const subnets = flatten(options.subnets.map(s => this.selectSubnets(s).subnets));
subnets.forEach((subnet) => {
this.createEgressRoute(subnet, egw, options.destination);
if (!processedSubnets.has(subnet.node.id)) {
this.createEgressRoute(subnet, egw, options.destination);
processedSubnets.add(subnet.node.id);
}
});
}
}
Expand Down Expand Up @@ -476,9 +488,23 @@ export abstract class VpcV2Base extends Resource implements IVpcV2 {
this._internetConnectivityEstablished.add(igw);
this._internetGatewayId = igw.routerTargetId;

// If there are no public subnets defined, no default route will be added
if (this.publicSubnets) {
this.publicSubnets.forEach( (s) => this.addDefaultInternetRoute(s, igw, options));
// Add routes for subnets defined as an input
if (options?.subnets) {
// Use Set to ensure unique subnets
const processedSubnets = new Set<string>();
const subnets = flatten(options.subnets.map(s => this.selectSubnets(s).subnets));
subnets.forEach((subnet) => {
if (!processedSubnets.has(subnet.node.id)) {
if (!this.publicSubnets.includes(subnet)) {
Annotations.of(this).addWarningV2('InternetGatewayWarning',
`Subnet ${subnet.node.id} is not a public subnet. Internet Gateway should be added only to public subnets.`);
}
this.addDefaultInternetRoute(subnet, igw, options);
processedSubnets.add(subnet.node.id);
}
}); // If there are no input subnets defined, default route will be added to all public subnets
} else if (!options?.subnets && this.publicSubnets) {
this.publicSubnets.forEach((publicSubnets) => this.addDefaultInternetRoute(publicSubnets, igw, options));
}
}

Expand All @@ -488,10 +514,6 @@ export abstract class VpcV2Base extends Resource implements IVpcV2 {
*/
private addDefaultInternetRoute(subnet: ISubnetV2, igw: InternetGateway, options?: InternetGatewayOptions): void {

if (subnet.subnetType !== SubnetType.PUBLIC) {
throw new Error('No public subnets defined to add route for internet gateway');
}

// Add default route to IGW for IPv6
if (subnet.ipv6CidrBlock) {
new Route(this, `${subnet.node.id}-DefaultIPv6Route`, {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ec2-alpha/test/integ.ipam.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const app = new cdk.App();
const stack = new cdk.Stack(app, 'aws-cdk-vpcv2-alpha-integ-ipam');

const ipam = new Ipam(stack, 'IpamTest', {
operatingRegion: ['us-west-2'],
operatingRegions: ['us-west-2'],
});

/** Test Ipam Pool Ipv4 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const natgw = vpc.addNatGateway({
natgw.node.addDependency(vpnGateway);

const ipam = new Ipam(stack, 'IpamIntegTest', {
operatingRegion: ['us-west-2'],
operatingRegions: ['us-west-2'],
ipamName: 'CDKIpamTestTag',
});

Expand Down
111 changes: 108 additions & 3 deletions packages/@aws-cdk/aws-ec2-alpha/test/ipam.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('IPAM Test', () => {
env: envUSA,
});
ipam = new Ipam(stack, 'Ipam', {
operatingRegion: ['us-west-2'],
operatingRegions: ['us-west-2'],
});
});

Expand Down Expand Up @@ -82,7 +82,7 @@ describe('IPAM Test', () => {
test('Creates IPAM CIDR pool under public scope for IPv6', () => {
// Create IPAM resources
const ipamIpv6 = new Ipam(stack, 'TestIpam', {
operatingRegion: ['us-west-2'],
operatingRegions: ['us-west-2'],
});
const poolOptions: vpc.PoolOptions = {
addressFamily: AddressFamily.IP_V6,
Expand Down Expand Up @@ -116,7 +116,7 @@ describe('IPAM Test', () => {
test('Get region from stack env', () => {
// Create IPAM resources
const ipamRegion = new Ipam(stack, 'TestIpam', {
operatingRegion: ['us-west-2'],
operatingRegions: ['us-west-2'],
});
const poolOptions: vpc.PoolOptions = {
addressFamily: AddressFamily.IP_V6,
Expand Down Expand Up @@ -155,4 +155,109 @@ describe('IPAM Test', () => {
);
});

test('IPAM throws error if awsService is not provided for IPv6 address', () => {
// Create IPAM resources
const ipamRegion = new Ipam(stack, 'TestIpam', {
operatingRegions: ['us-west-2'],
});
const poolOptions: vpc.PoolOptions = {
addressFamily: AddressFamily.IP_V6,
publicIpSource: IpamPoolPublicIpSource.AMAZON,
locale: 'us-west-2',
};
expect(() => ipamRegion.publicScope.addPool('TestPool', poolOptions)).toThrow('awsService is required when addressFamily is set to ipv6');
});

test('IPAM throws error if operating region is provided as an empty array', () => {
const app = new cdk.App();
const stack_new = new cdk.Stack(app, 'TestStack');
expect(() => new Ipam(stack_new, 'TestIpam', {
operatingRegions: [],
})).toThrow('Please provide at least one operating region');
});

test('IPAM infers region from provided operating region correctly', () => {
const app = new cdk.App();
const stack_new = new cdk.Stack(app, 'TestStack');
new Ipam(stack_new, 'TestIpam', {
operatingRegions: ['us-west-2'],
});
Template.fromStack(stack_new).hasResourceProperties(
'AWS::EC2::IPAM', {
OperatingRegions: [
{
RegionName: 'us-west-2',
},
],
},
);
});

test('IPAM infers region from stack if not provided under IPAM class object', () => {
const app = new cdk.App();
const stack_new = new cdk.Stack(app, 'TestStack', {
env: {
region: 'us-west-2',
},
});
new Ipam(stack_new, 'TestIpam', {});
Template.fromStack(stack_new).hasResourceProperties(
'AWS::EC2::IPAM', {
OperatingRegions: [
{
RegionName: 'us-west-2',
},
],
},
);
});

test('IPAM refers to stack region token', () => {
const app = new cdk.App();
const stack_new = new cdk.Stack(app, 'TestStack');
new Ipam(stack_new, 'TestIpam', {});
Template.fromStack(stack_new).hasResourceProperties(
'AWS::EC2::IPAM', {
OperatingRegions: [
{
RegionName: {
Ref: 'AWS::Region',
},
},
],
},
);
});

test('IPAM throws error if locale(region) of pool is not one of the operating regions', () => {
const ipamRegion = new Ipam(stack, 'TestIpam', {
operatingRegions: ['us-west-2'],
});
const poolOptions: vpc.PoolOptions = {
addressFamily: AddressFamily.IP_V6,
awsService: vpc.AwsServiceName.EC2,
publicIpSource: IpamPoolPublicIpSource.AMAZON,
locale: 'us-west-1', // Incorrect Region
};
expect(() => ipamRegion.publicScope.addPool('TestPool', poolOptions)).toThrow("The provided locale 'us-west-1' is not in the operating regions.");
});

test('IPAM handles operating regions correctly', () => {
const new_app = new cdk.App();
const testStack = new cdk.Stack(new_app, 'TestStack', {
env: {
region: 'us-west-1',
},
});
new Ipam(testStack, 'TestIpamNew', {});
Template.fromStack(testStack).hasResourceProperties(
'AWS::EC2::IPAM', {
OperatingRegions: [
{
RegionName: 'us-west-1',
},
],
},
);
});
});// End Test
Loading

0 comments on commit bcb7f9b

Please sign in to comment.