add some docstring and fix redundant code

Signed-off-by: nayuta-ai <[email protected]>

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
 }

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) => {

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<bool, std::io::Error> {
     // 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<bool, std::io::Error> {
                         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),
     }
 }