Merge pull request #289 from cgwalters/generalize-install-config

install: Generalize `root-fs-type` into `install.filesystem.root.type`

docs/install.md

@@ -103,15 +103,15 @@ To enable `bootc install` as part of your OS/distribution base image,
 create a file named `/usr/lib/bootc/install/00-<osname>.toml` with the contents of the form:
 
 ```toml
-[install]
-root-fs-type = "xfs"
+[install.filesystem.root]
+type = "xfs"
 ```
 
-The `root-fs-type` value **MUST** be set.
+The `install.filesystem.root` value **MUST** be set.
 
 Configuration files found in this directory will be merged, with higher alphanumeric values
 taking precedence.  If for example you are building a derived container image from the above OS,
-you could create a `50-myos.toml`  that sets `root-fs-type = "btrfs"` which will override the
+you could create a `50-myos.toml`  that sets `type = "btrfs"` which will override the
 prior setting.
 
 Other available options, also under the `[install]` section:
@@ -121,6 +121,8 @@ This option is particularly useful when creating derived/layered images; for exa
 image may want to have its default `console=` set, in contrast with a default base image.
 The values in this field are space separated.
 
+`root-fs-type`: This value is the same as `install.filesystem.root.type`.
+
 ## Installing an "unconfigured" image
 
 The bootc project aims to support generic/general-purpose operating

lib/src/install/baseline.rs

@@ -339,7 +339,10 @@ pub(crate) fn install_create_rootfs(
     // Initialize rootfs
     let root_filesystem = opts
         .filesystem
-        .or(state.install_config.root_fs_type)
+        .or(state
+            .install_config
+            .filesystem_root()
+            .and_then(|r| r.fstype))
         .ok_or_else(|| anyhow::anyhow!("No root filesystem specified"))?;
     let root_uuid = mkfs(&rootdev, root_filesystem, Some("root"), [])?;
     let rootarg = format!("root=UUID={root_uuid}");

lib/src/install/config.rs

@@ -14,32 +14,115 @@ pub(crate) struct InstallConfigurationToplevel {
     pub(crate) install: Option<InstallConfiguration>,
 }
 
+/// Configuration for a filesystem
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
+pub(crate) struct RootFS {
+    #[serde(rename = "type")]
+    pub(crate) fstype: Option<super::baseline::Filesystem>,
+}
+
+/// This structure should only define "system" or "basic" filesystems; we are
+/// not trying to generalize this into e.g. supporting `/var` or other ones.
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
+pub(crate) struct BasicFilesystems {
+    pub(crate) root: Option<RootFS>,
+    // TODO allow configuration of these other filesystems too
+    // pub(crate) xbootldr: Option<FilesystemCustomization>,
+    // pub(crate) esp: Option<FilesystemCustomization>,
+}
+
 /// The serialized [install] section
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
 #[serde(rename = "install", rename_all = "kebab-case", deny_unknown_fields)]
 pub(crate) struct InstallConfiguration {
     /// Root filesystem type
     pub(crate) root_fs_type: Option<super::baseline::Filesystem>,
+    pub(crate) filesystem: Option<BasicFilesystems>,
     /// Kernel arguments, applied at installation time
     #[serde(skip_serializing_if = "Option::is_none")]
     pub(crate) kargs: Option<Vec<String>>,
 }
 
-impl InstallConfiguration {
-    /// Apply any values in other, overriding any existing values in `self`.
-    fn merge(&mut self, other: Self) {
-        fn mergeopt<T>(s: &mut Option<T>, o: Option<T>) {
-            if let Some(o) = o {
-                *s = Some(o);
+fn merge_basic<T>(s: &mut Option<T>, o: Option<T>) {
+    if let Some(o) = o {
+        *s = Some(o);
+    }
+}
+
+trait Mergeable {
+    fn merge(&mut self, other: Self)
+    where
+        Self: Sized;
+}
+
+impl<T> Mergeable for Option<T>
+where
+    T: Mergeable,
+{
+    fn merge(&mut self, other: Self)
+    where
+        Self: Sized,
+    {
+        if let Some(other) = other {
+            if let Some(s) = self.as_mut() {
+                s.merge(other)
+            } else {
+                *self = Some(other);
             }
         }
-        mergeopt(&mut self.root_fs_type, other.root_fs_type);
+    }
+}
+
+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)
+    }
+}
+
+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)
+    }
+}
+
+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);
+        self.filesystem.merge(other.filesystem);
         if let Some(other_kargs) = other.kargs {
             self.kargs
                 .get_or_insert_with(Default::default)
                 .extend(other_kargs)
         }
     }
+}
+
+impl InstallConfiguration {
+    /// Some fields can be specified multiple ways.  This synchronizes the values of the fields
+    /// to ensure they're the same.
+    ///
+    /// - install.root-fs-type is synchronized with install.filesystems.root.type; if
+    ///   both are set, then the latter takes precedence
+    pub(crate) fn canonicalize(&mut self) {
+        // New canonical form wins.
+        if let Some(rootfs_type) = self.filesystem_root().and_then(|f| f.fstype.as_ref()) {
+            self.root_fs_type = Some(*rootfs_type)
+        } else if let Some(rootfs) = self.root_fs_type.as_ref() {
+            let fs = self.filesystem.get_or_insert_with(Default::default);
+            let root = fs.root.get_or_insert_with(Default::default);
+            root.fstype = Some(*rootfs);
+        }
+    }
+
+    /// Convenience helper to access the root filesystem
+    pub(crate) fn filesystem_root(&self) -> Option<&RootFS> {
+        self.filesystem.as_ref().and_then(|fs| fs.root.as_ref())
+    }
 
     // Remove all configuration which is handled by `install to-filesystem`.
     pub(crate) fn filter_to_external(&mut self) {
@@ -73,7 +156,9 @@ pub(crate) fn load_config() -> Result<InstallConfiguration> {
             config = c.install;
         }
     }
-    config.ok_or_else(|| anyhow::anyhow!("No bootc/install config found; this operating system must define a default configuration to be installable"))
+    let mut config = config.ok_or_else(|| anyhow::anyhow!("No bootc/install config found; this operating system must define a default configuration to be installable"))?;
+    config.canonicalize();
+    Ok(config)
 }
 
 #[test]
@@ -92,11 +177,23 @@ root-fs-type = "xfs"
     let other = InstallConfigurationToplevel {
         install: Some(InstallConfiguration {
             root_fs_type: Some(Filesystem::Ext4),
+            filesystem: None,
             kargs: None,
         }),
     };
     install.merge(other.install.unwrap());
-    assert_eq!(install.root_fs_type.unwrap(), Filesystem::Ext4);
+    assert_eq!(
+        install.root_fs_type.as_ref().copied().unwrap(),
+        Filesystem::Ext4
+    );
+    // This one shouldn't have been set
+    assert!(install.filesystem_root().is_none());
+    install.canonicalize();
+    assert_eq!(install.root_fs_type.as_ref().unwrap(), &Filesystem::Ext4);
+    assert_eq!(
+        install.filesystem_root().unwrap().fstype.unwrap(),
+        Filesystem::Ext4
+    );
 
     let c: InstallConfigurationToplevel = toml::from_str(
         r##"[install]
@@ -110,6 +207,7 @@ kargs = ["console=ttyS0", "foo=bar"]
     let other = InstallConfigurationToplevel {
         install: Some(InstallConfiguration {
             root_fs_type: None,
+            filesystem: None,
             kargs: Some(
                 ["console=tty0", "nosmt"]
                     .into_iter()
@@ -130,3 +228,35 @@ kargs = ["console=ttyS0", "foo=bar"]
         )
     )
 }
+
+#[test]
+fn test_parse_filesystems() {
+    use super::baseline::Filesystem;
+    let c: InstallConfigurationToplevel = toml::from_str(
+        r##"[install.filesystem.root]
+type = "xfs"
+"##,
+    )
+    .unwrap();
+    let mut install = c.install.unwrap();
+    assert_eq!(
+        install.filesystem_root().unwrap().fstype.unwrap(),
+        Filesystem::Xfs
+    );
+    let other = InstallConfigurationToplevel {
+        install: Some(InstallConfiguration {
+            root_fs_type: None,
+            filesystem: Some(BasicFilesystems {
+                root: Some(RootFS {
+                    fstype: Some(Filesystem::Ext4),
+                }),
+            }),
+            kargs: None,
+        }),
+    };
+    install.merge(other.install.unwrap());
+    assert_eq!(
+        install.filesystem_root().unwrap().fstype.unwrap(),
+        Filesystem::Ext4
+    );
+}

lib/src/privtests.rs

@@ -100,8 +100,9 @@ pub(crate) fn impl_run_container() -> Result<()> {
     assert!(stderr.contains("requires root privileges"));
 
     let config = cmd!(sh, "bootc install print-configuration").read()?;
-    let config: InstallConfiguration =
+    let mut config: InstallConfiguration =
         serde_json::from_str(&config).context("Parsing install config")?;
+    config.canonicalize();
     assert_eq!(
         config.root_fs_type.unwrap(),
         crate::install::baseline::Filesystem::Xfs

manpages-md-extra/bootc-install-config.md

@@ -0,0 +1,49 @@
+% bootc-install-config(5)
+
+# NAME
+
+bootc-install-config.toml
+
+# DESCRIPTION
+
+The `bootc install` process supports some basic customization.  This configuration file
+is in TOML format, and will be discovered by the installation process in via "drop-in"
+files in `/usr/lib/bootc/install` that are processed in alphanumerical order.
+
+The individual files are merged into a single final installation config, so it is
+supported for e.g. a container base image to provide a default root filesystem type,
+that can be overridden in a derived container image.
+
+# install
+
+This is the only defined toplevel table.
+
+The `install`` section supports two subfields:
+
+- `filesystem`: See below.
+- `kargs`: An array of strings; this will be appended to the set of kernel arguments.
+
+# filesystem
+
+There is one valid field:
+
+- `root`: An instance of "filesystem-root"; see below
+
+# filesystem-root
+
+There is one valid field:
+
+`type`: This can be any basic Linux filesystem with a `mkfs.$fstype`.  For example, `ext4`, `xfs`, etc.
+
+# Examples
+
+```toml
+[install.filesystem.root]
+type = "xfs"
+[install]
+kargs = ["nosmt", "console=tty0"]
+```
+
+# SEE ALSO
+
+**bootc(1)**