diff --git a/.gitignore b/.gitignore index 468e4aff..9205dc8e 100644 --- a/.gitignore +++ b/.gitignore @@ -13,7 +13,9 @@ tags.temp youki !youki/ youki_integration_test +runtimetest_tool runtimetest +!runtimetest/ .vscode diff --git a/Cargo.lock b/Cargo.lock index d3ece6bf..23217e50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1290,14 +1290,6 @@ version = "0.6.25" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f497285884f3fcff424ffc933e56d7cbca511def0c9831a7f9b5f6153e3cc89b" -[[package]] -name = "runtimetest" -version = "0.0.1" -dependencies = [ - "nix", - "oci-spec 0.5.2 (git+https://github.com/containers/oci-spec-rs?rev=54c5e386f01ab37c9305cc4a83404eb157e42440)", -] - [[package]] name = "rustc_version" version = "0.4.0" diff --git a/build.sh b/build.sh index bd99d05f..336e694b 100755 --- a/build.sh +++ b/build.sh @@ -11,18 +11,16 @@ if [ "$1" == "--release" ]; then VERSION=release fi -# We have to build the three binaries seprately for the following reason : -# The runtimetest MUST be compiled from its own directory, if compiled from root, -# it will not work as intended to test the runtime from inside -# So we just compile all thre binaries separately. -# To see why runtime test must be compiled in its own directory, see its Readme or its docs -cargo build --bin youki --verbose $TGT $1 -cargo build --bin integration_test --verbose $TGT $1 -cd crates/runtimetest +# Runtimetest must be compiled in its dorectory and is +# not a part of youki workspace. For the reasoning behind this, +# please check the docs and readme + cargo build --verbose $TGT $1 -cd ../../ +cd ./runtimetest +cargo build --verbose $TGT $1 +cd .. cp target/$TARGET/$VERSION/youki . cp target/$TARGET/$VERSION/integration_test ./youki_integration_test -cp target/$TARGET/$VERSION/runtimetest ./runtimetest +cp runtimetest/target/$TARGET/$VERSION/runtimetest ./runtimetest_tool diff --git a/crates/integration_test/tests.sh b/crates/integration_test/tests.sh index a78a9758..d5eb5238 100755 --- a/crates/integration_test/tests.sh +++ b/crates/integration_test/tests.sh @@ -3,7 +3,7 @@ cd ../../ ./build.sh --release cp ./youki ./crates/integration_test/youki cp ./youki_integration_test ./crates/integration_test/youki_integration_test -cp ./runtimetest ./crates/integration_test/runtimetest +cp ./runtimetest_tool ./crates/integration_test/runtimetest cd ./crates/integration_test RUNTIME=./youki diff --git a/docs/src/developer/runtimetest.md b/docs/src/developer/runtimetest.md index d5fc89af..57d3014d 100644 --- a/docs/src/developer/runtimetest.md +++ b/docs/src/developer/runtimetest.md @@ -6,7 +6,14 @@ This crate provides a binary which is used by integration tests to verify that t This binary must be compiled with the option of static linking to crt0 given to the rustc. If compiled without it, it will add a linking to /lib64/ld-linux-x86-64.so . The binary compiled this way cannot be run inside the container process, as they do not have access to /lib64/... Thus the runtime test must be statically linked to crt0. -Also this option can be given through .cargo/config.toml rustflags option, but this works only if the cargo build is invoked within the runtimetest directory. If invoked from the project root, the .cargo/config in the project root will take preference and the rustflags will be ignored. +While developing, originally this was added to the common workspace of all crates in youki. But then it was realized that this was quite inefficient because : + +- All packages except runtimetest will be compiled with dynamic linking +- Runtimetest will be compiled with static linking + +Now runtimetest needs at least `oci-spec` and `nix` package for its operations, which are also dependencies of other packages in the workspace. Thus both of these, and recursively their dependencies must be compiled twice, each time, once for dynamic linking and once for static. The took a long time in the compilation stage, especially when developing / adding new tests. Separating runtimetest from the workspace allows it to have a separate target/ directory, where it can store the statically compiled dependencies, and the workspace can have its target/ directory, where it can store its dynamically compiled dependencies. That way only the crates which have changes need to be compiled (runtimetest or integration test), and not their dependencies. + +In case in future this separation is not required, or some other configuration is chosen, make sure the multiple compilation issue does not arise, or the advantages of new method outweigh the time spent in double compilation. To see if a binary can be run inside the container process, run diff --git a/runtimetest/.cargo/config.toml b/runtimetest/.cargo/config.toml new file mode 100644 index 00000000..746c5182 --- /dev/null +++ b/runtimetest/.cargo/config.toml @@ -0,0 +1,3 @@ +[build] +target = "x86_64-unknown-linux-gnu" +rustflags = ["-C", "target-feature=+crt-static"] diff --git a/runtimetest/Cargo.lock b/runtimetest/Cargo.lock new file mode 100644 index 00000000..0c6ac10b --- /dev/null +++ b/runtimetest/Cargo.lock @@ -0,0 +1,300 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "autocfg" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a" + +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + +[[package]] +name = "cc" +version = "1.0.72" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22a9137b95ea06864e018375b72adfb7db6e6f68cfc8df5a04d00288050485ee" + +[[package]] +name = "cfg-if" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" + +[[package]] +name = "darling" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f2c43f534ea4b0b049015d00269734195e6d3f0f6635cb692251aca6f9f8b3c" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e91455b86830a1c21799d94524df0845183fa55bafd9aa137b01c7d1065fa36" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29b5acf0dea37a7f66f7b25d2c5e93fd46f8f6968b1a5d7a3e02e97768afc95a" +dependencies = [ + "darling_core", + "quote", + "syn", +] + +[[package]] +name = "derive_builder" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d13202debe11181040ae9063d739fa32cfcaaebe2275fe387703460ae2365b30" +dependencies = [ + "derive_builder_macro", +] + +[[package]] +name = "derive_builder_core" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "66e616858f6187ed828df7c64a6d71720d83767a7f19740b2d1b6fe6327b36e5" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "derive_builder_macro" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "58a94ace95092c5acb1e97a7e846b310cfbd499652f72297da7493f618a98d73" +dependencies = [ + "derive_builder_core", + "syn", +] + +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + +[[package]] +name = "getset" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e45727250e75cc04ff2846a66397da8ef2b3db8e40e0cef4df67950a07621eb9" +dependencies = [ + "proc-macro-error", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + +[[package]] +name = "itoa" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1aab8fc367588b89dcee83ab0fd66b72b50b72fa1904d7095045ace2b0c81c35" + +[[package]] +name = "libc" +version = "0.2.112" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b03d17f364a3a042d5e5d46b053bbbf82c92c9430c592dd4c064dc6ee997125" + +[[package]] +name = "memoffset" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5aa361d4faea93603064a027415f07bd8e1d5c88c9fbf68bf56a285428fd79ce" +dependencies = [ + "autocfg", +] + +[[package]] +name = "nix" +version = "0.23.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f866317acbd3a240710c63f065ffb1e4fd466259045ccb504130b7f668f35c6" +dependencies = [ + "bitflags", + "cc", + "cfg-if", + "libc", + "memoffset", +] + +[[package]] +name = "oci-spec" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8057bb0f33d7ecdf1f0f7cc74ea5cced7c6c694245e2a8d14700507c3bde32e3" +dependencies = [ + "derive_builder", + "getset", + "serde", + "serde_json", + "thiserror", +] + +[[package]] +name = "proc-macro-error" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c" +dependencies = [ + "proc-macro-error-attr", + "proc-macro2", + "quote", + "syn", + "version_check", +] + +[[package]] +name = "proc-macro-error-attr" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869" +dependencies = [ + "proc-macro2", + "quote", + "version_check", +] + +[[package]] +name = "proc-macro2" +version = "1.0.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7342d5883fbccae1cc37a2353b09c87c9b0f3afd73f5fb9bba687a1f733b029" +dependencies = [ + "unicode-xid", +] + +[[package]] +name = "quote" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47aa80447ce4daf1717500037052af176af5d38cc3e571d9ec1c7353fc10c87d" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "runtimetest" +version = "0.0.1" +dependencies = [ + "nix", + "oci-spec", +] + +[[package]] +name = "ryu" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73b4b750c782965c211b42f022f59af1fbceabdd026623714f104152f1ec149f" + +[[package]] +name = "serde" +version = "1.0.133" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97565067517b60e2d1ea8b268e59ce036de907ac523ad83a0475da04e818989a" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.133" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed201699328568d8d08208fdd080e3ff594e6c422e438b6705905da01005d537" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "serde_json" +version = "1.0.74" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee2bb9cd061c5865d345bb02ca49fcef1391741b672b54a0bf7b679badec3142" +dependencies = [ + "itoa", + "ryu", + "serde", +] + +[[package]] +name = "strsim" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" + +[[package]] +name = "syn" +version = "1.0.85" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a684ac3dcd8913827e18cd09a68384ee66c1de24157e3c556c9ab16d85695fb7" +dependencies = [ + "proc-macro2", + "quote", + "unicode-xid", +] + +[[package]] +name = "thiserror" +version = "1.0.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "854babe52e4df1653706b98fcfc05843010039b406875930a70e4d9644e5c417" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa32fd3f627f367fe16f893e2597ae3c05020f8bba2666a4e6ea73d377e5714b" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "unicode-xid" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" + +[[package]] +name = "version_check" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" diff --git a/crates/runtimetest/Cargo.toml b/runtimetest/Cargo.toml similarity index 62% rename from crates/runtimetest/Cargo.toml rename to runtimetest/Cargo.toml index 3d8fc4d3..e70a999a 100644 --- a/crates/runtimetest/Cargo.toml +++ b/runtimetest/Cargo.toml @@ -3,8 +3,12 @@ name = "runtimetest" version = "0.0.1" edition = "2021" +# This package is not part of any workspace, so we specify it so +[workspace] +members = [] + # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -oci-spec = { git = "https://github.com/containers/oci-spec-rs", rev = "54c5e386f01ab37c9305cc4a83404eb157e42440" } +oci-spec = "0.5.3" nix = "0.23.1" \ No newline at end of file diff --git a/crates/runtimetest/README.md b/runtimetest/README.md similarity index 82% rename from crates/runtimetest/README.md rename to runtimetest/README.md index 05752ddb..45b66d54 100644 --- a/crates/runtimetest/README.md +++ b/runtimetest/README.md @@ -19,8 +19,6 @@ This package must be compiled as a statically linked binary, as otherwise the ru **Note** that the dynamically linked binary does not give a `segmentation fault` or similar error when tried to run inside the container, but instead gives `no such file or directory found` or `executable not found` error, even though the executable exists in the container. This made this tricky to debug correctly when originally developing, so if you decide on chaing the compilation or configuration of this , please make absolutely sure that the changes work and do not accidentally break something. -**Another Note** is that `cargo build` must be run from inside this directory only to make the binary be a statically linked binary. This is due to the fact that to make it statically linked, appropriate rustflags must be set in the .cargo/config.toml . But in case of a workspace, as like this, when run `cargo build` from project root, it ignores crate specific config, and only checks the project root .cargo/config , which currently does not allow setting rustflags for specific crate, which means either all binaries must be statically linked, or none can be. Thus currently the only way is to run the build command inside this directory. - you can use ```bash diff --git a/crates/runtimetest/src/main.rs b/runtimetest/src/main.rs similarity index 100% rename from crates/runtimetest/src/main.rs rename to runtimetest/src/main.rs diff --git a/runtimetest/src/tests.rs b/runtimetest/src/tests.rs new file mode 100644 index 00000000..ff740b9f --- /dev/null +++ b/runtimetest/src/tests.rs @@ -0,0 +1,56 @@ +use crate::utils::{test_read_access, test_write_access, AccessibilityStatus}; +use oci_spec::runtime::Spec; + +pub fn validate_readonly_paths(spec: &Spec) { + let linux = spec.linux().as_ref().unwrap(); + let ro_paths = match linux.readonly_paths() { + Some(p) => p, + None => { + eprintln!("in readonly paths, expected some readonly paths to be set, found none"); + return; + } + }; + if ro_paths.is_empty() { + eprintln!("in readonly paths, expected some readonly paths to be set, found none"); + return; + } + + for path in ro_paths { + match test_read_access(path) { + std::io::Result::Err(e) => { + eprintln!( + "in readonly paths, error in testing read access for path {} : {:?}", + path, e + ); + return; + } + Ok(readability) => { + match readability { + AccessibilityStatus::Accessible => { /* This is expected */ } + AccessibilityStatus::Blocked => { + eprintln!("in readonly paths, path {} expected to be readable, found non readable",path); + return; + } + } + } + } + match test_write_access(path) { + std::io::Result::Err(e) => { + eprintln!( + "in readonly paths, error in testing write access for path {} : {:?}", + path, e + ); + return; + } + Ok(readability) => { + match readability { + AccessibilityStatus::Accessible => { + eprintln!("in readonly paths, path {} expected to not be writable, found writable",path); + return; + } + AccessibilityStatus::Blocked => { /* This is expected */ } + } + } + } + } +} diff --git a/runtimetest/src/utils.rs b/runtimetest/src/utils.rs new file mode 100644 index 00000000..428446db --- /dev/null +++ b/runtimetest/src/utils.rs @@ -0,0 +1,134 @@ +use std::path::PathBuf; + +use nix::sys::stat::stat; +use nix::sys::stat::SFlag; + +pub enum AccessibilityStatus { + Accessible, + Blocked, +} + +fn test_file_read_access(path: &str) -> Result { + match std::fs::OpenOptions::new() + .create(false) + .read(true) + .open(path) + { + Ok(_) => { + // we can directly return accessible, as if we are allowed to open with read access, + // we can read the file + Ok(AccessibilityStatus::Accessible) + } + Err(e) => { + if e.kind() == std::io::ErrorKind::PermissionDenied { + // we can get permission denied error if we try to read a + // file which we do not have read read access for, in + // which case that is not an error, but a valid accessibility status + Ok(AccessibilityStatus::Blocked) + } else { + Err(e) + } + } + } +} + +fn test_dir_read_access(path: &str) -> Result { + match std::fs::read_dir(path) { + Ok(_) => Ok(AccessibilityStatus::Accessible), + Err(e) => { + if e.kind() == std::io::ErrorKind::PermissionDenied { + Ok(AccessibilityStatus::Blocked) + } else { + Err(e) + } + } + } +} + +fn is_file_like(mode: u32) -> bool { + // for this please refer + // https://stackoverflow.com/questions/40163270/what-is-s-isreg-and-what-does-it-do + // https://linux.die.net/man/2/stat + mode & SFlag::S_IFREG.bits() != 0 + || mode & SFlag::S_IFBLK.bits() != 0 + || mode & SFlag::S_IFCHR.bits() != 0 +} + +fn is_dir(mode: u32) -> bool { + mode & SFlag::S_IFDIR.bits() != 0 +} + +pub fn test_read_access(path: &str) -> Result { + let fstat = stat(path)?; + let mode = fstat.st_mode; + if is_file_like(mode) { + // we have a file or a char/block device + return test_file_read_access(path); + } else if is_dir(mode) { + return test_dir_read_access(path); + } + + Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!( + "cannot test read access for {:?}, has mode {:x}", + path, mode + ), + )) +} + +fn test_file_write_access(path: &str) -> Result { + match std::fs::OpenOptions::new().write(true).open(path) { + std::io::Result::Ok(_) => { + // we don't have to check if we can actually write or not, as + // if we are allowed to open file with write access, we can write to it + Ok(AccessibilityStatus::Accessible) + } + std::io::Result::Err(e) => { + if e.kind() == std::io::ErrorKind::PermissionDenied { + return Ok(AccessibilityStatus::Blocked); + } else { + return Err(e); + } + } + } +} + +fn test_dir_write_access(path: &str) -> Result { + match std::fs::OpenOptions::new() + .create(true) + .write(true) + .open(PathBuf::from(path).join("test.txt")) + { + std::io::Result::Ok(_) => Ok(AccessibilityStatus::Accessible), + std::io::Result::Err(e) => { + if e.kind() == std::io::ErrorKind::PermissionDenied { + // technically we can still get permission denied even if + // we have write access, but do not have execute access, but by default + // dirs are created with execute access so that should not be an issue + return Ok(AccessibilityStatus::Blocked); + } else { + return Err(e); + } + } + } +} + +pub fn test_write_access(path: &str) -> Result { + let fstat = stat(path)?; + let mode = fstat.st_mode; + if is_file_like(mode) { + // we have a file or a char/block device + return test_file_write_access(path); + } else if is_dir(mode) { + return test_dir_write_access(path); + } + + Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!( + "cannot test write access for {:?}, has mode {:x}", + path, mode + ), + )) +}