Use HTTP path prefix of TCP connections to match Docker context behavior

[ben-krieger]

Currently podman system connection add allows providing TCP connections with paths. However, these paths are not actually used.

➤ podman system connection list
Name        URI         Identity    Default     ReadWrite
➤ podman system connection add foo tcp://localhost:8080/some/prefix
➤ podman system connection list
Name        URI                               Identity    Default     ReadWrite
foo         tcp://localhost:8080/some/prefix              true        true
➤ podman --log-level debug --connection foo ps
INFO[0000] podman filtering at log level debug
DEBU[0000] Called ps.PersistentPreRunE(podman --log-level debug --connection foo ps)
DEBU[0000] DoRequest Method: GET URI: http://d/v5.1.2/libpod/_ping
...

NB: The d value for host is an ignored placeholder value, because the client uses an *http.Transport that dials the correct host with the correct scheme (not necessarily TCP).

The docker client, however, does respect this path prefix. Upon parsing the connection/context URI, it saves the path as basePath: https://github.com/moby/moby/blob/ddea6b0fa8f816377b59e3f7d7b3c32fef629022/client/client.go#L401-L423

This PR introduces the same behavior:

➤ ./bin/podman --log-level=debug --connection foo ps
INFO[0000] ./bin/podman filtering at log level debug
DEBU[0000] Called ps.PersistentPreRunE(./bin/podman --log-level=debug --connection foo ps)
DEBU[0000] DoRequest Method: GET URI: http://localhost:8080/some/prefix/v5.2.0/libpod/_ping
...

Release Note

Changed TCP connections with paths in their URI to use the path as an HTTP path prefix to all API requests, matching the behavior of docker context.

[Luap99]

Are you running the service behind a reverse proxy or what is the use case of this?

[ben-krieger]

Are you running the service behind a reverse proxy or what is the use case of this?

Yes, exactly. Say if you had something like /container_runners/${RUNNER_ID}/ proxied directly through the API.

Without this change, you would need to create and run a small service that reverse proxied the base path to the prefixed path for every single prefixed connection.

My change won't work if the reverse proxy required any authentication, though. I suppose you could also use the c.URI.User to insert HTTP Basic Auth info back into the URI, but I didn't include this in the PR.

However, this change still helps in the case of connecting to a remote reverse proxy with auth, because you could run a single local reverse proxy that authenticates to the remote reverse proxy rather than one per prefix.

That was a little confusing, but I hope it makes sense. Reverse proxies are the use case here.

P.S. I'm looking into adding a test.

[Luap99]

LGTM once a test is added, not sure what a good test for it though. Adding a reverse proxy into our test setup seems way to overkill.
One option is test/e2e/system_connection_test.go and just create a http server there in the test via go std lib and then check the path in the request we got from the client.

[rhatdan]

LGTM

[baude]

i think we are just waiting on some form of a test? LGTM otherwise

[ben-krieger]

Added a quick and dirty test. I haven't been able to get integration tests running locally, so I doubt it works, but does this look like the right direction?

[ben-krieger]

Got tests passing outside of containerized environments. Inside a container, there's a failure in the BeforeEach when running SkipIfSystemdNotRunning, which does not make sense to me.

[Luap99]

Also can you squash both of your commits please

[ben-krieger]

This is now ready for re-review. The e2e test has been changed to 1) work and 2) set a boolean var when the reverse proxy path is actually triggered.

Further, podman-remote ps output is captured and compared to expected, because this is where bad HTTP status codes would show up if the reverse proxy were not correctly setup. If the check is too strict, I can change the gomega matcher.

Example logs from CI (https://api.cirrus-ci.com/v1/task/4624964236607488/logs/main.log) of the test running successfully:

[+0647s]   > Enter [It] add tcp:// connection with reverse proxy path - /var/tmp/go/src/github.com/containers/podman/test/e2e/system_connection_test.go:345 @ 08/21/24 12:29:55.077
[+0647s]   STEP: Proxying to unix:///run/podman/podman-45e5cce316e92b835f91d69736d256523a68d909877b074acea0391ae1998206.sock - /var/tmp/go/src/github.com/containers/podman/test/e2e/system_connection_test.go:360 @ 08/21/24 12:29:55.166
[+0647s]   CONTAINER ID  IMAGE       COMMAND     CREATED     STATUS      PORTS       NAMES
[+0647s]   CONTAINER ID  IMAGE       COMMAND     CREATED     STATUS      PORTS       NAMES
[+0647s]   < Exit [It] add tcp:// connection with reverse proxy path - /var/tmp/go/src/github.com/containers/podman/test/e2e/system_connection_test.go:345 @ 08/21/24 12:29:55.231 (153ms)

[rhatdan]

/approve
/lgtm
Thanks @ben-krieger

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ben-krieger, rhatdan

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:

• OWNERS [rhatdan]

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