docker / cli

The Docker CLI
Apache License 2.0
4.77k stars 1.89k forks source link

Support custom SSH config file for SSH connection #5082

Closed evolutics closed 2 months ago

evolutics commented 2 months ago

SSH Docker host connections support specifying an SSH username, hostname, port, and socket path with ssh://…. Furthermore, the default user SSH config file located at ~/.ssh/config is respected.

However, it would be useful to specify a path to a custom SSH config on the CLI. Use cases are per-project SSH configs or SSH configs generated on the fly (e.g., by vagrant ssh-config).

- What I did

With this commit, a path to a custom SSH config can be specified as part of the SSH Docker host URL.

This closes #1301. Related issues: #1463, #2539.

- How I did it

An SSH config file can be specified by appending a URL fragment component to the end of the SSH address (that is, behind a hash #). The given file path is passed to SSH using the ssh -F option. Example:

$ docker --host ssh://user@192.168.64.5#/path/to/ssh_config ps

- How to verify it

Set up:

$ docker buildx bake

$ cat <<EOF >Vagrantfile
Vagrant.configure("2") do |config|
  config.vm.box = "bento/ubuntu-24.04"
  config.vm.provision "docker"
end
EOF

$ vagrant up

$ vagrant ssh-config --host my-host >my_ssh_config

Test:

$ build/docker --host 'ssh://my-host#my_ssh_config' run -dit nginx

$ build/docker --host 'ssh://my-host#my_ssh_config' ps
CONTAINER ID   IMAGE     COMMAND                  CREATED         STATUS         PORTS     NAMES
856870d88719   nginx     "/docker-entrypoint.…"   5 seconds ago   Up 3 seconds   80/tcp    serene_mcclintock

$ build/docker ps
CONTAINER ID   IMAGE     COMMAND   CREATED   STATUS    PORTS     NAMES

Tear down:

$ vagrant destroy

$ rm -fr .vagrant/ my_ssh_config Vagrantfile

- Description for the changelog

Support custom SSH config file for SSH Docker host connections using URL of the form `ssh://host#/path/to/ssh_config`.

- A picture of a cute animal (not mandatory but encouraged)

custom_ssh_config

evolutics commented 2 months ago

I'm aware of this comment from something over a year ago:

We have no plans at the moment to extend the scope of this feature. So, I'm closing this issue for now but we can possibly reconsider in the future.

Thus, feel free to discard this PR.

I think the cost-benefit ratio of this change is quite OK though: not much code (cost) for the flexibility that this would give users (benefit).

Regarding the API, I'm not sure whether the URL fragment is best suited. An alternative would be the URL query string.

neersighted commented 2 months ago

I'm not sure this feature makes sense as-is, given that we already use ssh://url/path/to/docker.sock (the path component of the SSH URL is used for the remote socket path), it seems somewhat confusing.

In general, I feel very hesitant to add more microformat-like options here. I think a much more sane approach might be extending the context to include arbitrary SSH options, which allows both direct config, and specifying an alternate config path. You wouldn't be able to use those options with DOCKER_HOST, but contexts are encouraged over DOCKER_HOST anyway, and it would be a logical evolution of our rich 'remote daemon' config mechanism.

thaJeztah commented 2 months ago

Thought I commented already, but looks like I didn't; yes, also not comfortable with adding a new micro format at least. We accepted the /path/to/docker.sock as it followed existing formats (see https://github.com/docker/cli/pull/4073#pullrequestreview-1327204803), but I'd rather not come up with our own conventions otherwise.

evolutics commented 2 months ago

Thanks for your feedback, both. I agree with the confusion this uncommon format could cause. I'll think of other options; although I prefer the declarative nature of DOCKER_HOST over the stateful DOCKER_CONTEXT.