agent/image: merge container spec for images pulled inside guest

When being passed an image name through a container annotation, merge its corresponding bundle OCI specification and process into the passed container creation one. Signed-off-by: ChengyuZhu6 <chengyu.zhu@intel.com> Co-authored-by: Arron Wang <arron.wang@intel.com> Co-authored-by: Jiang Liu <gerry@linux.alibaba.com> Co-authored-by: stevenhorsman <steven@uk.ibm.com> Co-authored-by: wllenyj <wllenyj@linux.alibaba.com> Co-authored-by: jordan9500 <jordan.jackson@ibm.com>
2025-08-31 16:36:38 +00:00 · 2023-11-22 18:16:06 +08:00
parent cec1916196
commit 462051b067
4 changed files with 303 additions and 6 deletions
--- 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"
--- 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 = [
--- 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<String> {
        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<String>,
+        #[case] image_process_env: Vec<String>,
+        #[case] expected: Vec<String>,
+    ) {
+        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);
+    }
 }
--- 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