Skip to content
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

Allow use of Serverless Framework instead of CDK #43

Closed
john-tipper opened this issue Oct 6, 2022 · 3 comments · Fixed by #44
Closed

Allow use of Serverless Framework instead of CDK #43

john-tipper opened this issue Oct 6, 2022 · 3 comments · Fixed by #44

Comments

@john-tipper
Copy link
Contributor

Serverless Framework has a deployment approach of a serverless.yml file defining a single API Gateway and multiple lambdas. This is different to the current CDK implementation where each lambda is associated with a separate APIG and each APIG is associated with a separate CloudFront origin.

If you have 2 lambdas associated with a single APIG, then each lambda needs to be given a different path, e.g. the server-handler lambda being associated with /server/* paths and the image-handler lambda being associated with /image/* paths. CloudFront allows for originPaths where different origins will use a different prefix when making requests to the origins. Thus it is possible to user Serverless Framework to route traffic to different origins, where each origin uses the SAME shared APIG but with different prefixes.

PROBLEM 1: The server-handler lambda uses the full path of the request passed to it from the APIG to determine what Nextjs resources to serve (the image-handler lambda does not, it just uses the query string passed to it and doesn't care about the path). If the server-handler lambda has a /server prefix passed to it then the routes in Nextjs are wrong.

SOLUTION: The serverless-http library used in this project has support for defining a basePath configuration parameter (details here), which will strip the basePath off the supplied path before passing the value on to the consumer. e.g. a request path of /server/_next/data* with basePath: '/server' would be converted to /_next/data* before being passed to Nextjs.

PROBLEM 2: There is currently no means of injecting a configuration value into the options that are supplied to serverless-http, see here. Additionally, the current version of serverless-http (3.0.2) has a bug such that the Typescript type definition of the Options object passed to slsHttp does not support basePath. I have had a PR for serverless-http merged (dougmoscrop/serverless-http#246) which will add this support, but I am waiting for a release of 3.0.3.

SOLUTION: I would like to add a PR to allow for the basePath to be optionally specified by an env variable, here. e.g.:

basePath: process.env.NEXTJS_LAMBDA_BASE_PATH

If no variable is specified then basePath is not set and there is no behaviour change. If the variable is set then the base path is stripped before being passed to Nextjs. This will allow for both Serverless Framework and CDK to be used as a deployment tool.

@john-tipper
Copy link
Contributor Author

A new version of serverless-http has been release with my PR in it, which fixes the Typescript issue, so there is no need for the @ts-ignore workaround.

@sladg
Copy link
Owner

sladg commented Oct 7, 2022

Nice! So we could make the deployment more lean with just single APIGW.

I switched to CDK some time ago so I'm far from comfortable working with Serverless Framework, however, I'm more than happy to prepare CDK version of those changes and we can coordinate on adding SLS as second option?

serverless-http@3.0.3 indeed fixed the typings and I can see basePath as an option.

Let me experiment a bit with the base paths and I will get back to you 💯

@sladg sladg closed this as completed in #44 Oct 7, 2022
@sladg
Copy link
Owner

sladg commented Oct 8, 2022

New version released, simplified CDK construct (includes single ApiGateway) and simplified Image Optimization lambda (does not include unnecessary layers, way smaller).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants