From 48739f7323eddca02fc9b0443f91d2b4cb553da7 Mon Sep 17 00:00:00 2001 From: nayuta-ai Date: Sat, 2 Nov 2024 01:35:01 +0900 Subject: [PATCH] add some docstring and fix redundant code Signed-off-by: nayuta-ai --- .../tests/linux_masked_paths/masked_paths.rs | 43 +++++++------------ tests/contest/runtimetest/src/tests.rs | 2 - tests/contest/runtimetest/src/utils.rs | 20 +++------ 3 files changed, 20 insertions(+), 45 deletions(-) diff --git a/tests/contest/contest/src/tests/linux_masked_paths/masked_paths.rs b/tests/contest/contest/src/tests/linux_masked_paths/masked_paths.rs index a6f99345a1..432a4eb171 100644 --- a/tests/contest/contest/src/tests/linux_masked_paths/masked_paths.rs +++ b/tests/contest/contest/src/tests/linux_masked_paths/masked_paths.rs @@ -54,39 +54,23 @@ fn check_masked_paths() -> TestResult { test_inside_container(spec, &|bundle_path| { use std::fs; let test_dir = bundle_path.join(&masked_dir_sub); - if let Err(e) = fs::create_dir_all(&test_dir) { - bail!(e) - } + fs::create_dir_all(&test_dir)?; - if let Err(e) = fs::File::create(test_dir.join("tmp")) { - bail!(e) - } + fs::File::create(test_dir.join("tmp"))?; // runtimetest cannot check the readability of empty files, so // write something. let test_sub_sub_file = bundle_path.join(&masked_file_sub_sub); - if let Err(e) = fs::File::create(&test_sub_sub_file) { - bail!(e) - } - if let Err(e) = fs::write(&test_sub_sub_file, b"secrets") { - bail!(e) - } + fs::File::create(&test_sub_sub_file)?; + fs::write(&test_sub_sub_file, b"secrets")?; let test_sub_file = bundle_path.join(&masked_file_sub); - if let Err(e) = fs::File::create(&test_sub_file) { - bail!(e) - } - if let Err(e) = fs::write(&test_sub_file, b"secrets") { - bail!(e) - } + fs::File::create(&test_sub_file)?; + fs::write(&test_sub_file, b"secrets")?; let test_file = bundle_path.join(masked_file); - if let Err(e) = fs::File::create(&test_file) { - bail!(e) - } - if let Err(e) = fs::write(&test_file, b"secrets") { - bail!(e) - } + fs::File::create(&test_file)?; + fs::write(&test_file, b"secrets")?; Ok(()) }) @@ -99,6 +83,8 @@ fn check_masked_rel_paths() -> TestResult { let masked_paths = vec![masked_rel_path.to_string()]; let spec = get_spec(masked_paths); + // We expect the container creation to succeed, but don't mask the path because relative paths are not supported + // ref: https://github.com/opencontainers/runtime-tools/blob/master/validation/linux_masked_paths/linux_masked_paths.go#L67-L90 test_inside_container(spec, &|bundle_path| { use std::{fs, io}; let test_file = bundle_path.join(masked_rel_path); @@ -133,7 +119,7 @@ fn check_masked_symlinks() -> TestResult { let res = test_inside_container(spec, &|bundle_path| { use std::{fs, io}; let test_file = bundle_path.join(masked_symlink); - // ln -s .. /masked-symlink ; readlink -f /masked-symlink; ls -L /masked-symlink + // ln -s ../masked-symlink ; readlink -f /masked-symlink; ls -L /masked-symlink match std::os::unix::fs::symlink("../masked_symlink", &test_file) { io::Result::Ok(_) => { /* This is expected */ } io::Result::Err(e) => { @@ -148,6 +134,7 @@ fn check_masked_symlinks() -> TestResult { } }; + // It ensures that the symlink points not to exist. match fs::metadata(r_path) { io::Result::Ok(md) => { bail!( @@ -167,6 +154,7 @@ fn check_masked_symlinks() -> TestResult { } }); + // If the container creation succeeds, we expect an error since the masked paths does not support symlinks. if let TestResult::Passed = res { TestResult::Failed(anyhow!( "expected error in container creation with invalid symlink, found no error" @@ -176,7 +164,7 @@ fn check_masked_symlinks() -> TestResult { } } -fn test_node(mode: u32) -> TestResult { +fn test_mode(mode: u32) -> TestResult { let root = PathBuf::from("/"); let masked_device = "masked_device"; let masked_paths = vec![root.join(masked_device).to_string_lossy().to_string()]; @@ -225,11 +213,10 @@ fn check_masked_device_nodes() -> TestResult { SFlag::S_IFIFO.bits() | 0o666, ]; for mode in modes { - let res = test_node(mode); + let res = test_mode(mode); if let TestResult::Failed(_) = res { return res; } - std::thread::sleep(std::time::Duration::from_millis(1000)); } TestResult::Passed } diff --git a/tests/contest/runtimetest/src/tests.rs b/tests/contest/runtimetest/src/tests.rs index f7d4ff705f..86e7829672 100644 --- a/tests/contest/runtimetest/src/tests.rs +++ b/tests/contest/runtimetest/src/tests.rs @@ -561,8 +561,6 @@ pub fn validate_masked_paths(spec: &Spec) { return; } - // TODO when https://github.com/rust-lang/rust/issues/86442 stabilizes, - // change manual matching of i32 to e.kind() and match statement for path in masked_paths { match test_read_access(path) { Ok(true) => { diff --git a/tests/contest/runtimetest/src/utils.rs b/tests/contest/runtimetest/src/utils.rs index 1259367706..3cf16c3eb0 100644 --- a/tests/contest/runtimetest/src/utils.rs +++ b/tests/contest/runtimetest/src/utils.rs @@ -1,6 +1,6 @@ use std::fs; use std::fs::{metadata, symlink_metadata, OpenOptions}; -use std::io::{self, Read}; +use std::io::Read; use std::os::unix::prelude::MetadataExt; use std::path::PathBuf; use std::process::Command; @@ -13,16 +13,8 @@ fn test_file_read_access(path: &str) -> Result { // Create a buffer with a capacity of 1 byte let mut buffer = [0u8; 1]; match file.read(&mut buffer) { - Ok(_) => { - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - if contents.is_empty() { - // empty file - return Ok(false); - } - Ok(true) - } - Err(e) if e.kind() == io::ErrorKind::UnexpectedEof => Ok(false), + Ok(0) => Ok(false), // If the file is empty, then it's not readable. + Ok(_) => Ok(true), Err(e) => Err(e), } } @@ -40,12 +32,10 @@ fn test_dir_read_access(path: &str) -> Result { Err(_) => Ok(false), // If the entry is Err, then it's not readable } } - None => Ok(false), // If there are no entries, then it's not readable + None => Ok(false), // If there's an error, then it's not readable, or otherwise, it may indicate different conditions. } } - Err(_) => { - Ok(false) // If there's an error, then it's not readable - } + Err(e) => Err(e), } }