feat(elasticloadbalancingv2): transforms for ALB listener rule #35906
Conversation
github-actions
bot
added
the
distinguished-contributor
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
| */ | ||
| @MethodMetadata() | ||
| public addAction(id: string, props: AddApplicationActionProps): void { | ||
| checkAddRuleProps(this, props); |
There was a problem hiding this comment.
Because checkAddRuleProps() cannot accept transforms prop, I've added new validation function checkAddActionProps().
| } | ||
|
|
||
| private validateRewrites(rewrites: RewriteRule[]): void { | ||
| if (rewrites.length !== 1) { |
There was a problem hiding this comment.
In CloudFormation, an array of rewrite rules can be configured, but currently, deploying with multiple rules results in an error.
To prepare for future enhancements, the parameter is defined to accept an array, while validation restricts the number of elements to one.
transforms: [
elbv2.ListenerTransform.hostHeaderRewrite([
{
regex: '^(.*)\\.example\\.com$',
replace: '$1.internal.example.com',
},
{
regex: '^(.*)$',
replace: 'internal.example.com',
},
]),
],Resource handler returned message: "Rule transforms must have exactly one Rewrite (Service: ElasticLoadBalancingV2, Status Code: 400, Request ID: c2ecd108-daad-4c23-bc38-0c5b1348d04a) (SDK Attempt Count: 1)" (RequestToken: d66a6b69-d5c8-2d95-a439-4f422e5905c9, HandlerErrorCode: InvalidRequest)
|
|
||||||||||||||||||
|
|
||||||||||||||||||
mazyu36
left a comment
mazyu36
left a comment
There was a problem hiding this comment.
Thank you for the contribution.
I've added some comments.
| readonly removeSuffix?: boolean; | ||
|
|
||
| /** | ||
| * Transforms to apply to requests and responses |
There was a problem hiding this comment.
Just to confirm - responses aren't be transformed, correct?
There was a problem hiding this comment.
You're right!
| ); | ||
| } | ||
| } | ||
| if (hasTransforms && !hasPriority) { |
There was a problem hiding this comment.
Based on the error message, wouldn't !hasAnyConditions also be necessary?
There was a problem hiding this comment.
I've updated validation condition to if (hasTransforms && (!hasPriority || !hasAnyConditions)) {.
Validation for cases where conditions or priority are not set is already covered by existing tests, so I have not added new unit tests.
| // Rewrite host header | ||
| elbv2.ListenerTransform.hostHeaderRewrite([ | ||
| { | ||
| regex: '(.+).example.com', |
There was a problem hiding this comment.
Shouldn't the periods be escaped?
(.+)\\.example\\.com
There was a problem hiding this comment.
Yes, escaping is necessary. I've also added ^ and $ for better example.
aws-cdk-automation
removed
the
pr/needs-community-review
|
@mazyu36 Thank you very much for your review!! I've addressed all of your comments. |
|
@badmintoncryer |
|
@mazyu36 I’ve remembered that Rosetta cannot parse /^..$/ regular expressions. At this moment, I’ve avoided this expression and the build CI has passed. |
aws-cdk-automation
added
the
pr/needs-maintainer-review
|
@mazyu36 Thank you!! |
3 participants
Issue # (if applicable)
None
Reason for this change
ALB now supports for modifying host header and URL information. This feature has already supporrted by cloudformation.
https://docs.aws.amazon.com/ja_jp/elasticloadbalancing/latest/application/rule-transforms.html
Description of changes
ListenerTransformclass with factory methods:hostHeaderRewrite(): Transform host headers using regexpatterns
urlRewrite(): Transform URL paths using regex patternsAddApplicationActionPropsaddTransform()method toApplicationListenerRulerule
Describe any new or updated permissions being added
None
Description of how you validated changes
Add 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