fix(elasticloadbalancingv2): open, dual-stack-without-public-ipv4 ALB does not allow IPv6 inbound traffic (under feature flag) #32765
Conversation
github-actions
bot
added
bug
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
| @@ -218,7 +220,8 @@ Here is an example of a `cdk.json` file that restores v1 behavior for these flag | |||
| "@aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021": false, | |||
| "@aws-cdk/pipelines:reduceAssetRoleTrustScope": false, | |||
| "@aws-cdk/aws-stepfunctions-tasks:useNewS3UriParametersForBedrockInvokeModelTask": false, | |||
| "@aws-cdk/core:aspectStabilization": false | |||
| "@aws-cdk/core:aspectStabilization": false, | |||
| "@aws-cdk/aws-elasticloadbalancingV2:albDualstackWithoutPublicIpv4SecurityGroupRulesDefault": false | |||
There was a problem hiding this comment.
Feature flags with recommended value and default value set to true are getting added here even though they should not be. This section is for v1 feature flags where the behaviour changed. Will open a separate PR to fix this.
gracelu0
added
the
pr-linter/exempt-integ-test
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
@Mergifyio update |
✅ Branch has been successfully updated |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #32765 +/- ##
=======================================
Coverage 81.37% 81.37%
=======================================
Files 222 222
Lines 13693 13693
Branches 2412 2412
=======================================
Hits 11143 11143
Misses 2271 2271
Partials 279 279
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Co-authored-by: Clare Liguori <[email protected]>
gracelu0
force-pushed
the
dual-stack-ipv6-feature-flag
branch
from
9e01387 to
7c5dc35
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
aws-cdk-automation
added
the
pr/needs-maintainer-review
| @@ -303,7 +303,9 @@ export class ApplicationListener extends BaseListener implements IApplicationLis | |||
|
|
|||
| if (props.open !== false) { | |||
| this.connections.allowDefaultPortFrom(ec2.Peer.anyIpv4(), `Allow from anyone on port ${port}`); | |||
| if (this.loadBalancer.ipAddressType === IpAddressType.DUAL_STACK) { | |||
| if (this.loadBalancer.ipAddressType === IpAddressType.DUAL_STACK || | |||
| (this.loadBalancer.ipAddressType === IpAddressType.DUAL_STACK_WITHOUT_PUBLIC_IPV4 && | |||
There was a problem hiding this comment.
I recommend updating the doc for IpAddressType.DUAL_STACK_WITHOUT_PUBLIC_IPV4 to mention how it behaves differently depending on the feature flag.
| Using a feature flag to make sure existing customers who might be relying | ||
| on the overly restrictive permissions are not broken.`, | ||
| introducedIn: { v2: 'V2NEXT' }, | ||
| defaults: { v2: true }, |
There was a problem hiding this comment.
I believe setting the default to true means existing projects will automatically have this feature flag as true, which means they will get the new behaviour automatically, which is a breaking change. Can you double check?
There was a problem hiding this comment.
Thanks Samson, you're correct that they would automatically get the new ingress rules upon re-deploying. I set the default to true thinking we would want new projects to setup the ingress rules to allow IPv6 by default, but it looks like that is what the recommended value is for. Will update.
aws-cdk-automation
removed
the
pr/needs-maintainer-review
Issue # (if applicable)
Closes #32197 .
Reason for this change
Default generated security group ingress rules for open, dual-stack-without-public-ipv4 ALB does not allow IPv6 traffic. Only a rule for IPv4 ingress traffic is added to the security group rules currently.
Description of changes
Introduced a new feature flag which is enabled by default so that default generated security group ingress rules now have an additional rule that allows IPv6 ingress from anywhere.
Describe any new or updated permissions being added
No new IAM permissions. Added IPv6 security group ingress rules for open, internet-facing ALBs if IP address type is
dual-stack-without-public-ipv4and feature flag is set totrue(default).Description of how you validated changes
Added unit test which checks the security group rules for both cases where feature flag is enabled/disabled. Updated integration test snapshot.
Checklist
Co-authored-by: Clare Liguori [email protected]
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license