some changes

- rework to use cap-std
- add unit tests

Author: cgwalters

lib/src/boundimage.rs

@@ -1,37 +1,47 @@
 use crate::task::Task;
 use anyhow::{Context, Result};
 use camino::Utf8Path;
+use cap_std_ext::cap_std::fs::Dir;
+use cap_std_ext::dirext::CapStdExtDirExt;
 use fn_error_context::context;
 use ostree_ext::ostree::Deployment;
 use ostree_ext::sysroot::SysrootLock;
 use regex::Regex;
+use rustix::fd::BorrowedFd;
 use rustix::fs::{OFlags, ResolveFlags};
-use std::fs;
 use std::fs::File;
 use std::io::Read;
 use std::os::unix::io::AsFd;
-use std::path::Path;
 
 const BOUND_IMAGE_DIR: &'static str = "usr/lib/bootc-experimental/bound-images.d";
 
+// Access the file descriptor for a sysroot
+#[allow(unsafe_code)]
+pub(crate) fn sysroot_fd(sysroot: &ostree_ext::ostree::Sysroot) -> BorrowedFd {
+    unsafe { BorrowedFd::borrow_raw(sysroot.fd()) }
+}
+
 pub(crate) fn pull_bound_images(sysroot: &SysrootLock, deployment: &Deployment) -> Result<()> {
-    let deployment_root = format!("/{}", sysroot.deployment_dirpath(&deployment).to_string());
-    let spec_dir = format!("{}/{BOUND_IMAGE_DIR}", deployment_root);
+    let sysroot_fd = sysroot_fd(&sysroot);
+    let sysroot_fd = Dir::reopen_dir(&sysroot_fd)?;
+    let deployment_root_path = sysroot.deployment_dirpath(&deployment);
+    let deployment_root = &sysroot_fd.open_dir(&deployment_root_path)?;
 
-    if Path::new(&spec_dir).exists() {
-        let bound_images = parse_spec_dir(&spec_dir, &deployment_root)?;
-        pull_images(deployment_root, bound_images)?;
-    }
+    let bound_images = parse_spec_dir(&deployment_root, BOUND_IMAGE_DIR)?;
+    pull_images(deployment_root, bound_images)?;
 
     Ok(())
 }
 
 #[context("parse bound image spec dir")]
-fn parse_spec_dir(spec_dir: &String, deployment_root: &String) -> Result<Vec<BoundImage>> {
-    let entries = fs::read_dir(spec_dir)?;
+fn parse_spec_dir(root: &Dir, spec_dir: &str) -> Result<Vec<BoundImage>> {
+    let Some(bound_images_dir) = root.open_dir_optional(spec_dir)? else {
+        return Ok(Default::default());
+    };
+
     let mut bound_images = Vec::new();
 
-    for entry in entries {
+    for entry in bound_images_dir.entries()? {
         //validate entry is a symlink with correct extension
         let entry = entry?;
         let file_name = entry.file_name();
@@ -46,23 +56,19 @@ fn parse_spec_dir(spec_dir: &String, deployment_root: &String) -> Result<Vec<Bou
         }
 
         //parse the file contents
-        let file_path = entry.path();
-        let file_path = file_path.strip_prefix(format!("/{}", deployment_root)).context("Prefix not found in file path")?;
-
-        let root_dir = File::open(deployment_root).context("Unable to open deployment_root")?;
-        let root_fd = root_dir.as_fd();
-
+        let path = Utf8Path::new(spec_dir).join(file_name);
         let mut file: File = rustix::fs::openat2(
-            root_fd,
-            file_path,
-            OFlags::empty(),
+            root.as_fd(),
+            path.as_std_path(),
+            OFlags::CLOEXEC | OFlags::RDONLY,
             rustix::fs::Mode::empty(),
-            ResolveFlags::IN_ROOT,
+            ResolveFlags::IN_ROOT | ResolveFlags::BENEATH,
         )?
         .into();
 
         let mut file_contents = String::new();
-        file.read_to_string(&mut file_contents).context("Unable to read file contents")?;
+        file.read_to_string(&mut file_contents)
+            .context("Unable to read file contents")?;
 
         let file_ini = ini::Ini::load_from_str(&file_contents).context("Parse to ini")?;
         let file_extension = Utf8Path::new(file_name).extension();
@@ -103,14 +109,14 @@ fn parse_container_file(file_name: &str, file_contents: &ini::Ini) -> Result<Bou
 }
 
 #[context("pull bound images")]
-fn pull_images(deployment_root: String, bound_images: Vec<BoundImage>) -> Result<()> {
+fn pull_images(_deployment_root: &Dir, bound_images: Vec<BoundImage>) -> Result<()> {
     //TODO: do this in parallel
     for bound_image in bound_images {
         let mut task = Task::new("Pulling bound image", "/usr/bin/podman")
             .arg("pull")
             .arg(&bound_image.image);
         if let Some(auth_file) = &bound_image.auth_file {
-            task = task.arg("--authfile").arg(format!("/{deployment_root}/{auth_file}"));
+            task = task.arg("--authfile").arg(auth_file);
         }
         task.run()?;
     }
@@ -143,3 +149,26 @@ fn validate_spec_value(value: &String) -> Result<()> {
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use cap_std_ext::cap_std;
+
+    #[test]
+    fn test_parse_bound_images() -> Result<()> {
+        let td = &cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
+        let images = parse_spec_dir(td, &BOUND_IMAGE_DIR).unwrap();
+        assert_eq!(images.len(), 0);
+
+        td.create_dir_all(BOUND_IMAGE_DIR).unwrap();
+        let images = parse_spec_dir(td, &BOUND_IMAGE_DIR).unwrap();
+        assert_eq!(images.len(), 0);
+
+        td.symlink("../blah", format!("{BOUND_IMAGE_DIR}/foo.image"))
+            .unwrap();
+        assert!(parse_spec_dir(td, &BOUND_IMAGE_DIR).is_err());
+
+        Ok(())
+    }
+}

lib/src/lib.rs

@@ -17,8 +17,8 @@
 #![allow(clippy::needless_borrow)]
 #![allow(clippy::needless_borrows_for_generic_args)]
 
-pub mod cli;
 mod boundimage;
+pub mod cli;
 pub(crate) mod deploy;
 pub(crate) mod generator;
 mod image;

lib/src/task.rs

@@ -1,5 +1,7 @@
 use std::{
-    ffi::OsStr, io::{Seek, Write}, process::{Command, Stdio}
+    ffi::OsStr,
+    io::{Seek, Write},
+    process::{Command, Stdio},
 };
 
 use anyhow::{Context, Result};