Fix image-create with ACLs. Fixes #1394.

[dhbaird]

Relevant Issue (if applicable)

Fixes:
#1394

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation Update (if none of the other choices apply)

Checklist

• I have updated the documentation accordingly.
• I have added tests to cover my changes.

[anolis-bot]

@dhbaird , a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/89576

[codecov]

Codecov Report

Merging #1395 (67c8c88) into master (34ee825) will increase coverage by 0.24%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1395      +/-   ##
==========================================
+ Coverage   46.05%   46.30%   +0.24%     
==========================================
  Files         122      122              
  Lines       38077    38077              
  Branches    38077    38077              
==========================================
+ Hits        17538    17632      +94     
+ Misses      19618    19519      -99     
- Partials      921      926       +5     

Files Changed	Coverage Δ
rafs/src/metadata/layout/mod.rs	93.67% <100.00%> (ø)

... and 9 files with indirect coverage changes

[anolis-bot]

@dhbaird , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

[imeoer]

Thanks for the fixup, it seems we need also to handle this?
https://github.com/dragonflyoss/image-service/blob/34ee8255dea73819f4d634f83260d1fa4fe68c97/rafs/src/metadata/layout/v6.rs#L2088

[dhbaird]

Perhaps so; I am not 100% sure. I apologize that I have yet produced a test in bats to correspond to testing this. Thank you for the time and consideration.

[dhbaird]

*That I have not yet produced, I mean!

[imeoer]

Hi @dhbaird, you can add a related test case here, it will be used by the smoke test automatically.
https://github.com/dragonflyoss/image-service/blob/34ee8255dea73819f4d634f83260d1fa4fe68c97/smoke/tests/texture/layer.go#L89

[imeoer]

@dhbaird Please also add a signed-off-by for the commit by following https://github.com/dragonflyoss/image-service/pull/1395/checks?check_run_id=15694892551

[dhbaird]

Thanks for the guidance @imeoer! I confirmed that the line you indicated above should change: https://github.com/dragonflyoss/image-service/blob/34ee8255dea73819f4d634f83260d1fa4fe68c97/rafs/src/metadata/layout/v6.rs#L2088

I am working on the tests now and double checking the changes, and will also added signed-off-by.

[dhbaird]

Hi again @imeoer, When I tried to update the test indicated in previous comments, I got blocked by a new issue: toOCITar() drops all xattrs except for security.capability:

https://github.com/dragonflyoss/image-service/blob/34ee8255dea73819f4d634f83260d1fa4fe68c97/smoke/tests/tool/layer.go#L119

I read the code and traced it to containerd/archive, where it appears to ignore all xattrs except for security.capability:

https://github.com/containerd/containerd/blob/70a2c95ae8c02d7a4e448f0e4fb8bb0e6344b5c7/archive/tar.go#L644

Does this look like an issue in containerd/archive now too? Would you recommend that I create an issue over there too?

Thanks!

[imeoer]

Does this look like an issue in containerd/archive now too? Would you recommend that I create an issue over there too?

@dhbaird It seems only the security.capability xattr is supported in OCI tar, so we can't add the test case into the build of oci tar -> nydus rafs, but still can support it in the build of directory -> nydus rafs.

But to keep it simple, I think we just need to add some unit tests for RafsXAttrs, what do you think?

[dhbaird]

Hi @imeoer, the code is again ready for review. May you restart the "nydus-unit-test-coverage" automation? Thanks for your helpful feedback and time so far. Any ideas / suggestions / criticisms are welcome!

[imeoer]

Thanks @dhbaird for the excellent tests and fixup!