set up default IDMappings when none are set and userns=auto#27988
Conversation
vmsh0
force-pushed
the
idmappings_defaults
branch
from
11d8520 to
7ee48a0
Compare
| return nil, nil, nil, err | ||
| } | ||
| s.IDMappings = mappings | ||
| s.Annotations[define.UserNsAnnotation] = string(s.UserNS.NSMode) |
There was a problem hiding this comment.
I'm thinking if we can guarantee that s.Annotations is not nil in all cases in this function. Quick search shows there are places in the code checking if Annotations is nil, so it probably can happen.
Maybe you could add
if s.Annotations == nil {
s.Annotations = make(map[string]string)
}
There was a problem hiding this comment.
Ok, pushed. Now I'm also unconditionally setting up the annotation, so it also works when a client requests creation with explicit IDMappings but no annotation.
vmsh0
force-pushed
the
idmappings_defaults
branch
from
7ee48a0 to
8353072
Compare
|
I have also submitted #27998, which will fail the test added to this PR, so if the plan is to accept the other PR I will update the test in this one |
|
A friendly reminder that this PR had no activity for 30 days. |
Honny1
left a comment
Honny1
left a comment
There was a problem hiding this comment.
Thanks, LGTM
PTAL @containers/podman-maintainers
|
@vmsh0, can you please rebase on main? |
Signed-off-by: Riccardo Paolo Bestetti <pbl@bestov.io>
vmsh0
force-pushed
the
idmappings_defaults
branch
from
8353072 to
c38fd80
Compare
|
LGTM if tests pass. |
|
The usual tests + some new ones are still failing for this and #27998, but the failures still look unrelated by these patches imho |
|
I re-run failed tests. |
4 participants
This PR originates from podman-py#499.
The rationale is to make the behavior of userns=auto a bit more consistent, by setting up default IDMappings when none are provided. Before this patch, Podman would just ignore the userns parameter silently, resulting in a container with non-private userns. This is not ideal from the point of view of security, and it can also be confusing (it was to me at least).
After this patch, e.g. API call
POST libpod/containers/create UserNS='{"NSMode":"auto"}'results in a container with private userns, as imo one could reasonably expect.Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?