Unit test for S3BucketOrigin.withOriginAccessIdentity #31190
Conversation
| 'See the "Using pre-existing S3 buckets" section of module\'s README for more info. [ack: @aws-cdk/aws-cloudfront-origins:updateImportedBucketPolicy]'); | ||
| }); | ||
|
|
||
| it('should create OAI in bucket stack and output it, then reference the output in the distribution stack', () => { |
There was a problem hiding this comment.
This is specifically for testing the cross stack logic in the OAI bind method: https://github.com/samson-keung/aws-cdk/blob/9f5c1430bec9dd41246b83085cd4350b1ab1dffb/packages/aws-cdk-lib/aws-cloudfront-origins/lib/s3-bucket-origin.ts#L195-L199
Such logic is not in withOriginAccessControl, do we need it there?
There was a problem hiding this comment.
We won't be able to handle cross-stack references as easily for OAC because the bucket policy references the distribution, so the solution is to create the bucket and distribution in the same stack. Versus OAI where it just references the OAI user (so creating the OAI in the bucket stack easily resolves the circular dependency).
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.
| if (!result.statementAdded) { | ||
| Annotations.of(scope).addWarningV2('@aws-cdk/aws-cloudfront-origins:updateImportedBucketPolicy', | ||
| 'Cannot update bucket policy of an imported bucket. You may need to update the policy manually instead.\n' + | ||
| 'See the "Using pre-existing S3 buckets" section of module\'s README for more info.'); |
There was a problem hiding this comment.
Thanks for adding this! Note to self we will need to add a section to README for updating OAI imported bucket policies as well (and probably update this warning).
| 'See the "Using pre-existing S3 buckets" section of module\'s README for more info. [ack: @aws-cdk/aws-cloudfront-origins:updateImportedBucketPolicy]'); | ||
| }); | ||
|
|
||
| it('should create OAI in bucket stack and output it, then reference the output in the distribution stack', () => { |
There was a problem hiding this comment.
We won't be able to handle cross-stack references as easily for OAC because the bucket policy references the distribution, so the solution is to create the bucket and distribution in the same stack. Versus OAI where it just references the OAI user (so creating the OAI in the bucket stack easily resolves the circular dependency).
|
Comments on closed issues and PRs are hard for our team to see. |
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing |
Changes:
S3BucketOrigin.withOriginAccessIdentitywith different use caseS3BucketOrigin.withOriginAccessIdentity.bindwhen bucket is imported. So the behaviour is same asS3BucketOrigin.withOriginAccessControl