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

Node Dynamic Provider fails to launch when ./node_modules/@pulumi/pulumi does not exist #2261

Closed
ellismg opened this issue Nov 30, 2018 · 0 comments · Fixed by #2274
Closed
Assignees
Milestone

Comments

@ellismg
Copy link
Contributor

ellismg commented Nov 30, 2018

The launcher scripts for the dynamic provider, which ship with the CLI itself, do the following to kick off a node process to launch the dynamic provider

#!/bin/sh
node ./node_modules/@pulumi/pulumi/cmd/dynamic-provider $@

(a similar strategy is used on Windows).

Because we use a relative PATH here, we don't get all the node module loading semantics. This leads to cases like what we saw in this message: https://pulumi-community.slack.com/archives/C84L4E3N1/p1543598350013100 where a user had tried to have two pulumi projects that shared a central node_modules folder, yielding this tree structure:

├── bastion // pulumi project
├── dliver //pulumi project
├── modules // shared custom modules
├── node_modules // shared node_modules
└── static

A similar tree structure would be used if you were using lerna or yarn workspaces or the like.

When we faced this problem in the language plugin, we first used node -e console.log(require.resolve('@pulumi/pulumi/...')) to construct the path that we should execute and only then invoked node on this path. We should do something similar for the dynamic provider. This will be pretty straight-forward on *nix, as we can do something like:

#!/bin/sh
node $(node -e "console.log(require.resolve('@pulumi/pulumi/cmd/dynamic-provider'))") $@

On windows, this will be a little less straight-forward.

I am wondering if we should bite the bullet and move the resource plugin to be a single binary (instead of a script) like we have for all our other plugins (both languages and resources). Long term, it feels like this plugin should be delivered via our normal mechanimism (e.g. pulumi plugin install).

@pgavlin I'm interested in your thoughts here. Do we need to tightly couple the rest of the runtime with the dynamic provider? I understand we use it for testing core parts of the engine, so pulling these apart may be tricky in at our lowest level, but it feels like at some point we should have something like @pulumi/dynamic-node or some other package that just has the dynamic provider in it.

@ellismg ellismg added this to the 0.20 milestone Nov 30, 2018
ellismg added a commit that referenced this issue Dec 5, 2018
Previously, we assumed that the dynamic provider was located in
`./node_modules/@pulumi/pulumi/../` which is correct in the majority
of cases. However, tools like lerna or yarn workspaces (or custom
workflows) allow the node_modules folder to be located elsewhere on
disk, and node will still find it because of its algorithm for module
resolution.

So, do what we do in the language host itself, first launch node and
ask it to tell us where it resolves a require statement to on disk and
then launch node against that script.

Fixes #2261
ellismg added a commit that referenced this issue Dec 5, 2018
Previously, we assumed that the dynamic provider was located in
`./node_modules/@pulumi/pulumi/../` which is correct in the majority
of cases. However, tools like lerna or yarn workspaces (or custom
workflows) allow the node_modules folder to be located elsewhere on
disk, and node will still find it because of its algorithm for module
resolution.

So, do what we do in the language host itself, first launch node and
ask it to tell us where it resolves a require statement to on disk and
then launch node against that script.

Fixes #2261
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 a pull request may close this issue.

1 participant