feat: static s3 website construct proposal#19
feat: static s3 website construct proposal#19MarcioMeier wants to merge 3 commits intoflaviostutz:mainfrom
Conversation
flaviostutz
left a comment
flaviostutz
left a comment
There was a problem hiding this comment.
For me the component interface is nice. What do you think @erik-am @sergioflores-j?
docs/static-s3-website.md
Outdated
| @@ -0,0 +1,78 @@ | |||
| ## Construct StaticS3Website | |||
|
|
|||
| This construct creates an S3 bucket and upload any content into it. | |||
There was a problem hiding this comment.
Todo: change to "This construct creates a S3 bucket and uploads the contents of a local folder to it"
docs/static-s3-website.md
Outdated
| - [AWS S3 Bucket lib](https://github.com/aws/aws-cdk/tree/main/packages/aws-cdk-lib/aws-s3) | ||
| - [AWS S3 Bucket deployment lib](https://github.com/aws/aws-cdk/tree/main/packages/aws-cdk-lib/aws-s3-deployment) | ||
|
|
||
| Why to use it over the CDK constructs? |
There was a problem hiding this comment.
discussion: I would remove this paragraph as there isn't another construct with a similar behavior, so this is not "better", it's simply a new composition.
| deployments: [ | ||
| { | ||
| sources: [Source.asset('./dist', { exclude: ['*.html'] })], | ||
| cacheControl: [CacheControl.fromString('public,max-age=31536000,immutable')], |
There was a problem hiding this comment.
Question: this will make the GET of the files on S3 to have response header "cache-control" set? If so, pls add this comment to the example
There was a problem hiding this comment.
It will add the cache control metadata to the objects that will be uploaded by the sources selection.
In short, yes, it will add the cache-control headers to the files
|
|
||
| // Attaches the policy to the deployment bucket | ||
| staticWebsite.bucket.addToResourcePolicy(ipLimitPolicy) | ||
| ``` |
There was a problem hiding this comment.
Praise: The "complex" example is very nice!
| ```ts | ||
| // This will create the bucket `my-website-bucket` and upload the files in your local `dist` folder into it | ||
| new StaticS3Website(stack, 'my-website', { | ||
| bucketName: 'my-website-bucket', |
There was a problem hiding this comment.
Do the bucket name defaults to the id of the construct or it's a required prop?
There was a problem hiding this comment.
I'd rather have it as a required prop, so it be more explicit when building/reading the construct.
But having it as default is indeed an option.
2 participants
Summary
Proposal usage for #17