-
Notifications
You must be signed in to change notification settings - Fork 300
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
linux: support options to idmap #874
Conversation
[NO NEW TESTS NEEDED] the feature is still being worked on crun: containers/crun#874 Signed-off-by: Giuseppe Scrivano <[email protected]>
[NO NEW TESTS NEEDED] the feature is still being worked in crun: containers/crun#874 also needs: containers/common#927 Signed-off-by: Giuseppe Scrivano <[email protected]>
The current implementation is a bit problematic since it would be difficult to be used together with
This behavior is more from the perspective of the underlying filesystem (on_disk_uid-host_uid-amount) than from the perspective of the container. The behavior that I would prefer is the same as
With this behavior omitting
|
so you'd like that the specified This should be doable but adds some complexity, we need to deal with non-contiguous ranges, as well as handle an existing user namespace (e.g., |
If a container user namespace is non-contiguous and unmapped ids are included in the idmap option then that should just return an error.
But a user namespace that is contiguous but its mapping is not contiguous should still just work.
I have tested the following with the unshare/newgidmap/nsenter commands (uses nested user namespaces).
Examples unshare/newgidmap/nsenter: |
e48836d
to
9a1ab46
Compare
This pull request introduces 3 alerts when merging 9a1ab46 into d1acf9d - view on LGTM.com new alerts:
|
I've added a new flag that can help you with your use case. If you prepend
Would this work for you? |
9a1ab46
to
7ff74bf
Compare
This pull request introduces 3 alerts when merging 7ff74bf into d1acf9d - view on LGTM.com new alerts:
|
b7fe7e1
to
7b273a4
Compare
[NO NEW TESTS NEEDED] the feature is still being worked in crun: containers/crun#874 also needs: containers/common#927 Signed-off-by: Giuseppe Scrivano <[email protected]>
56ceec8
to
777d6f7
Compare
777d6f7
to
fe8cdc7
Compare
fe8cdc7
to
69df948
Compare
LGTM |
69df948
to
edc3fd9
Compare
a37d89d
to
edc3fd9
Compare
allow to specify what mapping must be used for idmapped mounts. The mapping can be specified after the `idmap` option like: `idmap=uids=0-1-10;gids=0-100-10`. When `uids` and `gids` are specified, then a new user namespace is created and used for the bind mount. Closes: containers#873 Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
edc3fd9
to
5c7165a
Compare
finally centos:8 tests are green. I think this is ready to merge |
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.
LGTM,
Just waiting for tests to pass.
allow to specify what mapping must be used for idmapped mounts.
The mapping can be specified after the
idmap
option like:idmap=uids=0-1-10;gids=0-100-10
.When
uids
andgids
are specified, then a new user namespace iscreated and used for the bind mount.
Closes: #873
Signed-off-by: Giuseppe Scrivano [email protected]