feat(elasticloadbalancingv2): application load balancer attributes#29586
feat(elasticloadbalancingv2): application load balancer attributes#29586mergify[bot] merged 12 commits intoaws:mainfrom
Conversation
aws-cdk-automation
left a comment
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.
eba9c25 to
9f869dd
Compare
| readonly clientKeepAlive?: Duration; | ||
|
|
||
| /** | ||
| * Indicates whether the Application Load Balancer should preserve the Host header in the HTTP request and send it to the target without any change. |
There was a problem hiding this comment.
nit: can you shorten these descriptions and put them on newlines?
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
| /** | ||
| * Application Load Balancer removes the X-Forwarded-For header in the HTTP request before it sends it to targets. | ||
| */ | ||
| REMOVE = 'remove', |
There was a problem hiding this comment.
nit: can you shorten these docs and move to multiple lines?
| * | ||
| * @default false | ||
| */ | ||
| readonly xAmznTlsVersionAndCipherSuiteHeaders?: boolean; |
There was a problem hiding this comment.
can you add @see values with links to AWS docs?
There was a problem hiding this comment.
@badmintoncryer Since the urls are the same, I think it'd make more sense to add the @see one time under Properties for defining an Application Load Balancer ?
There was a problem hiding this comment.
@msambol I was unable to retrieve links for each attribute and ended up entering the same URL...
I made the revisions just as you suggested!
| * | ||
| * @default false | ||
| */ | ||
| readonly preserveXffClientPort?: boolean; |
There was a problem hiding this comment.
likewise for this and below, can you add @see ?
|
@msambol Thank you for your review! I have updated comments. |
| denyAllIgwTraffic: false | ||
| denyAllIgwTraffic: false, | ||
|
|
||
| // Whether to preserve Host header in the request to the target |
| readonly clientKeepAlive?: Duration; | ||
|
|
||
| /** | ||
| * Indicates whether the Application Load Balancer should preserve the Host header in the HTTP request |
msambol
left a comment
There was a problem hiding this comment.
Two small nits. Looks great 👍🏼
|
@msambol Thank you very much! |
|
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). |
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). |
Issue # (if applicable)
Closes #29585.
Reason for this change
ALB supports some attributes that is not configurable from CDK
routing.http.preserve_host_header.enabledrouting.http.x_amzn_tls_version_and_cipher_suite.enabledrouting.http.xff_client_port.enabledrouting.http.xff_header_processing.modewaf.fail_open.enabledDescription of changes
Added some props to
ApplicationLoadBalancerProps.preserveHostHeaderxAmznTlsVersionAndCipherSuiteHeaderspreserveXffClientPortxffHeaderProcessingModewafFailOpenDescription of how you validated changes
Added both unit and integ tests.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license