Skip to content

Conversation

@justinmchase
Copy link
Contributor

@justinmchase justinmchase commented Dec 10, 2018

When building a nodejs lambda it is really common for dependencies under the node_modules folder to actually be symlink'd to other folders on your system. Currently, when creating a lambda with a dependencies linked this way, it will package the symlink itself and not the files within it.

For example, if you create a lambda function like this:

const example = new Function(this, 'Example'), {
  runtime: Runtime.NodeJS810,
  handler: "index.handler",
  code: Code.directory("example"))
});

And if that example code directory has a dependency like this in its package.json:

"dependencies": {
  "helpers": "../helpers"
}

When calling npm i before deployment, it will actually generate a symlink to the helpers directory node_modules/helpers -> ../helpers. Trying to deploy such a lambda function results in an error when you execute the lambda since the files within the symlink'd folder were not included in the zip and therefore were not deployed up to lambda.

This change simply configures the archiver to follow symlinks when building the zip file. It is not a breaking change as all existing directory structures should work, there is no scenario where a previously working zipdirectory would now not be working. None that I know of at least.

Fixes #731


Testing

  • Unit test and/or integration test added
  • Toolkit change?: N\A
  • Init template change?: N\A

Documentation

  • README: README and/or documentation topic updated
  • jsdocs: All public APIs documented

Title and description

  • Change type: Title is prefixed with change type:
    • fix(module): <title> bug fix (patch)
    • feat(module): <title> feature/capability (minor)
    • chore(module): <title> won't appear in changelog
    • build(module): <title> won't appear in changelog
  • Title format: Title uses lower case and doesn't end with a period
  • Breaking change?: Last paragraph of description is: BREAKING CHANGE: <describe exactly what changed and how to achieve similar behavior + link to documentation/gist/issue if more details are required>
  • References: Indicate issues fixed via: Fixes #xxx or Closes #xxx

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

@justinmchase justinmchase requested a review from a team as a code owner December 10, 2018 19:07
@rix0rrr
Copy link
Contributor

rix0rrr commented Dec 11, 2018

I'm slightly concerned about in-archive symlinks (wasted space) and recursive symlinks (infinite recursion?)

I feel we could optimize those some more. But at the same time, I will concede it's probably not a big concern.

@rix0rrr rix0rrr merged commit 2dfd593 into aws:master Dec 11, 2018
@eladb
Copy link
Contributor

eladb commented Dec 11, 2018

How hard it is to preserve symlinks for links inside the same directory? Does the glob library support something like this?

@justinmchase
Copy link
Contributor Author

justinmchase commented Dec 11, 2018

@rix0rrr glob will prevent infinite reference loops:
isaacs/node-glob#252

@eladb I believe if you link to the same folder twice it will copy the files into the zip twice. I don't think its supported in glob to have a partial solution, preserve symlinks that are subdirs and follow those that are not... but it seems like that would actually be doable from looking in glob. It would require adding the feature to glob I think but it would probably be a medium-low complexity feature I would guess.

@justinmchase justinmchase deleted the fix/731-follow-symlinks branch December 11, 2018 15:23
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 this pull request may close these issues.

Directory assets: symlinks

3 participants