Allow arbitrary sshOptions to be passed to ssh2

[steverice]

This allows for an arbitrary sshOptions to be passed in Modem options and be used when passed to the ssh constructor to get an agent from the ssh2 library.

Tests are added to be sure we're still respecting the SSH_AUTH_SOCK env var.

[bwateratmsft]

I think we need to keep sshAuthAgent, and override it with sshOptions.agent if present, otherwise this becomes a breaking change and would require a new major version.

[steverice]

Good point @bwateratmsft. Does bde24db address what you were thinking?

[bwateratmsft]

Good point @bwateratmsft. Does e226c99 address what you were thinking?

Yes, I think so. I guess I'd make the override happen in the other direction, i.e. only do this.sshOptions.agent = options.sshAuthAgent if this.sshOptions.agent was undefined. Essentially, new setting (if present) overrides old setting.

[steverice]

@bwateratmsft is supporting a custom agent option that overrides SSH_AUTH_SOCK something we actually need to support? The fact that that is possible today via the sshAuthAgent option seems unintentional, based on it not being possible in #108 and only introduced in #109 where the goal was unrelated.

Definitely agree that the behavior needs to stay the same for sshAuthAgent so this isn't a breaking change, but I think we get to decide how we want sshOptions.agent to be used when SSH_AUTH_SOCK is also set.

[bwateratmsft]

That's true. In that case, I think it's fine as you have it.

[steverice]

For the record, here's how I see this breaking down philosophically.

On the one hand, we want to say that sshOptions is an opaque blob as far as docker-modem is concerned — it's intended for use by ssh2 and we will just give it whatever the user provided.

On the other hand, there's some interest that docker-modem has in setting these options so that its own options (like host, protocol, and port) are used in a consistent way regardless of protocol. Before #108 I think it could be argued that those are the only "authoritative" options that should override the "opaque blob" when it comes to ssh options, but since then we've added "authoritative" options that are specific to SSH in #108, #109, and #117. Leaving agent to be one of those "authoritative" options managed by docker-modem fits the pattern.

IMO it'd be entirely reasonable to change this so that sshOptions instead overrides all of those, but I think that'd be a larger shift in philosophy that should come with a major version bump, perhaps along with support for #113 which will require similar decisions about option precedence.

I personally don't have a strong opinion one way or the other, just hoping this summary helps @apocas decide what is right here.

[apocas]

I would like to maintain the common options outside the sshOptions, in order to maintain some standardization between the different protocols. Just like you said. Everything exclusively specific to ssh should come inside the sshOptions. This implies a breaking change due to sshAuthAgent. We bump the major version.

I believe this is better than leaving a few options outside in order to avoid a major version bump.

[bwateratmsft]

This is my opinion on the matter: https://www.youtube.com/watch?v=ussCHoQttyQ :smile:

[steverice]

@apocas I opened #120 which does as you suggest, but needs to wait for a major version bump.

May we please still merge this PR or #118 into the 2.x branch? 🙏 I don't see it hurting anything and would really like to use agentForward with vscode-docker. 🙂

[apocas]

Yeah sure :) Will tackle it right away.

[mcary]

Here's a work-around I used to be able to pass arbitrary options to ssh2. In my case, I wanted to use the privateKey option without having to deploy ssh-agent:

  const Docker = require('dockerode');
  const ssh = require('docker-modem/lib/ssh');

  const keyFile = "./.vagrant/machines/default/virtualbox/private_key"
  const privateKey = require('fs').readFileSync(keyFile)
  const agent = ssh({
    host: '127.0.0.1',
    port: 2222,
    username: "root",
    privateKey,
  })
  const docker = new Docker({
    protocol: 'http', // kind of a hack, see below
    username: "root",
    agent,
  });

Passing an explicit connection agent is not possible when the protocol is "ssh". docker-modem will generate its own agent on the fly, overriding one I pass. But if I pass protocol "http", that rule isn't triggered, and I can pass the ssh-based agent myself.

The ssh agent passes its options through to ssh2.

This side-steps the need to wait for a breaking-change major version bump for #120.