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

Replace netlify's zip-it-and-ship-it for serverless deploy #3782

Conversation

Irev-Dev
Copy link
Contributor

@Irev-Dev Irev-Dev commented Nov 27, 2021

Replaced using vercel's nft (node file trace)

Related to #3686

Labeling WIP as I have a few questions.

@thedavidprice in regards to the following two points:

does this process create the files functions-manifest.json as well as a nft.json?
we won't want to add those deps to Redwood packages; instead we'll want to install them when setup is run

Deps should only be added on setup. 👍

No it doesn't create a function-manifest.json or nft.json, are these needed (as far as I know they are not needed for SLS but maybe there's context I'm missing)?


I also added a new option to the deploy command, it --stage and essentially just passes it along to the serverless deploy command. Imo opinion it's essential otherwise you're force into testing things live. Maybe I shouldn't have snuck it it though, should I remove it and make a seperate PR?

I also added a verbose option for @dac09, and will follow up and add this verbose to other deploy commands that are missing it.


Here's the process I used to test the changes:

created a test project

Ran

yarn rwfw project:sync
yarn rw g function exampleFunc
yarn rw setup deploy aws-serverless
yarn rwfw project:sync # again after setup since it runs yarn install

change api/src/services/posts/posts.ts posts function to:

export const posts = () => {
  return [
    {
      id: 5,
      title: 'test title',
      body: 'test body',
      createdAt: '2021-11-25T09:02:36.394Z',
      __typename: 'Post',
    },
  ]
}

i.e. remove db access from this call so we can test the function runs when deployed without setting up a live db

Deployed with yarn rw deploy aws --stage test

once deployed I could test the example function (i.e. https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions/exampleFunc)

update redwood.toml apiUrl to the deployed url (should look like https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions)

yarn rw dev

The test project homepage should have a single blog post with the dumby data we added (note navigating anywhere else in the app will cause errors since it doesn't have db access.)

image

Replaced using vercel's nft (node file trace)
@@ -0,0 +1,63 @@
import path from 'path'
Copy link
Contributor Author

@Irev-Dev Irev-Dev Nov 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is packages/cli/src/commands/deploy/aws-providers/packing.js a good home? not sure where this file should live.

Or should these functions be rolled into packages/cli/src/commands/deploy/aws-providers/serverless.js?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference: move packing to a separate folder and called in "packaging/nft.js"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now in packages/cli/src/commands/deploy/packing/nft.js.

Is that what you intended, or did you want it in packages/cli/src/commands/deploy/aws-providers/packing/nft.js or other?

@@ -7,25 +7,10 @@ export const preRequisites = [
'Please follow the steps at https://www.serverless.com/framework/docs/providers/aws/guide/installation/ to install Serverless.',
],
},
{
title: 'Checking if @netlify/zip-it-and-ship-it is installed...',
command: ['yarn', ['zip-it-and-ship-it', '--version']],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new deps that have been added are @vercel/nft, archiver and fs-extra. They are not cli tools so can't use --version as a test, is there an easy way to check deps?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it is easy, but in the past to verify dependencies like this are there, I have loaded package.json in code and inspected the JSON object to verify that there is the expected key/value under dependencies/devDependencies

@Irev-Dev Irev-Dev marked this pull request as draft November 28, 2021 23:56
const mapCommandsToListr = ({ title, command, task, errorMessage }) => {
return {
title: title,
task: task
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In cases where the task is not a cli command a task function can be provided directly.

await executingCommand
} catch (error) {
if (errorMessage) {
error.message = error.message + '\n' + errorMessage.join(' ')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error message only extended if an errorMessage array is provided.

const executingCommand = execa(...command, {
cwd: BASE_DIR,
})
executingCommand.stdout.pipe(process.stdout)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this might be a problem in making all of the commands very noise, but does not, only logging out the severless cli stdout, currently this doesn't respect the verbose flag on purpose because I think these details always need to be shown, in particular when the deploy is finished the last thing to be logged out is:

Serverless: Stack update finished...
Service Information
service: app
stage: test
region: us-east-2
stack: app-test
resources: 19
api keys:
  None
endpoints:
  GET - https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions/exampleFunc
  POST - https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions/exampleFunc
  GET - https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions/graphql
  POST - https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions/graphql
functions:
  exampleFunc: app-test-exampleFunc
  graphql: app-test-graphql
layers:
  None
Serverless: Removing old service artifacts from S3...

This info always needs to be shown because the urls are generated by aws, without these logs you can't know what they are without digging into aws console.
That's just my opinion, should I make this respect the verbose flag?

for context here's the whole terminal output for my test project.

➜  rw2 git:(master) ✗ yarn rw deploy aws --verbose --stage test
yarn run v1.23.0-20210726.1745
$ /Users/kurthutten/rw2/node_modules/.bin/rw deploy aws --verbose --stage test
[21:06:42] Building and Packaging... [started]
[21:06:42] Building API... [started]
$ /Users/kurthutten/rw2/node_modules/.bin/rw build api
[21:06:46] Generating Prisma Client... [started]
[21:06:49] Generating Prisma Client... [completed]
[21:06:49] Verifying graphql schema... [started]
[21:06:49] Verifying graphql schema... [completed]
[21:06:49] Building API... [started]
[21:06:50] Building API... [completed]
[21:06:50] Building API... [completed]
[21:06:50] Packing Functions... [started]
[21:06:59] Packing Functions... [completed]
[21:06:59] Building and Packaging... [completed]
[21:06:59] Deploying to AWS [started]
[21:06:59] Deploying... [started]
Serverless: Deprecation warning: Detected ".env" files. In the next major release variables from ".env" files will be automatically loaded into the serverless build process. Set "useDotenv: true" to adopt that behavior now.
            More Info: https://www.serverless.com/framework/docs/deprecations/#LOAD_VARIABLES_FROM_ENV_FILES
Serverless: DOTENV: Loading environment variables from .env:
Serverless: Deprecation warning: Resolution of lambda version hashes was improved with better algorithm, which will be used in next major release.
            Switch to it now by setting "provider.lambdaHashingVersion" to "20201221"
            More Info: https://www.serverless.com/framework/docs/deprecations/#LAMBDA_HASHING_VERSION_V2
Serverless: Deprecation warning: Starting with next major version, the provider tags will be applied to Http Api Gateway by default. 
            Set "provider.httpApi.useProviderTags" to "true" to adapt to the new behavior now.
            More Info: https://www.serverless.com/framework/docs/deprecations/#AWS_HTTP_API_USE_PROVIDER_TAGS
Serverless: Packaging service...
Serverless: Uploading CloudFormation file to S3...
Serverless: Uploading artifacts...
Serverless: Uploading service exampleFunc.zip file to S3 (279.71 KB)...
Serverless: Uploading service graphql.zip file to S3 (29.21 MB)...
Serverless: Validating template...
Serverless: Updating Stack...
Serverless: Checking Stack update progress...
...............
Serverless: Stack update finished...
Service Information
service: app
stage: test
region: us-east-2
stack: app-test
resources: 19
api keys:
  None
endpoints:
  GET - https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions/exampleFunc
  POST - https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions/exampleFunc
  GET - https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions/graphql
  POST - https://5mdfm18azg.execute-api.us-east-2.amazonaws.com/.redwood/functions/graphql
functions:
  exampleFunc: app-test-exampleFunc
  graphql: app-test-graphql
layers:
  None
Serverless: Removing old service artifacts from S3...
[21:09:03] Deploying... [completed]
[21:09:03] Deploying to AWS [completed]
✨  Done in 146.36s.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is getting this logged out to the console might be why the original author ran it outside of listr, but don't know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! A couple of notes:

  • in Serverless Framework v3 the --verbose flag will exist and be supported, not sure if that could be of any use for you here
  • in v3 we are redesigning the entire CLI output to make it less verbose/more helpful, you can read about that here: https://www.serverless.com/blog/serverless-framework-v3-beta

That v3 is currently in beta (see the blog post above) if you want to give it a try.

@Irev-Dev
Copy link
Contributor Author

Irev-Dev commented Nov 30, 2021

Hey @dac09, have a look at 9b8192c

I think I understood the spirit of what you were after with commiting to listr, normally I would like to follow conventions I can already see in a codebase, but in this case I felt like I need to change some things up to come to a reasonable tidy solution. The things I changed are:

  1. The commands in packages/cli/src/commands/deploy/aws-providers/serverless.js are no longer arrays, but instead functions that return an array, I did this so that extra information could be passed to them, namely the staging argument for the serverless deploy command.
  2. Each element in the array can optionally contain a task function for cases like the packing which is not run with a cli command.

Putting those two together meant I was able to run each stage for the deploy command with something along the lines of providerData.preRequisites(yargs).map(mapCommandsToListr).

Like I said this was my attempt at a tidy solution, open to trying a different direction.

@Irev-Dev Irev-Dev changed the title WIP Replace netlify's zip-it-and-ship-it for serverless deploy Replace netlify's zip-it-and-ship-it for serverless deploy Dec 3, 2021
@Irev-Dev Irev-Dev marked this pull request as ready for review December 3, 2021 06:47
@Irev-Dev
Copy link
Contributor Author

Irev-Dev commented Dec 3, 2021

I removed the draft label as personally happy with this PR now.

Happy to make more changes of course.

@dac09
Copy link
Contributor

dac09 commented Dec 3, 2021

This looks much cleaner @Irev-Dev than it was, all for it. Will need a bit of time to try it out first, but looking positive!

@thedavidprice
Copy link
Contributor

@mnapoli Are there any "go-to" function bundlers you see being used? And/or is it common practice to need to handle this via a custom implementation as we've done here?

Just looking for best practices in case there are any.

@mnapoli
Copy link
Contributor

mnapoli commented Dec 7, 2021

@thedavidprice there isn't a built-in default. There are plugins to build with:

By using a plugin like this the whole build is run in serverless deploy, without needing to call a separate command. That's also called when you run serverless package.

@Irev-Dev
Copy link
Contributor Author

Irev-Dev commented Dec 8, 2021

@thedavidprice I thought one perks of "doing it ourselves" is that running yarn rw build && yarn rw serve will give an accurate representation of how the app will behave when deployed with serverless as the build is the same, it's only the nft-and zip that would be different.

It doesn't happen often but every once in a while I'll find a bug that can only be replicated with build and serve, in which case I glad to be able to replicate it locally.

@dac09
Copy link
Contributor

dac09 commented Dec 8, 2021

Hey both - so I took this for a spin. Thank you so much for the effort into this - long post coming 💤

✅ NFT build process working amazingly
Top job on this @Irev-Dev - and I think it'll pay dividends!

This is the first time I've used the rw serverless integration - and it seems overdue for an overhaul! I wonder if we should point the PR to a different branch and chip away at making it more complete. The setup commands are meant to get your project deployable, or very close to a deployable state, where you can git push and that's it.

Here's what I'm thinking:

  1. Add script hook for before:deploy
    e.g.
plugins:
  - serverless-plugin-scripts
custom:
  scripts:
    hooks:
      'before:deploy:deploy': yarn rw deploy aws serverless --build-only

to run the build and package process, when sls deploy is run. This will allow us to use serverless's ci/cd https://www.serverless.com/framework/docs/guides/cicd/custom-scripts. Notice the build-only flag (or maybe --no-deploy, but that seems weird since the command is called deploy), that would package up everything.

  1. Update runtime to nodejs14.x
    Currently runtime is nodejs12.x, we should change it to nodejs14.x otherwise CI/CD builds fail

  2. Having dotenv plugin fails deployment in CI/CD mode, because .env is in .gitignore
    Is there a way we could ignore the dotenv plugin in CI/CD mode? I don't think we need this at all in fact, since .env support is built into serverless

  3. Add environment DATABASE_URL
    I think this could be default

    tags: # Tags for this specific lambda function
      endpoint: /.redwood/functions/auth
+params
+    environment:
+      DATABASE_URL: ${param:DATABASE_URL}

This will allow us to set the DATABASE_URL in the serverless dashboard and deploy using CI/CD

  1. Remove ./redwood/functions' from path
    I think this was a mistake that we originally had this. There's really no reason to do this!

  2. Add serverless-finch plugin?
    This would make using the serverless deploy option a lot more compelling, as both the web and api sides would be deployed! We'd need to change the build command to also build the web side too

I tried this plugin with the following config in serverless.yml and it works well.

plugins:
  - serverless-finch

custom:
  client:
    bucketName: redwood-sls-finch-deploy
    distributionFolder: web/dist

Only problems I see here are:

  • you need to deploy with sls client deploy. Why? No idea
  • Ideally we'd be able to setup a proxy so that requests to {host}/.redwood/functions would be proxied to the endpoints for the API side. I'm not quite sure if this would be possible, or if it would be a manual step. @mnapoli thoughts?
  1. I don't think we need the get endpoints for graphql
-      - httpApi:
-          path: /.redwood/functions/auth
-         method: GET
      - httpApi:
          path: /.redwood/functions/auth
          method: POST

Incidentally - I don't understand the difference between http and httpApi?

  1. I found it very difficult to find the docs for the available options on serverless.yaml. I still don't know where it is. Is it just me?
    Maybe we should link to the reference for available configuration options for the file.

  2. CORS config should come default
    I tried doing this, but was unsuccessful at this. Would it be possible to add CORS headers to the generated functions so that the whole deployed application would out of the box? (that's the point of rw setup, after all)

Here's what I tried:

     - http:
          path: /.redwood/functions/auth
          method: POST
          cors:
            origin: '*'
            headers:
              - Content-Type
              - X-Amz-Date
              - Authorization
              - Auth-provider
              - X-Api-Key
              - X-Amz-Security-Token
              - X-Amz-User-Agent

But unsuccessful... maybe someone more versed in this framework would know?

9. Not doing a prisma migrate during the deploy step

@dac09
Copy link
Contributor

dac09 commented Dec 9, 2021

Here's my hacky attempt at getting both sides working:
https://github.com/dac09/serverless-rw-deploy/blob/main/serverless.yml

What I have to do is:

  1. yarn rw deploy aws serverless so the api side deploys.
  2. I then grabbed the endpoint URL, and put it in the redwood.toml (see point 6 in above post, is there a way we could automatically grab the endpoint?)
  3. Deploy the web side: sls client deploy

End result: http://redwood-sls-finch-deploy.s3-website.us-east-2.amazonaws.com/

@ajcwebdev
Copy link
Collaborator

Thanks @dac09, this is really great stuff! I've linked your comment to #3755 where we've got a conversation going about overhauling the serverless integration.

@Irev-Dev Irev-Dev deleted the kurt/replace-zip-it-and-ship-it-rebase branch December 21, 2021 18:11
@Irev-Dev
Copy link
Contributor Author

Awesome, thanks @dac09

dac09 pushed a commit to dac09/redwood that referenced this pull request Jan 6, 2022
@jtoar jtoar modified the milestones: chore, test May 13, 2022
@Josh-Walker-GM Josh-Walker-GM modified the milestones: chore, v8.0.0 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

9 participants