mwilliamson / spur.py

Run commands and manipulate files locally or over SSH using the same interface
BSD 2-Clause "Simplified" License
267 stars 37 forks source link

Add proxy command capabilities to SshShell #41

Closed lindycoder closed 8 years ago

lindycoder commented 8 years ago

Hello,

This is sort a feature request / proposition.

When using paramiko's client.connect, you can can specify a sock=, this parameter can be passed an instance of paramiko.proxy.ProxyCommand instanciated with a proxy command such as... "ssh -q -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null user@proxyname nc -q0 1.2.3.4 22" Which would connect to 1.2.3.4 through the given proxy

So i would like to propose this change but wanted to know how you would see it implemented:

What do you think?

mwilliamson commented 8 years ago

Hello! Thanks for the suggestion. I've never used the sock argument, but from a quick read it looks like a socket-like argument -- that is, the interface isn't really specific to paramiko, but is whatever subset of the socket interface that Paramiko uses. So, it's not entirely implausible for people to provide their own implementation.

Perhaps there's a way we can get the simplicity of option 1, to keep the interaction with Paramiko simple (and more likely to be correct), with the user-friendliness of option 3. Specifically, what about something like (naming needs more thought):

spur.SshShell(sock=spur.proxy.ssh_command(username=...))

Where spur.proxy.ssh_command(...) creates the instance of ProxyCommand that paramiko expects?

lindycoder commented 8 years ago

Sure sounds nice... i'll change my PR to have a new sock= parameter to which you can feed anything, that would unblock me

and in a second pass the ssh_command helper could be added

lindycoder commented 8 years ago

PR has been updated

lindycoder commented 8 years ago

Okay here is some interesting news :

Paramiko's proxy command is NOT python 3 compatible but it's only partially part of paramiko, because as a user you could give anything that is socket like to sock=, there is an issue about that : paramiko/paramiko#673 which contains a PR with a python 3 comptable version of ProxyCommand.

So to add the proxy command building helper in spur i would need a test, that both work in py2 and py3, py2 would use the ProxyCommand (stock) and the py3 would use a copy made from the PR.

So we could venture into creating a new ProxyCommand that works for both version and include it in your package, but i fear that would include some "if py2 then... if py3 then" in the code which is not the most elegant. Or we could push to paramiko but that looks hard, there's 65 PR open and some are a years old.

My suggestion : leave it with the sock= i already proposed and no builders, so users such as me can have their own fixed proxy command that they use in their project that may not require py2 compatibility

mwilliamson commented 8 years ago

I think your suggestion makes sense: trying to provide builders is probably out of the scope of this library given that Paramiko itself doesn't provide an implementation of ProxyCommand that works across Python 2 and 3. Thanks again for the pull request!

lindycoder commented 8 years ago

Wonderful, thanks for your time, keep up the good work!

mwilliamson commented 8 years ago

No problem at all, thanks for the code and accommodating my requests!