Share podman sock bindings with other WSL distros #19705
Conversation
openshift-ci
bot
added
release-note
approved
|
PTAL @containers/podman-maintainers |
There was a problem hiding this comment.
The whole disconnect between the constant and the use worries me, this is hard to follow.
What uses printf() stlye what uses the weird [USER] syntax. AT the caller it is impossible to know I would have to look this up every single time.
The me a much easier to understand approach would be to create functions that return the string, i.e. create function getBindMountUserService(dist string) string then I automatically know what argument to use.
This file is definitely due some cleanup. There was originally a very small number of constants and a very limited number of replacements, but it's grown over time to be a lot. In a future PR I want to refactor this a bit and split things up so it's easier to follow. For this PR I will improve the aspects related to this change though |
|
Yes, I am not blocking on that I am only adding these comments because it costs me time to understand/review the change because I constantly need jump around to see what arg is given, etc... |
|
LGTM once comments from @Luap99 are addressed |
n1hility
force-pushed
the
shared-bind-mnts
branch
from
f1a06fb to
53dde02
Compare
Yeah agree, the code should aim to minimize review time. I did some improvements for this change, will definitely look at a more complete follow-up in the future. |
|
It looks like the bud and centos stream failures are unrelated. All feedback should be addressed |
| RemainAfterExit=true | ||
| Type=oneshot | ||
| # Ensure user services can register sockets as well | ||
| ExecStartPre=mkdir -p -m 777 /mnt/wsl/podman-sockets |
There was a problem hiding this comment.
is 777 realy correct? Seems way to open to me, but....
There was a problem hiding this comment.
Yeah it’s certainly an odd setup. So in this case /mnt/wsl (outside of our control is 777) since it’s a shaded space (sorta like /tmp) where all WSl distros cross (different Linux distributions, each running in a linux namespace). This subdir underneath just mirrors that permission scheme, and is also necessary since there is both a user systemd service and a root systemd service writing into the same directory.
Signed-off-by: Jason T. Greene <[email protected]>
Registers a rootless and rootful socket underneath /mnt/wsl/podman-sockets/[machine name]/ This allows podman remote clients on other Linux distributions to access podman. This also registers the podman root socket under the wheel group, to allow for rootful linking against /var/run/docker.sock, a use case expected by some clients and APIs. While this is not recommended practice on a Linux host, a WSL guest is user-isolated and already enables escalation trivially. [NO NEW TESTS NEEDED] Signed-off-by: Jason T. Greene <[email protected]>
n1hility
force-pushed
the
shared-bind-mnts
branch
from
53dde02 to
adf34cb
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: n1hility, vrothberg 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 |
github-actions
bot
added
the
locked - please file new issue/PR
Resolves #15190
Registers a rootless and rootful socket underneath /mnt/wsl/podman-sockets/[machine name]/ This allows podman remote clients on other Linux distributions to access podman.
This also registers the podman root socket under the wheel group, to allow for rootful linking against /var/run/docker.sock, a use case expected by some clients and APIs. While this is not recommended practice on a Linux host, a WSL guest is user-isolated and already enables escalation trivially.