Add SSH agent forwarding support to windows

[tbalthazar]

Minor refactoring of SSH support for windows.
Move the sshConnect function from ssh.go to ssh_internal.go (makes more
sense IMHO).
Refactor tests for ssh_internal.go.

[tbalthazar]

I was wondering if there was a reason to use a copy of the term package in the pkg/term directory or if it was just legacy code. I have a commit ready that removes it and replaces its usage (only 3 times here) by the equivalent functions from the golang.org/x/crypto/ssh/terminal package.

Please let me know if that makes senses.
Thanks!

[bryanl]

@tbalthazar at the time, I wasn't also testing on windows, and the term stuff was a suggestion from a contributor. I've also felt weird about it, and am happy someone is ready to help remove it.

[tbalthazar]

@bryanl Done :)

[bryanl]

@tbalthazar I was manually running through this, and hit a snag. What if you have a droplet that wasn't created with a pub key, and you need to enter a password? Right now, this will error if there isn't a private key.

[tbalthazar]

@bryanl Good catch! The internal SSH client now mimics the ssh command on Linux: it warns the user and asks for password.

[aybabtme]

@tbalthazar

I have a commit ready that removes it and replaces its usage (only 3 times here) by the equivalent functions from the golang.org/x/crypto/ssh/terminal package.

The authors of that package don't really like that people be using it, it was created before the introduction of the internal/ mechanism, the authors have mentioned regret at not having put the internal/ label on it. For those reasons, I recommend against using it. There's work elsewhere in the stdblib + exp trees to bring proper term support to Go.

[tbalthazar]

@aybabtme I ignored that, thanks for rising the issue. I did some research to try to get the context of this and found this discussion where mattn says that it should be an internal package. Is that what you are referring to? My understanding of this discussion is that the issue seems to be "where to put/how to best name that package?". It seems they agreed to move it to x/term in November 2015. Then I found another issue (July 2016) where the discussion is about removing the x/term package because it's empty and nobody took the time to move the terminal related code there. The x/crypto/ssh/terminal package seems to still be maintained though.
So, my understanding of the issue you raised is that terminal related code might be moved to another package at some point. But since the package is vendored in the doctl project, I guess it shouldn't be a problem to keep on using it. Please let me know what you think about that and tell me if I'm missing something. Thanks!

[aybabtme]

@tbalthazar sgtm!