Merge pull request #290 from yihuaf/yihuaf/youki

bump libcontainer to tip of tree
containerd · Aug 30, 2023 · 6232e5c · 6232e5c
2 parents 2653451 + 7cb992f
commit 6232e5c
Show file tree

Hide file tree

Showing 7 changed files with 219 additions and 180 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -33,7 +33,8 @@ thiserror = "1.0"
 libc = "0.2.147"
 oci-spec = { version = "0.6.1", features = ["runtime"] }
 sha256 = "1.4.0"
-libcontainer = { version = "0.1", default-features = false }
+# TODO: once lincontainer releases 0.2, switch to released version. The current commit is the tip of the tree from `youki` and a release candidate.
+libcontainer = { git = "https://github.com/containers/youki", rev = "09e67372a892f22a89eeef62ff429c3cbcac6d41", default-features = false }
 windows-sys = { version = "0.48" }
 
 [profile.release]

diff --git a/crates/containerd-shim-wasm/src/libcontainer_instance/container_executor.rs b/crates/containerd-shim-wasm/src/libcontainer_instance/container_executor.rs
@@ -3,12 +3,12 @@ use std::io::Read;
 use std::path::PathBuf;
 
 use libcontainer::workload::default::DefaultExecutor;
-use libcontainer::workload::{Executor, ExecutorError};
+use libcontainer::workload::{Executor, ExecutorError, ExecutorValidationError};
 use oci_spec::runtime::Spec;
 
 use crate::sandbox::{oci, Stdio};
 
-#[derive(Default)]
+#[derive(Clone)]
 pub struct LinuxContainerExecutor {
     stdio: Stdio,
     default_executor: DefaultExecutor,
@@ -18,7 +18,7 @@ impl LinuxContainerExecutor {
     pub fn new(stdio: Stdio) -> Self {
         Self {
             stdio,
-            ..Default::default()
+            default_executor: DefaultExecutor {},
         }
     }
 }
@@ -32,82 +32,86 @@ impl Executor for LinuxContainerExecutor {
         self.default_executor.exec(spec)
     }
 
-    fn can_handle(&self, spec: &Spec) -> bool {
-        let args = oci::get_args(spec);
+    fn validate(&self, spec: &Spec) -> Result<(), ExecutorValidationError> {
+        self.default_executor.validate(spec)?;
 
-        if args.is_empty() {
-            return false;
-        }
+        can_handle(spec)
+            .then_some(())
+            .ok_or(ExecutorValidationError::InvalidArg)
+    }
+}
 
-        let executable = args[0].as_str();
+fn can_handle(spec: &Spec) -> bool {
+    let args = oci::get_args(spec);
 
-        // mostly follows youki's verify_binary implementation
-        // https://github.com/containers/youki/blob/2d6fd7650bb0f22a78fb5fa982b5628f61fe25af/crates/libcontainer/src/process/container_init_process.rs#L106
-        let path = if executable.contains('/') {
-            PathBuf::from(executable)
-        } else {
-            let path = std::env::var("PATH").unwrap_or_default();
-            // check each path in $PATH
-            let mut found = false;
-            let mut found_path = PathBuf::default();
-            for p in path.split(':') {
-                let path = PathBuf::from(p).join(executable);
-                if path.exists() {
-                    found = true;
-                    found_path = path;
-                    break;
-                }
-            }
-            if !found {
-                return false;
-            }
-            found_path
-        };
+    if args.is_empty() {
+        return false;
+    }
 
-        // check execute permission
-        use std::os::unix::fs::PermissionsExt;
-        let metadata = path.metadata();
-        if metadata.is_err() {
-            log::info!("failed to get metadata of {:?}", path);
-            return false;
+    let executable = args[0].as_str();
+
+    // mostly follows youki's verify_binary implementation
+    // https://github.com/containers/youki/blob/2d6fd7650bb0f22a78fb5fa982b5628f61fe25af/crates/libcontainer/src/process/container_init_process.rs#L106
+    let path = if executable.contains('/') {
+        PathBuf::from(executable)
+    } else {
+        let path = std::env::var("PATH").unwrap_or_default();
+        // check each path in $PATH
+        let mut found = false;
+        let mut found_path = PathBuf::default();
+        for p in path.split(':') {
+            let path = PathBuf::from(p).join(executable);
+            if path.exists() {
+                found = true;
+                found_path = path;
+                break;
+            }
         }
-        let metadata = metadata.unwrap();
-        let permissions = metadata.permissions();
-        if !metadata.is_file() || permissions.mode() & 0o001 == 0 {
-            log::info!("{} is not a file or has no execute permission", executable);
+        if !found {
             return false;
         }
+        found_path
+    };
 
-        // check the shebang and ELF magic number
-        // https://en.wikipedia.org/wiki/Executable_and_Linkable_Format#File_header
-        let mut buffer = [0; 4];
+    // check execute permission
+    use std::os::unix::fs::PermissionsExt;
+    let metadata = path.metadata();
+    if metadata.is_err() {
+        log::info!("failed to get metadata of {:?}", path);
+        return false;
+    }
+    let metadata = metadata.unwrap();
+    let permissions = metadata.permissions();
+    if !metadata.is_file() || permissions.mode() & 0o001 == 0 {
+        log::info!("{} is not a file or has no execute permission", executable);
+        return false;
+    }
+
+    // check the shebang and ELF magic number
+    // https://en.wikipedia.org/wiki/Executable_and_Linkable_Format#File_header
+    let mut buffer = [0; 4];
 
-        let file = OpenOptions::new().read(true).open(path);
-        if file.is_err() {
-            log::info!("failed to open {}", executable);
+    let file = OpenOptions::new().read(true).open(path);
+    if file.is_err() {
+        log::info!("failed to open {}", executable);
+        return false;
+    }
+    let mut file = file.unwrap();
+    match file.read_exact(&mut buffer) {
+        Ok(_) => {}
+        Err(err) => {
+            log::info!("failed to read shebang of {}: {}", executable, err);
             return false;
         }
-        let mut file = file.unwrap();
-        match file.read_exact(&mut buffer) {
-            Ok(_) => {}
-            Err(err) => {
-                log::info!("failed to read shebang of {}: {}", executable, err);
-                return false;
-            }
-        }
-        match buffer {
-            // ELF magic number
-            [0x7f, 0x45, 0x4c, 0x46] => true,
-            // shebang
-            [0x23, 0x21, ..] => true,
-            _ => {
-                log::info!("{} is not a valid script or elf file", executable);
-                false
-            }
-        }
     }
-
-    fn name(&self) -> &'static str {
-        self.default_executor.name()
+    match buffer {
+        // ELF magic number
+        [0x7f, 0x45, 0x4c, 0x46] => true,
+        // shebang
+        [0x23, 0x21, ..] => true,
+        _ => {
+            log::info!("{} is not a valid script or elf file", executable);
+            false
+        }
     }
 }