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

AWS API Gateway / Lambda Support (via AWS CDK) #26

Closed
m14t opened this issue Nov 2, 2020 · 9 comments
Closed

AWS API Gateway / Lambda Support (via AWS CDK) #26

m14t opened this issue Nov 2, 2020 · 9 comments

Comments

@m14t
Copy link

m14t commented Nov 2, 2020

First, thank you for all the work you've already done here! I'm really excited about Remix. I primarily use AWS for my infrastructure, and so I wanted to see if I could get this working with Lambdas deployed via CDK.

Current Progress

Thanks in part to some code posted by @shortjared on Discord (who has started this repo https://github.com/shortjared/remix-cdk-starter), I was able to make some pretty good progress. You can see it running here: https://y9beilufg0.execute-api.us-east-1.amazonaws.com/

I posted the code for this at m14t/starter-aws-cdk, which is a mono-repo with 3 projects:

  • remix-run-apigateway - Remix bindings for AWS Lambda
    • I think this is a good start, but there are a few areas that could be improved:
      1. serveStaticFileIfExists - It would be great if this could stat the files to send the last modified header and calculate an etag, and to take those into account in the response
      2. calculating the build directory - the publicPath seems to be a part of the browserBuildDirectory, which required some funky path/string manipulation to get working -- I'm sure this can be improved
      3. Decide if its ok/useful to expose the error in some cases to the end user [1, 2
      4. Stream support - The @remix-run/express binding supported streams, but API Gateway doesn't. I'm not sure if its possible for handleRequest to return a stream here, but if it is, we'll need to consolidate it here.
      5. Duplicate Headers - ApiGateway has headers (and query parameters) parsed in two different ways: event.headers and event.multiValueHeaders. The difference can be seen when someone sends the same header name twice, but with different values. Right now this code is only handling non-repeated headers.
  • remix-starter-apigateway - An example project using remix-run-apigateway
  • cdk - An AWS CDK project to deploy remix-starter-apigateway

Issues

Edit: Solved! See comment below.

However there seems to be one major limitation with API Gateway. From https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-known-issues.html:

Path segments can only contain alphanumeric characters, hyphens, periods, commas, and curly braces.

This is an issue for the way we are serving the build/_shared/node_modules that contain an @ character in their package namespace. This effects the @babel/runtime and @remis-run/react dependencies specifically. If you look at the CloudWatch logs, the lambda function is never ever triggered for these requests, API Gateway seems to just drops them.

image

Potential Solution

One thing that would make this setup slightly more complex, but follow best practices, is if we deployed the built assets to S3 and served them via CloudFront. I don't believe that CloudFront has the same path character limitations, which should allow us to work around this issue, and avoid a lambda execution for a static file.

That being said, we might need some support in Remix to be able to serve these files from a different (sub)domain.

@m14t
Copy link
Author

m14t commented Nov 2, 2020

@shortjared had the idea to use multiple CloudFront origins. With this we can serve all public/* files from an S3 bucket and then fallback to serving all other requests from API Gateway.

This has been implemented in m14t/starter-aws-cdk@1e32ff2 and can be seen working at https://d3cmgcnia7ll9p.cloudfront.net. 🎉

The favicon.ico is still being served from the lambda (via the CDN), so we can either choose to add another "Behavior" to also serve that from the S3 bucket or leave things as is. It would be nice to be able to remove the serveStaticFileIfExists function, but this would mean needing to keep this Behavior part of deployment config in sync any time we added a new static asset which I'm not a fan of.


With that all being said, there are certainly improvements that could be made, but I think this is a workable solution.

Happy to hear any feedback, questions, or suggestions!

@m14t m14t changed the title AWS CDK Support AWS API Gateway / Lambda Support (via AWS CDK) Nov 3, 2020
@fwiech
Copy link

fwiech commented Dec 16, 2021

Thank you for providing an initial aws-cdk setup! I think the idea with CloudFront for static assets is great.

Since over time some things have changed (added remix AWS Architect deployment option, aws-cdk v2 release, ...), I have built an example deployment based on the current project versions:
remix-aws-cdk-example

It is based on the AWS Architect remix template (and still uses it for the local development). The deployment can be done via aws-cdk v2.

@sixman9
Copy link

sixman9 commented Jan 4, 2022

@florianwiech , apologies for the newbie-type questions:

  1. Could the 'makefile' and 'deploy.sh' calls be integrated directly into the package.json file, in theory (what are the benefits of having those files?)?

BTW, thank you VERY much for this offering 🙏🏾

@fwiech
Copy link

fwiech commented Jan 5, 2022

@florianwiech , apologies for the newbie-type questions:

  1. Could the 'makefile' and 'deploy.sh' calls be integrated directly into the package.json file, in theory (what are the benefits of having those files?)?

BTW, thank you VERY much for this offering 🙏🏾

Sure, it is totally fine to use other utilities for running scripts (eg. providing scripts inside the package.json as you mentioned).
I also started with package.json scripts but eventually these scripts increased in size and parameter length. It basically bloated the package.json scripts section. That was the reason I moved to the makefile and deploy.sh layout.

In my case I chose the makefile to group cli commands so that I do not have to retype then while developing.
The deploy.sh basically wraps the cdk deploy command with all the needed parameters and directory structure it needs.


If there are other questions regarding some remix-aws-cdk-example specifics I would suggest opening a ticket in the remix-aws-cdk-example repo. This way we can keep this thread targeted to the remix aws-cdk integration.

@ryan-mars
Copy link

Does anyone have a working version of Remix deployed via CDK? None of the projects mentioned in this thread seem to be maintained and work with latest Remix.

@fwiech
Copy link

fwiech commented Jan 19, 2022

@ryan-mars Thank you for pointing out that the latest release of remix introduced some breaking changes.
I updated the example and it should work with the latest remix version now. (fwiech/remix-aws-cdk-example@f6d5cd1)


Moreover, I introduced yarn workspaces for better project separation and extended the readme.

@ryan-mars
Copy link

@florianwiech Thanks! I got it to mostly work by disabling minifcation but even with your changes I still get bundler warnings... do you see this too?

Bundling asset remix/RequestHandler/Code/Stage...
▲ [WARNING] Non-relative path "packages/file/src/lib.js" is not allowed when "baseUrl" is not set (did you forget a leading "./"?)

    node_modules/@web-std/file/tsconfig.json:6:24:
      6 │       "@web-std/file": ["packages/file/src/lib.js"]
        ╵                         ~~~~~~~~~~~~~~~~~~~~~~~~~~

▲ [WARNING] Non-relative path "packages/blob/src/lib.js" is not allowed when "baseUrl" is not set (did you forget a leading "./"?)

    node_modules/@web-std/blob/tsconfig.json:6:24:
      6 │       "@web-std/blob": ["packages/blob/src/lib.js"]
        ╵                         ~~~~~~~~~~~~~~~~~~~~~~~~~~

2 warnings

@fwiech
Copy link

fwiech commented Jan 20, 2022

@ryan-mars You're right, I get the same warnings.
I think we need to give it some more time until those warnings might be resolved by itself through dependency updates.

Here are some infos I could gather regarding the status of those warnings:
FYI: We use the @remix-run/architect package. Yes it is called architect, but it is actually a generic adapter for AWS Lambda behind API Gateway (#1038).
If we have a quick look at our dependency tree we can see the following: @remix-run/architect > @remix-run/node > @web-std/file.
I was able to find a merge request regarding those warnings in their repo. The fix is already merged, although I was not able to find a release on npm yet. If that is shipped we could update the lockfile and might be good to go without the warnings.


Note to myself: Gosh I really have to add a dependabot to the example repo that there is some sort of automation on keeping it up-to-date.

@ryan-mars
Copy link

@florianwiech Thank you so much!

fwiech added a commit to fwiech/remix-aws-cdk-example that referenced this issue Mar 1, 2022
@chaance chaance closed this as completed Apr 19, 2022
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

No branches or pull requests

6 participants