Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test root readonly #2976

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions tests/contest/contest/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use crate::tests::mounts_recursive::get_mounts_recursive_test;
use crate::tests::no_pivot::get_no_pivot_test;
use crate::tests::pidfile::get_pidfile_test;
use crate::tests::readonly_paths::get_ro_paths_test;
use crate::tests::root_readonly_true::get_root_readonly_test;
use crate::tests::scheduler::get_scheduler_test;
use crate::tests::seccomp::get_seccomp_test;
use crate::tests::seccomp_notify::get_seccomp_notify_test;
Expand Down Expand Up @@ -114,6 +115,7 @@ fn main() -> Result<()> {
let scheduler = get_scheduler_test();
let io_priority_test = get_io_priority_test();
let devices = get_devices_test();
let root_readonly = get_root_readonly_test();
let no_pivot = get_no_pivot_test();

tm.add_test_group(Box::new(cl));
Expand All @@ -138,6 +140,7 @@ fn main() -> Result<()> {
tm.add_test_group(Box::new(sysctl));
tm.add_test_group(Box::new(scheduler));
tm.add_test_group(Box::new(devices));
tm.add_test_group(Box::new(root_readonly));
tm.add_test_group(Box::new(no_pivot));

tm.add_test_group(Box::new(io_priority_test));
Expand Down
1 change: 1 addition & 0 deletions tests/contest/contest/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub mod mounts_recursive;
pub mod no_pivot;
pub mod pidfile;
pub mod readonly_paths;
pub mod root_readonly_true;
pub mod scheduler;
pub mod seccomp;
pub mod seccomp_notify;
Expand Down
2 changes: 2 additions & 0 deletions tests/contest/contest/src/tests/root_readonly_true/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
mod root_readonly_tests;
pub use root_readonly_tests::get_root_readonly_test;
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use anyhow::{Context, Ok, Result};
use oci_spec::runtime::{ProcessBuilder, RootBuilder, Spec, SpecBuilder};
use test_framework::{test_result, Test, TestGroup, TestResult};

use crate::utils::test_inside_container;

fn create_spec(readonly: bool) -> Result<Spec> {
let spec = SpecBuilder::default()
.root(RootBuilder::default().readonly(readonly).build().unwrap())
.process(
ProcessBuilder::default()
.args(vec!["runtimetest".to_string(), "root_readonly".to_string()])
.build()
.expect("error in creating config"),
)
.build()
.context("failed to build spec")?;

Ok(spec)
}

fn root_readonly_test() -> TestResult {
let spec_true = test_result!(create_spec(true));
let spec_false = test_result!(create_spec(false));
test_inside_container(spec_true, &|_| Ok(()));
test_inside_container(spec_false, &|_| Ok(()))
YJDoc2 marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn get_root_readonly_test() -> TestGroup {
let mut root_readonly_test_group = TestGroup::new("root_readonly");

let test = Test::new("root_readonly_test", Box::new(root_readonly_test));
root_readonly_test_group.add(vec![Box::new(test)]);

root_readonly_test_group
}
2 changes: 2 additions & 0 deletions tests/contest/runtimetest/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ fn main() {
"io_priority_class_be" => tests::test_io_priority_class(&spec, IoprioClassBe),
"io_priority_class_idle" => tests::test_io_priority_class(&spec, IoprioClassIdle),
"devices" => tests::validate_devices(&spec),
"root_readonly" => tests::test_validate_root_readonly(&spec),
"no_pivot" => tests::validate_rootfs(),

YJDoc2 marked this conversation as resolved.
Show resolved Hide resolved
_ => eprintln!("error due to unexpected execute test name: {execute_test}"),
}
}
23 changes: 22 additions & 1 deletion tests/contest/runtimetest/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use nix::unistd::getcwd;
use oci_spec::runtime::IOPriorityClass::{self, IoprioClassBe, IoprioClassIdle, IoprioClassRt};
use oci_spec::runtime::{LinuxDevice, LinuxDeviceType, LinuxSchedulerPolicy, Spec};

use crate::utils::{self, test_read_access, test_write_access};
use crate::utils::{self, test_dir_write_access, test_read_access, test_write_access};

////////// ANCHOR: example_hello_world
pub fn hello_world(_spec: &Spec) {
Expand Down Expand Up @@ -546,6 +546,27 @@ pub fn test_io_priority_class(spec: &Spec, io_priority_class: IOPriorityClass) {
}
}

pub fn test_validate_root_readonly(spec: &Spec) {
let root = spec.root().as_ref().unwrap();
if root.readonly().unwrap() {
if let Err(e) = test_dir_write_access("/") {
let errno = Errno::from_raw(e.raw_os_error().unwrap());
if errno == Errno::EROFS {
/* This is expected */
} else {
eprintln!("readonly root filesystem, error in testing write access for path /");
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ok, but also check for read access is Ok and not err if possible. For the != EROFS case, also add the actual errno in the eprintl for debugging

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed this.
c3c62fb

} else if let Err(e) = test_dir_write_access("/") {
let errno = Errno::from_raw(e.raw_os_error().unwrap());
if errno == Errno::EROFS {
eprintln!("readt only root filesystem is false but write access for path / is err");
} else {
/* This is expected */
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for readonly = false, write access should not give any errors at all. So if it not ok, that should be considered error irrespective of errno.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed this.
c3c62fb

}
}

// the validate_rootfs function is used to validate the rootfs of the container is
// as expected. This function is used in the no_pivot test to validate the rootfs
pub fn validate_rootfs() {
Expand Down
2 changes: 1 addition & 1 deletion tests/contest/runtimetest/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ fn test_file_write_access(path: &str) -> Result<(), std::io::Error> {
Ok(())
}

fn test_dir_write_access(path: &str) -> Result<(), std::io::Error> {
pub fn test_dir_write_access(path: &str) -> Result<(), std::io::Error> {
let _ = std::fs::OpenOptions::new()
.create(true)
.truncate(true)
Expand Down