podman icon indicating copy to clipboard operation
podman copied to clipboard

Add support for ssh_config for connection

Open afbjorklund opened this issue 1 year ago • 10 comments

Previously had to use podman system connection

Normal ssh://<alias> would fail to work properly

e.g. "default", podman --host ssh://default --ssh native ps

Host default
  HostName 127.0.0.1
  User vagrant
  Port 2222
  UserKnownHostsFile /dev/null
  StrictHostKeyChecking no
  PasswordAuthentication no
  IdentityFile $PWD/.vagrant/machines/default/virtualbox/private_key
  IdentitiesOnly yes
  LogLevel FATAL

Does this PR introduce a user-facing change?

ssh connection URL now supports regular ssh_config

Also adds support for default ssh user and path.

Tested manually with Lima. No unit tests found.

$ limactl start --name podman template://podman
$ ssh -F ~/.lima/podman/ssh.config lima-podman podman

$ limactl show-ssh --format=config podman >>~/.ssh/config
$ CONTAINER_HOST=ssh://lima-podman podman-remote version

Closes #23831 Fixes #17452

afbjorklund avatar Sep 03 '24 18:09 afbjorklund

Ephemeral COPR build failed. @containers/packit-build please check.

Added required files to the "vendor" directory (by using go1.22.0 mod vendor)

bloat approved

***************************************************************************
* bin/podman grew by 110536 bytes; max allowed is 51200.
*
* Please investigate, and fix if possible.
*
* A repo admin can override by setting the bloat_approved label
***************************************************************************

no new tests

pr-should-include-tests: PR does not include changes in the 'tests' directory

Please write a regression test for what you're fixing. Even if it
seems trivial or obvious, try to add a test that will prevent
regressions.

If your change is minor, feel free to piggyback on already-written
tests, possibly just adding a small step to a similar existing test.
Every second counts in CI.

If your commit really, truly does not need tests, you can proceed
by asking a repo maintainer to set the 'No New Tests' github label.
This will only be done when there's no reasonable alternative.

BTW; There is no "tests" directory, but there is a "test" directory?

  • https://github.com/containers/podman/pull/15094

The "secure" parameter doesn't seem to do anything anymore?

ssh://<user>@<host>[:port]/run/podman/podman.sock?secure=True

afbjorklund avatar Sep 03 '24 19:09 afbjorklund

This will fix trying to nslookup the alias, but it will not add support for ProxyJump

In order to support that, Podman needs to use more of containers/common SSHMode

Like it said in the original PR #15094 (that added --ssh=native):

Overall, this is a v1 of a long process of offering native ssh


     ProxyCommand
             Specifies the command to use to connect to the server.  The com‐
             mand string extends to the end of the line, and is executed using
             the user's shell ‘exec’ directive to avoid a lingering shell
             process.
             ...

     ProxyJump
             Specifies one or more jump proxies as either [user@]host[:port]
             or an ssh URI.  Multiple proxies may be separated by comma char‐
             acters and will be visited sequentially.  Setting this option
             will cause ssh(1) to connect to the target host by first making a
             ssh(1) connection to the specified ProxyJump host and then estab‐
             lishing a TCP forwarding to the ultimate target from there.
             ...
  • https://github.com/containers/podman/issues/21113

The current workaround is to tunnel a temporary unix: socket with a ssh -L command

afbjorklund avatar Sep 03 '24 20:09 afbjorklund

* bin/podman grew by 110536 bytes; max allowed is 51200.

110kb from a 1700 line diffstat, ouch. Bloat approved label added still.

mheon avatar Sep 05 '24 18:09 mheon

Few nits, looks fine overall. @baude PTAL when you get back.

mheon avatar Sep 05 '24 18:09 mheon

* bin/podman grew by 110536 bytes; max allowed is 51200.

110kb from a 1700 line diffstat, ouch. Bloat approved label added still.

Not sure if there was any obvious culprit, it seemed to be evenly spread...

11714	runtime
1323	os
806	fmt
698	regexp
521	strings
501	strconv
437	unicode
430	bytes
429	sync
366	io
243	os/user
214	path/filepath
73	errors

EDIT: I take that back, the use of "runtime" seems suspicious and bloated

https://github.com/kevinburke/ssh_config/blob/v1.2.0/config.go#L312

                if r := recover(); r != nil {
                        if _, ok := r.(runtime.Error); ok {
                                panic(r)
                        }
                        if e, ok := r.(error); ok && e == ErrDepthExceeded {
                                err = e
                                return
                        }
                        err = errors.New(r.(string))
                }

https://go.dev/wiki/CodeReviewComments#dont-panic https://go.dev/blog/defer-panic-and-recover

So maybe the bloat checker was onto something anyway?

afbjorklund avatar Sep 05 '24 18:09 afbjorklund

Note that if providing a fully qualified URL, this will be a no-op (assuming an empty/small ~/.ssh/config)

export CONTAINER_HOST=ssh://[email protected]:2222/run/user/1000/podman/podman.sock
export CONTAINER_SSHKEY=$PWD/.vagrant/machines/default/virtualbox/private_key

It will only kick in, if there is an "alias" defined or if there are any parameters missing (like user or path)

  • https://wiki.debian.org/SshAliases
  • https://lima-vm.io/docs/reference/limactl_show-ssh/

afbjorklund avatar Sep 06 '24 10:09 afbjorklund

There is more talk about performance in the issue, beyond using a legacy --host=ssh://

  • https://github.com/containers/podman/issues/23831#issuecomment-2323491165

How to reuse the same ssh connection (and path), and how to choose a faster encryption...

For lima there is a "host agent" that sets up a persistent tunnel, and provides a unix socket

afbjorklund avatar Sep 06 '24 10:09 afbjorklund

Also can you squash your commits please.

Luap99 avatar Sep 24 '24 16:09 Luap99

Cockpit tests failed for commit b174663c2f1354c5b3f32c234f780303b177a2ac. @martinpitt, @jelly, @mvollmer please check.

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afbjorklund, Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci[bot] avatar Oct 30 '24 13:10 openshift-ci[bot]

/lgtm

mheon avatar Oct 30 '24 14:10 mheon