From 46516cd659c3937e474380143cf7ba4ded88fda8 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Wed, 10 Jul 2024 20:38:26 -0400
Subject: [PATCH] kargs: Use merge deployment

This is in general for us right now a no-op, but is closer
to technically correct. In ostree the "merge deployment"
concept started out as "deployment we use for the original /etc"
which *may be different* from the booted deployment. For example,
when one wants to do a "factory reset", we want to start
from a fresh /etc.

The other important case here is when we're doing a fresh
install (or into a new stateroot) there may be no merge
deployment at all! It wouldn't be correct to look at `/`.

We only sidestep this issue right now because the install
logic bypasses this to directly gather kargs...and doesn't
use the same `deploy` API as inplace updates. But we
want to get closer to doing things that way.

Signed-off-by: Colin Walters <walters@verbum.org>
---
 lib/src/deploy.rs |  6 +-----
 lib/src/kargs.rs  | 18 ++++++++++--------
 lib/src/utils.rs  | 20 ++++++++++++++++++++
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/lib/src/deploy.rs b/lib/src/deploy.rs
index 7d50a31cc..ff66562c3 100644
--- a/lib/src/deploy.rs
+++ b/lib/src/deploy.rs
@@ -335,11 +335,7 @@ async fn deploy(
     // is a distinct minor issue, but not super important as right now the install path
     // doesn't use this API).
     let override_kargs = if let Some(deployment) = merge_deployment {
-        Some(crate::kargs::get_kargs(
-            &sysroot.repo(),
-            &deployment,
-            image,
-        )?)
+        Some(crate::kargs::get_kargs(sysroot, &deployment, image)?)
     } else {
         None
     };
diff --git a/lib/src/kargs.rs b/lib/src/kargs.rs
index 5dac9158e..eb87b05a7 100644
--- a/lib/src/kargs.rs
+++ b/lib/src/kargs.rs
@@ -1,6 +1,5 @@
 use anyhow::{Context, Result};
 use camino::Utf8Path;
-use cap_std_ext::cap_std;
 use cap_std_ext::cap_std::fs::Dir;
 use cap_std_ext::dirext::CapStdExtDirExt;
 use ostree::gio;
@@ -9,6 +8,7 @@ use ostree_ext::ostree::Deployment;
 use ostree_ext::prelude::Cast;
 use ostree_ext::prelude::FileEnumeratorExt;
 use ostree_ext::prelude::FileExt;
+use ostree_ext::sysroot::SysrootLock;
 use serde::Deserialize;
 
 use crate::deploy::ImageState;
@@ -101,25 +101,26 @@ fn get_kargs_from_ostree(
 /// karg, but applies the diff between the bootc karg files in /usr/lib/bootc/kargs.d
 /// between the booted deployment and the new one.
 pub(crate) fn get_kargs(
-    repo: &ostree::Repo,
-    booted_deployment: &Deployment,
+    sysroot: &SysrootLock,
+    merge_deployment: &Deployment,
     fetched: &ImageState,
 ) -> Result<Vec<String>> {
     let cancellable = gio::Cancellable::NONE;
+    let repo = &sysroot.repo();
     let mut kargs: Vec<String> = vec![];
     let sys_arch = std::env::consts::ARCH;
 
-    // Get the running kargs of the booted system
-    if let Some(bootconfig) = ostree::Deployment::bootconfig(booted_deployment) {
+    // Get the kargs used for the merge in the bootloader config
+    if let Some(bootconfig) = ostree::Deployment::bootconfig(merge_deployment) {
         if let Some(options) = ostree::BootconfigParser::get(&bootconfig, "options") {
             let options = options.split_whitespace().map(|s| s.to_owned());
             kargs.extend(options);
         }
     };
 
-    // Get the kargs in kargs.d of the booted system
-    let root = &cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
-    let existing_kargs: Vec<String> = get_kargs_in_root(root, sys_arch)?;
+    // Get the kargs in kargs.d of the merge
+    let merge_root = &crate::utils::deployment_fd(sysroot, merge_deployment)?;
+    let existing_kargs: Vec<String> = get_kargs_in_root(merge_root, sys_arch)?;
 
     // Get the kargs in kargs.d of the pending image
     let (fetched_tree, _) = repo.read_commit(fetched.ostree_commit.as_str(), cancellable)?;
@@ -179,6 +180,7 @@ fn parse_kargs_toml(contents: &str, sys_arch: &str) -> Result<Vec<String>> {
 
 #[cfg(test)]
 mod tests {
+    use cap_std_ext::cap_std;
     use fn_error_context::context;
     use rustix::fd::{AsFd, AsRawFd};
 
diff --git a/lib/src/utils.rs b/lib/src/utils.rs
index bc50fdfeb..45c72fdbb 100644
--- a/lib/src/utils.rs
+++ b/lib/src/utils.rs
@@ -1,9 +1,11 @@
 use std::future::Future;
 use std::io::Write;
+use std::os::fd::BorrowedFd;
 use std::process::Command;
 use std::time::Duration;
 
 use anyhow::{Context, Result};
+use cap_std_ext::cap_std::fs::Dir;
 use ostree::glib;
 use ostree_ext::container::SignatureSource;
 use ostree_ext::ostree;
@@ -21,6 +23,24 @@ pub(crate) fn origin_has_rpmostree_stuff(kf: &glib::KeyFile) -> bool {
     false
 }
 
+// Access the file descriptor for a sysroot
+#[allow(unsafe_code)]
+pub(crate) fn sysroot_fd(sysroot: &ostree::Sysroot) -> BorrowedFd {
+    unsafe { BorrowedFd::borrow_raw(sysroot.fd()) }
+}
+
+// Return a cap-std `Dir` type for a deployment.
+// TODO: in the future this should perhaps actually mount via composefs
+#[allow(unsafe_code)]
+pub(crate) fn deployment_fd(
+    sysroot: &ostree::Sysroot,
+    deployment: &ostree::Deployment,
+) -> Result<Dir> {
+    let sysroot_dir = &Dir::reopen_dir(&sysroot_fd(sysroot))?;
+    let dirpath = sysroot.deployment_dirpath(deployment);
+    sysroot_dir.open_dir(&dirpath).map_err(Into::into)
+}
+
 /// Given an mount option string list like foo,bar=baz,something=else,ro parse it and find
 /// the first entry like $optname=
 /// This will not match a bare `optname` without an equals.