-
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
chore(cloudfront): prevent WebACL from being created in regions other than us-east-1 #32252
chore(cloudfront): prevent WebACL from being created in regions other than us-east-1 #32252
Conversation
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.
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.
if (webAclId.startsWith('arn:')) { | ||
const arnParts = Stack.of(this).splitArn(webAclId, ArnFormat.SLASH_RESOURCE_NAME); | ||
if (!Token.isUnresolved(arnParts.region) && arnParts.region !== 'us-east-1') { | ||
throw new Error(`WebACL for CloudFront distributions must be created in the us-east-1 region; received ${arnParts.region}`); | ||
} | ||
} |
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.
If a Web ACL created using AWS WAF Classic is specified, this validation will not be performed because the ACL ID received is not arn
.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #32252 +/- ##
==========================================
+ Coverage 78.48% 78.50% +0.01%
==========================================
Files 106 106
Lines 7201 7201
Branches 1321 1321
==========================================
+ Hits 5652 5653 +1
+ Misses 1362 1361 -1
Partials 187 187
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Exemption Request Because this MR does not have any modifications that would change the existing snapshot test, |
…ate-cloud-front-waf-region
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
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.
Thank you for the contribution.
Just one question.
* @param webAclId The WAF WebACL to associate with this distribution | ||
*/ | ||
public attachWebAclId(webAclId: string) { | ||
if (this.webAclId) { | ||
throw new Error('A WebACL has already been attached to this distribution'); | ||
} | ||
if (webAclId.startsWith('arn:')) { |
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.
Isn't this validation also necessary in the place where the webacl props is being passed?
this.webAclId = props.webAclId; |
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.
…ate-cloud-front-waf-region
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.
Thanks! LGTM
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). |
…ate-cloud-front-waf-region
Pull request has been modified.
I updated the branch because CI failed.(The cause is unknown...) Could you please approve it again? The modifications are as follows. |
Thank you. Please wait as approval from a maintainer is needed (I'm just a community reviewer, not a maintainer). |
…ate-cloud-front-waf-region
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). |
Pull request has been modified.
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. |
Reason for this change
When attaching a WebACL to CloudFront Distribution, the region must be
us-east-1
, but no validation was done.see: https://docs.aws.amazon.com/waf/latest/developerguide/web-acl-creating.html
Description of changes
Add validation to the
attachWebAclId
method of CloudFront DistributionDescription of how you validated changes
Unit and integ testing
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license