-
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
[@aws-cdk/aws-route53-patterns]: HttpsRedirect throws 'The parameter Comment is too big' #14020
Comments
Thanks for the bug report, @fbaba-nibtravel . This appears to be due to the CloudFront distribution comment having an (undocumented) limit of 128 characters. While we could fix this within the route53-patterns construct, I think a better solution would be to fix this in the Distribution (and CloudFrontWebDistribution) constructs themselves. The fix here is quite simple -- if the comment passed in is more than 128 characters, trim it down. This would be a great first issue for someone wanting to get started contributing to the CDK. We would just need to trim the comment in two places: https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts#L290 And add a test for each, of course! PRs welcome! |
@fbaba-nibtravel Do you want to take this? Otherwise, I would look into it. |
Hi @njlynch, thanks for the reply. I'll give it a go and will shout out if run into any trouble. |
|
I'm trying to create a HttpsRedirect on Route53 with CloudFront. When deploying, the CDK is throwing the below error.
The comment itself is auto-generated and the props interface for the
HttpsRedirect
does not allow to customize the comment. The comment pattern is:Redirect to <target> from <recordName>
.Reproduction Steps
What did you expect to happen?
Should create the route redirect using AWS CloudFront from a dns to a bucket target url
What actually happened?
The route fails to be created because of a comment property, not an invalid redirect.
Environment
Other
This is 🐛 Bug Report
The text was updated successfully, but these errors were encountered: