Skip to content
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

Unpack on NFS and Extended File Attributes #248

Closed
besnardjb opened this issue Aug 9, 2018 · 3 comments
Closed

Unpack on NFS and Extended File Attributes #248

besnardjb opened this issue Aug 9, 2018 · 3 comments

Comments

@besnardjb
Copy link

Hi all,

Issue

I tried to use umoci to extract images on an NFS file-system and encountered various errors linked to extended file attributes. Umoci fails to unpack in both rootless and default cases.

Reproducer

You need to be on an NFS mount.

#Get alpine
$ skopeo copy docker://alpine oci:alpine_image:latest
#!
#! Unpack in rootless
#!
$ umoci --log debug  unpack --rootless --image ./alpine_image ./alpine_bundle           1 ↵
   • parsed mappings           map.gid=[{1000 0 1}] map.uid=[{1000 0 1}]
   • -> ws.recurse             digest=sha256:8327fe6785fae2e722377350939dc07cd83674d285f537e1a91893248b925bc3
   • <- ws.recurse             digest=sha256:8327fe6785fae2e722377350939dc07cd83674d285f537e1a91893248b925bc3
   • casext.ResolveReference(latest) got these descriptors refs=[{[{application/vnd.oci.image.manifest.v1+json sha256:8327fe6785fae2e722377350939dc07cd83674d285f537e1a91893248b925bc3 348 [] map[org.opencontainers.image.ref.name:latest] 0xc420088900}]}]
   • umoci: unpacking OCI image bundle=./alpine_bundle image=./alpine_image ref=latest rootfs=rootfs
   • unpacking bundle ...     
   • unpack rootfs: alpine_bundle/rootfs
   • unpack layer: sha256:8e3ba11ec2a2b39ab372c60c16b421536e50e5ce64a0bc81765c2e38381bcff6
   • unpacking entry           path=bin root=alpine_bundle/rootfs type=53
   ⨯ create runtime bundle: unpack rootfs: unpack layer: unpack entry: bin: apply hdr metadata: clear xattr metadata: alpine_bundle/rootfs/bin: unpriv.lclearxattrs: unpriv.lremovexattr: input/output error
#!
#! Unpack as root
#!
(as root) umoci --log debug  unpack --image ./alpine_image ./alpine_bundle
[...] #Same error
   ⨯ create runtime bundle: unpack rootfs: unpack layer: unpack entry: bin: apply hdr metadata: clear xattr metadata: alpine_bundle/rootfs/bin: unpriv.lclearxattrs: unpriv.lremovexattr: input/output error

Discussion

This is linked to the fact that NFS does not support extended file attributes. When removing 'system.nfs4_acl' in Lclearxattrs you get EIO and overall when setting any xattr in Lsetxattr you get ENOTSUP.

Proposed Fix

I humbly tempted an initial fix which seems to solve the issue on my system in this branch:

https://github.com/besnardjb/umoci/tree/nfs

However, as it alters the resulting image (i.e. losing xattr data) it might deserve a warning as some data of the image was lost even if not representable in the target fs?

Thanks,

Jean-Baptiste.

@cyphar
Copy link
Member

cyphar commented Aug 9, 2018

We should ignore ENOTSUP when setting xattrs (I'm surprised I don't already do that because I added code like that to Docker to handle AUFS issues). As for the clearing of xattrs we need to have a blacklist of bad xattrs to clear when extracting an image -- which system.nfs4_acl will be added to. I also should take a look at the kernel source to see if there are any other special xattrs I should be aware of.

I humbly tempted an initial fix which seems to solve the issue on my system in this branch:

I took a look at that fix, and it has a couple of issues (mainly how extensible it is) -- but don't worry, I have my own proposed fix I started working on when I saw your other issue. I'll post that one, and ping you when it's ready to test. But thanks for writing a sample PR anyway! 😸

@besnardjb
Copy link
Author

Thank you very much! Indeed it was just a quick fix allowing me to test the umoci integration meanwhile and also to check that I was not reporting something dumb 😄

@cyphar
Copy link
Member

cyphar commented Aug 28, 2018

Sorry for the delay. The current update is that at the moment I have it implemented, but I'm having trouble coming up with a way of testing it -- because all of the current always-ignore xattrs are not really writable as a user.

I will publish a PR in a bit which should have it tested, as soon as I figure out how to make certain xattrs only ignored when running a test binary (it turns out that _test.go files aren't even compiled in dependencies of the package you build -- meaning you cannot use func init() to do dodgy stuff like that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants