Skip to content

Commit

Permalink
chore(elbv2): backwards compatible on securityGroups
Browse files Browse the repository at this point in the history
  • Loading branch information
WinterYukky committed Jan 4, 2024
1 parent bea84c0 commit 0aa750c
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { BaseNetworkListenerProps, NetworkListener } from './network-listener';
import * as cloudwatch from '../../../aws-cloudwatch';
import * as ec2 from '../../../aws-ec2';
import * as cxschema from '../../../cloud-assembly-schema';
import { Lazy, Resource } from '../../../core';
import { FeatureFlags, Lazy, Resource } from '../../../core';
import * as cxapi from '../../../cx-api';
import { NetworkELBMetrics } from '../elasticloadbalancingv2-canned-metrics.generated';
import { BaseLoadBalancer, BaseLoadBalancerLookupOptions, BaseLoadBalancerProps, ILoadBalancerV2 } from '../shared/base-load-balancer';
Expand All @@ -24,7 +24,8 @@ export interface NetworkLoadBalancerProps extends BaseLoadBalancerProps {
/**
* Security groups to associate with this load balancer
*
* @default - No security groups associated with the load balancer.
* @default - Create a new seucrity group which allows all outbound traffic
* when `createDefaultSecurityGroup` is true, otherwise no security groups.
*/
readonly securityGroups?: ec2.ISecurityGroup[];

Expand All @@ -37,6 +38,13 @@ export interface NetworkLoadBalancerProps extends BaseLoadBalancerProps {
* @default IpAddressType.IPV4
*/
readonly ipAddressType?: IpAddressType;

/**
* Create a new security group when not specify `securityGroups`.
*
* @default - true when set `@aws-cdk/aws-elasticloadbalancingv2:nlbCreateDefaultSecurityGroup` feature flag, otherwise false.
*/
readonly createDefaultSecurityGroup?: boolean;
}

/**
Expand Down Expand Up @@ -199,22 +207,41 @@ export class NetworkLoadBalancer extends BaseLoadBalancer implements INetworkLoa
}

public readonly metrics: INetworkLoadBalancerMetrics;
public readonly securityGroups?: string[];
public readonly ipAddressType?: IpAddressType;
public readonly connections: ec2.Connections;
public get securityGroups() {
/**
* With the implementation of IConnectable, connections security groups are now used instead of the security group passed in props,
* however, since connections always return arrays that are not undefined and NLB does not allow mutual changes of undefined and empty array,
* when the connections security groups is an empty array, the original security group is specified for backwards compatible. (original can be undefined or empty array)
* https://github.com/aws/aws-cdk/pull/28494
*/
return this.connections.securityGroups.length > 0
? this.connections.securityGroups.map(sg => sg.securityGroupId)
: this.originalSecurityGroups?.map(sg => sg.securityGroupId);
}
/**
* The security groups that were passed in Props.
*/
private readonly originalSecurityGroups?: ec2.ISecurityGroup[];

constructor(scope: Construct, id: string, props: NetworkLoadBalancerProps) {
super(scope, id, props, {
type: 'network',
securityGroups: Lazy.list({
produce: () => this.connections.securityGroups.length >= 1 ? this.connections.securityGroups.map(sg => sg.securityGroupId) : undefined,
}),
securityGroups: Lazy.list({ produce: () => this.securityGroups }),
ipAddressType: props.ipAddressType,
});

this.metrics = new NetworkLoadBalancerMetrics(this, this.loadBalancerFullName);
this.securityGroups = props.securityGroups?.map(sg => sg.securityGroupId);
this.connections = new ec2.Connections({ securityGroups: props.securityGroups });
const createDefaultSecurityGroup = props.createDefaultSecurityGroup
?? FeatureFlags.of(this).isEnabled(cxapi.NLB_CREATE_DEFAULT_SECURITY_GROUP);
if (!props.securityGroups && createDefaultSecurityGroup) {
this.addSecurityGroup(new ec2.SecurityGroup(this, 'SecurityGroup', {
vpc: props.vpc,
}));
}
this.originalSecurityGroups = props.securityGroups;
this.ipAddressType = props.ipAddressType ?? IpAddressType.IPV4;
if (props.crossZoneEnabled) { this.setAttribute('load_balancing.cross_zone.enabled', 'true'); }
}
Expand All @@ -236,7 +263,6 @@ export class NetworkLoadBalancer extends BaseLoadBalancer implements INetworkLoa
*/
public addSecurityGroup(securityGroup: ec2.ISecurityGroup) {
this.connections.addSecurityGroup(securityGroup);
this.securityGroups?.push(securityGroup.securityGroupId);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as ec2 from '../../../aws-ec2';
import * as route53 from '../../../aws-route53';
import * as s3 from '../../../aws-s3';
import * as cdk from '../../../core';
import { NLB_CREATE_DEFAULT_SECURITY_GROUP } from '../../../cx-api';
import * as elbv2 from '../../lib';

describe('tests', () => {
Expand Down Expand Up @@ -656,18 +657,18 @@ describe('tests', () => {
test('Trivial construction: security groups', () => {
// GIVEN
const stack = new cdk.Stack();
stack.node.setContext(NLB_CREATE_DEFAULT_SECURITY_GROUP, true);
const vpc = new ec2.Vpc(stack, 'Stack');
const sg1 = new ec2.SecurityGroup(stack, 'SG1', { vpc });
const sg2 = new ec2.SecurityGroup(stack, 'SG2', { vpc });
const securityGroup = new ec2.SecurityGroup(stack, 'SG', { vpc });

// WHEN
const nlb = new elbv2.NetworkLoadBalancer(stack, 'LB', {
vpc,
internetFacing: true,
securityGroups: [sg1],
});
const defaultSecurityGroup = nlb.connections.securityGroups[0];
nlb.connections.allowFromAnyIpv4(ec2.Port.tcp(80));
nlb.addSecurityGroup(sg2);
nlb.addSecurityGroup(securityGroup);

// THEN
const template = Template.fromStack(stack);
Expand All @@ -680,13 +681,13 @@ describe('tests', () => {
SecurityGroups: [
{
'Fn::GetAtt': [
stack.getLogicalId(sg1.node.findChild('Resource') as cdk.CfnElement),
stack.getLogicalId(defaultSecurityGroup.node.findChild('Resource') as cdk.CfnElement),
'GroupId',
],
},
{
'Fn::GetAtt': [
stack.getLogicalId(sg2.node.findChild('Resource') as cdk.CfnElement),
stack.getLogicalId(securityGroup.node.findChild('Resource') as cdk.CfnElement),
'GroupId',
],
},
Expand All @@ -706,29 +707,61 @@ describe('tests', () => {
}, 2);
});

test('Trivial construction: no security groups', () => {
describe('Trivial construction: no security groups', () => {
test('Using createDefaultSecurityGroup props', () => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Stack');
const stack = new cdk.Stack();
stack.node.setContext(NLB_CREATE_DEFAULT_SECURITY_GROUP, true);
const vpc = new ec2.Vpc(stack, 'Stack');

// WHEN
const nlb = new elbv2.NetworkLoadBalancer(stack, 'LB', {
vpc,
internetFacing: true,
// WHEN
const nlb = new elbv2.NetworkLoadBalancer(stack, 'LB', {
vpc,
internetFacing: true,
createDefaultSecurityGroup: false,
});
nlb.connections.allowFromAnyIpv4(ec2.Port.tcp(80));

// THEN
const template = Template.fromStack(stack);
template.hasResourceProperties('AWS::ElasticLoadBalancingV2::LoadBalancer', {
Scheme: 'internet-facing',
Subnets: [
{ Ref: 'StackPublicSubnet1Subnet0AD81D22' },
{ Ref: 'StackPublicSubnet2Subnet3C7D2288' },
],
SecurityGroups: Match.absent(),
});
template.resourceCountIs('AWS::EC2::SecurityGroup', 0);
expect(nlb.securityGroups).toBeUndefined();
});
nlb.connections.allowFromAnyIpv4(ec2.Port.tcp(80));

// THEN
const template = Template.fromStack(stack);
template.hasResourceProperties('AWS::ElasticLoadBalancingV2::LoadBalancer', {
Scheme: 'internet-facing',
Subnets: [
{ Ref: 'StackPublicSubnet1Subnet0AD81D22' },
{ Ref: 'StackPublicSubnet2Subnet3C7D2288' },
],
SecurityGroups: Match.absent(),
test('Using feature flag', () => {
// GIVEN
const stack = new cdk.Stack();
stack.node.setContext(NLB_CREATE_DEFAULT_SECURITY_GROUP, false);
const vpc = new ec2.Vpc(stack, 'Vpc');

// WHEN
const nlb = new elbv2.NetworkLoadBalancer(stack, 'LB', {
vpc,
internetFacing: true,
});
nlb.connections.allowFromAnyIpv4(ec2.Port.tcp(80));

// THEN
const template = Template.fromStack(stack);
template.hasResourceProperties('AWS::ElasticLoadBalancingV2::LoadBalancer', {
Scheme: 'internet-facing',
Subnets: [
{ Ref: 'VpcPublicSubnet1Subnet5C2D37C4' },
{ Ref: 'VpcPublicSubnet2Subnet691E08A3' },
],
SecurityGroups: Match.absent(),
});
template.resourceCountIs('AWS::EC2::SecurityGroup', 0);
expect(nlb.securityGroups).toBeUndefined();
});
template.resourceCountIs('AWS::EC2::SecurityGroup', 0);
});

describe('lookup', () => {
Expand Down
21 changes: 20 additions & 1 deletion packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Flags come in three types:
| [@aws-cdk/aws-rds:auroraClusterChangeScopeOfInstanceParameterGroupWithEachParameters](#aws-cdkaws-rdsauroraclusterchangescopeofinstanceparametergroupwitheachparameters) | When enabled, a scope of InstanceParameterGroup for AuroraClusterInstance with each parameters will change. | 2.97.0 | (fix) |
| [@aws-cdk/aws-rds:preventRenderingDeprecatedCredentials](#aws-cdkaws-rdspreventrenderingdeprecatedcredentials) | When enabled, creating an RDS database cluster from a snapshot will only render credentials for snapshot credentials. | 2.98.0 | (fix) |
| [@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource](#aws-cdkaws-codepipeline-actionsusenewdefaultbranchforcodecommitsource) | When enabled, the CodeCommit source action is using the default branch name 'main'. | 2.103.1 | (fix) |
| [@aws-cdk/aws-elasticloadbalancingv2:nlbCreateDefaultSecurityGroup](#aws-cdkaws-elasticloadbalancingv2nlbcreatedefaultsecuritygroup) | When not provided security groups of props, then create a new security group. | V2NEXT | (default) |

<!-- END table -->

Expand Down Expand Up @@ -116,7 +117,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-rds:auroraClusterChangeScopeOfInstanceParameterGroupWithEachParameters": true,
"@aws-cdk/aws-appsync:useArnForSourceApiAssociationIdentifier": true,
"@aws-cdk/aws-rds:preventRenderingDeprecatedCredentials": true,
"@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource": true
"@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource": true,
"@aws-cdk/aws-elasticloadbalancingv2:nlbCreateDefaultSecurityGroup": true
}
}
```
Expand Down Expand Up @@ -1193,4 +1195,21 @@ However, with the activation of this feature flag, the default branch is updated
| 2.103.1 | `false` | `true` |


### @aws-cdk/aws-elasticloadbalancingv2:nlbCreateDefaultSecurityGroup

*When not provided security groups of props, then create a new security group.* (default)

This flag create a new security group and associate it to the load balancer when not provided security groups of props.
If this flag is not set, `NetworkLoadBalancer` will not create security group.
A network load balancer that not associated security group can not associate security groups until recreate.


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

**Compatibility with old behavior:** Pass `createDefaultSecurityGroup: false` to `NetworkLoadBalancer` construct to restore the old behavior.


<!-- END details -->
15 changes: 15 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export const RDS_PREVENT_RENDERING_DEPRECATED_CREDENTIALS = '@aws-cdk/aws-rds:pr
export const AURORA_CLUSTER_CHANGE_SCOPE_OF_INSTANCE_PARAMETER_GROUP_WITH_EACH_PARAMETERS = '@aws-cdk/aws-rds:auroraClusterChangeScopeOfInstanceParameterGroupWithEachParameters';
export const APPSYNC_ENABLE_USE_ARN_IDENTIFIER_SOURCE_API_ASSOCIATION = '@aws-cdk/aws-appsync:useArnForSourceApiAssociationIdentifier';
export const CODECOMMIT_SOURCE_ACTION_DEFAULT_BRANCH_NAME = '@aws-cdk/aws-codepipeline-actions:useNewDefaultBranchForCodeCommitSource';
export const NLB_CREATE_DEFAULT_SECURITY_GROUP = '@aws-cdk/aws-elasticloadbalancingv2:nlbCreateDefaultSecurityGroup';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -976,6 +977,20 @@ export const FLAGS: Record<string, FlagInfo> = {
introducedIn: { v2: '2.103.1' },
recommendedValue: true,
},

//////////////////////////////////////////////////////////////////////
[NLB_CREATE_DEFAULT_SECURITY_GROUP]: {
type: FlagType.ApiDefault,
summary: 'When not provided security groups of props, then create a new security group.',
detailsMd: `
This flag create a new security group and associate it to the load balancer when not provided security groups of props.
If this flag is not set, \`NetworkLoadBalancer\` will not create security group.
A network load balancer that not associated security group can not associate security groups until recreate.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
compatibilityWithOldBehaviorMd: 'Pass `createDefaultSecurityGroup: false` to `NetworkLoadBalancer` construct to restore the old behavior.',
},
};

const CURRENT_MV = 'v2';
Expand Down

0 comments on commit 0aa750c

Please sign in to comment.