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

Support a configured SSHClient #1748

Closed
Dreamsorcerer opened this issue Sep 13, 2020 · 8 comments
Closed

Support a configured SSHClient #1748

Dreamsorcerer opened this issue Sep 13, 2020 · 8 comments

Comments

@Dreamsorcerer
Copy link

It would be nice to have a new class, something like ConfiguredSSHClient which automatically configures everything from an ssh config file.

The documentation says that users of the library can use the config themselves, but it seems unnecessary to require everyone to reimplement this code. It would be ideal of Paramiko just worked closer to the Openssh client out of the box.

I've started an implementation of the suggested class, so would you consider including this if I produce a PR for it?

Suggested interface would be to initialise a new client with:
client = ConfiguredSSHClient(user_config, system_config)
It would then work the same as the SSHClient, except any arguments missing from the connect() call would be filled in by the config if they have a matching hostname.

@MrMino
Copy link
Contributor

MrMino commented Sep 14, 2020

If this is OpenSSH specific, the functionality should reference it by name in the identifiers.

Don't be fooled by prevalence of OpenSSH - while a good idea to use it as a reference, tying a project to it is IMO a bad idea.

@Dreamsorcerer
Copy link
Author

Well, SSHConfig doesn't reference OpenSSH in the name, and this would essentially just be a wrapper around that object and SSHClient. It wouldn't be adding anything new that's specific to OpenSSH (and could potentially support other config formats if the SSHConfig class chooses to).

Regardless, naming is a minor implementation detail. I just want to know if it's likely to be accepted if I spend the time to flesh it out and create a PR?

@MrMino
Copy link
Contributor

MrMino commented Sep 14, 2020

@Dreamsorcerer you're right, I guess the naming is not that big of a deal.

That question has to be answered by @bitprophet though.

@Dreamsorcerer
Copy link
Author

To give an idea of the implementation, this is my proof-of-concept I'm using in my project, which supports hostname, user and proxycommand from the config:

SSH_PATH_USER = Path.home() / ".ssh/config"
SSH_PATH_SYSTEM = Path("/etc/ssh/ssh_config")

class ConfiguredSSHClient(SSHClient):
    def __init__(self, user_path=SSH_PATH_USER, system_path=SSH_PATH_SYSTEM):
        super().__init__()
        self._ssh_config = SSHConfig()
        with suppress(FileNotFoundError):
            self._ssh_config.parse(user_path.open())
        with suppress(FileNotFoundError):
            self._ssh_config.parse(system_path.open())

        self.load_system_host_keys()

    def connect(self, hostname, **kwargs):
        host_config = self._ssh_config.lookup(hostname)
        connect_options = {}
        with suppress(KeyError):
            connect_options["username"] = host_config["user"]
        with suppress(KeyError):
            connect_options["sock"] = ProxyCommand(host_config["proxycommand"])
        connect_options.update(kwargs)

        super().connect(host_config["hostname"], **connect_options)

@bitprophet
Copy link
Member

That level of thing tends to live in Fabric (fabfile.org) - it has a handy Connection class that will load up ssh config files by default: https://docs.fabfile.org/en/2.5/concepts/configuration.html#ssh-config - I'm open to improvements there, but in Paramiko's own Client (which is older and predates Fabric 2) I only really intend to put in outright bugfixes.

@Dreamsorcerer
Copy link
Author

I initially skipped Fabric, as it looks like a much larger, complex library, when I'm only looking for a small layer of abstraction (i.e. paramiko already has >90% of what's needed for this). But, I'll take another look.

@bitprophet
Copy link
Member

@Dreamsorcerer FWIW Fabric version 2 is being designed to be much more API friendly, meaning you can get away with just using a handful of classes instead of feeling forced into its entire way of doing things (as in v1). In this case, Connection is largely a more powerful wrapper around SSHClient.

@Dreamsorcerer
Copy link
Author

Any chance of some help with: fabric/fabric#2135

Still struggling to get my project converted over to Fabric due to this one problem of streaming input into a Fabric Connection. It works without issue using Paramiko, but Fabric just won't stream it over the connection.

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