-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(elasticloadbalancingv2): alb dualstack without public ipv4 #30248
Conversation
To cdkteam, why not keep the IpAddressType.DUAL_STACK_WITHOUT_PUBLIC_IPV4 identifier but add the IpAddressType.IPV6 alias? This is essentially easy to understand. |
@@ -186,6 +186,11 @@ export class ApplicationLoadBalancer extends BaseLoadBalancer implements IApplic | |||
}); | |||
|
|||
this.ipAddressType = props.ipAddressType ?? IpAddressType.IPV4; | |||
|
|||
if (props.ipAddressType === IpAddressType.DUAL_STACK_WITHOUT_PUBLIC_IPV4 && !props.internetFacing) { | |||
throw new Error('dual-stack without public IPv4 address can only be used with internet-facing scheme.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have the link of the document about this restriction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pahud
Thank you for your comment.
That restriction is not mentioned in the documentation, but it is stated on the management console.
If the internal scheme is selected, the "Dual-stack without public IPv4" option disappears and cannot be configured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DUAL_STACK_WITHOUT_PUBLIC_IPV4
supports ALB only, should we have a check that if the elbv2 with that ipAddressType is not ALB we should throw the error?
Should we add this in base-load-balancer.ts?
// DUAL_STACK_WITHOUT_PUBLIC_IPV4 only available for ALB
if (additionalProps.ipAddressType === IpAddressType.DUAL_STACK_WITHOUT_PUBLIC_IPV4 &&
additionalProps.type !== cxschema.LoadBalancerType.APPLICATION) {
throw new Error(`'ipAddressType' DUAL_STACK_WITHOUT_PUBLIC_IPV4 can only be used with ALB, got ${additionalProps.type}`);
}
At this moment, CFN does not have |
@pahud |
Featured in the weekly roundup, adding this quickly will help teams drive lower costs. |
packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts
Outdated
Show resolved
Hide resolved
...mework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.dualstack-without-public-ipv4.ts
Outdated
Show resolved
Hide resolved
…balancingv2/test/integ.alb.dualstack-without-public-ipv4.ts Co-authored-by: Calvin Combs <[email protected]>
@comcalvi |
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
@Mergifyio update |
❌ Mergify doesn't have permission to updateFor security reasons, Mergify can't update this pull request. Try updating locally. |
@Mergifyio refresh |
✅ Pull request refreshed |
@Mergifyio requeue |
❌ Command disallowed due to command restrictions in the Mergify configuration.
|
@Mergifyio update |
✅ Branch has been successfully updated |
@Mergifyio refresh |
✅ Pull request refreshed |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #30256.
Reason for this change
ALB supported a new dual-stack ALB without public IPv4.
https://aws.amazon.com/jp/about-aws/whats-new/2024/05/application-load-balancer-ipv6-internet-clients/
Description of changes
Add
IpAddressType.DUAL_STACK_WITHOUT_PUBLIC_IPV4
Description of how you validated changes
Add unit tests & integ tests
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license