diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index 6ed81e7215..03a3ea2d72 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -101,6 +101,16 @@ version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" +[[package]] +name = "async-attributes" +version = "1.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a3203e79f4dd9bdda415ed03cf14dae5a2bf775c683a00f94e9cd1faf0f596e5" +dependencies = [ + "quote", + "syn 1.0.109", +] + [[package]] name = "async-broadcast" version = "0.5.1" @@ -164,6 +174,21 @@ dependencies = [ "futures-lite", ] +[[package]] +name = "async-global-executor" +version = "2.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1b6f5d7df27bd294849f8eec66ecfc63d11814df7a4f5d74168a2394467b776" +dependencies = [ + "async-channel", + "async-executor", + "async-io", + "async-lock", + "blocking", + "futures-lite", + "once_cell", +] + [[package]] name = "async-io" version = "1.13.0" @@ -233,6 +258,33 @@ dependencies = [ "syn 2.0.52", ] +[[package]] +name = "async-std" +version = "1.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "62565bb4402e926b29953c785397c6dc0391b7b446e45008b0049eb43cec6f5d" +dependencies = [ + "async-attributes", + "async-channel", + "async-global-executor", + "async-io", + "async-lock", + "crossbeam-utils", + "futures-channel", + "futures-core", + "futures-io", + "futures-lite", + "gloo-timers", + "kv-log-macro", + "log", + "memchr", + "once_cell", + "pin-project-lite", + "pin-utils", + "slab", + "wasm-bindgen-futures", +] + [[package]] name = "async-task" version = "4.3.0" @@ -902,6 +954,16 @@ dependencies = [ "zeroize", ] +[[package]] +name = "ctor" +version = "0.1.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d2301688392eb071b0bf1a37be05c469d3cc4dbbd95df672fe28ab021e6a096" +dependencies = [ + "quote", + "syn 1.0.109", +] + [[package]] name = "curve25519-dalek" version = "4.1.2" @@ -1452,6 +1514,12 @@ version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "38d84fa142264698cdce1a9f9172cf383a0c82de1bddcf3092901442c4097004" +[[package]] +name = "futures-timer" +version = "3.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e64b03909df88034c26dc1547e8970b91f98bdb65165d6a4e9110d94263dbb2c" + [[package]] name = "futures-util" version = "0.3.30" @@ -1506,9 +1574,34 @@ dependencies = [ [[package]] name = "glob" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574" +checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" + +[[package]] +name = "globset" +version = "0.4.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "759c97c1e17c55525b57192c06a267cda0ac5210b222d6b82189a2338fa1c13d" +dependencies = [ + "aho-corasick", + "bstr", + "fnv", + "log", + "regex", +] + +[[package]] +name = "gloo-timers" +version = "0.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b995a66bb87bebce9a0f4a95aed01daca4872c050bfcb21653361c03bc35e5c" +dependencies = [ + "futures-channel", + "futures-core", + "js-sys", + "wasm-bindgen", +] [[package]] name = "group" @@ -1995,6 +2088,7 @@ version = "0.1.0" dependencies = [ "anyhow", "async-recursion 0.3.2", + "async-std", "async-trait", "capctl", "cfg-if 1.0.0", @@ -2023,6 +2117,7 @@ dependencies = [ "protocols", "regex", "reqwest", + "rstest", "rtnetlink", "rustjail", "safe-path", @@ -2101,6 +2196,15 @@ dependencies = [ "toml", ] +[[package]] +name = "kv-log-macro" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0de8b303297635ad57c9f5059fd9cee7a47f8e8daa09df0fcd07dd39fb22977f" +dependencies = [ + "log", +] + [[package]] name = "lazy_static" version = "1.4.0" @@ -2276,6 +2380,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "abb12e687cfb44aa40f41fc3978ef76448f9b6038cad6aef4259d3c095a2382e" dependencies = [ "cfg-if 1.0.0", + "value-bag", ] [[package]] @@ -3553,6 +3658,12 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dbb5fb1acd8a1a18b3dd5be62d25485eb770e05afb408a9627d14d451bae12da" +[[package]] +name = "relative-path" +version = "1.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e898588f33fdd5b9420719948f9f2a32c922a246964576f71ba7f24f80610fbc" + [[package]] name = "remove_dir_all" version = "0.5.3" @@ -3697,6 +3808,35 @@ dependencies = [ "zeroize", ] +[[package]] +name = "rstest" +version = "0.18.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97eeab2f3c0a199bc4be135c36c924b6590b88c377d416494288c14f2db30199" +dependencies = [ + "futures", + "futures-timer", + "rstest_macros", + "rustc_version", +] + +[[package]] +name = "rstest_macros" +version = "0.18.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d428f8247852f894ee1be110b375111b586d4fa431f6c46e64ba5a0dcccbe605" +dependencies = [ + "cfg-if 1.0.0", + "glob", + "proc-macro2", + "quote", + "regex", + "relative-path", + "rustc_version", + "syn 2.0.50", + "unicode-ident", +] + [[package]] name = "rtnetlink" version = "0.8.1" @@ -5064,9 +5204,9 @@ checksum = "92888ba5573ff080736b3648696b70cafad7d250551175acbaa4e0385b3e1460" [[package]] name = "unicode-ident" -version = "1.0.1" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5bd2fe26506023ed7b5e1e315add59d6f584c621d037f9368fea9cfb988f368c" +checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" [[package]] name = "unicode-normalization" @@ -5144,6 +5284,16 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "830b7e5d4d90034032940e4ace0d9a9a057e7a45cd94e6c007832e39edb82f6d" +[[package]] +name = "value-bag" +version = "1.0.0-alpha.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2209b78d1249f7e6f3293657c9779fe31ced465df091bbd433a1cf88e916ec55" +dependencies = [ + "ctor", + "version_check", +] + [[package]] name = "vcpkg" version = "0.2.15" diff --git a/src/agent/Cargo.toml b/src/agent/Cargo.toml index 1919730d52..3ee6ea8142 100644 --- a/src/agent/Cargo.toml +++ b/src/agent/Cargo.toml @@ -83,6 +83,8 @@ image-rs = { git = "https://github.com/confidential-containers/guest-components" tempfile = "3.1.0" test-utils = { path = "../libs/test-utils" } which = "4.3.0" +rstest = "0.18.0" +async-std = { version = "1.12.0", features = ["attributes"] } [workspace] members = [ diff --git a/src/agent/src/image.rs b/src/agent/src/image.rs index 9dd78df8e9..3ad89fa6a4 100644 --- a/src/agent/src/image.rs +++ b/src/agent/src/image.rs @@ -12,14 +12,17 @@ use std::fs; use std::sync::Arc; use std::path::PathBuf; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; use image_rs::image::ImageClient; use tokio::sync::Mutex; use crate::rpc::CONTAINER_BASE; use crate::AGENT_CONFIG; +// A marker to merge container spec for images pulled inside guest. +const ANNO_K8S_IMAGE_NAME: &str = "io.kubernetes.cri.image-name"; const KATA_IMAGE_WORK_DIR: &str = "/run/kata-containers/image/"; +const CONFIG_JSON: &str = "config.json"; #[rustfmt::skip] lazy_static! { @@ -91,7 +94,7 @@ impl ImageService { ) -> Result { info!(sl(), "image metadata: {image_metadata:?}"); Self::set_proxy_env_vars(); - + let bundle_base_dir = scoped_join(CONTAINER_BASE, cid)?; fs::create_dir_all(&bundle_base_dir)?; let bundle_path = scoped_join(&bundle_base_dir, "images")?; @@ -124,4 +127,141 @@ impl ImageService { let image_bundle_path = scoped_join(&bundle_path, "rootfs")?; Ok(image_bundle_path.as_path().display().to_string()) } + + /// When being passed an image name through a container annotation, merge its + /// corresponding bundle OCI specification into the passed container creation one. + pub async fn merge_bundle_oci(&self, container_oci: &mut oci::Spec) -> Result<()> { + if let Some(image_name) = container_oci.annotations.get(ANNO_K8S_IMAGE_NAME) { + let images = self.images.lock().await; + if let Some(container_id) = images.get(image_name) { + let image_oci_config_path = Path::new(CONTAINER_BASE) + .join(container_id) + .join(CONFIG_JSON); + debug!( + sl(), + "Image bundle config path: {:?}", image_oci_config_path + ); + + let image_oci = + oci::Spec::load(image_oci_config_path.to_str().ok_or_else(|| { + anyhow!( + "Invalid container image OCI config path {:?}", + image_oci_config_path + ) + })?) + .context("load image bundle")?; + + if let (Some(container_root), Some(image_root)) = + (container_oci.root.as_mut(), image_oci.root.as_ref()) + { + let root_path = Path::new(CONTAINER_BASE) + .join(container_id) + .join(image_root.path.clone()); + container_root.path = String::from(root_path.to_str().ok_or_else(|| { + anyhow!("Invalid container image root path {:?}", root_path) + })?); + } + + if let (Some(container_process), Some(image_process)) = + (container_oci.process.as_mut(), image_oci.process.as_ref()) + { + self.merge_oci_process(container_process, image_process); + } + } + } + + Ok(()) + } + + /// Partially merge an OCI process specification into another one. + fn merge_oci_process(&self, target: &mut oci::Process, source: &oci::Process) { + // Override the target args only when the target args is empty and source.args is not empty + if target.args.is_empty() && !source.args.is_empty() { + target.args.append(&mut source.args.clone()); + } + + // Override the target cwd only when the target cwd is blank and source.cwd is not blank + if target.cwd == "/" && source.cwd != "/" { + target.cwd = String::from(&source.cwd); + } + + for source_env in &source.env { + if let Some((variable_name, variable_value)) = source_env.split_once('=') { + debug!( + sl(), + "source spec environment variable: {variable_name:?} : {variable_value:?}" + ); + if !target.env.iter().any(|i| i.contains(variable_name)) { + target.env.push(source_env.to_string()); + } + } + } + } +} +#[cfg(test)] +mod tests { + use super::ImageService; + use rstest::rstest; + + #[rstest] + // TODO - how can we tell the user didn't specifically set it to `/` vs not setting at all? Is that scenario valid? + #[case::image_cwd_should_override_blank_container_cwd("/", "/imageDir", "/imageDir")] + #[case::container_cwd_should_override_image_cwd("/containerDir", "/imageDir", "/containerDir")] + #[case::container_cwd_should_override_blank_image_cwd("/containerDir", "/", "/containerDir")] + async fn test_merge_cwd( + #[case] container_process_cwd: &str, + #[case] image_process_cwd: &str, + #[case] expected: &str, + ) { + let image_service = ImageService::new(); + let mut container_process = oci::Process { + cwd: container_process_cwd.to_string(), + ..Default::default() + }; + let image_process = oci::Process { + cwd: image_process_cwd.to_string(), + ..Default::default() + }; + image_service.merge_oci_process(&mut container_process, &image_process); + assert_eq!(expected, container_process.cwd); + } + + #[rstest] + #[case::pods_environment_overrides_images( + vec!["ISPRODUCTION=true".to_string()], + vec!["ISPRODUCTION=false".to_string()], + vec!["ISPRODUCTION=true".to_string()] + )] + #[case::multiple_environment_variables_can_be_overrided( + vec!["ISPRODUCTION=true".to_string(), "ISDEVELOPMENT=false".to_string()], + vec!["ISPRODUCTION=false".to_string(), "ISDEVELOPMENT=true".to_string()], + vec!["ISPRODUCTION=true".to_string(), "ISDEVELOPMENT=false".to_string()] + )] + #[case::not_override_them_when_none_of_variables_match( + vec!["ANOTHERENV=TEST".to_string()], + vec!["ISPRODUCTION=false".to_string(), "ISDEVELOPMENT=true".to_string()], + vec!["ANOTHERENV=TEST".to_string(), "ISPRODUCTION=false".to_string(), "ISDEVELOPMENT=true".to_string()] + )] + #[case::a_mix_of_both_overriding_and_not( + vec!["ANOTHERENV=TEST".to_string(), "ISPRODUCTION=true".to_string()], + vec!["ISPRODUCTION=false".to_string(), "ISDEVELOPMENT=true".to_string()], + vec!["ANOTHERENV=TEST".to_string(), "ISPRODUCTION=true".to_string(), "ISDEVELOPMENT=true".to_string()] + )] + async fn test_merge_env( + #[case] container_process_env: Vec, + #[case] image_process_env: Vec, + #[case] expected: Vec, + ) { + let image_service = ImageService::new(); + let mut container_process = oci::Process { + env: container_process_env, + ..Default::default() + }; + let image_process = oci::Process { + env: image_process_env, + ..Default::default() + }; + image_service.merge_oci_process(&mut container_process, &image_process); + assert_eq!(expected, container_process.env); + } } diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index 1d5558fecd..d919df76d5 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -200,6 +200,11 @@ impl AgentService { "receive createcontainer, storages: {:?}", &req.storages ); + // In case of pulling image inside guest, we need to merge the image bundle OCI spec + // into the container creation request OCI spec. + let image_service = image::ImageService::singleton().await?; + image_service.merge_bundle_oci(&mut oci).await?; + // Some devices need some extra processing (the ones invoked with // --device for instance), and that's what this call is doing. It // updates the devices listed in the OCI spec, so that they actually