CDK Construct #878

kirkness · 2021-01-18T18:19:07Z

This may be wildly out of scope for this project at the moment(⁉️) as per #777. I've been using CDK + Next.js with the sls-next Builder for around a year now on a couple of different projects. To make this possible I've evolved this CDK Construct which is by now fairly well battle-tested; all the while trying to keep it in line with the way that the serverless-component works. I'm quite keen on open-sourcing it and think it makes the most sense to exist in this project. Keen to know whether this is something that is of interest including in the project at the moment?

Todo:

Documentation
Unit tests
Reuse cache logic from serverless component
Function names should be more descriptive
Fix custom behaviors (should use new format)
Fix static image bevaviour
Identify why minify handlers returns 503

Future possibilities:

Abstract infrastructure synthesis so that logic is shared between serverless and cdk deployments
Add built-in optional builder
Write using Jsii so that this construct can be used within other CDK languages

Side-note: I'm also keen on getting more involved with the maintenance and support for this project if there still a need (especially helping on the work outlined in #777).

codecov · 2021-01-18T18:55:59Z

Codecov Report

Merging #878 (5d0a370) into master (9b89a3b) will increase coverage by 0.57%.
The diff coverage is 97.63%.

@@            Coverage Diff             @@
##           master     #878      +/-   ##
==========================================
+ Coverage   80.73%   81.31%   +0.57%     
==========================================
  Files          64       68       +4     
  Lines        2403     2488      +85     
  Branches      561      591      +30     
==========================================
+ Hits         1940     2023      +83     
+ Misses        399      398       -1     
- Partials       64       67       +3

Impacted Files	Coverage Δ
...rless-components/nextjs-cdk-construct/src/index.ts	`94.54% <94.54%> (ø)`
packages/libs/s3-static-assets/src/index.ts	`100.00% <100.00%> (ø)`
...ibs/s3-static-assets/src/lib/readDirectoryFiles.ts	`100.00% <100.00%> (+11.11%)`	⬆️
...tjs-cdk-construct/src/utils/readAssetsDirectory.ts	`100.00% <100.00%> (ø)`
...uct/src/utils/readInvalidationPathsFromManifest.ts	`100.00% <100.00%> (ø)`
...cdk-construct/src/utils/reduceInvalidationPaths.ts	`100.00% <100.00%> (ø)`
...s/nextjs-cdk-construct/src/utils/toLambdaOption.ts	`100.00% <100.00%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b89a3b...5d0a370. Read the comment docs.

danielcondemarin · 2021-01-18T21:00:58Z

This is awesome @kirkness 🙌

Actually I think this aligns well with what @dphang outlined in #777. There is the question of CDK vs CDK for Terraform, but I don't see why we shouldn't make a start with plain CDK if you've done all the hard work already.
The most important thing is that CDK solves the infrastructure state problem we've always had with serverless components beta. It also opens up the possibility of supporting serverless components GA via CloudFormation which I've briefly discussed with @dphang. Plus CDK is not going away any time soon as AWS is investing a lot in it.

I will try to test out this branch over the weekend and will feedback.

'm also keen on getting more involved with the maintenance and support for this project if there still a need

We do need more contributors and I can add you to the repo's maintainers if that's something you're interested in 🙂

CC'ing @dphang for any thoughts on this.

dphang · 2021-01-19T02:07:51Z

packages/serverless-components/nextjs-cdk-construct/src/index.ts

+        destinationKeyPrefix: staticAsset,
+        distributionPaths: ["/*"],
+        sources: [s3Deploy.Source.asset(assetPath)],
+        cacheControl: [


Static Pages would need different cache control here: public, max-age=0, s-maxage=2678400, must-revalidate.

And also in the latest 1.19 alpha, we are now versioning pages so only the last two deployments are kept to save S3 space.

See the code here:

serverless-next.js/packages/libs/s3-static-assets/src/index.ts

Line 371 in da2da19

const deleteOldStaticAssets = async (

. Would be good to have this as well.

I will have a look into how static pages are being versioned, but I presume this means no invalidation is required for these either which is nice! This is something I wanted to revisit as it's currently creating an invalidation on all resources every deploy which is unnecessary given everything is versioned?

And also in the latest 1.19 alpha, we are now versioning pages so only the last two deployments are kept to save S3 space.

I believe this is actually handled by CDK itself as it automatically 'syncs' source assets, deleting ones that no longer exist. I'm assuming the reason you are keeping the last 2 deployments is for safety and thus this behavior is not needed for the construct?

@dphang - update on this here. I've refactored the uploadStaticAssetsFromBuild which was determining the cache-control for files so that it can be used in the CDK construct.

dphang · 2021-01-19T02:11:17Z

...-components/nextjs-cdk-construct/__tests__/fixtures/next-boilerplate/default-lambda/index.js

@@ -0,0 +1 @@
+exports.handler = async () => ({ statusCode: 200 });


For better readability of all these added JS/JSON files, could you please format these test files using prettier?

Done in ea6cc16

Although, prettier has not updated __tests__/fixtures/next-boilerplate/default-lambda/index.js...

dphang · 2021-01-19T02:19:03Z

@kirkness thanks a lot for this! Yea, we have been talking about adding CDK for a while now but just haven't had resources. So this is a good first step! We are also trying to upgrade to Serverless Components GA, if you are interested as mentioned it might go hand-in-hand with this as we are trying to make deployments more seamless in the near future (+ there is significant financial sponsorship (like $1k+) from Serverless Framework for doing so, which we would be happy to split based on resourcing). Do let @danielcondemarin know and we can add you to the Slack too.

kirkness · 2021-01-19T11:23:47Z

Great news, would be awesome to join you on Slack to kick this off! My email is just my first name (Henry) + my company domain (planes.studio).

In the meantime, I'll revert this PR to a draft and flesh out a plan of attack with some of the additional work that's needed to go into it.

packages/libs/s3-static-assets/package.json

kirkness · 2021-01-20T21:57:21Z

packages/libs/s3-static-assets/src/index.ts

-    bucketName,
-    credentials: credentials
-  });
+const getAssetDirectoryFileCachePolicies = (


I've taken all the logic from uploadStaticAssetsFromBuild that determines the cache-control for each file in /assets and refactored it to be agnostic to the deployment mechanism. This file is imported into the CDK construct now, whilst also used below in uploadStaticAssetsFromBuild. I think we could do with a grander refactor of this form that removes the deployment code from the synthesis.

This didn't make sense to use in the end as the data structure required to work with the constructs didn't fit this output enough to make it worth it. Happy to revert the changes to this module if you think its needed.

I think it should be ok to keep it.

kirkness · 2021-01-20T21:58:22Z

packages/libs/s3-static-assets/src/lib/filterOutDirectories.ts

@@ -1,3 +0,0 @@
-import { Item } from "klaw";
-
-export default (fileItem: Item): boolean => !fileItem.stats.isDirectory();


Not needed anymore as we get fast-glob to only read files.

iDVB · 2021-01-25T13:58:20Z

Thanks for working on this @kirkness!
Our company may be interested in the plain construct and helping out if we can.

One thing to think about is, have you considered the potential order of events that may be needed before the frontend assets are deployed? One think I've come up against when using CDK to deploy frontend assets is that there is currently no way that I saw to have "backend" infra deploy first and to pass API endpoints, outputs, into the frontend construct so they can be bundled into the frontend assets. Have you found away around this? Is there anything I might be able to help out with in these efforts?

kirkness · 2021-01-25T15:15:06Z

One thing to think about is, have you considered the potential order of events that may be needed before the frontend assets are deployed? One think I've come up against when using CDK to deploy frontend assets is that there is currently no way that I saw to have "backend" infra deploy first and to pass API endpoints, outputs, into the frontend construct so they can be bundled into the frontend assets. Have you found away around this? Is there anything I might be able to help out with in these efforts?

👍, sure I think I know what you mean, as you might:

Build the next app (this only point in time in which you can set Next.js env variables)
Synth + Deploy your app via CDK (this is the point in time in which you have the values that you might use for your env vars, e.g. API endpoints, S3 bucket names...)

I think at the very least, if the bundling was done within the construct, then you'd be able to pass in non-tokenized values as env vars, all inline with your CDK code.

iDVB · 2021-01-25T15:39:50Z

I think at the very least, if the bundling was done within the construct, then you'd be able to pass in non-tokenized values as env vars, all inline with your CDK code.

This would be SUPER ideal. Any work towards this working now?

kirkness · 2021-01-25T16:29:03Z

I think at the very least, if the bundling was done within the construct, then you'd be able to pass in non-tokenized values as env vars, all inline with your CDK code.

This would be SUPER ideal. Any work towards this working now?

I think all that would be needed is to set up Asset Bundeling, which would allow running the Builder in a Docker container... By the looks of it you can pass environment into the Docker container. If this accepts tokenized values, it solves the entire problem, if not, then still pretty neat.

Might have a look at a POC now. Watch this space. 👍

kirkness · 2021-01-25T18:21:39Z

Might have a look at a POC now. Watch this space. 👍

Parking this now, the CDK asset bundling process starts after the CDK constructs are created. At the point when we create the Next.js construct we need the next.js app to be bundled, this is because the infrastructure bases itself on the shape of the bundled output. So similar in part to the issue you outlined with env variables (that this was trying to resolve). On top of this, the env vars passed into the asset bundling process are not resolved, and so you end up with tokenized values.

More research needed - an easy way would be just to bundle without the use of the asset bundling construct, although keen to find a best-practice approach!

kirkness · 2021-02-10T13:30:14Z

I think LGTM in general, just a few minor comments and not sure if it's working with i18n subpaths (that was a recent change I made)

Cheers @dphang - just ran through those changes. Is i18n a blocker/substantial change? Will put in follow up PR if all good with you?

dphang · 2021-02-11T02:50:52Z

I think LGTM in general, just a few minor comments and not sure if it's working with i18n subpaths (that was a recent change I made)

Cheers @dphang - just ran through those changes. Is i18n a blocker/substantial change? Will put in follow up PR if all good with you?

I think it should probably work as is since it was just modifying the output HTML/JSON files to .serverless_nextjs to add the locale files, and also I updated the default Lambda handler. As long as you are uploading those, it should work. Yea, you can follow up in another PR.

Also, looks like some of the new tests in the CI build is failing now.

Thanks for this!

kirkness · 2021-02-11T18:18:38Z

I think LGTM in general, just a few minor comments and not sure if it's working with i18n subpaths (that was a recent change I made)

Cheers @dphang - just ran through those changes. Is i18n a blocker/substantial change? Will put in follow up PR if all good with you?

I think it should probably work as is since it was just modifying the output HTML/JSON files to .serverless_nextjs to add the locale files, and also I updated the default Lambda handler. As long as you are uploading those, it should work. Yea, you can follow up in another PR.

Also, looks like some of the new tests in the CI build is failing now.

Thanks for this!

Nice, sounds like it'll work then as each directory of the output is uploaded to s3/lambda as the sls component does. Ive updated the test snapshots so that they pass.

…also allow users to pass custom invalidationPaths

dphang · 2021-02-14T08:05:55Z

I think LGTM in general, just a few minor comments and not sure if it's working with i18n subpaths (that was a recent change I made)

Cheers @dphang - just ran through those changes. Is i18n a blocker/substantial change? Will put in follow up PR if all good with you?

I think it should probably work as is since it was just modifying the output HTML/JSON files to .serverless_nextjs to add the locale files, and also I updated the default Lambda handler. As long as you are uploading those, it should work. Yea, you can follow up in another PR.
Also, looks like some of the new tests in the CI build is failing now.
Thanks for this!

Nice, sounds like it'll work then as each directory of the output is uploaded to s3/lambda as the sls component does. Ive updated the test snapshots so that they pass.

Thanks, I ran e2e tests here: https://github.com/serverless-nextjs/serverless-next.js/actions/runs/565470296

dphang · 2021-02-16T06:24:46Z

@kirkness looks like everything is good. LMK if ready to merge?

kirkness · 2021-02-16T07:33:00Z

@kirkness looks like everything is good. LMK if ready to merge?

Cheers @dphang, good to merge from my perspective!

ibrahimcesar · 2021-02-28T01:40:57Z

Great work! I am looking towards this for some time. There are some example repo for me take a look? Tried to add to a codebase and even started from scratch but cdk deploy only generates a new config, cdk.out remains empty and the command stalled. When I added the builder debug flag true it just showme the log for the application starting.

Just a barebones example besides the code provided so far in the docs would be great, Because of my limits I was not able to use it 😓

Thanks in advance!

kirkness · 2021-02-28T08:40:27Z

Hey @ibrahimcesar - sounds odd, I don't suppose you could push your example up to a repo (just the one you did from scratch)?

ibrahimcesar · 2021-02-28T12:00:51Z

Sure @kirkness !
Please note I maybe missing something crucial, just trying to understand how to use. I think is a really great work. This is my PoC repo: https://github.com/ibrahimcesar/nextjs-starter-cdk-constructor

Thanks!

kirkness · 2021-02-28T12:52:06Z

Sure @kirkness !
Please note I maybe missing something crucial, just trying to understand how to use. I think is a really great work. This is my PoC repo: https://github.com/ibrahimcesar/nextjs-starter-cdk-constructor

Thanks!

Cheers @ibrahimcesar, I've updated the docs in this PR, it looks like you may have a couple of other unrelated issues in that repo, but hopefully they'll be obvious when it actually starts to build.

ibrahimcesar · 2021-02-28T15:21:42Z

Thanks @kirkness ,

Yes, there are some configs missing, but "life finds a way"
I was able to deploy as expected, thanks a lot for the help! 👍🏼

One hookie mistake I made, and l'll leave here as reference, is that I changed my next.config.js to

module.exports = {
  distDir: 'build',
}

Since I saw it was accessing the files at "build", but just leave the default build, or will fail because it will lookup in .next.

Updated my simple repo with a working version! Thanks a lot!

iDVB · 2021-02-28T15:41:09Z

Wow, greet work @kirkness !

Now, my specific interest is in the CDK construct alone.
What is the current state of that now since this merge?

does it work on its own?
Did you overcome the issue we spoke about in regards to baking stack values into the frontend build?
Does it have feature parody with hosting on Vercel? What’s missing?

donaldpipowitch · 2021-03-01T08:07:43Z

(FYI for others: Not sure if it's my fault, but I tried to play around with this CDK Construct and always hit a s3:PutBucketPolicy Access Denied error. As far as I can tell I should have this permission (also tried with {Allow:*, Resource:*}). There are some other Access Denied errors reported recently, but they seem to be a different problem. I tried to downgrade several libs - either from @sls-next or @aws-cdk and tried different regions (us and eu), but no luck. Just wanted to share this in case others have this problem as well.

Update: So apparently it is a config problem on my side, but not related to role permissions, but the account wide "Block Public Access settings" in S3. It looks like those settings would overrule anything I'd configure on the bucket, so I need to change them.)

iDVB · 2021-03-07T02:19:19Z

@kirkness When using the CDK construct outside of serverless components. what is the best way to organize your
CDK stack files, package.json etc, vs all the files as part of the nextjs src/build files.

I thought it would just be as simple as building in completely separate directory and just copying in the build files, but the CDK build process needed all the next apps deps and now its saying it needs the src pages directory too.

Not sure if its possible or the best way to keep some separation between the CDK stack files and the nextjs app its deploying?

I'm trying to run things in separate directories and leverage the Builder's config to point things around... and its running now, and starting to build the .next dir of files...

However it err's our with...
pages-manifest not found. Check if next.config.js target is set to 'serverless'

next.config.js

module.exports = {
  target: "serverless",
}

/bin/cdk-nextjs.ts

...
const nextConfigDir = path.join(__dirname, '../dvb-next');
const outputDir = path.join(nextConfigDir, ".serverless_nextjs");
const builder = new Builder(nextConfigDir, outputDir,
  {
    cmd: './node_modules/.bin/next',
    cwd: '../dvb-next',
    env: {},
    args: ['build']
  });
...

UPDATE -
Figured out my issue was with my directories. this works

...
const nextConfigDir = path.join(__dirname, '../../dvb-next');
const outputDir = path.join(nextConfigDir, ".serverless_nextjs");
const builder = new Builder(nextConfigDir, outputDir,
  {
    cmd: './node_modules/.bin/next',
    cwd: nextConfigDir,
    env: {},
    args: ['build']
  });
...

kirkness · 2021-03-07T13:17:12Z

Glad you got it working @iDVB, feel free to create issues if you have any other problems!

revmischa · 2021-07-06T11:57:30Z

Curious, is there a supported/recommended way to deploy a NextJS app with CDK but without Serverless?

ibrahimcesar · 2021-07-06T14:40:51Z

@revmischa What do you mean by supported? Like a package? Because you only need to build your own constructor but there's a lot for React applications as whole in community. CDK is a general purpose tool so you can create an EC2 Instance and run NextJS inside or use ECS if your application is on Docker or even Fargate, but this service is considered serverless. But CDK gives you access to create any kind of infrastructure on AWS so you can do as you want.

revmischa · 2021-07-06T15:01:58Z

@revmischa What do you mean by supported? Like a package? Because you only need to build your own constructor but there's a lot for React applications as whole in community. CDK is a general purpose tool so you can create an EC2 Instance and run NextJS inside or use ECS if your application is on Docker or even Fargate, but this service is considered serverless. But CDK gives you access to create any kind of infrastructure on AWS so you can do as you want.

Yeah ideally deploying it using the https://docs.aws.amazon.com/cdk/api/latest/docs/aws-lambda-nodejs-readme.html construct. Just not sure on what to put in my handler.

ibrahimcesar · 2021-07-06T15:09:12Z

@revmischa I don't think you gonna achieve what you want with this constructor in particular. It is serverless and is a Lambda. The architecture is a bit different, I don't think this is the right forum. You can ask me directly and I can help you.

kirkness added 4 commits January 18, 2021 17:53

feat: add intial cdk component

5f492b6

fix: add @sls-next/lambda-at-edge dep link

9c40bb4

fix: add cdk assert lib

3799b10

fix: update failing snapshot

b12f6db

dphang reviewed Jan 19, 2021

View reviewed changes

kirkness marked this pull request as draft January 19, 2021 11:37

kirkness added 2 commits January 19, 2021 11:37

Merge branch 'master' into master

50da04f

fix: updates cdk asset caches to match sls-component caches

ed731a2

kirkness commented Jan 20, 2021

View reviewed changes

packages/libs/s3-static-assets/package.json Show resolved Hide resolved

kirkness commented Jan 20, 2021

View reviewed changes

dphang mentioned this pull request Jan 22, 2021

Setup all default cloudfront behaviors (_next/* etc) to custom behaviors #875

Closed

danielcondemarin and others added 7 commits January 24, 2021 09:23

Merge branch 'master' into master

00e930d

refactor: run prettier on fixture files

ea6cc16

docs: add intial cdk documentation

2a3ef61

test: update test snapshots

2fd6258

docs: add cdk info to readmes

bdfcd5c

docs: fix syntax on docs

b86e3c0

test: add unit tests for readDirectoryFiles

9fe27fd

test: add tests to CDK construct

a9a2210

kirkness added 2 commits February 10, 2021 13:11

fix: remove unneeded async from test

b918d1e

fix: format all remaning fixture files

b811058

test: update cdk snapshots

a2af6f7

arjunnaha mentioned this pull request Feb 12, 2021

Server-side rendering support / SSR aws-amplify/amplify-hosting#412

Closed

kirkness added 4 commits February 13, 2021 09:56

fix: reduce number of invalidations created by stripping duplicates, …

7aeaec9

…also allow users to pass custom invalidationPaths

Merge branch 'master' into master

19bf59c

fix: typo

989c8a6

Merge branch 'master' of https://github.com/kirkness/serverless-next.js

5d0a370

dphang approved these changes Feb 16, 2021

View reviewed changes

dphang merged commit d38e2aa into serverless-nextjs:master Feb 16, 2021

kirkness mentioned this pull request Feb 28, 2021

docs: adds the build arg to Builder in CDK docs #908

Merged

		@@ -0,0 +1 @@
		exports.handler = async () => ({ statusCode: 200 });

		@@ -1,3 +0,0 @@
		import { Item } from "klaw";

		export default (fileItem: Item): boolean => !fileItem.stats.isDirectory();

CDK Construct #878

CDK Construct #878

Conversation

kirkness commented Jan 18, 2021 • edited Loading

codecov bot commented Jan 18, 2021 • edited Loading

Codecov Report

danielcondemarin commented Jan 18, 2021

dphang Jan 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dphang commented Jan 19, 2021 • edited Loading

kirkness commented Jan 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iDVB commented Jan 25, 2021

kirkness commented Jan 25, 2021

iDVB commented Jan 25, 2021

kirkness commented Jan 25, 2021 • edited Loading

kirkness commented Jan 25, 2021

kirkness commented Feb 10, 2021

dphang commented Feb 11, 2021

kirkness commented Feb 11, 2021

dphang commented Feb 14, 2021

dphang commented Feb 16, 2021

kirkness commented Feb 16, 2021

ibrahimcesar commented Feb 28, 2021

kirkness commented Feb 28, 2021

ibrahimcesar commented Feb 28, 2021

kirkness commented Feb 28, 2021

ibrahimcesar commented Feb 28, 2021

iDVB commented Feb 28, 2021

donaldpipowitch commented Mar 1, 2021 • edited Loading

iDVB commented Mar 7, 2021 • edited Loading

kirkness commented Mar 7, 2021

revmischa commented Jul 6, 2021

ibrahimcesar commented Jul 6, 2021

revmischa commented Jul 6, 2021

ibrahimcesar commented Jul 6, 2021

kirkness commented Jan 18, 2021 •

edited

Loading

codecov bot commented Jan 18, 2021 •

edited

Loading

dphang Jan 19, 2021 •

edited

Loading

dphang commented Jan 19, 2021 •

edited

Loading

kirkness commented Jan 25, 2021 •

edited

Loading

donaldpipowitch commented Mar 1, 2021 •

edited

Loading

iDVB commented Mar 7, 2021 •

edited

Loading