Skip to content

Commit

Permalink
fix(elbv2): unable to deploy template with IPv4 load balancer when de…
Browse files Browse the repository at this point in the history
…nyAllIgwTraffic set (#29956)

### Issue # (if applicable)

Closes #30247 .

### Reason for this change

Integ test for NLB attributes ([integ.nlb-attributes.ts](https://github.com/aws/aws-cdk/blob/4f1c94b27ef7f4ceccea0ff39625c0e8add31c9f/packages/%40aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.nlb-attributes.ts)) fails to deploy due to an error. The error occurs when `denyAllIgwTraffic` is explicitly set for load balancers with Ipv4 addressing, the `ipv6.deny_all_igw_traffic` attribute is set.

### Description of changes

- Remove the denyAllIgwTraffic setting from integ.nlb-attribute.ts
- Instead, set denyAllIgwTraffic in integ.nlb.dualstack.internal.ts.
- Raise an error during synthesis if `denyAllIgwTraffic` is set on a load balancer that does not use dual stack addressing.

### Description of how you validated changes

- Added new unit tests for different combinations of `denyAllIgwTraffic` and `ipAddressType`
- Updated existing integration test

### 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)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Michae1CC authored May 17, 2024
1 parent 1ba6e87 commit 42d424e
Show file tree
Hide file tree
Showing 20 changed files with 134 additions and 34 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -403,10 +403,6 @@
"Key": "load_balancing.cross_zone.enabled",
"Value": "true"
},
{
"Key": "ipv6.deny_all_igw_traffic",
"Value": "true"
},
{
"Key": "dns_record.client_routing_policy",
"Value": "partial_availability_zone_affinity"
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ new elbv2.NetworkLoadBalancer(stack, 'NLB', {
vpc,
crossZoneEnabled: true,
deletionProtection: false,
denyAllIgwTraffic: true,
clientRoutingPolicy: elbv2.ClientRoutingPolicy.PARTIAL_AVAILABILITY_ZONE_AFFINITY,
});

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@
{
"Key": "deletion_protection.enabled",
"Value": "false"
},
{
"Key": "ipv6.deny_all_igw_traffic",
"Value": "true"
}
],
"Scheme": "internal",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const subnetIpv6CidrBlocks = cdk.Fn.cidr(vpcIpv6CidrBlock, 256, '64');

const lb = new elbv2.NetworkLoadBalancer(stack, 'LB', {
vpc,
denyAllIgwTraffic: true,
ipAddressType: elbv2.IpAddressType.DUAL_STACK,
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Construct } from 'constructs';
import { IpAddressType } from './enums';
import { Attributes, ifUndefined, mapTagMapToCxschema, renderAttributes } from './util';
import * as ec2 from '../../../aws-ec2';
import * as iam from '../../../aws-iam';
Expand Down Expand Up @@ -251,7 +252,11 @@ export abstract class BaseLoadBalancer extends Resource {
}

if (baseProps.denyAllIgwTraffic !== undefined) {
this.setAttribute('ipv6.deny_all_igw_traffic', baseProps.denyAllIgwTraffic.toString());
if (additionalProps.ipAddressType === IpAddressType.DUAL_STACK) {
this.setAttribute('ipv6.deny_all_igw_traffic', baseProps.denyAllIgwTraffic.toString());
} else {
throw new Error(`'denyAllIgwTraffic' may only be set on load balancers with ${IpAddressType.DUAL_STACK} addressing.`);
}
}

this.loadBalancerCanonicalHostedZoneId = resource.attrCanonicalHostedZoneId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ describe('tests', () => {
idleTimeout: cdk.Duration.seconds(1000),
dropInvalidHeaderFields: true,
clientKeepAlive: cdk.Duration.seconds(200),
denyAllIgwTraffic: true,
preserveHostHeader: true,
xAmznTlsVersionAndCipherSuiteHeaders: true,
preserveXffClientPort: true,
Expand All @@ -99,10 +98,6 @@ describe('tests', () => {
Key: 'deletion_protection.enabled',
Value: 'true',
},
{
Key: 'ipv6.deny_all_igw_traffic',
Value: 'true',
},
{
Key: 'routing.http2.enabled',
Value: 'false',
Expand Down Expand Up @@ -171,6 +166,26 @@ describe('tests', () => {
}).toThrow('\'clientKeepAlive\' must be between 60 and 604800 seconds. Got: 100 milliseconds');
});

test.each([
[false, undefined],
[true, undefined],
[false, elbv2.IpAddressType.IPV4],
[true, elbv2.IpAddressType.IPV4],
])('throw error for denyAllIgwTraffic set to %s for Ipv4 (default) addressing.', (denyAllIgwTraffic, ipAddressType) => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Stack');

// THEN
expect(() => {
new elbv2.ApplicationLoadBalancer(stack, 'LB', {
vpc,
denyAllIgwTraffic: denyAllIgwTraffic,
ipAddressType: ipAddressType,
});
}).toThrow(`'denyAllIgwTraffic' may only be set on load balancers with ${elbv2.IpAddressType.DUAL_STACK} addressing.`);
});

describe('Desync mitigation mode', () => {
test('Defensive', () => {
// GIVEN
Expand Down Expand Up @@ -971,6 +986,27 @@ describe('tests', () => {
});
});

test('Can create internet-facing dualstack ApplicationLoadBalancer with denyAllIgwTraffic set to false', () => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Stack');

// WHEN
new elbv2.ApplicationLoadBalancer(stack, 'LB', {
vpc,
denyAllIgwTraffic: false,
internetFacing: true,
ipAddressType: elbv2.IpAddressType.DUAL_STACK,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::LoadBalancer', {
Scheme: 'internet-facing',
Type: 'application',
IpAddressType: 'dualstack',
});
});

test('Can create internal dualstack ApplicationLoadBalancer', () => {
// GIVEN
const stack = new cdk.Stack();
Expand All @@ -989,5 +1025,26 @@ describe('tests', () => {
IpAddressType: 'dualstack',
});
});

test.each([undefined, false])('Can create internal dualstack ApplicationLoadBalancer with denyAllIgwTraffic set to true', (internetFacing) => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Stack');

// WHEN
new elbv2.ApplicationLoadBalancer(stack, 'LB', {
vpc,
denyAllIgwTraffic: true,
internetFacing: internetFacing,
ipAddressType: elbv2.IpAddressType.DUAL_STACK,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::LoadBalancer', {
Scheme: 'internal',
Type: 'application',
IpAddressType: 'dualstack',
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ describe('tests', () => {
new elbv2.NetworkLoadBalancer(stack, 'LB', {
vpc,
crossZoneEnabled: true,
denyAllIgwTraffic: true,
clientRoutingPolicy: elbv2.ClientRoutingPolicy.PARTIAL_AVAILABILITY_ZONE_AFFINITY,
});

Expand All @@ -91,10 +90,6 @@ describe('tests', () => {
Key: 'load_balancing.cross_zone.enabled',
Value: 'true',
},
{
Key: 'ipv6.deny_all_igw_traffic',
Value: 'true',
},
{
Key: 'dns_record.client_routing_policy',
Value: 'partial_availability_zone_affinity',
Expand Down Expand Up @@ -488,6 +483,26 @@ describe('tests', () => {
}).toThrow('Load balancer name: "my load balancer" must contain only alphanumeric characters or hyphens.');
});

test.each([
[false, undefined],
[true, undefined],
[false, elbv2.IpAddressType.IPV4],
[true, elbv2.IpAddressType.IPV4],
])('throw error for denyAllIgwTraffic set to %s for Ipv4 (default) addressing.', (denyAllIgwTraffic, ipAddressType) => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Stack');

// THEN
expect(() => {
new elbv2.NetworkLoadBalancer(stack, 'NLB', {
vpc,
denyAllIgwTraffic: denyAllIgwTraffic,
ipAddressType: ipAddressType,
});
}).toThrow(`'denyAllIgwTraffic' may only be set on load balancers with ${elbv2.IpAddressType.DUAL_STACK} addressing.`);
});

test('imported network load balancer with no vpc specified throws error when calling addTargets', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down Expand Up @@ -1074,14 +1089,37 @@ describe('tests', () => {
});
});

test('Can create internal dualstack NetworkLoadBalancer', () => {
test('Can create internet-facing dualstack NetworkLoadBalancer with denyAllIgwTraffic set to false', () => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Stack');

// WHEN
new elbv2.NetworkLoadBalancer(stack, 'LB', {
vpc,
denyAllIgwTraffic: false,
internetFacing: true,
ipAddressType: elbv2.IpAddressType.DUAL_STACK,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::LoadBalancer', {
Scheme: 'internet-facing',
Type: 'network',
IpAddressType: 'dualstack',
});
});

test.each([undefined, false])('Can create internal dualstack NetworkLoadBalancer with denyAllIgwTraffic set to true', (internetFacing) => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Stack');

// WHEN
new elbv2.NetworkLoadBalancer(stack, 'LB', {
vpc,
denyAllIgwTraffic: true,
internetFacing: internetFacing,
ipAddressType: elbv2.IpAddressType.DUAL_STACK,
});

Expand Down

0 comments on commit 42d424e

Please sign in to comment.