Skip to content

NonZero userids in docker breaks using mounted ssh keys #182

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

Closed
robtandy opened this issue Apr 27, 2018 · 6 comments
Closed

NonZero userids in docker breaks using mounted ssh keys #182

robtandy opened this issue Apr 27, 2018 · 6 comments
Labels

Comments

@robtandy
Copy link

Changes in the 4.0.0 beta that tell the docker container to run as a particular user break using the mounted SSH keys for pip. Specifically, #145

part of the cmdOptions that are sent to docker include mounting ~/.ssh/id_rsa as /root/.ssh/id_rsa within the docker container. An ssh key present is required when a line in pip requirements.txt looks like

git+ssh://git@github.com/....

Mounting it as /root/ requires that the docker user also be root.

Looking through the code, and experimenting with different versions of serverless-python-requirements, it looks like setting the docker user to be root all the time (the default) is fine. the .serverless directory always seems owned by the current host user. We have tested using v4.0.x, and v3.3.1 on windows, linux and mac. and we were fine.

Could it be that using a non-root user in docker is not the best way to handle the cache-dir issue for pip? One thought here is to provide --cache-dir=.serverless/pipCache in order to have the cache exist outside docker and serve a useful purpose over multiple builds.

@dschep
Copy link
Contributor

dschep commented Apr 27, 2018

Damn. Pesky permissions.

Could it be that using a non-root user in docker is not the best way to handle the cache-dir issue for pip?

Yup. That's the reason the change was proposed.

One thought here is to provide --cache-dir=.serverless/pipCache in order to have the cache exist outside docker and serve a useful purpose over multiple builds.

@AndrewFarley your caching work in progress (#165) would provide exactly that, right?

@AndrewFarley
Copy link
Contributor

Yeah it does, it precisely solves all problems like this by operating docker completely in a temp directory not related to the local folder. I’d ask to try my pull request to see if that solves the problem. It’s fully functional as far as I can tell but I need to rebase against master and fix a test and a few things. I’ll try to get around to finishing it soon I hope.

@cgrimal
Copy link

cgrimal commented May 31, 2018

Hi guys, I ran into the same issue as described by @robtandy
I'm the one who implemented the dockerSsh option, so I'm a bit sad it is now broken...

I was glad to hear that @AndrewFarley's PR would solve that, but I'm afraid it does not (even though I really like the features implemented, but that's another subject).
There is even a TODO entry: "Make sure dockerSsh works? (I don't quite understand what it does, so someone help, or in a test hopefully (didn't look))". So I guess I should be the one looking into it maybe!

@dschep: Is there a way you want this feature to be implemented? Basically, the Docker user must be root to be able to use the ssh keys, but must be the current user to use pip option (such as --cache-dir).
This commit is the 'culprit' (if I may): c0be594

Since @AndrewFarley's static cache feature would render the pip --cache-dir option useless, I guess we could go back to my implementation?

@AndrewFarley
Copy link
Contributor

^ Thanks for the comment, indeed if things get merged from that PR it will largely solve this problem, but my MR will need some help, as I don't think this feature works. Will chat about it in my MR with the author, and maybe that will retroactively fix this, and a handful of longstanding other bugs.

@AndrewFarley
Copy link
Contributor

@robtandy I think we've largely fixed this in the upcoming merge/PR of mine. If you have a second, could you try it?

rm -Rf node_modules/serverless-python-requirements
npm i github:andrewfarley/serverless-python-requirements#master

Then do your thing, sls package, or sls deploy, or whatever your usual pattern to cause this problem to occur. We had @cgrimal help work on this and confirm this should/might fix your problem. Could you try, and if it doesn't fix it, then one of us will have to look into this issue separately/directly. If this doesn't fix your problem, could you provide your serverless.yml? Thanks!

@pgrzesik
Copy link
Contributor

Hey 👋 I'm closing this ticket as it looks like it's heavily outdated, we can of course reopen it if needed 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants