-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
volumes: add new option idmap #12298
volumes: add new option idmap #12298
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe 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 |
36a8b8a
to
3170904
Compare
/hold |
needed by containers/podman#12298 Signed-off-by: Giuseppe Scrivano <[email protected]>
c/common PR: containers/common#827 |
pkg/util/mountOpts.go
Outdated
@@ -33,6 +33,10 @@ func ProcessOptions(options []string, isTmpfs bool, sourcePath string) ([]string | |||
// Some options have parameters - size, mode | |||
splitOpt := strings.SplitN(opt, "=", 2) | |||
switch splitOpt[0] { | |||
case "idmap": | |||
if len(options) > 1 { | |||
return nil, errors.Wrapf(ErrDupeMntOption, "'idmap' option can not be used with other options") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've copied the same logic as O
.
I'll drop it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am just wondering if this would be an issue with Z, or RO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-v foo:/foo:idmap,Z
seems to work fine
3170904
to
8eeb0bc
Compare
8eeb0bc
to
018805f
Compare
docs/source/markdown/podman-run.1.md
Outdated
@@ -649,7 +651,9 @@ Current supported mount TYPEs are **bind**, **volume**, **image**, **tmpfs** and | |||
|
|||
. relabel: shared, private. | |||
|
|||
. U, chown: true or false (default). Change recursively the owner and group of the source volume based on the UID and GID of the container. | |||
· idmap: true or false (default). If specified create an idmapped mount to the target user namespace in the container. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
· idmap: true or false (default). If specified create an idmapped mount to the target user namespace in the container. | |
· idmap: true or false (default). If specified, create an idmapped mount to the target user namespace in the container. |
018805f
to
1bc19be
Compare
1bc19be
to
a18624d
Compare
comments addressed and pushed a new version |
Tests are failing. |
pass down the "idmap" mount option to the OCI runtime. Needs: containers/crun#780 Closes: containers#12154 [NO NEW TESTS NEEDED] there is no crun version yet that support the new feature. Test case (must run as root): podman run --rm -v foo:/foo alpine touch /foo/bar podman run --uidmap 0:1:1000 --rm -v foo:/foo:idmap alpine ls -l /foo total 0 -rw-r--r-- 1 root root 0 Nov 15 14:01 bar Signed-off-by: Giuseppe Scrivano <[email protected]>
a18624d
to
e83d366
Compare
rebased |
All green |
LGTM |
/lgtm |
pass down the "idmap" mount option to the OCI runtime.
Needs: containers/crun#780
Closes: #12154
[NO NEW TESTS NEEDED] there is no crun version yet that support the
new feature.
Test case (must run as root):
podman run --rm -v foo:/foo alpine touch /foo/bar
podman run --uidmap 0:1:1000 --rm -v foo:/foo:idmap alpine ls -l /foo
total 0
-rw-r--r-- 1 root root 0 Nov 15 14:01 bar
Signed-off-by: Giuseppe Scrivano [email protected]