From 870b529aba92ccd82a8f857dd7527266eb8e42cf Mon Sep 17 00:00:00 2001 From: Chris Kyrouac Date: Thu, 11 Jul 2024 13:25:51 -0400 Subject: [PATCH] deploy: Retrieve bound images when staging new image This parses any file pointed to by a symlink with a .container or .image extension found in /usr/lib/bootc/bound-images.d. An error is thrown if a systemd specifier is found in the parsed fields. It currently only supports the Image and AuthFile fields. Some known shortcomings are that each image is pulled synchronously. It does not do any cleanup during a rollback or if the switch fails after pulling an image. The install path also needs to pull bound images. Co-authored-by: Colin Walters Signed-off-by: Chris Kyrouac Signed-off-by: Colin Walters --- Cargo.lock | 7 + lib/Cargo.toml | 3 +- lib/src/boundimage.rs | 296 ++++++++++++++++++++++++++++++++++++++++++ lib/src/deploy.rs | 5 +- lib/src/lib.rs | 1 + 5 files changed, 310 insertions(+), 2 deletions(-) create mode 100644 lib/src/boundimage.rs diff --git a/Cargo.lock b/Cargo.lock index 38787fe63..595993180 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -195,6 +195,7 @@ dependencies = [ "serde_yaml", "similar-asserts", "tempfile", + "tini", "tokio", "tokio-util", "toml", @@ -2037,6 +2038,12 @@ dependencies = [ "num_cpus", ] +[[package]] +name = "tini" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e004df4c5f0805eb5f55883204a514cfa43a6d924741be29e871753a53d5565a" + [[package]] name = "tinyvec" version = "1.6.0" diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 6afd0b2d0..3bc65c871 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -30,7 +30,7 @@ liboverdrop = "0.1.0" libsystemd = "0.7" openssl = "^0.10.64" # TODO drop this in favor of rustix -nix = { version = "0.29", features = ["ioctl", "sched"] } +nix = { version = "0.29", features = ["ioctl", "sched", "fs"] } regex = "1.10.4" rustix = { "version" = "0.38.34", features = ["thread", "fs", "system", "process"] } schemars = { version = "0.8.17", features = ["chrono"] } @@ -45,6 +45,7 @@ tempfile = "3.10.1" toml = "0.8.12" xshell = { version = "0.2.6", optional = true } uuid = { version = "1.8.0", features = ["v4"] } +tini = "1.3.0" [dev-dependencies] similar-asserts = { version = "1.5.0" } diff --git a/lib/src/boundimage.rs b/lib/src/boundimage.rs new file mode 100644 index 000000000..68126543f --- /dev/null +++ b/lib/src/boundimage.rs @@ -0,0 +1,296 @@ +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 rustix::fd::BorrowedFd; +use rustix::fs::{OFlags, ResolveFlags}; +use std::fs::File; +use std::io::Read; +use std::os::unix::io::AsFd; + +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 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)?; + + 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(root: &Dir, spec_dir: &str) -> Result> { + 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 bound_images_dir + .entries() + .context("Unable to read entries")? + { + //validate entry is a symlink with correct extension + let entry = entry?; + let file_name = entry.file_name(); + let file_name = if let Some(n) = file_name.to_str() { + n + } else { + anyhow::bail!("Invalid non-UTF8 filename: {file_name:?} in {}", spec_dir); + }; + + if !entry.file_type()?.is_symlink() { + anyhow::bail!("Not a symlink: {file_name}"); + } + + //parse the file contents + let path = Utf8Path::new(spec_dir).join(file_name); + let mut file: File = rustix::fs::openat2( + root.as_fd(), + path.as_std_path(), + OFlags::CLOEXEC | OFlags::RDONLY, + rustix::fs::Mode::empty(), + ResolveFlags::IN_ROOT, + ) + .context("Unable to openat")? + .into(); + + let mut file_contents = String::new(); + file.read_to_string(&mut file_contents) + .context("Unable to read file contents")?; + + let file_ini = tini::Ini::from_string(&file_contents).context("Parse to ini")?; + let file_extension = Utf8Path::new(file_name).extension(); + let bound_image = match file_extension { + Some("image") => parse_image_file(file_name, &file_ini), + Some("container") => parse_container_file(file_name, &file_ini), + _ => anyhow::bail!("Invalid file extension: {file_name}"), + }?; + + bound_images.push(bound_image); + } + + Ok(bound_images) +} + +#[context("parse image file {file_name}")] +fn parse_image_file(file_name: &str, file_contents: &tini::Ini) -> Result { + let image: String = file_contents + .get("Image", "Image") + .ok_or_else(|| anyhow::anyhow!("Missing Image field in {file_name}"))?; + + //TODO: auth_files have some semi-complicated edge cases that we need to handle, + // so for now let's bail out if we see one since the existence of an authfile + // will most likely result in a failure to pull the image + let auth_file: Option = file_contents.get("Image", "AuthFile"); + if !auth_file.is_none() { + anyhow::bail!("AuthFile is not supported by bound bootc images"); + } + + let bound_image = BoundImage::new(image.to_string(), None)?; + Ok(bound_image) +} + +#[context("parse container file {file_name}")] +fn parse_container_file(file_name: &str, file_contents: &tini::Ini) -> Result { + let image: String = file_contents + .get("Container", "Image") + .ok_or_else(|| anyhow::anyhow!("Missing Image field in {file_name}"))?; + + let bound_image = BoundImage::new(image.to_string(), None)?; + Ok(bound_image) +} + +#[context("pull bound images")] +fn pull_images(_deployment_root: &Dir, bound_images: Vec) -> 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(auth_file); + } + task.run()?; + } + + Ok(()) +} + +#[derive(PartialEq, Eq, PartialOrd)] +struct BoundImage { + image: String, + auth_file: Option, +} + +impl BoundImage { + fn new(image: String, auth_file: Option) -> Result { + validate_spec_value(&image).context("Invalid image value")?; + + if let Some(auth_file) = &auth_file { + validate_spec_value(auth_file).context("Invalid auth_file value")?; + } + + Ok(BoundImage { image, auth_file }) + } +} + +impl Ord for BoundImage { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.image.cmp(&other.image) + } +} + +fn validate_spec_value(value: &String) -> Result<()> { + let mut number_of_percents = 0; + for char in value.chars() { + if char == '%' { + number_of_percents += 1; + } else if number_of_percents % 2 != 0 { + anyhow::bail!("Systemd specifiers are not supported by bound bootc images: {value}"); + } else { + number_of_percents = 0; + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use cap_std_ext::cap_std; + use std::io::Write; + + #[test] + fn test_parse_spec_dir() -> Result<()> { + const CONTAINER_IMAGE_DIR: &'static str = "usr/share/containers/systemd"; + + // Empty dir should return an empty vector + 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(); + td.create_dir_all(CONTAINER_IMAGE_DIR).unwrap(); + let images = parse_spec_dir(td, &BOUND_IMAGE_DIR).unwrap(); + assert_eq!(images.len(), 0); + + // Should return BoundImages + let mut foo_file = td + .create(format!("{CONTAINER_IMAGE_DIR}/foo.image")) + .unwrap(); + foo_file.write_all(b"[Image]\n").unwrap(); + foo_file.write_all(b"Image=quay.io/foo/foo:latest").unwrap(); + td.symlink_contents( + format!("/{CONTAINER_IMAGE_DIR}/foo.image"), + format!("{BOUND_IMAGE_DIR}/foo.image"), + ) + .unwrap(); + + let mut bar_file = td + .create(format!("{CONTAINER_IMAGE_DIR}/bar.image")) + .unwrap(); + bar_file.write_all(b"[Image]\n").unwrap(); + bar_file.write_all(b"Image=quay.io/bar/bar:latest").unwrap(); + td.symlink_contents( + format!("/{CONTAINER_IMAGE_DIR}/bar.image"), + format!("{BOUND_IMAGE_DIR}/bar.image"), + ) + .unwrap(); + + let mut images = parse_spec_dir(td, &BOUND_IMAGE_DIR).unwrap(); + images.sort(); + assert_eq!(images.len(), 2); + assert_eq!(images[0].image, "quay.io/bar/bar:latest"); + assert_eq!(images[1].image, "quay.io/foo/foo:latest"); + + // Invalid symlink should return an error + td.symlink("./blah", format!("{BOUND_IMAGE_DIR}/blah.image")) + .unwrap(); + assert!(parse_spec_dir(td, &BOUND_IMAGE_DIR).is_err()); + + // Invalid image contents should return an error + let mut error_file = td.create("error.image").unwrap(); + error_file.write_all(b"[Image]\n").unwrap(); + td.symlink_contents("/error.image", format!("{BOUND_IMAGE_DIR}/error.image")) + .unwrap(); + assert!(parse_spec_dir(td, &BOUND_IMAGE_DIR).is_err()); + + Ok(()) + } + + #[test] + fn test_validate_spec_value() -> Result<()> { + //should not return an error with no % characters + let value = String::from("[Image]\nImage=quay.io/foo/foo:latest"); + validate_spec_value(&value).unwrap(); + + //should return error when % is NOT followed by another % + let value = String::from("[Image]\nImage=quay.io/foo/%foo:latest"); + assert!(validate_spec_value(&value).is_err()); + + //should not return an error when % is followed by another % + let value = String::from("[Image]\nImage=quay.io/foo/%%foo:latest"); + validate_spec_value(&value).unwrap(); + + //should not return an error when %% is followed by a specifier + let value = String::from("[Image]\nImage=quay.io/foo/%%%foo:latest"); + assert!(validate_spec_value(&value).is_err()); + + Ok(()) + } + + #[test] + fn test_parse_image_file() -> Result<()> { + //should return BoundImage when no auth_file is present + let file_contents = + tini::Ini::from_string("[Image]\nImage=quay.io/foo/foo:latest").unwrap(); + let bound_image = parse_image_file("foo.image", &file_contents).unwrap(); + assert_eq!(bound_image.image, "quay.io/foo/foo:latest"); + assert_eq!(bound_image.auth_file, None); + + //should error when auth_file is present + let file_contents = tini::Ini::from_string( + "[Image]\nImage=quay.io/foo/foo:latest\nAuthFile=/etc/containers/auth.json", + ) + .unwrap(); + assert!(parse_image_file("foo.image", &file_contents).is_err()); + + //should return error when missing image field + let file_contents = tini::Ini::from_string("[Image]\n").unwrap(); + assert!(parse_image_file("foo.image", &file_contents).is_err()); + + Ok(()) + } + + #[test] + fn test_parse_container_file() -> Result<()> { + //should return BoundImage + let file_contents = + tini::Ini::from_string("[Container]\nImage=quay.io/foo/foo:latest").unwrap(); + let bound_image = parse_container_file("foo.container", &file_contents).unwrap(); + assert_eq!(bound_image.image, "quay.io/foo/foo:latest"); + assert_eq!(bound_image.auth_file, None); + + //should return error when missing image field + let file_contents = tini::Ini::from_string("[Container]\n").unwrap(); + assert!(parse_container_file("foo.container", &file_contents).is_err()); + + Ok(()) + } +} diff --git a/lib/src/deploy.rs b/lib/src/deploy.rs index ff66562c3..6d8a640e8 100644 --- a/lib/src/deploy.rs +++ b/lib/src/deploy.rs @@ -383,7 +383,7 @@ pub(crate) async fn stage( ) -> Result<()> { let merge_deployment = sysroot.merge_deployment(Some(stateroot)); let origin = origin_from_imageref(spec.image)?; - crate::deploy::deploy( + let deployment = crate::deploy::deploy( sysroot, merge_deployment.as_ref(), stateroot, @@ -391,6 +391,9 @@ pub(crate) async fn stage( &origin, ) .await?; + + crate::boundimage::pull_bound_images(sysroot, &deployment)?; + crate::deploy::cleanup(sysroot).await?; println!("Queued for next boot: {:#}", spec.image); if let Some(version) = image.version.as_deref() { diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 9f8d4ac51..600856f9d 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -17,6 +17,7 @@ #![allow(clippy::needless_borrow)] #![allow(clippy::needless_borrows_for_generic_args)] +mod boundimage; pub mod cli; pub(crate) mod deploy; pub(crate) mod generator;