From c6cac6397fcf0a2f434dc591033cde68adbb9f80 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Mon, 27 May 2024 09:13:27 -0400 Subject: [PATCH] Pass env properties to merge function Required for https://github.com/containers/bootc/pull/401 Adds a `EnvProperties` struct to allow for conditional merging of kargs depending on whether the archiecture matches. Future properties such as `platform.id` can be added in the future. Signed-off-by: Luke Yang --- lib/src/install/config.rs | 81 +++++++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 21 deletions(-) diff --git a/lib/src/install/config.rs b/lib/src/install/config.rs index 730d18a2f..f5e24316d 100644 --- a/lib/src/install/config.rs +++ b/lib/src/install/config.rs @@ -8,6 +8,12 @@ use serde::{Deserialize, Serialize}; use super::baseline::BlockSetup; +/// Properties of the environment, such as the system architecture +/// Left open for future properties such as `platform.id` +pub(crate) struct EnvProperties { + pub(crate) sys_arch: String, +} + /// The toplevel config entry for installation configs stored /// in bootc/install (e.g. /etc/bootc/install/05-custom.toml) #[derive(Debug, Clone, Serialize, Deserialize, Default)] @@ -47,16 +53,18 @@ pub(crate) struct InstallConfiguration { /// Kernel arguments, applied at installation time #[serde(skip_serializing_if = "Option::is_none")] pub(crate) kargs: Option>, + /// Supported architectures for this configuration + pub(crate) supported_arch: Option>, } -fn merge_basic(s: &mut Option, o: Option) { +fn merge_basic(s: &mut Option, o: Option, env: &EnvProperties) { if let Some(o) = o { *s = Some(o); } } trait Mergeable { - fn merge(&mut self, other: Self) + fn merge(&mut self, other: Self, env: &EnvProperties) where Self: Sized; } @@ -65,13 +73,13 @@ impl Mergeable for Option where T: Mergeable, { - fn merge(&mut self, other: Self) + fn merge(&mut self, other: Self, env: &EnvProperties) where Self: Sized, { if let Some(other) = other { if let Some(s) = self.as_mut() { - s.merge(other) + s.merge(other, env) } else { *self = Some(other); } @@ -81,28 +89,38 @@ where impl Mergeable for RootFS { /// Apply any values in other, overriding any existing values in `self`. - fn merge(&mut self, other: Self) { - merge_basic(&mut self.fstype, other.fstype) + fn merge(&mut self, other: Self, env: &EnvProperties) { + merge_basic(&mut self.fstype, other.fstype, env) } } impl Mergeable for BasicFilesystems { /// Apply any values in other, overriding any existing values in `self`. - fn merge(&mut self, other: Self) { - self.root.merge(other.root) + fn merge(&mut self, other: Self, env: &EnvProperties) { + self.root.merge(other.root, env) } } impl Mergeable for InstallConfiguration { /// Apply any values in other, overriding any existing values in `self`. - fn merge(&mut self, other: Self) { - merge_basic(&mut self.root_fs_type, other.root_fs_type); - merge_basic(&mut self.block, other.block); - self.filesystem.merge(other.filesystem); + fn merge(&mut self, other: Self, env: &EnvProperties) { + merge_basic(&mut self.root_fs_type, other.root_fs_type, env); + merge_basic(&mut self.block, other.block, env); + self.filesystem.merge(other.filesystem, env); if let Some(other_kargs) = other.kargs { - self.kargs - .get_or_insert_with(Default::default) - .extend(other_kargs) + // if arch is specified, only apply kargs if it matches the current arch + // if arch is not specified, apply kargs unconditionally + if let Some(other_arch) = other.supported_arch { + if other_arch.contains(&env.sys_arch) { + self.kargs + .get_or_insert_with(Default::default) + .extend(other_kargs.clone()); + } + } else { + self.kargs + .get_or_insert_with(Default::default) + .extend(other_kargs) + } } } } @@ -154,6 +172,9 @@ impl InstallConfiguration { #[context("Loading configuration")] /// Load the install configuration, merging all found configuration files. pub(crate) fn load_config() -> Result> { + let env = EnvProperties { + sys_arch: std::env::consts::ARCH.to_string(), + }; const SYSTEMD_CONVENTIONAL_BASES: &[&str] = &["/usr/lib", "/usr/local/lib", "/etc", "/run"]; let fragments = liboverdrop::scan(SYSTEMD_CONVENTIONAL_BASES, "bootc/install", &["toml"], true); let mut config: Option = None; @@ -161,7 +182,7 @@ pub(crate) fn load_config() -> Result> { let buf = std::fs::read_to_string(&path)?; let mut unused = std::collections::HashSet::new(); let de = toml::Deserializer::new(&buf); - let c: InstallConfigurationToplevel = serde_ignored::deserialize(de, |path| { + let mut c: InstallConfigurationToplevel = serde_ignored::deserialize(de, |path| { unused.insert(path.to_string()); }) .with_context(|| format!("Parsing {path:?}"))?; @@ -171,9 +192,18 @@ pub(crate) fn load_config() -> Result> { if let Some(config) = config.as_mut() { if let Some(install) = c.install { tracing::debug!("Merging install config: {install:?}"); - config.merge(install); + config.merge(install, &env); } } else { + // If arch is specified and doesn't match the current arch, remove kargs + // We need to add this since the merge function isn't applied for the first config file + if let Some(ref mut install) = c.install { + if let Some(arch) = install.supported_arch.as_ref() { + if !arch.contains(&env.sys_arch) { + install.kargs = None; + } + } + } config = c.install; } } @@ -188,6 +218,9 @@ pub(crate) fn load_config() -> Result> { fn test_parse_config() { use super::baseline::Filesystem; + let env = EnvProperties { + sys_arch: "x86_64".to_string(), + }; let c: InstallConfigurationToplevel = toml::from_str( r##"[install] root-fs-type = "xfs" @@ -202,7 +235,7 @@ root-fs-type = "xfs" ..Default::default() }), }; - install.merge(other.install.unwrap()); + install.merge(other.install.unwrap(), &env); assert_eq!( install.root_fs_type.as_ref().copied().unwrap(), Filesystem::Ext4 @@ -236,7 +269,7 @@ kargs = ["console=ttyS0", "foo=bar"] ..Default::default() }), }; - install.merge(other.install.unwrap()); + install.merge(other.install.unwrap(), &env); assert_eq!(install.root_fs_type.unwrap(), Filesystem::Ext4); assert_eq!( install.kargs, @@ -252,6 +285,9 @@ kargs = ["console=ttyS0", "foo=bar"] #[test] fn test_parse_filesystems() { use super::baseline::Filesystem; + let env = EnvProperties { + sys_arch: "x86_64".to_string(), + }; let c: InstallConfigurationToplevel = toml::from_str( r##"[install.filesystem.root] type = "xfs" @@ -273,7 +309,7 @@ type = "xfs" ..Default::default() }), }; - install.merge(other.install.unwrap()); + install.merge(other.install.unwrap(), &env); assert_eq!( install.filesystem_root().unwrap().fstype.unwrap(), Filesystem::Ext4 @@ -282,6 +318,9 @@ type = "xfs" #[test] fn test_parse_block() { + let env = EnvProperties { + sys_arch: "x86_64".to_string(), + }; let c: InstallConfigurationToplevel = toml::from_str( r##"[install.filesystem.root] type = "xfs" @@ -301,7 +340,7 @@ type = "xfs" ..Default::default() }), }; - install.merge(other.install.unwrap()); + install.merge(other.install.unwrap(), &env); // Should be set, but zero length assert_eq!(install.block.as_ref().unwrap().len(), 0); assert!(install.get_block_setup(None).is_err());