nx-plugins icon indicating copy to clipboard operation
nx-plugins copied to clipboard

[BUG] Zip file resolution error when deploying a single artifact

Open tstackhouse opened this issue 3 years ago • 18 comments

Describe the bug When overwriting servicePath with the packagePath in deploy.impl.ts:123 (and in earlier iterations) because of the relative pathing, inside serverless' packaging routines, the package path gets appended twice, resulting in a silent failure caused by an ENOENT when deploy attempts to read the generated zip artifacts:

https://github.com/serverless/serverless/blob/master/lib/plugins/package/lib/zipService.js#L67 https://github.com/serverless/serverless/blob/master/lib/plugins/aws/deploy/lib/checkForChanges.js#L158-L162

package.artifact is populated when package.individually is set to false in serverless.yml: https://github.com/serverless/serverless/blob/master/lib/plugins/package/lib/packageService.js#L99-L113

Inspecting the filePath that comes back in packageAll, we can see the following:

{
  zipFileName: 'my-app.zip',
  filePath: 'dist/.serverlessPackages/apps/my-app/.serverless/my-app.zip'
}

That filePath then gets set to service.package.artifact which we can see in checkIfDeploymentIsNecessary in checkForChanges.js we can inspect zipFiles and zipFilePaths and see the incorrect path resolution for artifact:

{
  artifact: 'dist/.serverlessPackages/apps/my-app/.serverless/my-app.zip',
  zipFiles: [
    'my-app.zip',
    '/home/my-user/my-repo/dist/.serverlessPackages/apps/my-app/dist/.serverlessPackages/apps/my-app/.serverless/my-app.zip'
  ],
  zipFilePaths: [
    '/home/my-user/my-repo/dist/.serverlessPackages/apps/my-app/.serverless/my-app.zip',
    '/home/my-user/my-repo/dist/.serverlessPackages/apps/my-app/dist/.serverlessPackages/apps/my-app/.serverless/my-app.zip'
  ]
}

We can see that the resolution on the prior step appends servicePath again which then on subsequent lines results in a silent ENOENT when serverless attempts to calculate the hash of the non-existant zip file. I already have tried to reproduce this through serverless directly, but as we are manipulating the runtime configuration in real time here, it is not easily reproducible through the command line for sls.

Suggested resolution After a lot of trial and error if we modify the servicePath override on https://github.com/flowaccount/nx-plugins/blob/master/libs/nx-serverless/src/builders/deploy/deploy.impl.ts#L123 to be like so:

        ServerlessWrapper.serverless.config.servicePath = path.resolve(packagePath);

This solves the path resolution issue by preventing path.resolve from incorrectly attempting to append the path:

{
  artifact: '/home/my-user/my-repo/dist/.serverlessPackages/apps/my-app/.serverless/my-app.zip',
  zipFiles: [
    'my-app.zip',
    '/home/my-user/my-repo/dist/.serverlessPackages/apps/my-app/.serverless/my-app.zip'
  ],
  zipFilePaths: [
    '/home/my-user/my-repo/dist/.serverlessPackages/apps/my-app/.serverless/my-app.zip',
    '/home/my-user/my-repo/dist/.serverlessPackages/apps/my-app/.serverless/my-app.zip'
  ]
}

I want to submit a PR, but I've already spent days on this, and would need to upgrade my fork to match the master branch which includes the beta work for supporting serverless 2, which is a whole other can of worms for my project.

My repo with the change is here: https://github.com/tstackhouse/nx-plugins along with a solo build branch so I can pull it into my project: https://github.com/tstackhouse/nx-plugins/tree/nx-serverless-2

I have it on my radar to plan for upgrades in the near future (I hope) and am excited to see this flourish. I've seen some of the work to support Nx11, which would be awesome as well.

Check which provider is affected: [x] AWS [?] Azure [?] Google Cloud Platform

Check which framework is affected: [ ] Angular [x] Nodejs [x] Serverless [ ] Lambda [ ] Infrastructure as a code

Additional context Currently using 0.5.3 of this plugin Node 12.18.3 npm 6.14.6 serverless: Framework Core: 1.79.0 Plugin: 3.7.1 SDK: 2.3.1 Components: 2.34.6

tstackhouse avatar May 26 '21 18:05 tstackhouse

Hey i just saw this bug. Is it happening to everyone who is trying to use the beta version then?

wickstargazer avatar Jun 03 '21 12:06 wickstargazer

@tstackhouse would this help? --> https://github.com/flowaccount/nx-plugins/pull/84/files#diff-83dc8d582b8fb37ed63b28f9d66778aaf1b91d48495c4085387fda2eed4afdef

also i merged the latest code into master although the test isn't working yet. Will try to find time to make them work. but if this helps will release beta.4

wickstargazer avatar Jun 03 '21 12:06 wickstargazer

It's a step in the right direction, though I was doing some more testing and found another issue in the upload artifacts step when not packaging individually: https://github.com/serverless/serverless/blob/master/lib/plugins/aws/deploy/lib/uploadArtifacts.js#L104-L116

That usage of packagePath there is the culprit, because that gets set way that the initialization of the library here: https://github.com/serverless/serverless/blob/master/lib/plugins/aws/deploy/index.js#L22-L25

And because of that, it's not actually overridden by the time we do the override in deploy.impl.ts. Using package.individually avoids going into that if statement and sidesteps the issue.

tstackhouse avatar Jun 03 '21 19:06 tstackhouse

so should we set the package.artifact? and test it out to see if it works. To summarize a bit. It works if not packaging individually correct? since i tested out and that seems ok.

wickstargazer avatar Jun 04 '21 03:06 wickstargazer

The opposite actually, at least in my experience. Packaging individually works because it goes inside both if statements in that code in uploadArtifacts.js.

Not packaging individually only goes into the outer if, as the artifact is set in https://github.com/serverless/serverless/blob/master/lib/plugins/package/lib/packageService.js#L99-L113 during the packat step.

Commenting the return on line 115 allows the if statement to be exited and the deploy works correctly.

tstackhouse avatar Jun 04 '21 03:06 tstackhouse

Commenting the return on line 115 allows the if statement to be exited and the deploy works correctly. in which file?

wickstargazer avatar Jun 04 '21 04:06 wickstargazer

problem is i released 1.0.0-beta and the code has changed Alot....my bad for lack of communications. Want to make sure it works and we are talking about the same issue. I am testing out with packagingIndividually flags now

wickstargazer avatar Jun 04 '21 05:06 wickstargazer

ok i found out that if i set package.individually: false the error of the zip file occurs. I am on the right track right?

There was an error with the build. Error: ENOENT: no such file or directory, open dist\.serverlessPackages\apps\api\lambda\dist\.serverlessPackages\apps\api\lambda.user-analytics\.serverless\lambda-.zip

trying to solve it

wickstargazer avatar Jun 04 '21 06:06 wickstargazer

i traced it down to something in serverless itself

image

the servicePath is changed and not same as serviceDir in

serverless\lib\plugins\aws\deploy\index.js. In constructor it is set to ServiceDir but along the way its changed! ..... line 97 i log it and the value has changed.

still figuring out

wickstargazer avatar Jun 04 '21 13:06 wickstargazer

Commenting the return on line 115 allows the if statement to be exited and the deploy works correctly. in which file?

Sorry, I was on my phone and getting out a detailed reply is tricky. like 115 in uploadArtifacts.js: https://github.com/serverless/serverless/blob/master/lib/plugins/aws/deploy/lib/uploadArtifacts.js#L115

problem is i released 1.0.0-beta and the code has changed Alot....my bad for lack of communications. Want to make sure it works and we are talking about the same issue. I am testing out with packagingIndividually flags now

No worries about the 1.0.0-beta, I need to see what if anything I would need to do to use serverless 2 in my project over 1.x.

ok i found out that if i set package.individually: false the error of the zip file occurs. I am on the right track right?

There was an error with the build. Error: ENOENT: no such file or directory, open dist.serverlessPackages\apps\api\lambda\dist.serverlessPackages\apps\api\lambda.user-analytics.serverless\lambda-.zip

trying to solve it

That's exactly it. I did see that you fixed an issue in the beta that I was going to submit a fix for that I saw in 0.5.3:

In the beta: https://github.com/flowaccount/nx-plugins/blob/f22b58d1d087537513d9317da30b61a89c7fd705/libs/nx-serverless/src/utils/serverless.ts#L175 in 0.5.3: https://github.com/flowaccount/nx-plugins/blob/0.5.3/libs/nx-serverless/src/builders/deploy/deploy.impl.ts#L138-L141

There was no logging on deploy failure, so it would silently exit the nx command and the only indicator was an exit code of 1. I had added a log call so help me trace the failures in my debugging: https://github.com/tstackhouse/nx-plugins/blob/master/libs/nx-serverless/src/builders/deploy/deploy.impl.ts#L141-L147

tstackhouse avatar Jun 04 '21 15:06 tstackhouse

yeh I had to refactor the whole thing to work with nx-12 .. Its another pattern that is used with nx/dev-kit and generators etc etc as well.

So i am investigating why the path changed. I think it has something to do with the serverless lifeCycle hooks not sure which hook changed the variable.

wickstargazer avatar Jun 04 '21 16:06 wickstargazer

I'm investigating updating to 2.x and at least upon first impressions, the way this plugin handling initializing serverless, if we use an SSM reference in our serverless.yml, the framework attempts to parse the config and fails because the aws plugin isn't loaded, I tried commenting out all my SSM params from the config, but when it gets to the deploy step, it seems to blow out:


npx nx deploy my-api --stage=my-stage

// Build output removed

dist/.serverlessPackages/apps/my-api
Copying build output files from dist/apps/my-api to dist/.serverlessPackages/apps/my-api to be packaged
Done copying build output files.
running serverless commands
There was an error with the build. ServerlessError: Serverless command "deploy" not found. Did you mean "undefined"? Run "serverless help" for a list of all available commands..

tstackhouse avatar Jun 04 '21 17:06 tstackhouse

Replying from phone Check the first line of import Serverless 2 uses local serverless so when debugging have to use the path to the debugging api i commented it out as example

Would be good to reuse the serverless resolveLocal function

On Sat, Jun 5, 2021, 12:30 AM Tim Stackhouse, @.***> wrote:

I'm investigating updating to 2.x and at least upon first impressions, the way this plugin handling initializing serverless, if we use an SSM reference in our serverless.yml, the framework attempts to parse the config and fails because the aws plugin isn't loaded, I tried commenting out all my SSM params from the config, but when it gets to the deploy step, it seems to blow out:

npx nx deploy my-api --stage=my-stage

// Build output removed

dist/.serverlessPackages/apps/my-api Copying build output files from dist/apps/my-api to dist/.serverlessPackages/apps/my-api to be packaged Done copying build output files. running serverless commands There was an error with the build. ServerlessError: Serverless command "deploy" not found. Did you mean "undefined"? Run "serverless help" for a list of all available commands..

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/flowaccount/nx-plugins/issues/82#issuecomment-854891991, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALY2EXYFBDGXYA5UDROEHTTREET5ANCNFSM45SVIVWA .

wickstargazer avatar Jun 04 '21 17:06 wickstargazer

Researching a bit more, it almost seems like something is causing the AWS plugins to not load at all:

Serverless: Load command param:list
Serverless: Load command studio
Serverless: Configuration warning: Unrecognized provider 'aws'
Serverless:  
Serverless: You're relying on provider plugin which doesn't provide a validation schema for its config.

-- Continuing to dig around, it looks like the sls builder doesn't include compat in the 1.0.0-beta.3 dist: image image

10086$ npx nx run my-api:sls help
> nx run my-api:sls 
Cannot find module '/home/me/my-project/node_modules/@flowaccount/nx-serverless/src/builders/sls/compat'
Require stack:
- /home/me/my-project/node_modules/@angular-devkit/architect/node/node-modules-architect-host.js
...

-- More realtime debugging research: ~It looks like deploy might not be initializing serverless before attempting to run any commands (in your serverless.js file), build.impl.js doesn't yet use your refactored library.~ I'm still picking through things, though it still seems like the AWS plugin isn't being loaded... It looks like this could be something with my environment, but I'm not sure. The plugin has it's own node_modules and subsequently, it's own serverless. when I build, the project level Serverless gets called and initialized, but then when deploy is called, it's calling the serverless inside the plugin, so none of the commands are there and hence the error I saw in my prior comment. I'm not sure how best to fix it yet :/

tstackhouse avatar Jun 04 '21 17:06 tstackhouse

in serverless.ts you need to change the first import to something like

import * as Serverless from '<path to your debugging-workspace>/node_modules/serverless/lib/Serverless.js'; instead of 'serverless/lib/Serverless';

this will make thigs work

wickstargazer avatar Jun 07 '21 05:06 wickstargazer

@tstackhouse the deploy per function is working now in 1.0.0-beta.8 can we close this issue? its fixed in this PR #97

wickstargazer avatar Nov 14 '21 17:11 wickstargazer

I've been able to get back into this a bi, trying to update to 1.1.0 and serverless 2 in my project, and this bug is related to a slightly different issue, when using packageIndividually: false in serverless.yml a single zip file is built to be deployed, however due to the hotpatching necessary to support the .serverlessPackages paths, this file resolution fails. It also seems like my proposed solution in the original issue, using path.resolve() prior to the hotpatch on the serverless object doesn't seem to work properly with serverless 2+.

I'm kind of just putting words on paper at this point, so forgive me, I've been iterating on this and getting back into the headspace all day today. It seems to me that it should be possible through configuration or code changes to alleviate this, I keep coming back to the .serverlessPackages piece being one potential thread that contributes to this. Is there a reason to have this path over just packaging directly in dist?

tstackhouse avatar Dec 28 '21 23:12 tstackhouse

I've actually tracked down an easy potential fix for this, by changing the code here: https://github.com/flowaccount/nx-plugins/blob/master/libs/nx-serverless/src/utils/serverless.ts#L126-L132 from:

      if (
        deployOptions &&
        deployOptions.function &&
        deployOptions.function != ''
      ) {
        serverlessConfig.servicePath = getPackagePath(deployOptions);
      }

to

      if (deployOptions) {
        serverlessConfig.serviceDir = getPackagePath(deployOptions);
      }

Omitting the extra conditionals and correcting the config option name a project with pacakgeIndividually: false in serverless.yml allows for a single zip file deployment to go through properly by setting the config value earlier in the initialization process and avoiding the need to hotpatch the config.

I can try to pull together a PR tomorrow

tstackhouse avatar Dec 29 '21 00:12 tstackhouse