-
Notifications
You must be signed in to change notification settings - Fork 112
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
Server-side Website construct #44
Conversation
23c045a
to
0b18a6c
Compare
e218f2a
to
d81dcc9
Compare
d81dcc9
to
84fafe9
Compare
…gle custom subdomain
…rded-Host` using CloudFront Functions The `Host` header cannot be forwarded natively, because this breaks API Gateway. We use CloudFront Functions to set `X-Forwarded-Host` (using the original value in `Host`), and that header is forwarded to Lambda.
I'm not familiar enough with TypeScript to make a good code review, but I tested the construct (with Bref) on my end and it's working great! 🎉 The only thing I can notice is the |
Oh yes good catch, thank you! |
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.
Here are some things I spotted
Co-authored-by: Thibault RICHARD <thibault@widop.com>
Co-authored-by: Thibault RICHARD <thibault@widop.com>
# Conflicts: # src/classes/AwsProvider.ts
# Conflicts: # README.md # src/classes/AwsProvider.ts
src/constructs/ServerSideWebsite.ts
Outdated
"Origin", | ||
"Referer", | ||
// This header is set by our CloudFront Function | ||
"X-Forwarded-Host", |
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.
In #43 (comment) you said:
Some headers are whitelisted but you could easily run into cases where more headers are needed (in the case of the demo, it uses
X-Requested-With
to determine if the request is a XHR. Other headers likeX-Api-Key
or CORS related headers. Some custom headers might be needed too so I think it's important to allow configuration of the headers.
This is a good point. I've made headers configurable, but I'd love the construct to "get out of the way" and be usable with reasonable defaults.
Because of that, I'm seriously considering whitelisting more headers, i.e. the most common ones. For example X-Requested-With
, as well as CORS headers, but how can we build a reliable header list?
Do you see any other header we should probably include?
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.
Do you see any other header we should probably include?
I'm not sure what's the best approach here.
I think getting a list right is hard and the list of standard headers is very long. And some non-standard headers are more widespread than standard ones.
For example, x-api-key
or x-forwarded-*
headers are non standard, but their usage is probably higher than Range
or From
standard headers.
My point here is that any list we may provide will be biaised and the more we add, the more people will either want us to add more to the list, or ask us to remove some.
So i see two solutions
- We stay on the same track and maybe add some more headers to the mix like, but not limited to, CORS related,
X-Requested-With
, allAccept-*
,User-Agent
, allContent-*
- We allow every headers to be forwarded (maybe even include Cloudfront headers). This is less restrictive but is generally what is done by default by web servers like Apache or Nginx. We don't get in the way and this feels familiar for most users. We may still support a
forwardedHeaders
list but the user would be expected to provide each and every header they need : they choose to be picky and must know what they are doing.
Let me know what you think
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.
We allow every headers to be forwarded
That would be the ideal scenario, but we cannot forward the Host
header because it breaks API Gateway. (indeed, API Gateway uses the Host header to identify which API to call)
Ideally, we would "forward all except Host" but I don't think it's possible to exclude headers.
There's also this extreme solution, but it requires using a custom domain. And it's a bit weird.
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.
(FYI I've asked Eric Johnson from AWS, he's an API Gateway expert, maybe he has some ideas)
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.
And what about unsetting the host
header in the Cloudfront Function ? Would that work ?
That way we would allow all headers and remove just the host
header.
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.
Just tested this and Host
is a read-only header so we're unable to delete it in a Cloudfront function 😦
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.
And on top of that, OriginRequestHeaderBehavior.allowList
is limited to 10 headers. We're already setting 5 of them which leaves 5 headers for the end user to configure. That sucks but I don't see any other way to do this, that's an AWS limitation.
Let's wait for the feedback from Eric but if he has no proper solution, I think the current implementation is the way to go. And maybe add an Error when the user specify more than 5 custom headers (the CDK doesn't validate it, you'd get a Cloudformation error at deploy).
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.
No solution from the API Gateway team unfortunately :(
And on top of that, OriginRequestHeaderBehavior.allowList is limited to 10 headers.
This is aweful, where did you read that? I can't find any mention of it in the docs:
I keep coming back to this thinking this isn't ideal at all (to provide a generic component). Debugging why some headers are not received by a backend app will waste hours for developers (they'll never think about the serverless.yml config).
The 10
limit makes it much more complex: ideally I would want to whitelist the most common headers by default (even go up to 10 headers). But then if we make it configurable, that means user cannot add new headers, they would have to redefine the list completely to fit their needs. Which isn't great UX :/ I'm not sure what to do here TBH.
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.
This is aweful, where did you read that?
I tested some things out in the console and it's stated there. And that limit is also present when using Cloudformation. Didn't found useful docs either.
I think even with the 10 headers limit, it still has a great value. From my experience, most apps don't need that much headers.
I agree on the UX part and don't see an easy way to avoid this pitfall. Maybe one way could be to force users to write the list of headers and make it a required field. That way, it makes it explicit that headers need to be configured and we may provide a default list in the docs. Not ideal, I agree...
I check on serverlessland and found that they have a similar pattern implemented with CDK and don't worry about headers there apparently. I haven't looked at it in details but maybe it contains something valuable (or maybe they just don't care about headers...). Here it is : https://serverlessland.com/patterns/cloudfront-s3-lambda-cdk
Let me know if I can be of any help.
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.
OK thanks a lot @t-richard for all your advice.
You're right, the construct can still be very helpful to a lot of people, let's not stop on that.
What I did is:
- add
X-Requested-With
to the default list - make the YAML option an "override" instead of an "extend" -> that gives users control over 10 headers, instead of just 4. Later if needed we can add a
additionalHeadersToForward
option.
Last question on my mind: are there any other obvious header to forward? CloudFront has separate settings for cookies for examples, I need to test if they actually work correctly.
Except that, I think this problem is finally solved.
The reason for this change is that CloudFront only allows maximum 10 headers. If we stick with an "extend" behavior, then users will only be able to add 4 new headers (instead of controlling the 10 headers).
This PR is ready 🎉 |
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.
Just some minor comments but I'm happy with this PR 🎉
I reviewed the code and docs and I tested the construct again tonight and it looks great 👍
I've tested headers and cookies : it works as expected.
I have some more ideas to improve it but this can be added afterwards.
Thanks!
…gured to be forwarded See #44 (comment)
Thanks @t-richard, super useful feedback as usual! I've applied your comments, if the build is green that's going into a release! |
Let's go 🚀 |
This is an implementation of the Server-side Website construct described in the RFC.
Check out the full description below.
TODO:
The
server-side-website
construct deploys websites where the HTML is rendered "server-side", i.e. on AWS Lambda.This is usually done with backend frameworks like Laravel/Symfony (PHP), Ruby on Rails, Django/Flask (Python), Express (Node), etc.
To build a SPA or static website, use the Static Website construct instead.
Quick start
On
serverless deploy
, the example above will set up a website that serves both:https://<domain>/*
-> the website through API Gateway + Lambdahttps://<domain>/css/*
andhttps://<domain>/js/*
-> assets through S3Note: the first deployment takes 5 minutes.
The website is served over HTTPS and cached all over the world via the CloudFront CDN.
How it works
On the first
serverless deploy
, Lift creates:CloudFront serves the website over HTTPS with caching at the edge. It also provides a "HTTP to HTTPS" redirection which is not supported by API Gateway. For websites, this is problematic because it means someone typing
website.com
in a browser will get a blank page: API Gateway will not even redirect this to HTTPS.Finally, CloudFront also acts as a router:
The construct uses the API Gateway configured in functions defined in
serverless.yml
.Additionally, every time
serverless deploy
runs, Lift:Note: the S3 bucket is public and entirely managed by Lift. Do not store or upload files to the bucket, they will be removed by Lift on the next deployment. Instead, create a separate bucket to store any extra file.
CloudFront configuration
CloudFront is configured to cache static assets by default, but not cache dynamic content by default. It will forward cookies, query strings and most headers to the backend running on Lambda.
Website routes
To define website routes, create Lambda functions in
functions:
withhttpApi
events:Check out the official documentation on how to set up HTTP events.
When using backend frameworks that provide a routing feature, another option is to define a single Lambda function that captures all the HTTP routes:
Commands
serverless deploy
deploys everything configured inserverless.yml
and uploads assets.When iterating, it is possible to skip the CloudFormation deployment and directly publish changes via:
serverless deploy function -f <function-name>
to deploy a single Lambda functionserverless <construct-name>:assets:upload
to upload assets to S3 (the CloudFront cache will be cleared as well)Configuration reference
API Gateway
API Gateway provides 2 versions of APIs:
By default, the
server-side-website
construct supports v2 HTTP APIs.If your Lambda functions uses
http
events (v1 REST API) instead ofhttpApi
events (v2 HTTP API), use theapiGateway: "rest"
option:Assets
The
assets
section lets users define routing for static assets (like JavaScript, CSS, images, etc.).Assets can be either whole directories, or single files:
With the example above:
https://<domain>/*
-> Lambdahttps://<domain>/css/*
-> serves the files uploaded from the localdist/css
directoryhttps://<domain>/images/*
-> serves the files uploaded from the localassets/animations
directoryhttps://<domain>/favicon.ico
-> serves the file uploaded frompublic/favicon.ico
Custom domain
The configuration above will activate the custom domain
mywebsite.com
on CloudFront, using the provided HTTPS certificate.After running
serverless deploy
(orserverless info
), you should see the following output in the terminal:Create a CNAME DNS entry that points your domain to the
xxx.cloudfront.net
domain. After a few minutes/hours, the domain should be available.HTTPS certificate
To create the HTTPS certificate:
us-east-1
region (CDN certificates must be in us-east-1, regardless of where your application is hosted)admin@your-domain.com
After the certificate is created and validated, you should see the ARN of the certificate.
Multiple domains
It is possible to set up multiple domains:
Usually, we can retrieve which domain a user is visiting via the
Host
HTTP header. This doesn't work with API Gateway (Host
contains the API Gateway domain).The
server-side-website
construct offers a workaround: theX-Forwarded-Host
header is automatically populated via CloudFront Functions. Code running on Lambda will be able to access the originalHost
header via thisX-Forwarded-Host
header.Error pages
In case a web page throws an error, API Gateway's default
Internal Error
blank page shows up. This can be overridden by providing an HTML error page.Applications are of course free to catch errors and display custom error pages. However, sometimes even error pages and frameworks fail completely: this is where the API Gateway error page shows up.