From bdb8bed5b7c74b7a1fe4440ae86d31c8d4c1d59c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 18 Dec 2023 10:08:29 -0500 Subject: [PATCH] coreos: Change aleph API to take a root as a std path This makes it amenable to unit testing, and also since the previous code was always operating on `/` there's no good reason to use the `openat` APIs which don't follow symlinks by default, which breaks ergonomics. --- src/bootupd.rs | 2 +- src/coreos.rs | 92 +++++++++++++++++++++++++++++++------------------- src/efi.rs | 2 +- 3 files changed, 59 insertions(+), 37 deletions(-) diff --git a/src/bootupd.rs b/src/bootupd.rs index 6baa3246..f5a8529d 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -388,7 +388,7 @@ pub(crate) fn print_status(status: &Status) -> Result<()> { } } - if let Some(coreos_aleph) = coreos::get_aleph_version()? { + if let Some(coreos_aleph) = coreos::get_aleph_version(Path::new("/"))? { println!("CoreOS aleph version: {}", coreos_aleph.aleph.version); } diff --git a/src/coreos.rs b/src/coreos.rs index 2ee70024..9977acef 100644 --- a/src/coreos.rs +++ b/src/coreos.rs @@ -8,9 +8,8 @@ use anyhow::{Context, Result}; use chrono::prelude::*; -use openat_ext::OpenatDirExt; use serde::{Deserialize, Serialize}; -use std::fs::{canonicalize, symlink_metadata}; +use std::fs::File; use std::path::Path; #[derive(Serialize, Deserialize, Clone, Debug, Hash, Ord, PartialOrd, PartialEq, Eq)] @@ -28,35 +27,21 @@ pub(crate) struct AlephWithTimestamp { } /// Path to the file, see above -const ALEPH_PATH: &str = "/sysroot/.coreos-aleph-version.json"; +const ALEPH_PATH: &str = "sysroot/.coreos-aleph-version.json"; -pub(crate) fn get_aleph_version() -> Result> { - let sysroot = openat::Dir::open("/")?; - let mut path = ALEPH_PATH; - if !Path::new(ALEPH_PATH).exists() { +pub(crate) fn get_aleph_version(root: &Path) -> Result> { + let path = &root.join(ALEPH_PATH); + if !path.exists() { return Ok(None); } - let target; - let is_link = symlink_metadata(path) - .with_context(|| format!("reading metadata for {}", path))? - .file_type() - .is_symlink(); - if is_link { - target = - canonicalize(path).with_context(|| format!("getting absolute path to {}", path))?; - path = target.to_str().unwrap() - } - if let Some(statusf) = sysroot.open_file_optional(path)? { - let meta = statusf.metadata()?; - let bufr = std::io::BufReader::new(statusf); - let aleph: Aleph = serde_json::from_reader(bufr)?; - Ok(Some(AlephWithTimestamp { - aleph, - ts: meta.created()?.into(), - })) - } else { - Ok(None) - } + let statusf = File::open(path).with_context(|| format!("Opening {path:?}"))?; + let meta = statusf.metadata()?; + let bufr = std::io::BufReader::new(statusf); + let aleph: Aleph = serde_json::from_reader(bufr)?; + Ok(Some(AlephWithTimestamp { + aleph, + ts: meta.created()?.into(), + })) } #[cfg(test)] @@ -64,6 +49,49 @@ mod test { use super::*; use anyhow::Result; + const V1_ALEPH_DATA: &str = r##" + { + "version": "32.20201002.dev.2", + "ref": "fedora/x86_64/coreos/testing-devel", + "ostree-commit": "b2ea6159d6274e1bbbb49aa0ef093eda5d53a75c8a793dbe184f760ed64dc862" + }"##; + + #[test] + fn test_parse_from_root_empty() -> Result<()> { + // Verify we're a no-op in an empty root + let root: &tempfile::TempDir = &tempfile::tempdir()?; + let root = root.path(); + assert!(get_aleph_version(root).unwrap().is_none()); + Ok(()) + } + + #[test] + fn test_parse_from_root() -> Result<()> { + let root: &tempfile::TempDir = &tempfile::tempdir()?; + let root = root.path(); + let sysroot = &root.join("sysroot"); + std::fs::create_dir(sysroot).context("Creating sysroot")?; + std::fs::write(root.join(ALEPH_PATH), V1_ALEPH_DATA).context("Writing aleph")?; + let aleph = get_aleph_version(root).unwrap().unwrap(); + assert_eq!(aleph.aleph.version, "32.20201002.dev.2"); + Ok(()) + } + + #[test] + fn test_parse_from_root_linked() -> Result<()> { + let root: &tempfile::TempDir = &tempfile::tempdir()?; + let root = root.path(); + let sysroot = &root.join("sysroot"); + std::fs::create_dir(sysroot).context("Creating sysroot")?; + let target_name = ".new-ostree-aleph.json"; + let target = &sysroot.join(target_name); + std::fs::write(root.join(target), V1_ALEPH_DATA).context("Writing aleph")?; + std::os::unix::fs::symlink(target_name, root.join(ALEPH_PATH)).context("Symlinking")?; + let aleph = get_aleph_version(root).unwrap().unwrap(); + assert_eq!(aleph.aleph.version, "32.20201002.dev.2"); + Ok(()) + } + #[test] fn test_parse_old_aleph() -> Result<()> { // What the aleph file looked like before we changed it in @@ -82,13 +110,7 @@ mod test { #[test] fn test_parse_aleph() -> Result<()> { - let alephdata = r##" -{ - "version": "32.20201002.dev.2", - "ref": "fedora/x86_64/coreos/testing-devel", - "ostree-commit": "b2ea6159d6274e1bbbb49aa0ef093eda5d53a75c8a793dbe184f760ed64dc862" -}"##; - let aleph: Aleph = serde_json::from_str(alephdata)?; + let aleph: Aleph = serde_json::from_str(V1_ALEPH_DATA)?; assert_eq!(aleph.version, "32.20201002.dev.2"); Ok(()) } diff --git a/src/efi.rs b/src/efi.rs index 265999d3..7ffe69c4 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -218,7 +218,7 @@ impl Component for Efi { return Ok(None); }; // This would be extended with support for other operating systems later - if let Some(coreos_aleph) = crate::coreos::get_aleph_version()? { + if let Some(coreos_aleph) = crate::coreos::get_aleph_version(Path::new("/"))? { let meta = ContentMetadata { timestamp: coreos_aleph.ts, version: coreos_aleph.aleph.version,