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

main does not set working dir correctly for Lambda zip? #667

Closed
lukehoban opened this issue Dec 8, 2017 · 8 comments
Closed

main does not set working dir correctly for Lambda zip? #667

lukehoban opened this issue Dec 8, 2017 · 8 comments
Assignees
Milestone

Comments

@lukehoban
Copy link
Member

It appears that references to . in assets used for Lambdas will use the actual working directory, not the folder that main points to.

I believe the contract for main is that it sets the working directory for execution of the program. If so, it should use this for interpretation of . in Assets as well.

This leads to Lambda zip files being too large when used in setups with a Pulumi.yaml in a root folder.

See for example the failures in https://travis-ci.com/pulumi/pulumi-ppc/builds/61152492, based on layout in https://github.com/pulumi/pulumi-ppc/tree/464c475794ee36f5c80f744d70bd816731448a3c/infrastructure.

@lukehoban lukehoban added this to the 0.9.1 milestone Dec 8, 2017
@lukehoban
Copy link
Member Author

This is blocking being able to add tests for the PPC infrastructure, which I believe is critical to add in 0.9.1. Pulling this into the milestone as well, to either fix or identify a workaround.

Note that we could also "fix" this by getting RelativeWorkDir in the test framework working again (it was broken by the main addition). But in theory, main should be a replacement for that.

@lukehoban
Copy link
Member Author

Indeed - we only pass the Main through into the langhost Pwd. However, the langhost passes an asset which includes "." back to the engine, and the engine interprets that path based on it's own working directory, which is the working directory where the CLI was launched. So lambdas will end up including the full contents of the working direction where the CLI is launched, not what Main points at.

@joeduffy
Copy link
Member

IMHO we should probably require the langhost to pass an absolute path for all assets, rather than introducing a requirement that the processes share a working directory. /cc @pgavlin

@lukehoban
Copy link
Member Author

Perhaps - though we do not want to serialize an absolute path into the checkpoint file.

@joeduffy
Copy link
Member

@lukehoban Did you plan to take a look at this? The bug is currently unassigned and I wasn't sure if you were asking either @pgavlin or I to take it from here.

@pgavlin
Copy link
Member

pgavlin commented Dec 12, 2017

Perhaps - though we do not want to serialize an absolute path into the checkpoint file.

@lukehoban is correct; we definitely do not want to be serializing absolute paths into checkpoints. I think that it would be reasonable to require that all paths from the language host are either a) absolute, or b) relative to the directory that contains Pulumi.yaml. The former condition is pretty easy to validate; the engine can then translate these paths s.t. they are relative to the Pulumi.yaml directory.

@joeduffy
Copy link
Member

That sounds like a nice solution.

joeduffy added a commit that referenced this issue Dec 12, 2017
This change just flows the project's "main" directory all the way
through to the plugins, fixing #667.  In that work item, we discussed
alternative approaches, such as rewriting the asset paths, but this
is tricky because it's very tough to do without those absolute paths
somehow ending up in the checkpoint files.  Just launching the
processes with the right pwd is far easier and safer, and it turns
out that, conveniently, we set up the plugin context in exactly the
same place that we read the project information.
joeduffy added a commit that referenced this issue Dec 12, 2017
This change just flows the project's "main" directory all the way
through to the plugins, fixing #667.  In that work item, we discussed
alternative approaches, such as rewriting the asset paths, but this
is tricky because it's very tough to do without those absolute paths
somehow ending up in the checkpoint files.  Just launching the
processes with the right pwd is far easier and safer, and it turns
out that, conveniently, we set up the plugin context in exactly the
same place that we read the project information.
joeduffy added a commit that referenced this issue Dec 12, 2017
This change just flows the project's "main" directory all the way
through to the plugins, fixing #667.  In that work item, we discussed
alternative approaches, such as rewriting the asset paths, but this
is tricky because it's very tough to do without those absolute paths
somehow ending up in the checkpoint files.  Just launching the
processes with the right pwd is far easier and safer, and it turns
out that, conveniently, we set up the plugin context in exactly the
same place that we read the project information.
joeduffy added a commit that referenced this issue Dec 12, 2017
This change just flows the project's "main" directory all the way
through to the plugins, fixing #667.  In that work item, we discussed
alternative approaches, such as rewriting the asset paths, but this
is tricky because it's very tough to do without those absolute paths
somehow ending up in the checkpoint files.  Just launching the
processes with the right pwd is far easier and safer, and it turns
out that, conveniently, we set up the plugin context in exactly the
same place that we read the project information.
joeduffy added a commit that referenced this issue Dec 12, 2017
This change just flows the project's "main" directory all the way
through to the plugins, fixing #667.  In that work item, we discussed
alternative approaches, such as rewriting the asset paths, but this
is tricky because it's very tough to do without those absolute paths
somehow ending up in the checkpoint files.  Just launching the
processes with the right pwd is far easier and safer, and it turns
out that, conveniently, we set up the plugin context in exactly the
same place that we read the project information.
joeduffy added a commit that referenced this issue Dec 12, 2017
This change just flows the project's "main" directory all the way
through to the plugins, fixing #667.  In that work item, we discussed
alternative approaches, such as rewriting the asset paths, but this
is tricky because it's very tough to do without those absolute paths
somehow ending up in the checkpoint files.  Just launching the
processes with the right pwd is far easier and safer, and it turns
out that, conveniently, we set up the plugin context in exactly the
same place that we read the project information.
@joeduffy
Copy link
Member

Fixed in b6a3869.

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

No branches or pull requests

3 participants