Replace separate mounting functions with a Mount type

crates/carton_bin/src/bin/carton.rs

@@ -30,6 +30,7 @@ fn main() -> Result<()> {
     let mut container = ContainerBuilder::new()
         .rootfs(cli_args.rootfs_path)
         .command(cli_args.command, cli_args.arguments)
+        .add_default_mounts()
         .add_default_devices()
         .build()
         .context("building container")?;

crates/libcarton/src/container.rs

@@ -6,6 +6,7 @@ use std::path::{Path, PathBuf};
 
 use log::{error, info, warn};
 
+use nix::mount;
 use nix::sched::{self, CloneFlags};
 use nix::sys::signal::Signal::SIGCHLD;
 use nix::sys::wait;
@@ -86,13 +87,13 @@ impl Container {
 #[derive(Default, Debug)]
 pub(crate) struct ContainerConfiguration {
     /// The path to the root filesystem of the container.
-    pub(crate) rootfs: Option<MountSpecification>,
+    pub(crate) rootfs: Option<Mount>,
     /// Command to execute inside the container.
     pub(crate) command: Option<PathBuf>,
     /// Arguments to the command
     pub(crate) arguments: Vec<String>,
-    /// Additional paths on the "host" to bind mount inside the container.
-    pub(crate) mounts: Vec<MountSpecification>,
+    /// Vita paths (like /proc, /tmp, /dev) and paths from the "host" to bind mount inside the container.
+    pub(crate) mounts: Vec<Mount>,
     /// Device nodes to create in /dev.
     pub(crate) devices: Vec<DeviceNode>,
 }
@@ -102,7 +103,12 @@ impl ContainerConfiguration {
         match &self.rootfs {
             None => return Err(CartonError::MissingRequiredConfiguration("rootfs".into())),
             Some(root_spec) => {
-                if !root_spec.source.is_dir() {
+                if !root_spec
+                    .source
+                    .as_ref()
+                    .expect("rootfs source path should not be None")
+                    .is_dir()
+                {
                     return Err(CartonError::InvalidConfiguration(format!(
                         "rootfs does not exist or is not a directory: {:?}",
                         root_spec.source
@@ -134,10 +140,89 @@ pub enum ContainerState {
     Exited,
 }
 
-#[derive(Default, Debug)]
-pub(crate) struct MountSpecification {
-    pub source: PathBuf,
-    pub destination: PathBuf,
+#[derive(Debug)]
+pub struct Mount {
+    pub(crate) source: Option<PathBuf>,
+    relative_target: PathBuf,
+    fstype: Option<String>,
+    flags: mount::MsFlags,
+    data: Option<String>,
+}
+
+impl Mount {
+    /// Defines a "bind" mount which can be used to share a directory from outside the container
+    /// with the container.
+    pub(crate) fn bind(
+        source: PathBuf,
+        relative_target: PathBuf,
+        flags: Option<mount::MsFlags>,
+        data: Option<String>,
+    ) -> Self {
+        Mount {
+            source: Some(source),
+            relative_target,
+            fstype: None,
+            flags: flags.unwrap_or(mount::MsFlags::MS_BIND | mount::MsFlags::MS_PRIVATE),
+            data,
+        }
+    }
+
+    /// When the container runs in a separate PID namespace it also needs a separate /proc mount that
+    /// will contain only this PID namespace's processes.
+    pub(crate) fn procfs(relative_target: PathBuf) -> Self {
+        Mount {
+            source: None::<PathBuf>,
+            relative_target,
+            fstype: Some("proc".into()),
+            flags: mount::MsFlags::empty(),
+            data: None,
+        }
+    }
+
+    pub(crate) fn rootfs(source: PathBuf) -> Self {
+        Mount {
+            source: Some(source),
+            relative_target: "".into(),
+            fstype: None,
+            flags: mount::MsFlags::MS_BIND | mount::MsFlags::MS_PRIVATE,
+            data: None,
+        }
+    }
+
+    pub(crate) fn tmpfs(relative_target: PathBuf) -> Self {
+        Mount {
+            source: None::<PathBuf>,
+            relative_target,
+            fstype: Some("tmpfs".into()),
+            flags: mount::MsFlags::empty(),
+            data: None,
+        }
+    }
+
+    /// Returns the absolute path where the mount has been mounted
+    pub(crate) fn mount(&self, rootfs_path: &Path) -> Result<PathBuf, CartonError> {
+        let mount_path = rootfs_path.join(&self.relative_target);
+
+        info!(
+            "mount {} ({}) at {}",
+            &self
+                .source
+                .as_ref()
+                .map_or("(no source)", |p| p.to_str().unwrap()),
+            self.fstype.as_ref().map_or("bind mount", |f| f.as_str()),
+            &mount_path.display()
+        );
+
+        mount::mount(
+            self.source.as_ref(),
+            &mount_path,
+            self.fstype.as_deref(),
+            self.flags,
+            self.data.as_deref(),
+        )?;
+
+        Ok(mount_path)
+    }
 }
 
 #[derive(Debug)]
@@ -161,7 +246,7 @@ fn execute_command(command: &Path, arguments: &[String]) -> isize {
     c_args.insert(0, c_cmd.clone());
 
     // This syscall replaces the current process with the requested command. That means that this
-    // `run_command()` function will only return if went wrong with starting the command.
+    // `run_command()` function will only return if something went wrong with starting the command.
     // TODO execve()
     unistd::execv(&c_cmd, &c_args).and(Ok(0)).unwrap()
 }

crates/libcarton/src/container_builder.rs

@@ -6,9 +6,7 @@ use std::path::{Path, PathBuf};
 use nix::sys::resource;
 
 use crate::consts::DEFAULT_CONTAINER_STACK_SIZE;
-use crate::container::{
-    Container, ContainerBuffer, ContainerConfiguration, DeviceNode, MountSpecification,
-};
+use crate::container::{Container, ContainerBuffer, ContainerConfiguration, DeviceNode, Mount};
 use crate::error::CartonError;
 
 #[derive(Default, Debug)]
@@ -23,10 +21,7 @@ impl ContainerBuilder {
     }
 
     pub fn rootfs(mut self, path: PathBuf) -> Self {
-        self.config.rootfs = Some(MountSpecification {
-            source: path,
-            destination: "/".into(),
-        });
+        self.config.rootfs = Some(Mount::rootfs(path));
 
         self
     }
@@ -42,11 +37,21 @@ impl ContainerBuilder {
         self
     }
 
-    pub fn add_mount(mut self, source: PathBuf, destination: PathBuf) -> Self {
-        self.config.mounts.push(MountSpecification {
-            source,
-            destination,
-        });
+    /// Adds mounting configuration for some important mounts, in the correct order.
+    pub fn add_default_mounts(mut self) -> Self {
+        self.config.mounts.extend(vec![
+            Mount::procfs("proc".into()),
+            Mount::tmpfs("tmp".into()),
+            Mount::tmpfs("dev".into()),
+        ]);
+
+        self
+    }
+
+    pub fn add_mount(mut self, source: PathBuf, relative_target: PathBuf) -> Self {
+        self.config
+            .mounts
+            .push(Mount::bind(source, relative_target, None, None));
         self
     }
 

crates/libcarton/src/namespace.rs

@@ -1,13 +1,13 @@
 // Copyright 2023 Arjen Verstoep
 // SPDX-License-Identifier: Apache-2.0
 
-use log::info;
+use std::path::Path;
 
 use nix::mount;
 use nix::sys::stat;
 use nix::unistd;
 
-use crate::container::{ContainerConfiguration, DeviceNode, MountSpecification};
+use crate::container::{ContainerConfiguration, DeviceNode, Mount};
 use crate::error::CartonError;
 
 /// Does the entire dance of setting up all the elements of the new processes' namespace, like
@@ -25,15 +25,18 @@ fn setup_mount_namespace(config: &ContainerConfiguration) -> Result<(), CartonEr
         .rootfs
         .as_ref()
         .expect("rootfs should not be None at this point");
+    let rootfs_source = rootfs
+        .source
+        .as_ref()
+        .expect("rootfs source path should not be None");
 
     prepare_rootfs(rootfs)?;
 
-    mount_procfs(rootfs)?;
-    mount_tmp(rootfs)?;
-    mount_additional_binds(rootfs, &config.mounts)?;
+    for mount in config.mounts.iter() {
+        mount.mount(rootfs_source)?;
+    }
 
-    mount_dev(rootfs)?;
-    create_device_nodes(rootfs, &config.devices)?;
+    create_device_nodes(&rootfs_source.join("dev"), &config.devices)?;
 
     mount_rootfs(rootfs)?;
 
@@ -45,7 +48,7 @@ fn setup_mount_namespace(config: &ContainerConfiguration) -> Result<(), CartonEr
 ///
 /// If we don't do this first, further mounts will either not pass into the mount namespace after
 /// pivot_root() or affect the "host" system, messing up things.
-fn prepare_rootfs(root_spec: &MountSpecification) -> Result<(), CartonError> {
+fn prepare_rootfs(rootfs: &Mount) -> Result<(), CartonError> {
     // Remount root within our mount namespace and mark it as private, so that any changes to it
     // (like a umount) will not (try) to affect the real root partition.
     mount::mount(
@@ -58,8 +61,8 @@ fn prepare_rootfs(root_spec: &MountSpecification) -> Result<(), CartonError> {
 
     // Prepare the new root filesystem for mounting
     mount::mount(
-        Some(&root_spec.source),
-        &root_spec.source,
+        rootfs.source.as_ref(),
+        rootfs.source.as_ref().unwrap(),
         None::<&str>,
         mount::MsFlags::MS_BIND | mount::MsFlags::MS_PRIVATE,
         None::<&str>,
@@ -68,78 +71,7 @@ fn prepare_rootfs(root_spec: &MountSpecification) -> Result<(), CartonError> {
     Ok(())
 }
 
-/// When the container runs in a separate PID namespace it also needs a separate /proc mount that
-/// will contain only this PID namespace's processes.
-fn mount_procfs(root_mount: &MountSpecification) -> Result<(), CartonError> {
-    let proc_mount = &root_mount.source.join("proc");
-    info!("mounting proc at {:?}", proc_mount);
-
-    mount::mount(
-        None::<&str>,
-        proc_mount,
-        Some("proc"),
-        mount::MsFlags::empty(),
-        None::<&str>,
-    )?;
-
-    Ok(())
-}
-
-/// Mounts /tmp under the new rootfs
-fn mount_tmp(root_mount: &MountSpecification) -> Result<(), CartonError> {
-    let tmp_mount = &root_mount.source.join("tmp");
-    info!("mounting tmp at {:?}", tmp_mount);
-
-    mount::mount(
-        None::<&str>,
-        tmp_mount,
-        Some("tmpfs"),
-        mount::MsFlags::empty(),
-        None::<&str>,
-    )?;
-
-    Ok(())
-}
-
-fn mount_additional_binds(
-    root_mount: &MountSpecification,
-    mounts: &[MountSpecification],
-) -> Result<(), CartonError> {
-    for mount_spec in mounts {
-        mount::mount(
-            Some(&mount_spec.source),
-            &root_mount.source.join(&mount_spec.destination),
-            None::<&str>,
-            mount::MsFlags::MS_BIND | mount::MsFlags::MS_PRIVATE,
-            None::<&str>,
-        )?;
-    }
-
-    Ok(())
-}
-
-/// Mounts a clean /dev under the new rootfs
-fn mount_dev(root_spec: &MountSpecification) -> Result<(), CartonError> {
-    let dev_path = root_spec.source.join("dev");
-    info!("mount /dev at {:?}", &dev_path);
-
-    mount::mount(
-        None::<&str>,
-        &dev_path,
-        Some("tmpfs"),
-        mount::MsFlags::empty(),
-        None::<&str>,
-    )?;
-
-    Ok(())
-}
-
-fn create_device_nodes(
-    root_spec: &MountSpecification,
-    devices: &[DeviceNode],
-) -> Result<(), CartonError> {
-    let dev_path = root_spec.source.join("dev");
-
+fn create_device_nodes(dev_path: &Path, devices: &[DeviceNode]) -> Result<(), CartonError> {
     let device_perm = stat::Mode::from_bits(0o0666).unwrap();
     for node in devices {
         stat::mknod(
@@ -162,7 +94,7 @@ fn create_device_nodes(
 /// Replacing the root mount inside the contaier consists of a few steps. This function marks all
 /// mount points with the right flags and then does the all-important `pivot_root()` that replaces
 /// the root mount inside the container with the new root filesystem.
-fn mount_rootfs(root_spec: &MountSpecification) -> Result<(), CartonError> {
+fn mount_rootfs(rootfs: &Mount) -> Result<(), CartonError> {
     // Pivot root to the new bind mount
     //
     // Instead of using a temporary "put_old" directory to mount the current root on, like
@@ -171,7 +103,10 @@ fn mount_rootfs(root_spec: &MountSpecification) -> Result<(), CartonError> {
     // This stacks the mounts with the "old root" at the top of the stack. By umounting that layer
     // we get to the new "fake" root like we want, without having to create/delete a temporary
     // directory.
-    unistd::pivot_root(&root_spec.source, &root_spec.source)?;
+    unistd::pivot_root(
+        rootfs.source.as_ref().unwrap(),
+        rootfs.source.as_ref().unwrap(),
+    )?;
 
     // Re-mount the root yet again but mark it as "MS_SLAVE" so umount events will
     // in no circumstance propagate to outside the namespace.