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

fix(elasticloadbalancingv2): crossZoneEnabled does not support false for ALB #29907

Merged
merged 8 commits into from Apr 19, 2024

Conversation

pahud
Copy link
Contributor

@pahud pahud commented Apr 19, 2024

Issue # (if applicable)

Closes #29866

Reason for this change

crossZoneEnabled is not well handled when it's false with ALB. Because:

  1. When the L2 prop is set false, it will not pass down to the L1 and won't throw any error as ALB does not support being disabled. It just silently ignore it.
  2. When the prop is false for NLB, the L1 attribute will be undefined, which is having the same result but it should be explicitly set as false in L1.

This PR covers the following cases:

  1. When crossZoneEnabled is true, load_balancing.cross_zone.enabled should be true.
  2. When crossZoneEnabled is false, load_balancing.cross_zone.enabled should be false, rather than undefined.
  3. When crossZoneEnabled is false with ALB, cdk throws an error because ALB does not support disabling it per doc description.
  4. NLB supports either true or false.
  5. This prop can be undefined for ALB or NLB.
  6. Improve the doc string for the crossZoneEnabled prop.

Description of changes

Description of how you validated changes

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@aws-cdk-automation aws-cdk-automation requested a review from a team April 19, 2024 17:33
@github-actions github-actions bot added bug This issue is a bug. p2 labels Apr 19, 2024
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Apr 19, 2024
@pahud pahud marked this pull request as ready for review April 19, 2024 21:17
Copy link
Contributor

@GavinZZ GavinZZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

mergify bot commented Apr 19, 2024

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-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: d8936b9
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit f6c902e into aws:main Apr 19, 2024
9 checks passed
Copy link
Contributor

mergify bot commented Apr 19, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. contribution/core This is a PR that came from AWS. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws-elb: Disabling cross zone load balancing is not respected in Java CDK
4 participants