storage: make [l]chown errors clearer #303
Conversation
if os.[Lc,C]hown are failing with EINVAL, it might be related to an UID/GID not mapped in the user namespace we are currently using. It could be possible to detect this issue by inspecting /proc/self/uid_map or /proc/self/gid_map, but that won't be possible when we are pulling a new image and extracting it from a chroot where /proc is not mounted. Signed-off-by: Giuseppe Scrivano <[email protected]>
|
Any chance to use https://github.com/rootless-containers/proto , so that OCI runtime can emulate file permission using ptrace/seccomp ? |
|
that would work only with umoci and the rootless-containers fork of PRoot. Is there any other runtime honoring these xattrs? I don't think at the moment we want to support syscalls emulation either in Podman and Buildah. For containers/storage it is probably fine if we support it through an explicit configuration setting, if there is anyone using it. I am only opposed to silently doing it as a fallback when chown fails. |
|
|
||
| func checkChownErr(err error, name string, uid, gid int) error { | ||
| if e, ok := err.(*os.PathError); ok && e.Err == syscall.EINVAL { | ||
| return errors.Wrapf(err, "there might not be enough IDs available in the namespace (requested %d:%d for %s)", uid, gid, name) |
There was a problem hiding this comment.
How about
"%d:%d dones have have a valid mapping in the user namespace for %s"
There was a problem hiding this comment.
we are not sure that is the problem without checking /proc/self/uid_map, that is why I've left it as a possibility in the message. Even though I don't see other reasons why it could fail with that error code.
There was a problem hiding this comment.
Maybe: "(err, "A valid mapping could not be determined for the usernamespace %s with the requested uid:gid (%s:%s)", name, uid, gid)"
There was a problem hiding this comment.
I don't know if you want to add something about "verify the values in /proc/self/uid_map" or some such to that.
There was a problem hiding this comment.
there is need to be in the correct namespace to verify /proc/self/uid_map
There was a problem hiding this comment.
I just want something more specific for the user. then "Their might not be" ...
we were playing safe and not allowed any container to have less than 65536 mappings. There are a couple of reasons to change it: - it blocked libpod to work in an environment where newuidmap/newgidmap are not available, or not configured. - not allowed to use different partitions of subuids, where each user has less than 65536 ids available. Hopefully this change in containers/storage: containers/storage#303 will make error clearers if there are not enough IDs for the image that is being used. Closes: containers#1651 Signed-off-by: Giuseppe Scrivano <[email protected]>
|
LGTM |
if os.[Lc,C]hown are failing with EINVAL, it might be related to an
UID/GID not mapped in the user namespace we are currently using.
It could be possible to detect this issue by inspecting
/proc/self/uid_map or /proc/self/gid_map, but that won't be possible
when we are pulling a new image and extracting it from a chroot where
/proc is not mounted.
Signed-off-by: Giuseppe Scrivano [email protected]