Skip to content

Commit

Permalink
fix(ec2): deprecated SubnetType enums are treated incorrectly (#21140)
Browse files Browse the repository at this point in the history
In #19320, we changed the values of deprecated enums to include a `Deprecated_` prefix. This also meant we had to do some gymnastics in the code to change usages of the value to a switch case. 

However, there were also places in the code that do enum comparison, which uses the enum value. Those comparison points need to also explicitly consider the equivalent deprecated enums.  

Fixes #21131, Fixes #21138 

----

### All Submissions:

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

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-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
iliapolo authored Jul 14, 2022
1 parent 424b157 commit 0b5123a
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 2 deletions.
5 changes: 3 additions & 2 deletions packages/@aws-cdk/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ export class Vpc extends VpcBase {
this.createSubnets();

const allowOutbound = this.subnetConfiguration.filter(
subnet => (subnet.subnetType !== SubnetType.PRIVATE_ISOLATED)).length > 0;
subnet => (subnet.subnetType !== SubnetType.PRIVATE_ISOLATED && subnet.subnetType !== SubnetType.ISOLATED)).length > 0;

// Create an Internet Gateway and attach it if necessary
if (allowOutbound) {
Expand Down Expand Up @@ -2205,7 +2205,8 @@ class ImportedSubnet extends Resource implements ISubnet, IPublicSubnet, IPrivat
* They seem pointless but I see no reason to prevent it.
*/
function determineNatGatewayCount(requestedCount: number | undefined, subnetConfig: SubnetConfiguration[], azCount: number) {
const hasPrivateSubnets = subnetConfig.some(c => c.subnetType === SubnetType.PRIVATE_WITH_NAT && !c.reserved);
const hasPrivateSubnets = subnetConfig.some(c => (c.subnetType === SubnetType.PRIVATE_WITH_NAT
|| c.subnetType === SubnetType.PRIVATE) && !c.reserved);
const hasPublicSubnets = subnetConfig.some(c => c.subnetType === SubnetType.PUBLIC);

const count = requestedCount !== undefined ? Math.min(requestedCount, azCount) : (hasPrivateSubnets ? azCount : 0);
Expand Down
67 changes: 67 additions & 0 deletions packages/@aws-cdk/aws-ec2/test/vpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,73 @@ import {

describe('vpc', () => {
describe('When creating a VPC', () => {

test('SubnetType.PRIVATE is equivalent to SubnetType.PRIVATE_WITH_NAT', () => {

const stack1 = getTestStack();
const stack2 = getTestStack();
new Vpc(stack1, 'TheVPC', {
subnetConfiguration: [
{
subnetType: SubnetType.PRIVATE,
name: 'subnet',
},
{
subnetType: SubnetType.PUBLIC,
name: 'public',
},
],
});

new Vpc(stack2, 'TheVPC', {
subnetConfiguration: [
{
subnetType: SubnetType.PRIVATE_WITH_NAT,
name: 'subnet',
},
{
subnetType: SubnetType.PUBLIC,
name: 'public',
},
],
});

const t1 = Template.fromStack(stack1);
const t2 = Template.fromStack(stack2);

expect(t1.toJSON()).toEqual(t2.toJSON());

});

test('SubnetType.ISOLATED is equivalent to SubnetType.PRIVATE_ISOLATED', () => {

const stack1 = getTestStack();
const stack2 = getTestStack();
new Vpc(stack1, 'TheVPC', {
subnetConfiguration: [
{
subnetType: SubnetType.ISOLATED,
name: 'subnet',
},
],
});

new Vpc(stack2, 'TheVPC', {
subnetConfiguration: [
{
subnetType: SubnetType.PRIVATE_ISOLATED,
name: 'subnet',
},
],
});

const t1 = Template.fromStack(stack1);
const t2 = Template.fromStack(stack2);

expect(t1.toJSON()).toEqual(t2.toJSON());

});

describe('with the default CIDR range', () => {

test('vpc.vpcId returns a token to the VPC ID', () => {
Expand Down

0 comments on commit 0b5123a

Please sign in to comment.