train
train copied to clipboard
inspec
Need a ssh fork-and-exec transport
The benefits of fork+exec to command line ssh over net-ssh are:
- no bugs from net-ssh (constant problem with knife-ssh, particularly with new crypto algs)
- leverages all the users .ssh/config settings (constant problem with knife-ssh, again particularly with new crypto algs)
- MRI isn't multithreaded, so forking an ssh process allows all the crypto for the ssh process to run on a different CPU. With net-ssh and 50 ssh connections you'll be stuck in a single ruby process, no matter how many ruby threads you spawn, with fork+exec those 50 ssh connections run in their own process and can run on different cores.
@lamont-granquist @arlimus From my perspective we should add this as ssh-native transport. I would like to keep a ruby only implementation, but I agree with @lamont-granquist that there may be cases, where fork makes more sense.
Yeah, if net-ssh works fine it'll be less overhead, I wouldn't suggest replacing it. And ssh-native sounds like a good name.
Here's an example where net-ssh falls short for one of our customers:
https://learn.chef.io/install-and-manage-your-own-chef-server/linux/manage-a-node-on-your-chef-server/#comment-2356862847
@lamont-granquist I cannot see the comment. Getting a 404 error whether logged in or not.
For Inspec's perspective, there are severe issues with a multiprocess model which would need to be resolved - see https://github.com/inspec/inspec/issues/3010 .
Currently, plans to use Train as a transport library for other projects is very limited; Chef Workstation is the only known realistic consumer at the moment. Unless @cheeseplus says otherwise, I think this should probably be closed.
I'm going to actually throw down and state that this should be a requirement for workstation to use train.
We're currently dealing with issues with net-scp pinning old versions of net-ssh and therefore the chef-dk product being unable to use the latest net-ssh and unable to use ed25519 keys (https://github.com/net-ssh/net-scp/blob/master/net-scp.gemspec#L35) which is just the latest in a long line of issues caused by the net-ssh ecosystem lagging.
Those are all solved by fork-exec to the ssh binary on the box.
As Chef Workstation should subsume the old knife ssh workflow entirely and make that command go away, I think this is a requirement for both train and for workstation.
@clintoncwolfe Workstation does use train for chef-apply, and wherever chef bootstrap lands in Workstation will also use train. Really wherever we do transports we'll want to migrate to train eventually, because reimplementing bastion hosts, proxies, and all the edge cases in multiple projects is more tiring than the work to make one library work across our ecosystem.
I'm personally in favor of using Train wherever a transport library is needed; I was just reporting what I had been told by others.
So, if train has many consumers aside from InSpec, and those consumers desire a forking SSH transport; well, train has a plugin model now, and one could certainly be written.
Yes, and I'm making an argument that as a business we need to support this transport as a first-class citizen because of issues with net-ssh stretching back to the inception of the company.
A train plugin is a first-class citizen. We are moving in the direction to make every backend its own plugin. At this point, the inspec team has no capacity to drive the development of an train-ssh-native plugin but we're happy to help to get this off the ground.
This just blew up yet again.
The whole ed25519 tirefire once again has its roots in the fact that we were stuck on net-ssh 4.x for ages because net-scp was pinned to that and didn't support net-ssh 5.x while the libnacl-sodium author deprecated that in favor of libsodium from distros which broke our omnibus world and as a result we're wound up way the hell behind the curve for ed25519, causing what i'd characterize as real product damage -- all caused by the fact that the do-ssh-ecosystem-in-ruby set of gems is a never ending pile of tech debt.
Fork-and-exec would have been just like "your ssh supports ed25519 and your config specifies it, so I don't care..."
https://github.com/chef/chef/issues/8382 (including bonus fantastic user comment)
So long term here where I see train usage:
- knife for knife ssh / winrm
- knife for bootstrapping new chef nodes
- inspec
- workstation for apply
- test kitchen for everything
Given those uses it would be really great to get a performant SSH experience. We can still fall back to a ruby ssh implementation, but ideally we try to use native system binaries on modern platforms since those are going to be multithreaded and should give us some serious performance / compatibility wins.
At this point I know the Train codebase much better, and can attest that it is possible to write a federated Transport (here, one that manages a pool of SSH channels or processes, natively). I don't know that InSpec would be able to leverage it, but other products certainly could.
we get a fork-exec train transport i will be highly incentivized into getting it into the relevant codebases, particularly now that knife-bootstrap is train-ified
What’s happening? Why was this issue closed? This issue was closed due to some much needed review of legacy issues or issues that were spawned in older versions of InSpec, i.e. < v3.
Why do I care? You would care about this if this was an issue that you are still seeing and/or feel needs to be addressed in the current version of InSpec.
What do I need to do? If this issue is no longer important, no further action is necessary. However, if you think this is something that should be addressed, please open a new issue and refer to the original issue in the description.
The inability of Net::SSH to parse all .ssh/config options just bit us again.
@lamont-granquist we'll discuss this in our next prioritization meeting. As it affects fellow chefs, I'm motivated to understand the history of this issue, why it was hard to get this addressed in the past, and what we can do incrementally to move the ball forward as it still seems relevant. Stay tuned.