From 65670572aa2a538e5488f11d1d37072a5b0562ad Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Fri, 15 Dec 2023 23:25:25 -0500 Subject: [PATCH 1/3] coreos: update aleph version handling In https://github.com/osbuild/osbuild/pull/1475 we are updating aleph version to have more information and some of the fields are changing slightly. Notably here `imgid` is no longer going to be populated and `build` -> `version` now. Let's make these tweaks and also just drop any fields from the definition here that we aren't using to tolerate changes better. --- src/bootupd.rs | 2 +- src/coreos.rs | 29 +++++++++++++++++++---------- src/efi.rs | 2 +- tests/kola/test-bootupd | 2 +- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/bootupd.rs b/src/bootupd.rs index c323d68e..6baa3246 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -389,7 +389,7 @@ pub(crate) fn print_status(status: &Status) -> Result<()> { } if let Some(coreos_aleph) = coreos::get_aleph_version()? { - println!("CoreOS aleph image ID: {}", coreos_aleph.aleph.imgid); + println!("CoreOS aleph version: {}", coreos_aleph.aleph.version); } #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] diff --git a/src/coreos.rs b/src/coreos.rs index e8b91cdc..2b552908 100644 --- a/src/coreos.rs +++ b/src/coreos.rs @@ -15,11 +15,8 @@ use serde::{Deserialize, Serialize}; #[serde(rename_all = "kebab-case")] /// See https://github.com/coreos/fedora-coreos-tracker/blob/66d7d00bedd9d5eabc7287b9577f443dcefb7c04/internals/README-internals.md#aleph-version pub(crate) struct Aleph { - pub(crate) build: String, - #[serde(rename = "ref")] - pub(crate) ostree_ref: String, - pub(crate) ostree_commit: String, - pub(crate) imgid: String, + #[serde(alias = "build")] + pub(crate) version: String, } pub(crate) struct AlephWithTimestamp { @@ -52,7 +49,9 @@ mod test { use anyhow::Result; #[test] - fn test_parse_aleph() -> Result<()> { + fn test_parse_old_aleph() -> Result<()> { + // What the aleph file looked like before we changed it in + // https://github.com/osbuild/osbuild/pull/1475 let alephdata = r##" { "build": "32.20201002.dev.2", @@ -61,10 +60,20 @@ mod test { "imgid": "fedora-coreos-32.20201002.dev.2-qemu.x86_64.qcow2" }"##; let aleph: Aleph = serde_json::from_str(alephdata)?; - assert_eq!( - aleph.imgid, - "fedora-coreos-32.20201002.dev.2-qemu.x86_64.qcow2" - ); + assert_eq!(aleph.version, "32.20201002.dev.2"); + Ok(()) + } + + #[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)?; + assert_eq!(aleph.version, "32.20201002.dev.2"); Ok(()) } } diff --git a/src/efi.rs b/src/efi.rs index 3cc44ae5..265999d3 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -221,7 +221,7 @@ impl Component for Efi { if let Some(coreos_aleph) = crate::coreos::get_aleph_version()? { let meta = ContentMetadata { timestamp: coreos_aleph.ts, - version: coreos_aleph.aleph.imgid, + version: coreos_aleph.aleph.version, }; log::trace!("EFI adoptable: {:?}", &meta); return Ok(Some(Adoptable { diff --git a/tests/kola/test-bootupd b/tests/kola/test-bootupd index 8824c809..7402c6fb 100755 --- a/tests/kola/test-bootupd +++ b/tests/kola/test-bootupd @@ -44,7 +44,7 @@ bootupctl status > out.txt assert_file_has_content_literal out.txt 'Component EFI' assert_file_has_content_literal out.txt ' Installed: grub2-efi-x64-' assert_file_has_content_literal out.txt 'Update: At latest version' -assert_file_has_content out.txt '^CoreOS aleph image ID: .*-qemu.'$(arch)'.qcow2' +assert_file_has_content out.txt '^CoreOS aleph version:' ok status # Validate we auto-exited From 3c38e7eb911a8e456531ecfe0472bf5c8b3d4ed2 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Fri, 15 Dec 2023 23:27:33 -0500 Subject: [PATCH 2/3] coreos: let aleph file be a symlink Once https://github.com/osbuild/osbuild/pull/1475 gets picked up and starts getting used the .coreos-aleph-version.json will be a symlink. Let's canonicalize and find the actual file before dropping into sysroot.open_file_optional() because the underlying code for that uses O_NOFOLLOW and will give an ELOOP (too many symbolic links) error. --- src/coreos.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/coreos.rs b/src/coreos.rs index 2b552908..2ee70024 100644 --- a/src/coreos.rs +++ b/src/coreos.rs @@ -6,10 +6,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -use anyhow::Result; +use anyhow::{Context, Result}; use chrono::prelude::*; use openat_ext::OpenatDirExt; use serde::{Deserialize, Serialize}; +use std::fs::{canonicalize, symlink_metadata}; +use std::path::Path; #[derive(Serialize, Deserialize, Clone, Debug, Hash, Ord, PartialOrd, PartialEq, Eq)] #[serde(rename_all = "kebab-case")] @@ -30,7 +32,21 @@ const ALEPH_PATH: &str = "/sysroot/.coreos-aleph-version.json"; pub(crate) fn get_aleph_version() -> Result> { let sysroot = openat::Dir::open("/")?; - if let Some(statusf) = sysroot.open_file_optional(ALEPH_PATH)? { + let mut path = ALEPH_PATH; + if !Path::new(ALEPH_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)?; From 8d746cd8036365491872efcfd2279f91e330af82 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 18 Dec 2023 10:08:29 -0500 Subject: [PATCH 3/3] 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,