feat: allow ssh flag arguments

[znck]

closes#2539

- Description for the changelog
Support SSH command-line arguments.

[thaJeztah]

@tonistiigi@AkihiroSuda PTAL

[AkihiroSuda]

looks good but needs rebase

[AkihiroSuda]

Is this expected to be only used by Go pkg consumers, not by the docker CLI?

[tonistiigi]

Needs rebase as well.

[thaJeztah]

@znck could you reply to the question(s) above?

@tonistiigi do you think we should have an option to pass arguments from the docker CLI ?

[znck]

I submitted this PR from go package consumer perspective. If CLI is required, I can update the PR but it’s up to maintainers to decide.

[thaJeztah]

Sorry for the long delay; I meant to leave a reply, but forgot 😓. We discussed this PR in our maintainers meeting, and given that docker itself wouldn't be using these options, we thought that instead of modifying GetConnectionHelper, it would be better to have a separate function for this that allows additional parameters (e.g. GetConnectionHelperWithSSHOpts() - better name suggestions welcome). The main functionality of GetConnectionHelper could then be moved to a non-exported function, that would be used by both GetConnectionHelper and GetConnectionHelperWithSSHOpts. Something like;

// GetConnectionHelper returns Docker-specific connection helper for the given URL.// GetConnectionHelper returns nil without error when no helper is registered for the scheme.//// ssh://<user>@<host> URL requires Docker 18.09 or later on the remote host.funcGetConnectionHelper(daemonURLstring) (*ConnectionHelper, error) {
returngetConnectionHelper(daemonURL, nil)
}
// GetConnectionHelperWithSSHOpts returns Docker-specific connection helper for// the given URL, and accepts additional options for ssh connections.//// GetConnectionHelper returns nil without error when no helper is registered for the scheme.//// ssh://<user>@<host> URL requires Docker 18.09 or later on the remote host.funcGetConnectionHelperWithSSHOpts(daemonURLstring, sshFlags []string) (*ConnectionHelper, error) {
returngetConnectionHelper(daemonURL, sshFlags)
}
funcgetConnectionHelper(daemonURLstring, sshFlags []string) (*ConnectionHelper, error) {
u, err:=url.Parse(daemonURL)
iferr!=nil {
returnnil, err
}
switchscheme:=u.Scheme; scheme {
case"ssh":
sp, err:=ssh.ParseURL(daemonURL)
iferr!=nil {
returnnil, errors.Wrap(err, "ssh host connection is not valid")
}
return&ConnectionHelper{
Dialer: func(ctx context.Context, network, addrstring) (net.Conn, error) {
returncommandconn.New(ctx, "ssh", append(sshFlags, sp.Args("docker", "system", "dial-stdio")...)...)
},
Host: "http://docker",
}, nil
}
// Future version may support plugins via ~/.docker/config.json. e.g. "dind"// See docker/cli#889 for the previous discussion.returnnil, err
}

Patch;

diff --git a/cli/connhelper/connhelper.go b/cli/connhelper/connhelper.go
index ad1c0fa67..78ab024a6 100644
--- a/cli/connhelper/connhelper.go+++ b/cli/connhelper/connhelper.go@@ -21,7 +21,21 @@ type ConnectionHelper struct {
// GetConnectionHelper returns nil without error when no helper is registered for the scheme.
//
// ssh://<user>@<host> URL requires Docker 18.09 or later on the remote host.
-func GetConnectionHelper(daemonURL string, sshFlags ...string) (*ConnectionHelper, error) {+func GetConnectionHelper(daemonURL string) (*ConnectionHelper, error) {+ return getConnectionHelper(daemonURL, nil)+}++// GetConnectionHelperWithSSHOpts returns Docker-specific connection helper for+// the given URL, and accepts additional options for ssh connections.+//+// GetConnectionHelper returns nil without error when no helper is registered for the scheme.+//+// ssh://<user>@<host> URL requires Docker 18.09 or later on the remote host.+func GetConnectionHelperWithSSHOpts(daemonURL string, sshFlags []string) (*ConnectionHelper, error) {+ return getConnectionHelper(daemonURL, sshFlags)+}++func getConnectionHelper(daemonURL string, sshFlags []string) (*ConnectionHelper, error) {
u, err := url.Parse(daemonURL)
if err != nil {
return nil, err

[harryjubb]

Hi, is there any progress on this? This would be very valuable for automating Docker deployments.

[thaJeztah]

@harryjubb waiting for @znck to address the review comments, or for someone to carry the PR

[znck]

Please someone carry it.

[thaJeztah]

I pushed my proposed changes to this branch

[thaJeztah]

LGTM 😅

[silvin-lubecki]

LGTM