youki icon indicating copy to clipboard operation
youki copied to clipboard

linux_mount_label integration test

Open Gekko0114 opened this issue 1 year ago • 19 comments

Created linux_mount_label integration test.

Referred this linux_mount_label test. https://github.com/opencontainers/runtime-tools/tree/master/validation/linux_mount_label

#361

Gekko0114 avatar Feb 18 '24 08:02 Gekko0114

Hi @utam0k, Could you take a look? Though I think I've implemented this PR same as the integration test of runtime-tools, got an error below on my codespace environment. I've tried to fix it, but couldn't find how to fix it

# Start group linux_mount_label
1 / 1 : linux_mount_label : not ok
	container stderr was not empty, found : [31mERROR[0m [2mlibcontainer::process::container_init_process[0m[2m:[0m failed to mount path as masked using tempfs [3mpath[0m[2m=[0m"/proc/acpi" [3merr[0m[2m=[0mNix(EINVAL)
[31mERROR[0m [2mlibcontainer::process::container_init_process[0m[2m:[0m failed to set masked path [3merr[0m[2m=[0mMountPathMasked(Nix(EINVAL)) [3mpath[0m[2m=[0m"/proc/acpi"
[31mERROR[0m [2mlibcontainer::process::container_intermediate_process[0m[2m:[0m failed to initialize container process: failed to mount path as masked
[31mERROR[0m [2mlibcontainer::process::container_main_process[0m[2m:[0m failed to wait for init ready: failed to receive. "waiting for init ready". BrokenChannel
[31mERROR[0m [2mlibcontainer::container::builder_impl[0m[2m:[0m failed to run container process [3merr[0m[2m=[0mChannel(ReceiveError { msg: "waiting for init ready", source: BrokenChannel })
[31mERROR[0m [2myouki[0m[2m:[0m error in executing command: failed to receive. "waiting for init ready". BrokenChannel

Caused by:
    channel connection broken
Error: failed to receive. "waiting for init ready". BrokenChannel

Caused by:
    channel connection broken

# End group linux_mount_label

Gekko0114 avatar Feb 18 '24 08:02 Gekko0114

@Gekko0114 It seems there is some issue with the /proc/acpi path due to which we are getting EINVAL error. Can you check this path, its perms and related stuff? Thanks!

YJDoc2 avatar Feb 19 '24 05:02 YJDoc2

@Gekko0114 May I ask you to commit with your sign? https://github.com/containers/youki/pull/2688/checks?check_run_id=21940370118

utam0k avatar Feb 25 '24 12:02 utam0k

Codecov Report

Merging #2688 (1d77cf7) into main (919ff4a) will increase coverage by 0.00%. Report is 2 commits behind head on main. The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2688   +/-   ##
=======================================
  Coverage   65.40%   65.41%           
=======================================
  Files         133      133           
  Lines       16942    16942           
=======================================
+ Hits        11081    11082    +1     
+ Misses       5861     5860    -1     

codecov-commenter avatar Feb 25 '24 12:02 codecov-commenter

Hi @YJDoc2 @utam0k Though I have some questions, I created a PR and passed a test case. Could you review it?

Gekko0114 avatar Feb 25 '24 14:02 Gekko0114

Though I have some questions, I created a PR and passed a test case. Could you review it?

Hey, thanks! I'll take a look and answer the questions :+1:

YJDoc2 avatar Feb 26 '24 05:02 YJDoc2

Hey @utam0k , a couple of things :

  1. Currently our integration test validation CI is broken, opened https://github.com/containers/youki/pull/2707 for that. In that the test added in this PR is failing on Runc as well , see a run done in my branch : https://github.com/YJDoc2/youki/actions/runs/8050503893/job/21986255509
  2. I have some question regarding the MountLabel implementation itself : As per the oci spec, the MountLabel field marks the selinux label to be used for that mount. Thus in runc https://github.com/opencontainers/runc/blob/d5e4c33001d74176222fe8f48a323f3e8ad89999/libcontainer/rootfs_linux.go#L536 , they use the selinux package and set the label using that. However in youki, we are passing that from https://github.com/containers/youki/blob/main/crates/libcontainer/src/rootfs/rootfs.rs#L90 to the syscall implementation, which finally reaches https://github.com/containers/youki/blob/2681f9c63f2b70ef636c088222a92cccb9252995/crates/libcontainer/src/rootfs/mount.rs#L484 and then goes https://github.com/containers/youki/blob/main/crates/libcontainer/src/syscall/linux.rs#L471 . Where it finally calls nix::mount. I'm not sure how that is supposed to correspond to selinux. Can you check if there is some issue with out implementation , or am I doing some wrong code follow?

@Gekko0114 May I ask you to wait for sometime until the above are a bit more clear? Thanks :)

YJDoc2 avatar Feb 26 '24 15:02 YJDoc2

On further testing, the original oci go tests fails on runc as well here : https://github.com/YJDoc2/youki/actions/runs/8052111550/job/21991533730?pr=263 (ignore the passing status, I have removed all exit 1 to make sure all tests run ; check the not ok instead)

I think we need se linux setup and configured for this to work at all. Also another thing is that I'm not sure if the original oci tools tests are used anywhere currently (runc,crun etc) probably because they being broken and out of sync.

YJDoc2 avatar Feb 26 '24 16:02 YJDoc2

🤦‍♂Our implementation is wrong.

utam0k avatar Feb 27 '24 12:02 utam0k

@saschagrunert Do you know any good crate for SELinux without libselinux? I don't like adding a dependency.

utam0k avatar Feb 27 '24 12:02 utam0k

@saschagrunert Do you know any good crate for SELinux without libselinux? I don't like adding a dependency.

Which functionality do you need? I assume all of them require libselinux somehow.

saschagrunert avatar Feb 27 '24 12:02 saschagrunert

Which functionality do you need? I assume all of them require libselinux somehow.

I want a crate like opencontainers/selinux. It doesn't need libselinux, does it? https://github.com/opencontainers/selinux

It seems we need following features:

  • label.SetFileLabel https://github.com/opencontainers/runc/blob/d5e4c33001d74176222fe8f48a323f3e8ad89999/libcontainer/rootfs_linux.go#L536
  • https://github.com/opencontainers/runc/blob/d5e4c33001d74176222fe8f48a323f3e8ad89999/libcontainer/rootfs_linux.go#L667-L672
			if err := label.Validate(m.Relabel); err != nil {
				return err
			}
			shared := label.IsShared(m.Relabel)
			if err := label.Relabel(m.Source, mountLabel, shared); err != nil {
				return err
			}
  • label.FormatMountLabel https://github.com/opencontainers/runc/blob/d5e4c33001d74176222fe8f48a323f3e8ad89999/libcontainer/rootfs_linux.go#L1247

If there isn't in the world for now, is there any motivation to implement it in containers org? I think you are a specialist in this field.

utam0k avatar Feb 27 '24 12:02 utam0k

@utam0k that does not look like much code, do we want to create a module here and then consider moving it into a new crate?

saschagrunert avatar Feb 27 '24 13:02 saschagrunert

Yes, I think something like oci-spec-rs would be good. How about first implementing it in this repository to some extent, and then taking it to new repository in containers org? Maybe it can be used for other repositories/projects than youki?

utam0k avatar Feb 27 '24 13:02 utam0k

Yes, I think something like oci-spec-rs would be good. How about first implementing it in this repository to some extent, and then taking it to new repository in containers org? Maybe it can be used for other repositories/projects than youki?

Yes this sounds like a good idea. :+1:

saschagrunert avatar Feb 27 '24 13:02 saschagrunert

How about first implementing it in this repository to some extent, ... Maybe it can be used for other repositories/projects than youki?

After seeing the go implementation code, I was thinking along the same lines. Even if not a complete API, I think we can port the functions we need into youki, and use them :+1:

I can take a try at a basic initial implementation around the weekend, or if someone else want to try before that, let me know if you need any help!

YJDoc2 avatar Feb 27 '24 15:02 YJDoc2

@Gekko0114 Are you interested in this? If possible, I want you to give it a challenge with @YJDoc2 as a mentor and @saschagrunert as an advisor.

Yes, I think something like oci-spec-rs would be good. How about first implementing it in this repository to some extent, and then taking it to new repository in containers org? Maybe it can be used for other repositories/projects than youki?

utam0k avatar Feb 28 '24 11:02 utam0k

@utam0k Thanks for asking me! Yes, I would like to work on it. I will start working on this issue from this weekend. Also I will create a new issue for this.

Gekko0114 avatar Feb 28 '24 13:02 Gekko0114

Thanks for asking me! Yes, I would like to work on it. I will start working on this issue from this weekend. Also I will create a new issue for this.

That's great! Feel free to ping me on github or discord if you need any help :)

YJDoc2 avatar Mar 01 '24 18:03 YJDoc2