[Twisted-Python] potential connectSSH workflow
I've written up a potential workflow for connectSSH, a simplified way of using Twisted Conch. It's available at http://z3p.jot.com/WikiHome/SummerOfCode2007/connectSSH. If those who are interested could take a look, I'd love some feedback. thanks, -p -- Paul Swartz paulswartz at gmail dot com http://z3p.livejournal.com/ AIM: z3penguin
On 05:49 pm, paulswartz@gmail.com wrote:
I've written up a potential workflow for connectSSH, a simplified way of using Twisted Conch. It's available at http://z3p.jot.com/WikiHome/SummerOfCode2007/connectSSH. If those who are interested could take a look, I'd love some feedback.
Thanks a lot! Here are my initial impressions: The bundle of crud that lives in ~/.ssh should be represented as an object. connectSSH should really be a method on that object, so that things like host key verification can happen as normal and expected. This is especially true because that object can have references to other objects, such as the (Deferred-returning) UI for authorizing or denying an unexpected host key. That object could also be more easily tested because the reactor could be an attribute of it rather than a global import. Unfortunately there's a lot of code in Twisted that doesn't do this yet, but we should be building better use of that singleton. See http://glyf.livejournal.com/70684.html (On a tangent, this is similar to the fact that in the next-generation HTTP API, there should be a "browser" object for indirecting the reactor, but also for persisting cookies, HTTP authorization headers, and the like. A free function for doing a connectSSH is lame for many of the same reasons that getPage() is lame.) This also looks like an excellent use-case for endpoints. Please see this ticket: http://twistedmatrix.com/trac/ticket/1442 and see if you have any feedback there. What do you think?
On 7/26/07, glyph@divmod.com <glyph@divmod.com> wrote:
On 05:49 pm, paulswartz@gmail.com wrote:
I've written up a potential workflow for connectSSH, a simplified way of using Twisted Conch. It's available at http://z3p.jot.com/WikiHome/SummerOfCode2007/connectSSH. If those who are interested could take a look, I'd love some feedback.
Thanks a lot! Here are my initial impressions:
The bundle of crud that lives in ~/.ssh should be represented as an object. connectSSH should really be a method on that object, so that things like host key verification can happen as normal and expected. This is especially true because that object can have references to other objects, such as the (Deferred-returning) UI for authorizing or denying an unexpected host key. That object could also be more easily tested because the reactor could be an attribute of it rather than a global import. Unfortunately there's a lot of code in Twisted that doesn't do this yet, but we should be building better use of that singleton. See http://glyf.livejournal.com/70684.html
That makes sense: a ConchConnection object which wraps some of the nonsense that currently lives in t.c.scripts.conch like verifying a host key and finding keys to use. I still think that passing authentication data in connect() is the right idea, but it can default to using the keys that it finds in ~/.ssh or in a key agent.
This also looks like an excellent use-case for endpoints. Please see this ticket: http://twistedmatrix.com/trac/ticket/1442 and see if you have any feedback there.
I think making a ConchClientEndpoint which takes another ClientEndpoint and some authentication data is a good interface, but I'm not holding my breath for endpoints-1442 getting merged to trunk. I'm already depending on at least 1-2 of my branches getting merged :) -p -- Paul Swartz paulswartz at gmail dot com http://z3p.livejournal.com/ AIM: z3penguin
On 09:14 pm, paulswartz@gmail.com wrote:
That makes sense: a ConchConnection object which wraps some of the nonsense that currently lives in t.c.scripts.conch like verifying a host key and finding keys to use.
"Connection" seems like a weird name for that to me, as it applies it is attached to a particular ... well, connection. Connector, maybe? Client? ClientStorage? I guess those names have some pretty serious problems too, but it's worth thinking about a better one.
I still think that passing authentication data in connect() is the right idea, but it can default to using the keys that it finds in ~/.ssh or in a key agent.
I agree emphatically, but both the default and non-default cases are equally important. There's a direct analogue in the user interface: "ssh foo.com" will use your default key or agent, and your default username, and that's 99% of usage. That, of course, doesn't mean you can get rid of "ssh -i mykey.id_rsa foo.com" or "ssh otheruser@foo.com", they're both very necessary use-cases.
This also looks like an excellent use-case for endpoints. Please see this ticket: http://twistedmatrix.com/trac/ticket/1442 and see if you have any feedback there.
I think making a ConchClientEndpoint which takes another ClientEndpoint and some authentication data is a good interface, but I'm not holding my breath for endpoints-1442 getting merged to trunk. I'm already depending on at least 1-2 of my branches getting merged :)
Definitely. This should be a separate ticket. In fact, far from asking you to wait for it to be merged, I was wondering if you had any comments on the interface that might come from use-cases that would arise in the conch endpoint implementation that might even further delay it :).
On 7/26/07, glyph@divmod.com <glyph@divmod.com> wrote:
On 09:14 pm, paulswartz@gmail.com wrote:
That makes sense: a ConchConnection object which wraps some of the nonsense that currently lives in t.c.scripts.conch like verifying a host key and finding keys to use.
"Connection" seems like a weird name for that to me, as it applies it is attached to a particular ... well, connection. Connector, maybe? Client? ClientStorage? I guess those names have some pretty serious problems too, but it's worth thinking about a better one.
I actually meant Connector :) There's also Creator (in comparison with t.i.protocol.ClientCreator).
I still think that passing authentication data in connect() is the right idea, but it can default to using the keys that it finds in ~/.ssh or in a key agent.
I agree emphatically, but both the default and non-default cases are equally important. There's a direct analogue in the user interface: "ssh foo.com" will use your default key or agent, and your default username, and that's 99% of usage. That, of course, doesn't mean you can get rid of "ssh -i mykey.id_rsa foo.com" or "ssh otheruser@foo.com", they're both very necessary use-cases.
So how about this: class IConch<C word>(Interface): def connect(host, port=22, user=None, knownHosts=None, authentications=None): where user defaults to the current username, knownHosts defaults to the to hosts in known_hosts, and authentications defaults to a key agent or the keys in .ssh/id_*. To keep the current use case of verifying a host key with a function, knownHosts could be a filename string, in which case it would be read for host keys, or a function in which case it is called with the key and fingerprint. authentications would be a list functioning the same was as in my initial outline. This would make the default .connect(hostname) work equivalently to the default 'ssh hostname' but still give options for the other overrides. -p -- Paul Swartz paulswartz at gmail dot com http://z3p.livejournal.com/ AIM: z3penguin
On 26 Jul, 09:47 pm, paulswartz@gmail.com wrote:
On 7/26/07, glyph@divmod.com <glyph@divmod.com> wrote:
On 09:14 pm, paulswartz@gmail.com wrote:
Let me quote this point from the end first, because it is the central point of agreement, and basically the specification for this feature:
This would make the default .connect(hostname) work equivalently to the default 'ssh hostname' but still give options for the other overrides.
I don't think, however, that the distinction being proposed (function arguments vs. defaults) is quite the right way to go about it, though.
That makes sense: a ConchConnection object (snip)
"Connection" seems like a weird name for that to me, as it applies it is attached to a particular ... well, connection. Connector, maybe?
I actually meant Connector :) There's also Creator (in comparison with t.i.protocol.ClientCreator).
Let's call it ConchEndpoint for the moment. Hopefully why I say that will become clear in a moment.
I still think that passing authentication data in connect() is the right idea, but it can default to using the keys that it finds in ~/.ssh or in a key agent.
I agree emphatically, but both the default and non-default cases are equally important.
class IConch<C word>(Interface): def connect(host, port=22, user=None, knownHosts=None, authentications=None):
I think the interface here is probably superfluous. The interface definition, when there is one, should really just be an endpoint. These may make sense as constructor arguments or arguments to a method that builds an endpoint though. (More below...)
To keep the current use case of verifying a host key with a function, knownHosts could be a filename string, in which case it would be read for host keys, or a function in which case it is called with the key and fingerprint. authentications would be a list functioning the same was as in my initial outline.
OK, let's have a bit more depth. This interface isn't quite right. Starting at the bottom, "knownHosts" is the wrong name for this object. It should be "hostVerifier" and it should be an object with a method to verify the host. It should definitely _not_ accept a filename string; in the filename case, it should look something like this: OpenSSHFormatVerifier(FilePath("my-ssh-hosts-file") ui=GuiPasswordDialog()) In general, I think passing filenames around should be deprecated within Twisted; FilePath provides lots more flexibility, especially with a real interface to unify it with ZipPath. (A marginal benefit of this is that your tests can avoid a whole bunch of file I/O because you can have a mock memory-backed FilePath. This benefit becomes less marginal as more and more tests use it and the whole suite speeds up, though.) As far as object-vs-function, passing functions around is always tempting, but it tends to mean "This is an object with only one obvious method that I've thought of so far, which I'll call '__call__'". There are lots of places I've used this idiom and found that it falls down and gets ugly when it ends up that you actually want _two_ methods instead. To retain compatibility with the existing use-a-function-to-verify path, it would of course be trivial to have a SimpleFunctionVerifier. This might seem like a lot of typing, so to help with abbreviation, a higher-level object could roll them together: altdotssh = DotSSH(FilePath("my-dot-ssh-dir")) And this leads in to my idea for subtly changing the design. Instead of modifying stuff with arguments to the function: defaultConnector.connectSSH( factory, hostname, hostVerifier=altdotssh.hostVerifier) Building a different object takes roughly the same amount of work and does roughly the same thing, but yields a result which can be passed around and used as a connector: endpt = defaultConfig.endpointSSH( hostname, hostVerifier=altdotssh.hostVerifier) endpt.connect(factory) To push this flexibility even further, you could make the configuration object cloneable. cfg2 = defaultConfig.copy(hostVerifier=altdotssh.hostVerifier) endpt = cfg2.endpointSSH(hostname) endpt.connect(factory) although whether this last additional level of flexibility is worth it or not probably depends on all of the things that a "configuration" object can contain. Thoughts?
On 7/26/07, glyph@divmod.com <glyph@divmod.com> wrote:
This might seem like a lot of typing, so to help with abbreviation, a higher-level object could roll them together:
altdotssh = DotSSH(FilePath("my-dot-ssh-dir"))
And this leads in to my idea for subtly changing the design. Instead of modifying stuff with arguments to the function:
defaultConnector.connectSSH( factory, hostname, hostVerifier=altdotssh.hostVerifier)
Building a different object takes roughly the same amount of work and does roughly the same thing, but yields a result which can be passed around and used as a connector:
endpt = defaultConfig.endpointSSH( hostname, hostVerifier=altdotssh.hostVerifier) endpt.connect(factory)
So, is defaultConfig is some other object which can create a ConchEndpoint? Is defaultConfig a DotSSH? I'm not clear on what these different objects are here.
To push this flexibility even further, you could make the configuration object cloneable.
cfg2 = defaultConfig.copy(hostVerifier=altdotssh.hostVerifier) endpt = cfg2.endpointSSH(hostname) endpt.connect(factory)
although whether this last additional level of flexibility is worth it or not probably depends on all of the things that a "configuration" object can contain.
Thoughts?
This seems reasonable, at least for the case of host verification. I'm not sure where authentication fits into these things, though. It feels different from host key verification, but moving it to the connect() method makes the ConchEndpoint a different interface from the other Endpoints. I suppose with a copy method: defaultConfig.copy(user="new username", authentications=[KeyAuthentication(key), PasswordAuthentication('password'), InteractiveAuthentication(['answer 1'])]).endPointSSH(hostname).connect() is straightforward enough. But it seems more confusing than: defaultConfig.endPointSSH(hostname, user="name").connect() which seems more in line with my personal expectations. I think, however, my expectations might be changed if the above example was more clear to me. -p -- Paul Swartz paulswartz at gmail dot com http://z3p.livejournal.com/ AIM: z3penguin
On 7/26/07, glyph@divmod.com <glyph@divmod.com> wrote:
Thoughts?
I've updated the wiki (http://z3p.jot.com/WikiHome/SummerOfCode2007/connectSSH) with an interface that is more along the lines of what you presented here. I'm out for the weekend, but I'd love comments. -p -- Paul Swartz paulswartz at gmail dot com http://z3p.livejournal.com/ AIM: z3penguin
participants (2)
-
glyph＠divmod.com -
Paul Swartz