Skip to content
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

(aws-elasticloadbalancingv2): Prevent using denyAllIgwTraffic for load balancers that do not use dual stack addressing #30247

Closed
2 tasks
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 bug This issue is a bug. feature-request A feature should be added or improved. p2 testing Related to tests needed to be modified or added.

Comments

@moelasmar
Copy link
Contributor

Describe the feature

Setting denyAllIgwTraffic flag for Load balancers with IPV4 addressing is not allowed, and cause this deployment error Load balancer attribute key 'ipv6.deny_all_igw_traffic' is not supported on load balancers with IP address type 'ipv4'.

Sample usecase:

new elbv2.NetworkLoadBalancer(stack, 'NLB', {
  vpc,
  crossZoneEnabled: true,
  deletionProtection: false,
  denyAllIgwTraffic: true,
  clientRoutingPolicy: elbv2.ClientRoutingPolicy.PARTIAL_AVAILABILITY_ZONE_AFFINITY,
});

Also, there are some integration test cases that could not be deployed because of this issue.

Use Case

It is better to figure out this issue during synthesize time instead of deployment time.

Proposed Solution

Prevent customers form setting denyAllIgwTraffic flag is the Load balancers do not use dual stack addressing.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

v2.142.0

Environment details (OS name and version, etc.)

macos

@moelasmar moelasmar added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. bug This issue is a bug. labels May 17, 2024
@github-actions github-actions bot added the @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 label May 17, 2024
@moelasmar moelasmar added testing Related to tests needed to be modified or added. @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 and removed needs-triage This issue or PR still needs to be triaged. @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 labels May 17, 2024
@moelasmar moelasmar added the p2 label May 17, 2024
@mergify mergify bot closed this as completed in #29956 May 17, 2024
mergify bot pushed a commit that referenced this issue May 17, 2024
…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*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

atanaspam pushed a commit to atanaspam/aws-cdk that referenced this issue Jun 3, 2024
…nyAllIgwTraffic set (aws#29956)

### Issue # (if applicable)

Closes aws#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*
@aws-cdk-automation
Copy link
Collaborator

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.