feat: Add a construct to ease S3 bucket creation #635
Conversation
| import { GuMigratingS3Bucket } from "./migrating-bucket"; | ||
|
|
||
| describe("The GuMigratingS3Bucket construct", () => { | ||
| it("should auto-generate the logicalId by default", () => { |
There was a problem hiding this comment.
I don't think we'd ever want this construct to create a new logical id (since it is explicitly for migrating existing buckets)? Perhaps we could require the prop (or throw) instead of allowing this to happen?
There was a problem hiding this comment.
Good point! I actually don't think GuMigratingS3Bucket is in line with our other constructs, which are typically "Guardian flavoured versions of AWS constructs".
A Guardian flavoured bucket would be a tagged bucket with a retain policy. What if GuMigratingS3Bucket became GuS3Bucket, setting these props?
There was a problem hiding this comment.
Updated now. The diff is tiny as a result! 🎉
Guardian best practice is to retain S3 buckets as buckets share a global namespace. `GuS3Bucket` will set the correct policy on the bucket to simplify this. See: - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html
akash1810
force-pushed
the
aa-migrating-s3-construct
branch
from
5d3d094 to
9740afb
Compare
akash1810
changed the title
|
🎉 This PR is included in version 19.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this change?
Guardian best practice is to retain S3 buckets as buckets share a global namespace.
GuS3Bucketwill set the correct policy on the bucket to simplify this.See:
Does this change require changes to existing projects or CDK CLI?
No.
Does this change require changes to the library documentation?
Yes and it has been added.
How to test
See the added tests?
How can we measure success?
It is easier to create Guardian flavoured buckets.
Have we considered potential risks?
n/a