Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions podman/domain/containers_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,33 @@ def parse_host_port(_container_port, _protocol, _host):
if "userns_mode" in args:
params["userns"] = normalize_nsmode(args.pop("userns_mode"))

# proof-of-concept

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this comment a left over?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. So far this patch is just a workaround I have deployed in my software to make things work, but if it is something that you would include in the library I'll clean it up and perhaps see about adding tests

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jwhonce are you fine with it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vmsh0 I can live with the work-around if you add more detail on why we are doing it and under what conditions it should be removed. As for testing, more the better. :-)

@inknos The errors reported during the build don't seem to match the code in the patch. ideas?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are only linting errors. @vmsh0

diff --git a/podman/domain/containers_create.py b/podman/domain/containers_create.py
index 066902c..308549d 100644
--- a/podman/domain/containers_create.py
+++ b/podman/domain/containers_create.py
@@ -847,7 +847,9 @@ class CreateMixin:  # pylint: disable=too-few-public-methods
 
             default_idmappings = {
                 "AutoUserNs": False if params["userns"]["nsmode"] != "auto" else True,
-                "AutoUserNsOpts": None if params["userns"]["nsmode"] != "auto" else {
+                "AutoUserNsOpts": None
+                if params["userns"]["nsmode"] != "auto"
+                else {
                     "AdditionalUIDMappings": None,
                     "AdditionalGIDMappings": None,
                     "PasswdFile": "",

please reformat and change the subject line to < 72 characters so all linters are happy :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, my two cents: I am ok with the workaround with an open issue in Podman that cross-references the change in the code. @vmsh0 I am active on podman-py, so if you need help working on both repos feel free to tag me around or in our matrix channel and I'll be happy to help

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jwhonce

@vmsh0 I can live with the work-around if you add more detail on why we are doing it and under what conditions it should be removed.

Here, in the code, or somewhere else?

By the way, today I have validated that the workaround is still needed in recent Podman, as last year when I discovered this I was running Podman 4.

As for testing, more the better. :-)

👍

@inknos

also, my two cents: I am ok with the workaround with an open issue in Podman that cross-references the change in the code. @vmsh0 I am active on podman-py, so if you need help working on both repos feel free to tag me around or in our matrix channel and I'll be happy to help

The best case scenario for me would be if you could provide some support in getting podman to compile so I can still try to fix it there. But if that's too much, I would still appreciate your guidance in getting this patch ready for podman-py instead

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh. I tried to build latest Podman, and this time it looks like I have succeeded without any hiccups. Could also reproduce the issue there. Let me look into this a bit more, and I'll find you on Matrix if I need help.

Thank you!

def merge_dicts(d1, d2):
for dk in d2:
if dk in d1:
if not isinstance(d1[dk], dict) or not isinstance(d2[dk], dict):
d1[dk] = d2[dk]
else:
d1[dk] = merge_dicts(d1[dk], d2[dk])
else:
d1[dk] = d2[dk]
return d1

default_idmappings = {
"AutoUserNs": False if params["userns"]["nsmode"] != "auto" else True,
"AutoUserNsOpts": None if params["userns"]["nsmode"] != "auto" else {
"AdditionalUIDMappings": None,
"AdditionalGIDMappings": None,
"PasswdFile": "",
"GroupFile": "",
"InitialSize": 0,
"Size": 0,
},
}

old_idmappings = params.get("idmappings") or {}
params.update(idmappings=merge_dicts(default_idmappings, old_idmappings))

if "uts_mode" in args:
params["utsns"] = normalize_nsmode(args.pop("uts_mode"))

Expand Down
Loading