composefs: Implement bootc edit

crates/lib/src/bootc_composefs/switch.rs

@@ -11,6 +11,7 @@ use crate::{
     },
     cli::{SwitchOpts, imgref_for_switch},
     progress_jsonl::ProgressWriter,
+    spec::{Host, ImageReference},
     store::{BootedComposefs, Storage},
 };
 
@@ -27,7 +28,7 @@ pub(crate) async fn switch_composefs(
 
     let prog: ProgressWriter = opts.progress.clone().try_into()?;
 
-    let mut do_upgrade_opts = DoUpgradeOpts {
+    let do_upgrade_opts = DoUpgradeOpts {
         soft_reboot: opts.soft_reboot,
         apply: opts.apply,
         download_only: opts.download_opts.download_only,
@@ -42,6 +43,29 @@ pub(crate) async fn switch_composefs(
 
     let target = imgref_for_switch(&opts)?;
 
+    switch_composefs_to(
+        storage,
+        booted_cfs,
+        &host,
+        target,
+        do_upgrade_opts,
+        opts.unified_storage_exp,
+    )
+    .await
+}
+
+/// Switch the booted composefs system to `target`, staging a new deployment.
+///
+/// `unified_storage_exp` forces the use of unified storage; otherwise it is
+/// used when either the booted or the target image is already there.
+pub(crate) async fn switch_composefs_to(
+    storage: &Storage,
+    booted_cfs: &BootedComposefs,
+    host: &Host,
+    target: ImageReference,
+    mut do_upgrade_opts: DoUpgradeOpts,
+    unified_storage_exp: bool,
+) -> Result<()> {
     let new_spec = {
         let mut new_spec = host.spec.clone();
         new_spec.image = Some(target.clone());
@@ -50,7 +74,7 @@ pub(crate) async fn switch_composefs(
 
     if new_spec == host.spec {
         println!("Image specification is unchanged.");
-        if opts.apply && host.status.staged.is_some() {
+        if do_upgrade_opts.apply && host.status.staged.is_some() {
             crate::reboot::reboot()?;
         }
         return Ok(());
@@ -66,9 +90,8 @@ pub(crate) async fn switch_composefs(
         message_id = COMPOSEFS_SWITCH_JOURNAL_ID,
         bootc.operation = "switch",
         bootc.target_image = target_imgref.to_string(),
-        bootc.apply_mode = opts.apply,
-        bootc.download_only = opts.download_opts.download_only,
-        bootc.from_downloaded = opts.download_opts.from_downloaded,
+        bootc.apply_mode = do_upgrade_opts.apply,
+        bootc.download_only = do_upgrade_opts.download_only,
         "Starting composefs switch operation",
     );
 
@@ -78,7 +101,7 @@ pub(crate) async fn switch_composefs(
     // target image is already in bootc-owned containers-storage, OR the booted
     // image is — which means the user has opted into unified storage and all
     // subsequent operations (including switch to a new image) should use it.
-    do_upgrade_opts.use_unified = if opts.unified_storage_exp {
+    do_upgrade_opts.use_unified = if unified_storage_exp {
         true
     } else {
         let booted_imgref = host.spec.image.as_ref();
@@ -98,7 +121,7 @@ pub(crate) async fn switch_composefs(
         let action = validate_update(
             storage,
             booted_cfs,
-            &host,
+            host,
             img_config.manifest.config().digest().as_ref(),
             &cfg_verity,
             true,
@@ -114,7 +137,7 @@ pub(crate) async fn switch_composefs(
                 return do_upgrade(
                     storage,
                     booted_cfs,
-                    &host,
+                    host,
                     &target_imgref,
                     &do_upgrade_opts,
                     &img_config.manifest,
@@ -127,7 +150,7 @@ pub(crate) async fn switch_composefs(
     do_upgrade(
         storage,
         booted_cfs,
-        &host,
+        host,
         &target_imgref,
         &do_upgrade_opts,
         &img_config.manifest,

crates/lib/src/cli.rs

@@ -47,17 +47,18 @@ use crate::bootc_composefs::{
     finalize::{composefs_backend_finalize, get_etc_diff},
     rollback::composefs_rollback,
     state::composefs_usr_overlay,
-    switch::switch_composefs,
-    update::upgrade_composefs,
+    status::get_composefs_status,
+    switch::{switch_composefs, switch_composefs_to},
+    update::{DoUpgradeOpts, upgrade_composefs},
 };
 use crate::deploy::{MergeState, RequiredHostSpec};
 use crate::podstorage::set_additional_image_store;
 use crate::progress_jsonl::{ProgressWriter, RawProgressFd};
 use crate::spec::FilesystemOverlayAccessMode;
-use crate::spec::Host;
 use crate::spec::ImageReference;
+use crate::spec::{Host, HostSpec};
 use crate::status::get_host;
-use crate::store::{BootedOstree, Storage};
+use crate::store::{BootedComposefs, BootedOstree, Storage};
 use crate::store::{BootedStorage, BootedStorageKind};
 use crate::utils::sigpolicy_from_opt;
 use crate::{bootc_composefs, lints};
@@ -1763,6 +1764,39 @@ async fn rollback(opts: &RollbackOpts) -> Result<()> {
     }
 }
 
+/// Read the edited host definition for `bootc edit`, either from `filename` or
+/// by spawning an editor on the current one, and validate it with
+/// [`validate_edited_spec`].
+fn edited_host_spec(filename: Option<&str>, host: &Host) -> Result<Option<HostSpec>> {
+    let new_host: Host = if let Some(filename) = filename {
+        let f = std::fs::File::open(filename).with_context(|| format!("Opening {filename}"))?;
+        serde_yaml::from_reader(std::io::BufReader::new(f))
+            .with_context(|| format!("Parsing {filename}"))?
+    } else {
+        let tmpf = tempfile::NamedTempFile::with_suffix(".yaml")?;
+        serde_yaml::to_writer(std::io::BufWriter::new(tmpf.as_file()), host)?;
+        crate::utils::spawn_editor(&tmpf)?;
+        tmpf.as_file().seek(std::io::SeekFrom::Start(0))?;
+        serde_yaml::from_reader(&mut tmpf.as_file()).context("Parsing edited host")?
+    };
+
+    let r = validate_edited_spec(&host.spec, new_host.spec)?;
+    if r.is_none() {
+        println!("Edit cancelled, no changes made.");
+    }
+    Ok(r)
+}
+
+/// Returns `None` if the edited spec is unchanged; otherwise the new spec,
+/// after checking that it is a supported transition from the current one.
+fn validate_edited_spec(current: &HostSpec, new: HostSpec) -> Result<Option<HostSpec>> {
+    if &new == current {
+        return Ok(None);
+    }
+    current.verify_transition(&new)?;
+    Ok(Some(new))
+}
+
 /// Implementation of the `bootc edit` CLI command for ostree backend.
 #[context("Editing spec (ostree)")]
 async fn edit_ostree(
@@ -1773,32 +1807,19 @@ async fn edit_ostree(
     let repo = &booted_ostree.repo();
     let (_, host) = crate::status::get_status(booted_ostree)?;
 
-    let new_host: Host = if let Some(filename) = opts.filename {
-        let mut r = std::io::BufReader::new(std::fs::File::open(filename)?);
-        serde_yaml::from_reader(&mut r)?
-    } else {
-        let tmpf = tempfile::NamedTempFile::with_suffix(".yaml")?;
-        serde_yaml::to_writer(std::io::BufWriter::new(tmpf.as_file()), &host)?;
-        crate::utils::spawn_editor(&tmpf)?;
-        tmpf.as_file().seek(std::io::SeekFrom::Start(0))?;
-        serde_yaml::from_reader(&mut tmpf.as_file())?
-    };
-
-    if new_host.spec == host.spec {
-        println!("Edit cancelled, no changes made.");
+    let Some(new_spec) = edited_host_spec(opts.filename.as_deref(), &host)? else {
         return Ok(());
-    }
-    host.spec.verify_transition(&new_host.spec)?;
-    let new_spec = RequiredHostSpec::from_spec(&new_host.spec)?;
-
-    let prog = ProgressWriter::default();
+    };
 
     // We only support two state transitions right now; switching the image,
     // or flipping the bootloader ordering.
-    if host.spec.boot_order != new_host.spec.boot_order {
+    if host.spec.boot_order != new_spec.boot_order {
         return crate::deploy::rollback(storage).await;
     }
 
+    let new_spec = RequiredHostSpec::from_spec(&new_spec)?;
+    let prog = ProgressWriter::default();
+
     let fetched = crate::deploy::pull(
         repo,
         new_spec.image,
@@ -1820,6 +1841,47 @@ async fn edit_ostree(
     Ok(())
 }
 
+/// Implementation of the `bootc edit` CLI command for composefs backend.
+#[context("Editing spec (composefs)")]
+async fn edit_composefs(
+    opts: EditOpts,
+    storage: &Storage,
+    booted_cfs: &BootedComposefs,
+) -> Result<()> {
+    let host = get_composefs_status(storage, booted_cfs)
+        .await
+        .context("Getting composefs deployment status")?;
+
+    let Some(new_spec) = edited_host_spec(opts.filename.as_deref(), &host)? else {
+        return Ok(());
+    };
+
+    // As for ostree, the supported transitions are flipping the boot order
+    // (a rollback) or changing the image; verify_transition rejected doing both.
+    if host.spec.boot_order != new_spec.boot_order {
+        return composefs_rollback(storage, booted_cfs).await;
+    }
+
+    let new_spec = RequiredHostSpec::from_spec(&new_spec)?;
+    let do_upgrade_opts = DoUpgradeOpts {
+        apply: false,
+        soft_reboot: None,
+        download_only: false,
+        use_unified: false,
+        quiet: opts.quiet,
+        prog: ProgressWriter::default(),
+    };
+    switch_composefs_to(
+        storage,
+        booted_cfs,
+        &host,
+        new_spec.image.clone(),
+        do_upgrade_opts,
+        false,
+    )
+    .await
+}
+
 /// Implementation of the `bootc edit` CLI command.
 #[context("Editing spec")]
 async fn edit(opts: EditOpts) -> Result<()> {
@@ -1828,8 +1890,8 @@ async fn edit(opts: EditOpts) -> Result<()> {
         BootedStorageKind::Ostree(booted_ostree) => {
             edit_ostree(opts, storage, &booted_ostree).await
         }
-        BootedStorageKind::Composefs(_) => {
-            anyhow::bail!("Edit is not yet supported for composefs backend")
+        BootedStorageKind::Composefs(booted_cfs) => {
+            edit_composefs(opts, storage, &booted_cfs).await
         }
     }
 }
@@ -2781,6 +2843,43 @@ async fn run_from_opt(opt: Opt) -> Result<CliExitStatus> {
 mod tests {
     use super::*;
 
+    #[test]
+    fn test_validate_edited_spec() {
+        use crate::spec::BootOrder;
+
+        let host: Host =
+            serde_yaml::from_str(include_str!("fixtures/spec-staged-rollback.yaml")).unwrap();
+        let current = &host.spec;
+        let other_image = || ImageReference {
+            image: "quay.io/example/other:latest".into(),
+            transport: "registry".into(),
+            signature: None,
+        };
+        let spec = |image: Option<ImageReference>, boot_order| HostSpec { image, boot_order };
+
+        // (edited spec, whether a change is expected, or None for an error)
+        let cases = [
+            (current.clone(), Some(false)),
+            (spec(Some(other_image()), BootOrder::Default), Some(true)),
+            (spec(current.image.clone(), BootOrder::Rollback), Some(true)),
+            (spec(None, BootOrder::Default), Some(true)),
+            (spec(Some(other_image()), BootOrder::Rollback), None),
+        ];
+        for (new, expected) in cases {
+            let r = validate_edited_spec(current, new.clone());
+            match expected {
+                Some(changed) => {
+                    let r = r.unwrap();
+                    assert_eq!(r.is_some(), changed, "{new:?}");
+                    if let Some(r) = r {
+                        assert_eq!(r, new);
+                    }
+                }
+                None => assert!(r.is_err(), "{new:?}"),
+            }
+        }
+    }
+
     #[test]
     fn test_callname() {
         use std::os::unix::ffi::OsStrExt;

tmt/plans/integration.fmf

@@ -331,4 +331,11 @@ execute:
     enabled: true
   extra-try_bind_storage: true
   extra-skip_if_ostree: true
+
+/plan-50-edit:
+  summary: Test bootc edit for image changes and rollback
+  discover:
+    how: fmf
+    test:
+      - /tmt/tests/tests/test-50-edit
 # END GENERATED PLANS