From d756f52c73068848e54e46f66dd7553908cfb4f9 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Tue, 25 Aug 2020 15:54:34 -0500 Subject: [PATCH 01/12] rustjail: implement functions to mount and umount files Use conditional compilation (#[cfg]) to change mount and umount behaviours at compilation time. For example, such functions will just return `Ok(())` when the unit tests are being compiled, otherwise real mount and umount operations are performed. Signed-off-by: Julio Montes --- src/agent/rustjail/src/mount.rs | 65 +++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index c48946ba34..c63cccb8cf 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -102,6 +102,31 @@ lazy_static! { }; } +#[inline(always)] +fn mount( + source: Option<&P1>, + target: &P2, + fstype: Option<&P3>, + flags: MsFlags, + data: Option<&P4>, +) -> std::result::Result<(), nix::Error> { + #[cfg(not(test))] + return mount::mount(source, target, fstype, flags, data); + #[cfg(test)] + return Ok(()); +} + +#[inline(always)] +fn umount2( + target: &P, + flags: MntFlags, +) -> std::result::Result<(), nix::Error> { + #[cfg(not(test))] + return mount::umount2(target, flags); + #[cfg(test)] + return Ok(()); +} + pub fn init_rootfs( cfd_log: RawFd, spec: &Spec, @@ -124,11 +149,11 @@ pub fn init_rootfs( let root = fs::canonicalize(rootfs)?; let rootfs = root.to_str().unwrap(); - mount::mount(None::<&str>, "/", None::<&str>, flags, None::<&str>)?; + mount(None::<&str>, "/", None::<&str>, flags, None::<&str>)?; rootfs_parent_mount_private(rootfs)?; - mount::mount( + mount( Some(rootfs), rootfs, None::<&str>, @@ -157,7 +182,7 @@ pub fn init_rootfs( for o in &m.options { if let Some(fl) = PROPAGATION.get(o.as_str()) { let dest = format!("{}{}", &rootfs, &m.destination); - mount::mount(None::<&str>, dest.as_str(), None::<&str>, *fl, None::<&str>)?; + mount(None::<&str>, dest.as_str(), None::<&str>, *fl, None::<&str>)?; } } } @@ -196,7 +221,7 @@ fn mount_cgroups_v2(cfd_log: RawFd, m: &Mount, rootfs: &str, flags: MsFlags) -> if flags.contains(MsFlags::MS_RDONLY) { let dest = format!("{}{}", rootfs, m.destination.as_str()); - mount::mount( + mount( Some(dest.as_str()), dest.as_str(), None::<&str>, @@ -303,7 +328,7 @@ fn mount_cgroups( if flags.contains(MsFlags::MS_RDONLY) { let dest = format!("{}{}", rootfs, m.destination.as_str()); - mount::mount( + mount( Some(dest.as_str()), dest.as_str(), None::<&str>, @@ -336,7 +361,7 @@ pub fn pivot_rootfs(path: &P) -> Result<( // to races where we still have a reference to a mount while a process in // the host namespace are trying to operate on something they think has no // mounts (devicemapper in particular). - mount::mount( + mount( Some("none"), ".", Some(""), @@ -345,7 +370,7 @@ pub fn pivot_rootfs(path: &P) -> Result<( )?; // Preform the unmount. MNT_DETACH allows us to unmount /proc/self/cwd. - mount::umount2(".", MntFlags::MNT_DETACH).context("failed to do umount2")?; + umount2(".", MntFlags::MNT_DETACH).context("failed to do umount2")?; // Switch back to our shiny new root. unistd::chdir("/")?; @@ -368,7 +393,7 @@ fn rootfs_parent_mount_private(path: &str) -> Result<()> { } if options.contains("shared:") { - mount::mount( + mount( None::<&str>, mount_point.as_str(), None::<&str>, @@ -463,14 +488,14 @@ pub fn ms_move_root(rootfs: &str) -> Result { } // Be sure umount events are not propagated to the host. - mount::mount( + mount( None::<&str>, abs_mount_point, None::<&str>, MsFlags::MS_SLAVE | MsFlags::MS_REC, None::<&str>, )?; - match mount::umount2(abs_mount_point, MntFlags::MNT_DETACH) { + match umount2(abs_mount_point, MntFlags::MNT_DETACH) { Ok(_) => (), Err(e) => { if e.ne(&nix::Error::from(Errno::EINVAL)) && e.ne(&nix::Error::from(Errno::EPERM)) { @@ -479,7 +504,7 @@ pub fn ms_move_root(rootfs: &str) -> Result { // If we have not privileges for umounting (e.g. rootless), then // cover the path. - mount::mount( + mount( Some("tmpfs"), abs_mount_point, Some("tmpfs"), @@ -490,7 +515,7 @@ pub fn ms_move_root(rootfs: &str) -> Result { } } - mount::mount( + mount( Some(abs_root), "/", None::<&str>, @@ -584,7 +609,7 @@ fn mount_from( } } - match mount::mount( + match mount( Some(src.as_str()), dest.as_str(), Some(m.r#type.as_str()), @@ -608,7 +633,7 @@ fn mount_from( | MsFlags::MS_SLAVE), ) { - match mount::mount( + match mount( Some(dest.as_str()), dest.as_str(), None::<&str>, @@ -714,7 +739,7 @@ fn bind_dev(dev: &LinuxDevice) -> Result<()> { unistd::close(fd)?; - mount::mount( + mount( Some(&*dev.path), &dev.path[1..], None::<&str>, @@ -744,7 +769,7 @@ pub fn finish_rootfs(cfd_log: RawFd, spec: &Spec) -> Result<()> { if m.destination == "/dev" { let (flags, _) = parse_mount(m); if flags.contains(MsFlags::MS_RDONLY) { - mount::mount( + mount( Some("/dev"), "/dev", None::<&str>, @@ -758,7 +783,7 @@ pub fn finish_rootfs(cfd_log: RawFd, spec: &Spec) -> Result<()> { if spec.root.as_ref().unwrap().readonly { let flags = MsFlags::MS_BIND | MsFlags::MS_RDONLY | MsFlags::MS_NODEV | MsFlags::MS_REMOUNT; - mount::mount(Some("/"), "/", None::<&str>, flags, None::<&str>)?; + mount(Some("/"), "/", None::<&str>, flags, None::<&str>)?; } stat::umask(Mode::from_bits_truncate(0o022)); unistd::chdir(&olddir)?; @@ -773,7 +798,7 @@ fn mask_path(path: &str) -> Result<()> { //info!("{}", path); - match mount::mount( + match mount( Some("/dev/null"), path, None::<&str>, @@ -805,7 +830,7 @@ fn readonly_path(path: &str) -> Result<()> { //info!("{}", path); - match mount::mount( + match mount( Some(&path[1..]), path, None::<&str>, @@ -829,7 +854,7 @@ fn readonly_path(path: &str) -> Result<()> { Ok(_) => {} } - mount::mount( + mount( Some(&path[1..]), &path[1..], None::<&str>, From 3dc9452bc6837c82e62bc6b59346a5c66ae09cab Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Wed, 9 Sep 2020 10:39:23 -0500 Subject: [PATCH 02/12] agent/rustjail: add tempfile crate as depedency Add tempfile crate as depedency, it will be used in the following commits to create temporary directories for unit testing. Signed-off-by: Julio Montes --- src/agent/Cargo.lock | 1 + src/agent/rustjail/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index c99d63c873..2ebcfd3221 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -661,6 +661,7 @@ dependencies = [ "serde_json", "slog", "slog-scope", + "tempfile", ] [[package]] diff --git a/src/agent/rustjail/Cargo.toml b/src/agent/rustjail/Cargo.toml index 97c03d1e59..aea0f2fe8b 100644 --- a/src/agent/rustjail/Cargo.toml +++ b/src/agent/rustjail/Cargo.toml @@ -25,3 +25,4 @@ path-absolutize = "1.2.0" dirs = "3.0.1" anyhow = "1.0.32" cgroups = { git = "https://github.com/kata-containers/cgroups-rs", tag = "0.1.1"} +tempfile = "3.1.0" From 0a0714c9c38843a4c07a9c014f4d26d8573809fd Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Wed, 9 Sep 2020 10:46:32 -0500 Subject: [PATCH 03/12] agent/rustjail/mount: don't use unwrap Don't use unwrap in `init_rootfs` instead return an Error, this way we can write unit tests that don't panic. Signed-off-by: Julio Montes --- src/agent/rustjail/src/mount.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index c63cccb8cf..dba475bc33 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -138,16 +138,28 @@ pub fn init_rootfs( lazy_static::initialize(&PROPAGATION); lazy_static::initialize(&LINUXDEVICETYPE); - let linux = spec.linux.as_ref().unwrap(); + let linux = &spec + .linux + .as_ref() + .ok_or::(anyhow!("Could not get linux configuration from spec"))?; + let mut flags = MsFlags::MS_REC; match PROPAGATION.get(&linux.rootfs_propagation.as_str()) { Some(fl) => flags |= *fl, None => flags |= MsFlags::MS_SLAVE, } - let rootfs = spec.root.as_ref().unwrap().path.as_str(); - let root = fs::canonicalize(rootfs)?; - let rootfs = root.to_str().unwrap(); + let root = spec + .root + .as_ref() + .ok_or(anyhow!("Could not get rootfs path from spec")) + .and_then(|r| { + fs::canonicalize(r.path.as_str()).context("Could not canonicalize rootfs path") + })?; + + let rootfs = (*root) + .to_str() + .ok_or(anyhow!("Could not convert rootfs path to string"))?; mount(None::<&str>, "/", None::<&str>, flags, None::<&str>)?; From ab61cf7f9fe3fe2bd52ae632c3b287b88cb6f825 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Wed, 9 Sep 2020 10:49:06 -0500 Subject: [PATCH 04/12] agent/rustjail: add unit test for init_rootfs Add a unit test for `init_rootfs` increasing the code coverage of mount.rs from 0% to 44%. Signed-off-by: Julio Montes --- src/agent/rustjail/src/mount.rs | 91 +++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index dba475bc33..0fa6c66238 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -876,3 +876,94 @@ fn readonly_path(path: &str) -> Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::os::unix::io::AsRawFd; + use tempfile::tempdir; + + #[test] + fn test_init_rootfs() { + let stdout_fd = std::io::stdout().as_raw_fd(); + let mut spec = oci::Spec::default(); + let cpath = HashMap::new(); + let mounts = HashMap::new(); + + // there is no spec.linux, should fail + let ret = init_rootfs(stdout_fd, &spec, &cpath, &mounts, true); + assert!( + ret.is_err(), + "Should fail: there is no spec.linux. Got: {:?}", + ret + ); + + // there is no spec.Root, should fail + spec.linux = Some(oci::Linux::default()); + let ret = init_rootfs(stdout_fd, &spec, &cpath, &mounts, true); + assert!( + ret.is_err(), + "should fail: there is no spec.Root. Got: {:?}", + ret + ); + + let rootfs = tempdir().unwrap(); + let ret = fs::create_dir(rootfs.path().join("dev")); + assert!(ret.is_ok(), "Got: {:?}", ret); + + spec.root = Some(oci::Root { + path: rootfs.path().to_str().unwrap().to_string(), + readonly: false, + }); + + // there is no spec.mounts, but should pass + let ret = init_rootfs(stdout_fd, &spec, &cpath, &mounts, true); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + let ret = fs::remove_dir_all(rootfs.path().join("dev")); + let ret = fs::create_dir(rootfs.path().join("dev")); + + // Adding bad mount point to spec.mounts + spec.mounts.push(oci::Mount { + destination: "error".into(), + r#type: "bind".into(), + source: "error".into(), + options: vec!["shared".into(), "rw".into(), "dev".into()], + }); + + // destination doesn't start with /, should fail + let ret = init_rootfs(stdout_fd, &spec, &cpath, &mounts, true); + assert!( + ret.is_err(), + "Should fail: destination doesn't start with '/'. Got: {:?}", + ret + ); + spec.mounts.pop(); + let ret = fs::remove_dir_all(rootfs.path().join("dev")); + let ret = fs::create_dir(rootfs.path().join("dev")); + + // mounting a cgroup + spec.mounts.push(oci::Mount { + destination: "/cgroup".into(), + r#type: "cgroup".into(), + source: "/cgroup".into(), + options: vec!["shared".into()], + }); + + let ret = init_rootfs(stdout_fd, &spec, &cpath, &mounts, true); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + spec.mounts.pop(); + let ret = fs::remove_dir_all(rootfs.path().join("dev")); + let ret = fs::create_dir(rootfs.path().join("dev")); + + // mounting /dev + spec.mounts.push(oci::Mount { + destination: "/dev".into(), + r#type: "bind".into(), + source: "/dev".into(), + options: vec!["shared".into()], + }); + + let ret = init_rootfs(stdout_fd, &spec, &cpath, &mounts, true); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + } +} From 672da4d08c46466fafb3fe83e374f5a39890b905 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 10 Sep 2020 08:59:20 -0500 Subject: [PATCH 05/12] agent/rustjail: add unit test for mount_cgroups Add a unit test for `mount_cgroups` increasing the code coverage of mount.rs from 44% to 52% Signed-off-by: Julio Montes --- src/agent/rustjail/src/mount.rs | 41 +++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index 0fa6c66238..d9568ad5a6 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -966,4 +966,45 @@ mod tests { let ret = init_rootfs(stdout_fd, &spec, &cpath, &mounts, true); assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); } + + #[test] + fn test_mount_cgroups() { + let stdout_fd = std::io::stdout().as_raw_fd(); + let mount = oci::Mount { + destination: "/cgroups".to_string(), + r#type: "cgroup".to_string(), + source: "/cgroups".to_string(), + options: vec!["shared".to_string()], + }; + let tempdir = tempdir().unwrap(); + let rootfs = tempdir.path().to_str().unwrap().to_string(); + let flags = MsFlags::MS_RDONLY; + let mut cpath = HashMap::new(); + let mut cgroup_mounts = HashMap::new(); + + cpath.insert("cpu".to_string(), "cpu".to_string()); + cpath.insert("memory".to_string(), "memory".to_string()); + + cgroup_mounts.insert("default".to_string(), "default".to_string()); + cgroup_mounts.insert("cpu".to_string(), "cpu".to_string()); + cgroup_mounts.insert("memory".to_string(), "memory".to_string()); + + let ret = fs::create_dir_all(tempdir.path().join("cgroups")); + assert!(ret.is_ok(), "Should pass. Got {:?}", ret); + let ret = fs::create_dir_all(tempdir.path().join("cpu")); + assert!(ret.is_ok(), "Should pass. Got {:?}", ret); + let ret = fs::create_dir_all(tempdir.path().join("memory")); + assert!(ret.is_ok(), "Should pass. Got {:?}", ret); + + let ret = mount_cgroups( + stdout_fd, + &mount, + &rootfs, + flags, + "", + &cpath, + &cgroup_mounts, + ); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + } } From 7cf0fd95f19b714eb81c0cc2de4b04326f1eaebb Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 10 Sep 2020 10:32:43 -0500 Subject: [PATCH 06/12] agent/rustjail: implement functions to pivot_root Use conditional compilation (#[cfg]) to change pivot_root behaviour at compilation time. For example, such function will just return `Ok(())` when the unit tests are being compiled, otherwise real pivot_root operation is performed. Signed-off-by: Julio Montes --- src/agent/rustjail/src/mount.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index d9568ad5a6..78b0c02727 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -352,6 +352,16 @@ fn mount_cgroups( Ok(()) } +fn pivot_root( + new_root: &P1, + put_old: &P2, +) -> anyhow::Result<(), nix::Error> { + #[cfg(not(test))] + return unistd::pivot_root(new_root, put_old); + #[cfg(test)] + return Ok(()); +} + pub fn pivot_rootfs(path: &P) -> Result<()> { let oldroot = fcntl::open("/", OFlag::O_DIRECTORY | OFlag::O_RDONLY, Mode::empty())?; defer!(unistd::close(oldroot).unwrap()); @@ -360,7 +370,7 @@ pub fn pivot_rootfs(path: &P) -> Result<( // Change to the new root so that the pivot_root actually acts on it. unistd::fchdir(newroot)?; - unistd::pivot_root(".", ".").context(format!("failed to pivot_root on {:?}", path))?; + pivot_root(".", ".").context(format!("failed to pivot_root on {:?}", path))?; // Currently our "." is oldroot (according to the current kernel code). // However, purely for safety, we will fchdir(oldroot) since there isn't From 25c91afbea5788de80a004efa861eb84d583dbbd Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 10 Sep 2020 10:34:48 -0500 Subject: [PATCH 07/12] agent/rustjail: add unit test for pivot_rootfs Add unit test for pivot_rootfs increasing the code coverage of mount.rs Signed-off-by: Julio Montes --- src/agent/rustjail/src/mount.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index 78b0c02727..1ce746d72d 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -1017,4 +1017,10 @@ mod tests { ); assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); } + + #[test] + fn test_pivot_root() { + let ret = pivot_rootfs("/tmp"); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + } } From d79fad2dd8f225969ae122bfff4a823176ec6e56 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 10 Sep 2020 11:19:01 -0500 Subject: [PATCH 08/12] agent/rustjail: implement functions to chroot Use conditional compilation (#[cfg]) to change chroot behaviour at compilation time. For example, such function will just return `Ok(())` when the unit tests are being compiled, otherwise real chroot operation is performed. Signed-off-by: Julio Montes --- src/agent/rustjail/src/mount.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index 1ce746d72d..c8237ad8ed 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -483,6 +483,14 @@ fn parse_mount_table() -> Result> { Ok(infos) } +#[inline(always)] +fn chroot(path: &P) -> Result<(), nix::Error> { + #[cfg(not(test))] + return unistd::chroot(path); + #[cfg(test)] + return Ok(()); +} + pub fn ms_move_root(rootfs: &str) -> Result { unistd::chdir(rootfs)?; let mount_infos = parse_mount_table()?; @@ -544,7 +552,7 @@ pub fn ms_move_root(rootfs: &str) -> Result { MsFlags::MS_MOVE, None::<&str>, )?; - unistd::chroot(".")?; + chroot(".")?; unistd::chdir("/")?; Ok(true) From b99fefad7e3f7c58b11cc11b2a289e20566fb69c Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 10 Sep 2020 12:00:50 -0500 Subject: [PATCH 09/12] agent/rustjail: add unit tests for ms_move_rootfs and mask_path Increase code coverage of mount.rs Signed-off-by: Julio Montes --- src/agent/rustjail/src/mount.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index c8237ad8ed..af45903df9 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -1031,4 +1031,37 @@ mod tests { let ret = pivot_rootfs("/tmp"); assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); } + + #[test] + fn test_ms_move_rootfs() { + let ret = ms_move_root("/abc"); + assert!( + ret.is_err(), + "Should fail. path doesn't exist. Got: {:?}", + ret + ); + + let ret = ms_move_root("/tmp"); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + } + + #[test] + fn test_mask_path() { + let ret = mask_path("abc"); + assert!( + ret.is_err(), + "Should fail: path doesn't start with '/'. Got: {:?}", + ret + ); + + let ret = mask_path("abc/../"); + assert!( + ret.is_err(), + "Should fail: path contains '..'. Got: {:?}", + ret + ); + + let ret = mask_path("/tmp"); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + } } From 98cc979ae1cf00604d28022a33e823077f8cc44b Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 10 Sep 2020 12:01:58 -0500 Subject: [PATCH 10/12] agent/rustjail: remove makedev function remove `makedev` function, use `nix`'s implementation instead Signed-off-by: Julio Montes --- src/agent/rustjail/src/mount.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index af45903df9..0fa5b9d193 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -724,10 +724,6 @@ fn ensure_ptmx() -> Result<()> { Ok(()) } -fn makedev(major: u64, minor: u64) -> u64 { - (minor & 0xff) | ((major & 0xfff) << 8) | ((minor & !0xff) << 12) | ((major & !0xfff) << 32) -} - lazy_static! { static ref LINUXDEVICETYPE: HashMap<&'static str, SFlag> = { let mut m = HashMap::new(); @@ -748,7 +744,7 @@ fn mknod_dev(dev: &LinuxDevice) -> Result<()> { &dev.path[1..], *f, Mode::from_bits_truncate(dev.file_mode.unwrap_or(0)), - makedev(dev.major as u64, dev.minor as u64), + nix::sys::stat::makedev(dev.major as u64, dev.minor as u64), )?; unistd::chown( From cda7acf7da07d531d14f65ae6d56046cd68f8e0d Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 10 Sep 2020 14:46:54 -0500 Subject: [PATCH 11/12] agent/rustjail: add more unit tests Add unit tests for finish_root, read_only_path and mknod_dev increasing code coverage of mount.rs fixes #284 Signed-off-by: Julio Montes --- src/agent/rustjail/src/lib.rs | 11 ++++++ src/agent/rustjail/src/mount.rs | 63 +++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/src/agent/rustjail/src/lib.rs b/src/agent/rustjail/src/lib.rs index c77da3a309..6c14ed8a0e 100644 --- a/src/agent/rustjail/src/lib.rs +++ b/src/agent/rustjail/src/lib.rs @@ -580,4 +580,15 @@ mod tests { fn it_works() { assert_eq!(2 + 2, 4); } + + #[allow(unused_macros)] + #[macro_export] + macro_rules! skip_if_not_root { + () => { + if !nix::unistd::Uid::effective().is_root() { + println!("INFO: skipping {} which needs root", module_path!()); + return; + } + }; + } } diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index 0fa5b9d193..8bc3152dab 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -894,6 +894,7 @@ fn readonly_path(path: &str) -> Result<()> { #[cfg(test)] mod tests { use super::*; + use crate::skip_if_not_root; use std::os::unix::io::AsRawFd; use tempfile::tempdir; @@ -1060,4 +1061,66 @@ mod tests { let ret = mask_path("/tmp"); assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); } + + #[test] + fn test_finish_rootfs() { + let stdout_fd = std::io::stdout().as_raw_fd(); + let mut spec = oci::Spec::default(); + + spec.linux = Some(oci::Linux::default()); + spec.linux.as_mut().unwrap().masked_paths = vec!["/tmp".to_string()]; + spec.linux.as_mut().unwrap().readonly_paths = vec!["/tmp".to_string()]; + spec.root = Some(oci::Root { + path: "/tmp".to_string(), + readonly: true, + }); + spec.mounts = vec![oci::Mount { + destination: "/dev".to_string(), + r#type: "bind".to_string(), + source: "/dev".to_string(), + options: vec!["ro".to_string(), "shared".to_string()], + }]; + + let ret = finish_rootfs(stdout_fd, &spec); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + } + + #[test] + fn test_readonly_path() { + let ret = readonly_path("abc"); + assert!(ret.is_err(), "Should fail. Got: {:?}", ret); + + let ret = readonly_path("../../"); + assert!(ret.is_err(), "Should fail. Got: {:?}", ret); + + let ret = readonly_path("/tmp"); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + } + + #[test] + fn test_mknod_dev() { + skip_if_not_root!(); + + let tempdir = tempdir().unwrap(); + + let olddir = unistd::getcwd().unwrap(); + defer!(unistd::chdir(&olddir);); + unistd::chdir(tempdir.path()); + + let dev = oci::LinuxDevice { + path: "/fifo".to_string(), + r#type: "c".to_string(), + major: 0, + minor: 0, + file_mode: Some(0660), + uid: Some(unistd::getuid().as_raw()), + gid: Some(unistd::getgid().as_raw()), + }; + + let ret = mknod_dev(&dev); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + + let ret = stat::stat("fifo"); + assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); + } } From 96f8769a99e4cc8c125bc72910f8bb38766ee024 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Mon, 21 Sep 2020 15:48:02 -0500 Subject: [PATCH 12/12] travis: enable RUST_BACKTRACE RUST_BACKTRACE=1 will help us a lot to debug unit tests when a test is failing Signed-off-by: Julio Montes --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2bac58b583..672c209d64 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,9 @@ dist: bionic os: linux language: go go: 1.14.4 -env: target_branch=$TRAVIS_BRANCH +env: + - target_branch=$TRAVIS_BRANCH + - RUST_BACKTRACE=1 before_install: - git remote set-branches --add origin "${TRAVIS_BRANCH}"