Skip to content

fix(ec2): dual-stack vpc without private subnets creates EgressOnlyInternetGateway (under feature flag) #34437

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
8 changes: 7 additions & 1 deletion packages/aws-cdk-lib/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1649,7 +1649,13 @@ export class Vpc extends VpcBase {
}

// Create an Egress Only Internet Gateway and attach it if necessary
if (this.useIpv6 && this.privateSubnets) {

const redundantEgressOnlyGatewayRemovalFeatureFlag =
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: variable name

Suggested change
const redundantEgressOnlyGatewayRemovalFeatureFlag =
const isRequirePrivateSubnetsForEgressOnlyIgw =

FeatureFlags.of(this).isEnabled(cxapi.ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC);

if ((this.useIpv6 && !redundantEgressOnlyGatewayRemovalFeatureFlag && this.privateSubnets) ||
(this.useIpv6 && redundantEgressOnlyGatewayRemovalFeatureFlag && this.privateSubnets.length > 0)
) {
const eigw = new CfnEgressOnlyInternetGateway(this, 'EIGW6', {
vpcId: this.vpcId,
});
Expand Down
86 changes: 85 additions & 1 deletion packages/aws-cdk-lib/aws-ec2/test/vpc.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { testDeprecated } from '@aws-cdk/cdk-build-tools';
import { Annotations, Match, Template } from '../../assertions';
import { App, CfnOutput, CfnResource, Fn, Lazy, Stack, Tags } from '../../core';
import { EC2_RESTRICT_DEFAULT_SECURITY_GROUP } from '../../cx-api';
import { EC2_RESTRICT_DEFAULT_SECURITY_GROUP, ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC } from '../../cx-api';
import {
AclCidr,
AclTraffic,
Expand Down Expand Up @@ -2747,6 +2747,90 @@ describe('vpc', () => {
},
});
});
test('(default)EgressOnlyInternetGateWay is created when no private subnet configured in dual stack', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of (default) EgressOnlyIGW.... maybe change this to say "EgressOnlyIGW is created when....in dual stack when feature flag is enabled."

// GIVEN
const app = new App();
const stack = new Stack(app, 'DualStackStack');

// WHEN
const vpc = new Vpc(stack, 'Vpc', {
ipProtocol: IpProtocol.DUAL_STACK,
subnetConfiguration: [
{
subnetType: SubnetType.PUBLIC,
name: 'public',
},
],
});

// THEN
Template.fromStack(stack).resourceCountIs('AWS::EC2::EgressOnlyInternetGateway', 1);
});
test('(default)EgressOnlyInternetGateWay is created when private subnet configured in dual stack', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above test.

// GIVEN
const app = new App();
const stack = new Stack(app, 'DualStackStack');

// WHEN
const vpc = new Vpc(stack, 'Vpc', {
ipProtocol: IpProtocol.DUAL_STACK,
subnetConfiguration: [
{
subnetType: SubnetType.PUBLIC,
name: 'public',
},
{
subnetType: SubnetType.PRIVATE_WITH_EGRESS,
name: 'private',
},
],
});

// THEN
Template.fromStack(stack).resourceCountIs('AWS::EC2::EgressOnlyInternetGateway', 1);
});

test('(feature flag ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC)EgressOnlyInternetGateWay is created when private subnet configured in dual stack', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
test('(feature flag ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC)EgressOnlyInternetGateWay is created when private subnet configured in dual stack', () => {
test('EgressOnlyInternetGateWay is created when private subnet configured in dual stack when feature flag is enabled', () => {

// GIVEN
const app = new App();
const stack = new Stack(app, 'DualStackStack');
// WHEN
stack.node.setContext(ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC, true);
const vpc = new Vpc(stack, 'Vpc', {
ipProtocol: IpProtocol.DUAL_STACK,
subnetConfiguration: [
{
subnetType: SubnetType.PUBLIC,
name: 'public',
},
{
subnetType: SubnetType.PRIVATE_WITH_EGRESS,
name: 'private',
},
],
});

// THEN
Template.fromStack(stack).resourceCountIs('AWS::EC2::EgressOnlyInternetGateway', 1);
});
test(' (feature flag ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC)EgressOnlyInternetGateWay is not created when private subnet configured in dual stack', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
test(' (feature flag ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC)EgressOnlyInternetGateWay is not created when private subnet configured in dual stack', () => {
test(' (feature flag ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC)EgressOnlyInternetGateWay is not created when no private subnets are configured in dual stack', () => {

// GIVEN
const app = new App();
const stack = new Stack(app, 'DualStackStack');
stack.node.setContext(ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC, true);
// WHEN
const vpc = new Vpc(stack, 'Vpc', {
ipProtocol: IpProtocol.DUAL_STACK,
subnetConfiguration: [
{
subnetType: SubnetType.PUBLIC,
name: 'public',
},
],
});
// THEN
Template.fromStack(stack).resourceCountIs('AWS::EC2::EgressOnlyInternetGateway', 0);
});

test('error should occur if IPv6 properties are provided for a non-dual-stack VPC', () => {
// GIVEN
Expand Down
19 changes: 18 additions & 1 deletion packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ Flags come in three types:
| [@aws-cdk/pipelines:reduceCrossAccountActionRoleTrustScope](#aws-cdkpipelinesreducecrossaccountactionroletrustscope) | When enabled, scopes down the trust policy for the cross-account action role | 2.189.0 | new default |
| [@aws-cdk/core:aspectPrioritiesMutating](#aws-cdkcoreaspectprioritiesmutating) | When set to true, Aspects added by the construct library on your behalf will be given a priority of MUTATING. | 2.189.1 | new default |
| [@aws-cdk/s3-notifications:addS3TrustKeyPolicyForSnsSubscriptions](#aws-cdks3-notificationsadds3trustkeypolicyforsnssubscriptions) | Add an S3 trust policy to a KMS key resource policy for SNS subscriptions. | 2.195.0 | fix |
| [@aws-cdk/ec2:removeEgressOnlyGatewayFromPublicSubnetVPC](#aws-cdkec2removeegressonlygatewayfrompublicsubnetvpc) | Remove EgressOnlyGateway resource when a a double stack vpc has only public subnets | V2NEXT | fix |

<!-- END table -->

Expand Down Expand Up @@ -183,7 +184,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/core:aspectPrioritiesMutating": true,
"@aws-cdk/aws-dynamodb:retainTableReplica": true,
"@aws-cdk/aws-stepfunctions:useDistributedMapResultWriterV2": true,
"@aws-cdk/s3-notifications:addS3TrustKeyPolicyForSnsSubscriptions": true
"@aws-cdk/s3-notifications:addS3TrustKeyPolicyForSnsSubscriptions": true,
"@aws-cdk/ec2:removeEgressOnlyGatewayFromPublicSubnetVPC": true
}
}
```
Expand Down Expand Up @@ -2105,4 +2107,19 @@ When this feature flag is enabled, a S3 trust policy will be added to the KMS ke
| 2.195.0 | `false` | `true` |


### @aws-cdk/ec2:removeEgressOnlyGatewayFromPublicSubnetVPC

*Remove EgressOnlyGateway resource when a a double stack vpc has only public subnets*

Flag type: Backwards incompatible bugfix

When this feature flag is enabled, EgressOnlyGateway resource will not be created when you create a vpc with only public subnets. A


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| V2NEXT | `false` | `true` |


<!-- END details -->
13 changes: 13 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export const DYNAMODB_TABLE_RETAIN_TABLE_REPLICA = '@aws-cdk/aws-dynamodb:retain
export const LOG_USER_POOL_CLIENT_SECRET_VALUE = '@aws-cdk/cognito:logUserPoolClientSecretValue';
export const PIPELINE_REDUCE_CROSS_ACCOUNT_ACTION_ROLE_TRUST_SCOPE = '@aws-cdk/pipelines:reduceCrossAccountActionRoleTrustScope';
export const S3_TRUST_KEY_POLICY_FOR_SNS_SUBSCRIPTIONS = '@aws-cdk/s3-notifications:addS3TrustKeyPolicyForSnsSubscriptions';
export const ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC = '@aws-cdk/ec2:removeEgressOnlyGatewayFromPublicSubnetVPC';
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 know it's not consistent but the convention is to prefix the service here with aws-. Also we should prefix the constant with the module name EC2- and update the name of the feature flag to more accurately reflect the behaviour.

Suggested change
export const ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC = '@aws-cdk/ec2:removeEgressOnlyGatewayFromPublicSubnetVPC';
export const EC2_REQUIRE_PRIVATE_SUBNETS_FOR_EGRESSONLYINTERNETGATEWAY = '@aws-cdk/aws-ec2:requirePrivateSubnetsForEgressOnlyInternetGateway';

export const USE_RESOURCEID_FOR_VPCV2_MIGRATION = '@aws-cdk/aws-ec2-alpha:useResourceIdForVpcV2Migration';

export const FLAGS: Record<string, FlagInfo> = {
Expand Down Expand Up @@ -1576,6 +1577,17 @@ export const FLAGS: Record<string, FlagInfo> = {
},

//////////////////////////////////////////////////////////////////////
[ENABLE_E2_REMOVE_EGRESSONLYGATEWAY_FROM_PUBLIC_SUBNET_VPC]: {
type: FlagType.BugFix,
summary: 'Remove EgressOnlyGateway resource when a a double stack vpc has only public subnets',
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem entirely correct. Rather than "removing EgressOnlyGateway when a stack only has public subnets", a more accurate summary could be "When enabled, the EgressOnlyGateway resource is only created if private subnets are defined in the VPC."

detailsMd: `
When this feature flag is enabled, EgressOnlyGateway resource will not be created when you create a vpc with only public subnets.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
},

/// ///////////////////////////////////////////////////////////////////
[USE_RESOURCEID_FOR_VPCV2_MIGRATION]: {
type: FlagType.ApiDefault,
summary: 'When enabled, use resource IDs for VPC V2 migration',
Expand All @@ -1588,6 +1600,7 @@ export const FLAGS: Record<string, FlagInfo> = {
recommendedValue: false,
defaults: { v2: false },
compatibilityWithOldBehaviorMd: 'Disable the feature flag to use getAtt references for VPC V2 migration',

},
};

Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk-lib/recommended-feature-flags.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,6 @@
"@aws-cdk/core:aspectPrioritiesMutating": true,
"@aws-cdk/aws-dynamodb:retainTableReplica": true,
"@aws-cdk/aws-stepfunctions:useDistributedMapResultWriterV2": true,
"@aws-cdk/s3-notifications:addS3TrustKeyPolicyForSnsSubscriptions": true
"@aws-cdk/s3-notifications:addS3TrustKeyPolicyForSnsSubscriptions": true,
"@aws-cdk/ec2:removeEgressOnlyGatewayFromPublicSubnetVPC": true
}
Loading