Enable run_hooks to be passed cwd

Signed-off-by: utam0k <k0ma@utam0k.jp>

crates/libcontainer/src/container/builder_impl.rs

@@ -1,3 +1,9 @@
+use std::{fs, io::Write, os::unix::prelude::RawFd, path::PathBuf, rc::Rc};
+
+use libcgroups::common::CgroupManager;
+use nix::unistd::Pid;
+use oci_spec::runtime::Spec;
+
 use super::{Container, ContainerStatus};
 use crate::{
     error::{LibcontainerError, MissingSpecError},
@@ -13,10 +19,6 @@ use crate::{
     utils,
     workload::Executor,
 };
-use libcgroups::common::CgroupManager;
-use nix::unistd::Pid;
-use oci_spec::runtime::Spec;
-use std::{fs, io::Write, os::unix::prelude::RawFd, path::PathBuf, rc::Rc};
 
 pub(super) struct ContainerBuilderImpl {
     /// Flag indicating if an init or a tenant container should be created
@@ -82,7 +84,11 @@ impl ContainerBuilderImpl {
 
         if matches!(self.container_type, ContainerType::InitContainer) {
             if let Some(hooks) = self.spec.hooks() {
-                hooks::run_hooks(hooks.create_runtime().as_ref(), self.container.as_ref())?
+                hooks::run_hooks(
+                    hooks.create_runtime().as_ref(),
+                    self.container.as_ref(),
+                    None,
+                )?
             }
         }
 

crates/libcontainer/src/container/container_delete.rs

@@ -94,10 +94,12 @@ impl Container {
                     })?;
 
                     if let Some(hooks) = config.hooks.as_ref() {
-                        hooks::run_hooks(hooks.poststop().as_ref(), Some(self)).map_err(|err| {
-                            tracing::error!(err = ?err, "failed to run post stop hooks");
-                            err
-                        })?;
+                        hooks::run_hooks(hooks.poststop().as_ref(), Some(self), None).map_err(
+                            |err| {
+                                tracing::error!(err = ?err, "failed to run post stop hooks");
+                                err
+                            },
+                        )?;
                     }
                 }
                 Err(err) => {

crates/libcontainer/src/container/container_start.rs

@@ -1,15 +1,13 @@
-use std::env;
+use nix::sys::signal;
 
+use super::{Container, ContainerStatus};
 use crate::{
     config::YoukiConfig,
     error::LibcontainerError,
     hooks,
     notify_socket::{NotifySocket, NOTIFY_FILE},
 };
 
-use super::{Container, ContainerStatus};
-use nix::{sys::signal, unistd};
-
 impl Container {
     /// Starts a previously created container
     ///
@@ -51,7 +49,7 @@ impl Container {
             // While prestart is marked as deprecated in the OCI spec, the docker and integration test still
             // uses it.
             #[allow(deprecated)]
-            hooks::run_hooks(hooks.prestart().as_ref(), Some(self)).map_err(|err| {
+            hooks::run_hooks(hooks.prestart().as_ref(), Some(self), None).map_err(|err| {
                 tracing::error!("failed to run pre start hooks: {}", err);
                 // In the case where prestart hook fails, the runtime must
                 // stop the container before generating an error and exiting.
@@ -73,25 +71,12 @@ impl Container {
         // Run post start hooks. It runs after the container process is started.
         // It is called in the runtime namespace.
         if let Some(hooks) = config.hooks.as_ref() {
-            let original_dir = env::current_dir().map_err(|err| {
-                tracing::error!("failed to get current directory: {}", err);
-                LibcontainerError::Other(format!("failed to get current directory: {}", err))
-            })?;
-
-            unistd::chdir(self.root.as_os_str()).map_err(|err| {
-                tracing::error!("failed to change directory to container root: {}", err);
-                LibcontainerError::OtherSyscall(err)
-            })?;
-
-            hooks::run_hooks(hooks.poststart().as_ref(), Some(self)).map_err(|err| {
-                tracing::error!("failed to run post start hooks: {}", err);
-                err
-            })?;
-
-            unistd::chdir(original_dir.as_path()).map_err(|err| {
-                tracing::error!("failed to change directory to container root: {}", err);
-                LibcontainerError::OtherSyscall(err)
-            })?;
+            hooks::run_hooks(hooks.poststart().as_ref(), Some(self), Some(&self.root)).map_err(
+                |err| {
+                    tracing::error!("failed to run post start hooks: {}", err);
+                    err
+                },
+            )?;
         }
 
         Ok(())

crates/libcontainer/src/hooks.rs

@@ -1,14 +1,15 @@
-use nix::{sys::signal, unistd::Pid};
-use oci_spec::runtime::Hook;
 use std::{
     collections::HashMap,
-    io::ErrorKind,
-    io::Write,
+    env,
+    io::{ErrorKind, Write},
     os::unix::prelude::CommandExt,
-    process::{self},
-    thread, time,
+    path::Path,
+    process, thread, time,
 };
 
+use nix::{sys::signal, unistd::Pid};
+use oci_spec::runtime::Hook;
+
 use crate::{container::Container, utils};
 
 #[derive(Debug, thiserror::Error)]
@@ -27,16 +28,35 @@ pub enum HookError {
     MissingContainerState,
     #[error("failed to write container state to stdin")]
     WriteContainerState(#[source] std::io::Error),
+    #[error("io error")]
+    OtherIO(#[source] std::io::Error),
 }
 
 type Result<T> = std::result::Result<T, HookError>;
 
-pub fn run_hooks(hooks: Option<&Vec<Hook>>, container: Option<&Container>) -> Result<()> {
+pub fn run_hooks(
+    hooks: Option<&Vec<Hook>>,
+    container: Option<&Container>,
+    cwd: Option<&Path>,
+) -> Result<()> {
     let state = &(container.ok_or(HookError::MissingContainerState)?.state);
 
     if let Some(hooks) = hooks {
         for hook in hooks {
-            let mut hook_command = process::Command::new(hook.path());
+            let mut cmd = process::Command::new(hook.path());
+
+            let hook_command = if let Some(cwd) = cwd {
+                cmd.current_dir(cwd)
+            } else {
+                cmd.current_dir(
+                    env::current_dir()
+                        .map_err(|err| {
+                            tracing::error!("failed to get current directory: {}", err);
+                            HookError::OtherIO(err)
+                        })?
+                        .as_path(),
+                )
+            };
             // Based on OCI spec, the first argument of the args vector is the
             // arg0, which can be different from the path.  For example, path
             // may be "/usr/bin/true" and arg0 is set to "true". However, rust
@@ -165,7 +185,7 @@ mod test {
     fn test_run_hook() -> Result<()> {
         {
             let default_container: Container = Default::default();
-            run_hooks(None, Some(&default_container)).context("Failed simple test")?;
+            run_hooks(None, Some(&default_container), None).context("Failed simple test")?;
         }
 
         {
@@ -174,7 +194,7 @@ mod test {
 
             let hook = HookBuilder::default().path("true").build()?;
             let hooks = Some(vec![hook]);
-            run_hooks(hooks.as_ref(), Some(&default_container)).context("Failed true")?;
+            run_hooks(hooks.as_ref(), Some(&default_container), None).context("Failed true")?;
         }
 
         {
@@ -194,7 +214,27 @@ mod test {
                 .env(vec![String::from("key=value")])
                 .build()?;
             let hooks = Some(vec![hook]);
-            run_hooks(hooks.as_ref(), Some(&default_container)).context("Failed printenv test")?;
+            run_hooks(hooks.as_ref(), Some(&default_container), None)
+                .context("Failed printenv test")?;
+        }
+
+        {
+            assert!(is_command_in_path("pwd"), "The pwd was not found.");
+
+            let tmp = tempfile::tempdir()?;
+
+            let default_container: Container = Default::default();
+            let hook = HookBuilder::default()
+                .path("bash")
+                .args(vec![
+                    String::from("bash"),
+                    String::from("-c"),
+                    format!("test $(pwd) = {:?}", tmp.path()),
+                ])
+                .build()?;
+            let hooks = Some(vec![hook]);
+            run_hooks(hooks.as_ref(), Some(&default_container), Some(tmp.path()))
+                .context("Failed pwd test")?;
         }
 
         Ok(())
@@ -217,7 +257,7 @@ mod test {
             .timeout(1)
             .build()?;
         let hooks = Some(vec![hook]);
-        match run_hooks(hooks.as_ref(), Some(&default_container)) {
+        match run_hooks(hooks.as_ref(), Some(&default_container), None) {
             Ok(_) => {
                 bail!("The test expects the hook to error out with timeout. Should not execute cleanly");
             }

crates/libcontainer/src/process/container_init_process.rs

@@ -1,32 +1,40 @@
+use std::{
+    collections::HashMap,
+    env, fs, mem,
+    os::unix::io::AsRawFd,
+    path::{Path, PathBuf},
+};
+
 use super::args::{ContainerArgs, ContainerType};
-use crate::error::MissingSpecError;
-use crate::namespaces::NamespaceError;
-use crate::syscall::{Syscall, SyscallError};
-use crate::{apparmor, notify_socket, rootfs, workload};
+#[cfg(feature = "libseccomp")]
+use crate::seccomp;
 use crate::{
-    capabilities, hooks, namespaces::Namespaces, process::channel, rootfs::RootFS, tty,
-    user_ns::UserNamespaceConfig, utils,
+    apparmor, capabilities,
+    error::MissingSpecError,
+    hooks,
+    namespaces::{NamespaceError, Namespaces},
+    notify_socket,
+    process::channel,
+    rootfs,
+    rootfs::RootFS,
+    syscall::{Syscall, SyscallError},
+    tty,
+    user_ns::UserNamespaceConfig,
+    utils, workload,
 };
+
 use nc;
-use nix::mount::MsFlags;
-use nix::sched::CloneFlags;
-use nix::sys::stat::Mode;
-use nix::unistd::setsid;
-use nix::unistd::{self, Gid, Uid};
+use nix::{
+    mount::MsFlags,
+    sched::CloneFlags,
+    sys::stat::Mode,
+    unistd::setsid,
+    unistd::{self, Gid, Uid},
+};
 use oci_spec::runtime::{
     IOPriorityClass, LinuxIOPriority, LinuxNamespaceType, LinuxSchedulerFlag, LinuxSchedulerPolicy,
     Scheduler, Spec, User,
 };
-use std::collections::HashMap;
-use std::mem;
-use std::os::unix::io::AsRawFd;
-use std::{
-    env, fs,
-    path::{Path, PathBuf},
-};
-
-#[cfg(feature = "libseccomp")]
-use crate::seccomp;
 
 #[derive(Debug, thiserror::Error)]
 pub enum InitProcessError {
@@ -317,10 +325,12 @@ pub fn container_init_process(
         // create_container hook needs to be called after the namespace setup, but
         // before pivot_root is called. This runs in the container namespaces.
         if let Some(hooks) = hooks {
-            hooks::run_hooks(hooks.create_container().as_ref(), container).map_err(|err| {
-                tracing::error!(?err, "failed to run create container hooks");
-                InitProcessError::Hooks(err)
-            })?;
+            hooks::run_hooks(hooks.create_container().as_ref(), container, None).map_err(
+                |err| {
+                    tracing::error!(?err, "failed to run create container hooks");
+                    InitProcessError::Hooks(err)
+                },
+            )?;
         }
 
         let in_user_ns = utils::is_in_new_userns().map_err(InitProcessError::Io)?;
@@ -628,7 +638,7 @@ pub fn container_init_process(
     // before pivot_root is called. This runs in the container namespaces.
     if matches!(args.container_type, ContainerType::InitContainer) {
         if let Some(hooks) = hooks {
-            hooks::run_hooks(hooks.start_container().as_ref(), container).map_err(|err| {
+            hooks::run_hooks(hooks.start_container().as_ref(), container, None).map_err(|err| {
                 tracing::error!(?err, "failed to run start container hooks");
                 err
             })?;