Using upstream connection providers and the future of the ssh transport

[dmacvicar]

After seeing that @zaneb landed digitalocean/go-libvirt#209 which provides the ConnectionURI upstream, with all the basic implementations, and seeing #1155 from @jgooge, I am undecided how to move forward, especially around the ssh transport (with contributions from @scabala and @memetb, among others).

I'd like to keep the provider as simple as possible, which means I am all for moving to the upstream ConnectionURI and just do:

	url, err := url.Parse(c.URI)
	if err != nil {
		return nil, err
	}

	l, err := libvirt.ConnectToURI(url)
	if err != nil {
		return nil, fmt.Errorf("failed to connect: %w", err)
	}

The main problem I see with ssh, where the upstream one does not support all the user config options (and not sure all the bastion features).

On the other hand, I am not happy with the complexity of the current ssh transport and the issues it generates.

I see the following options:

a. Upstreaming our ssh transport features (which somebody would need to do)
b. Use the upstream ones, except for ssh (parse the transport early and follow a different code-path)

For both cases, the complexity problem does not go away. One option could be to create a "spawn ssh binary" version of the dialer which uses the ssh binary, and allow to use that in the case of the standard ssh (vs libssh) transport, or allow to enable it with some flag. This could be upstreamed or replace the one we use to override.

[jgooge]

While option (a) would be the simplest long-term solution, option (b) seems like the most actionable path forward.

One option could be to create a "spawn ssh binary" version of the dialer which uses the ssh binary, and allow to use that in the case of the standard ssh (vs libssh) transport, or allow to enable it with some flag. This could be upstreamed or replace the one we use to override.

I can't speak for @scabala and @memetb & all other ssh transport contributors, but refactoring the current dialer to make use of the standard OpenSSH binary could offer users a more predictable experience.

[memetb]

Strangely enough, I read the libvirt documentation here

The ssh transport uses the standard SSH protocol via the system installed binary.

and was looking at the ef7bc723d6257183e8f901115b5880621b4455df message

Resolve qemu+ssh:// URIs using the internal (golang ssh library--based)
ssh dialer. Note that this cannot respect the system ssh configuration,
as users of the libvirt client library would (since it shells out to the
ssh binary for these URIs, unlike for qemu+libssh:// or qemu+libssh2://
URIs).

and my personal preference is actually to do what libvirt does, which is to spawn out (b).

I have no objection to the code already written being factored out, so long as we can achieve the desired end result (which for me is "as close to feature parity with libvirt as possible"), however I do want to say that there is no free lunch: ssh is complicated and there will not be a free solution no matter what. It just needs to be done once and correctly. By this metric, I dislike that the upstream feature implementation does not make use of the ssh_config library and instead re-rolls out that functionality.

And while the purist in me also would love to see a golang implemented full solution upstream, the current upstream version is documented to have some of the exact shortcomings that were set out to be fixed in this project. Whether we like it or not, these issues will come here as bug reports (and we may feel relief because we can bump them upstream), we will not solve the end users' problems by doing so.

One final thought is that while the code did generate create a couple of bugs, and revealed a couple of latent ones, it overall looks like it is now working as it should. I am reluctant to make changes without a solid landing site in plain view.

So as it is right now:

a) while it is more purist in spirit, the upstream code looks just as complicated to my eyes, is documented to have missing features, and may yet reveal issues we aren't aware of
b) is my pragmatic preference, mainly because it matches library behaviour. However I'm concern this is more complicated than we are realizing it to be right now (mainly because I simply don't know and haven't yet investigated how to use ssh by spawning it)

[dmacvicar]

One final thought is that while the code did generate create a couple of bugs, and revealed a couple of latent ones, it overall looks like it is now working as it should. I am reluctant to make changes without a solid landing site in plain view.

Just for perspective, just the changes between v0.8.1 and v0.8.2 about ssh port configuration introduced a new bug that make ssh stop working #1165, and had to release v0.8.3 only with that bugfix.

[dmacvicar]

When this provider was born, it was using the C interface, and that made the provider dependent on the underlying operating system, plus prevented us from being published on the registry. I ported it to use the pure-Go version to overcome these limitations.

The upstream library has the same motivation and it is very likely used for automating infrastructure (Digital Ocean), even outside the context of terraform and a developer's laptop. You don't want to depend on a system ssh for that.

I think we could do it in this order, to minimize disruption and risk:

1. Make the terraform provider use the upstream providers for all transports except ssh, where we use our own (unless I am unaware of other shortcomings)
2. Develop a "ssh binary" alternative and introduce it behind some flag, as an alternative to the current one
3. Once things are stable, swap the ssh provider on our end
4. Attempt to upstream the "ssh binary" alternative to go-libvirt. I don't think why it would not make sense, given that there is a "libssh" transport which can continue to be based on the go ssh library.

[memetb]

This sounds sensible. I'm ever so slightly concerned about step 3, but this will take a while to get there.

As an aside, we should "lobby" the upstream maintainers to make use of ssh_config.

[zaneb]

I think a PR would go much further than lobbying 🙂

[scabala]

After some research and thinking, I do like idea of using upstream dialers - this is the way. However, as has been pointed out, using upstream ssh dialer will create ton of new issues. What I propose is to:

• for all URIs except ssh, use upstream cod
• for ssh URI, do use system SSH binary - it will solve a lot of issues for people that just run Terraform from their local laptop
• introduce a new URI, specific to this project - gossh (gotfssh? tfgossh? name up to discussion) where it will be all the code that provides current functionality with ssh dialer. So it will make it easy for people that do care to use pure Go SSH client easy to use it. It would also be beneficial to document it - it is aiming for SSH binary compatibility but it is not there yet. Expect bugs and missing features. Also, it would still allow to use this provider in Terraform Cloud, as I've seen some people are doing.

What do you think?

[zaneb]

If you're going to create separate 'true ssh binary' and 'pure-go ssh binary emulation' modes, then I would suggest selecting between them with a query parameter rather than a separate URI scheme.

[dmacvicar]

Got something working here. Should have something for testing soon.

[jgooge]

Looking forward to it!

[dmacvicar]

#1166

If anyone wants to give it an early try. Only tested against my own localhost without any custom params).