-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat(experimental): Add support for using a private S3 bucket #29
Conversation
✅ Your deploy preview is ready: https://96d03a4f.serverlessui.app |
Hey @JakePartusch, I have a need for deploying a static site without opening public access to the S3 bucket and this change seems to fit my needs perfectly. Is there any way I can help get this change merged into main? |
Hey @damc-dev, I didn't end up merging because of the issue here: #26 (reply in thread) I wasn't able to find a good workaround other than deploying an Edge Lambda... And the Edge lambda would be tricky because it would need to make some assumptions about how the routing in the app would work 😕 |
So I was able to get it working with a CloudFront function in my fork serverless-ui.construct.ts#L171 in a pretty generic way that works for both of your example applications. simple+api: Site | CloudFront to API gatsby: Site I am sure there are other cases that I am missing, would you be able to elaborate on the cases were this assumption wouldn't work? |
Yeah @damc-dev , I should've been more specific about the problem that I've seen when using OAI. For example:
As far as I understand, this happens because page-2 is not an object in the bucket, as the object is actually /page-2/index.html. Typical public S3 websites are smart enough to figure this out, but OAI breaks it 😕. So the way that I would normally go about fixing that is to use an Edge lambda to reroute the path to /page-2/index.html if the resource receives an error on the initial route. |
Interesting, I had no idea that Gatsby routing compensated for that, but would error when navigating there directly. I updated the CloudFront function to perform the same action that is done by default in S3 (copied from github.com/aws-samples/amazon-cloudfront-functions/url-rewrite-single-page-apps) I think it should solve the problem! updated simple+api: Site | CloudFront to API updated gatsby: Site |
I created a PR #38 to update your feature branch, if you would prefer the PR request was against main let me know! |
* updated to latest stable cdk * Added CloudFront function to rewrite URLs * Modified CloudFront function to add /index.html to any request without a filename Co-authored-by: David McElligott <dmmcelli@amazon.com>
Thanks, @damc-dev! I'm thinking about putting this behind a feature flag for now... Also, it looks like I have some conflicts to resolve. Hopefully I can get this updated over the weekend. Thanks for your help and feedback! |
Yeah I had to update to the latest CDK to support deploying CloudFront functions, in hindsight a separate PR to just update the CDK might have been a good idea. Sorry for dumping the extra integration work in your lap. I know a lot of organizations have control policies to disable the ability to make buckets public so this pattern will allow them to use your awesome solution. I wouldn't be surprised if there are other challenges introduced via this approach we haven't ran into yet, so I like the idea of a feature flag at-least temporarily! Thanks for all your hard work on this great solution @JakePartusch! |
…eature/oai-private-s3
…sch/serverlessui into feature/oai-private-s3
✅ Your deploy preview is ready: https://bd9abf26.serverlessui.app |
✅ Your deploy preview is ready: https://864b79d4.serverlessui.app |
✅ Your deploy preview is ready: https://9d7201b1.serverlessui.app |
✅ Your deploy preview is ready: https://57a34edc.serverlessui.app |
✅ Your deploy preview is ready: https://c3d7aede.serverlessui.app |
✅ Your deploy preview is ready: https://73405bac.serverlessui.app |
TODO: