Sshconfig missing bugfix - addresses issue #1105

[memetb]

This PR addresses issue #1105. It is a minimal codefix, marking as WIP as I've made the decision to make it a minimal fix as opposed to a larger rewrite.

@dmacvicar I am open to any directives you have to give. I will separately comment on my design choice.

[memetb]

Edit: OK, it appears path of least effort isn't going to work. I'm going to go over the code more closely. The solution below is possibly the wrong approach.

---

The log output provided by @mvrk69 has the following relevant output:

2024-10-03T14:45:39.141+0200 [INFO]  provider.terraform-provider-libvirt_v0.8.0: 2024/10/03 14:45:39 [WARN] Failed to open ssh config file: open /home/user/.ssh/config: no such file or directory: timestamp="2024-10-03T14:45:39.141+0200"
2024-10-03T14:45:39.141+0200 [INFO]  provider.terraform-provider-libvirt_v0.8.0: 2024/10/03 14:45:39 [WARN] Failed to parse ssh config file: invalid argument: timestamp="2024-10-03T14:45:39.141+0200"
2024-10-03T14:45:39.141+0200 [INFO]  provider.terraform-provider-libvirt_v0.8.0: 2024/10/03 14:45:39 [INFO] establishing ssh connection to 'XXX.XXX.XXX.XXX': timestamp="2024-10-03T14:45:39.141+0200"
2024-10-03T14:45:39.143+0200 [DEBUG] provider.terraform-provider-libvirt_v0.8.0: panic: runtime error: invalid memory address or nil pointer dereference
2024-10-03T14:45:39.143+0200 [DEBUG] provider.terraform-provider-libvirt_v0.8.0: [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x79911c]

This means that the loading of the sshconfig file initially fails here,

sshConfigFile, err := os.Open(os.ExpandEnv(defaultSSHConfigFile))

then code execution proceeds to here

sshcfg, err := ssh_config.Decode(sshConfigFile)

which returns a nil for the sshcfg object.

This object is then queried multiple times inside the dialHost call here.

It's important to note that dialHost is quite dynamic in that the same call (e.g. sshcfg.Get(target, "HostName")) might yield different results at multiple tunnel hop levels. Furthermore, there are several spots where sshcfg is queried.

Because of the above complexity, and because I didn't want to change all the sites where the sshcfg object is interacted with, I chose to load an empty configuration file and simply proceeding with a guarantee that dialHost will never receive a nil sshcfg arg.

Finally, there is a bit of very defensive programming here where I simply die if for some reason sshcfg fails to load an empty file - which I could only imagine would occur if there was some internal breakage in the sshconfig library itself.

[memetb]

Here is the updated code. Allows for graceful absence of .ssh/config file, unreadable ssh config file, and empty ssh config file.

[dmacvicar]

I am ok with the current fix. But in retrospective I think the whole introduction of ssh config file support in this way was a mistake.

What we should do is abstract the whole code flow to work with a final configuration. How this configuration is built should be in a single place. The ssConfig object being all over the place is not nice.

The code with the loading of the file, overriding etc, should all be in a single place, and that is where the code that loads a empty file could be as well.

[memetb]

@dmacvicar I understand the point you're making: there's an aspect of the current implementation that is messy.

The thing to keep in mind if we do refactor, is that the ssh configuration can't be pre-computed without some serious state management.

For instance, I've worked in environments where I had:

Match Host corp.asset-*
       User service
       Hostname %h
       ProxyJump mothership.corp.com

determining the outcome of these and pre-computing them into a dictionary is simply not possible.

I think a second area of concern is the "level" the query string applies to: does it apply to each host along the way or just the end host etc?

My vote would be to formalize the query string configuration into an object and formalize the library (terraform-provider-libvirt) api documentation to show the precedence of configuration, and then if it must be done, create a wrapper object which polls sshconfig and query string according to those rules.

The important thing is that the config must specify the target attribute every time it calls Get.