compose: change opt_usrlocal_overlays to be an enum

xref ostreedev/ostree-rs-ext#607

We need to support having `/opt` and `/usr/local` be directly
on the rootfs.  Change the existing knob for this to be more
configurable.  The stateoverlay path is now an internal-only
knob; we expect people to do that externally.

ci/prow/fcos-e2e.sh

@@ -9,9 +9,9 @@ ls -al /usr/bin/rpm-ostree
 rpm-ostree --version
 cd $(mktemp -d)
 cosa init https://github.com/coreos/fedora-coreos-config/
-# let's turn on opt-usrlocal-overlays in this test since CoreOS CI already
+# let's turn on stateoverlays in this test since CoreOS CI already
 # covers the off path
-echo -e '\nopt-usrlocal-overlays: true\n' >> src/config/manifest.yaml
+echo -e '\nopt-usrlocal: "stateoverlay"\n' >> src/config/manifest.yaml
 cp /cosa/component-rpms/*.rpm overrides/rpm
 # XXX: temporarily import new ostree until it makes it into FCOS
 (cd overrides/rpm && curl -L --remote-name-all https://kojipkgs.fedoraproject.org//packages/ostree/2024.2/1.fc39/x86_64/ostree-{,libs-}2024.2-1.fc39.x86_64.rpm)

docs/treefile.md

@@ -479,9 +479,9 @@ version of `rpm-ostree`.
    names to use when substituting variables in yum repo files. The `releasever`
    variable name is invalid. Use the `releasever` key instead. The `basearch`
    name is invalid; it is filled in automatically.
- * `opt-usrlocal-overlays`: boolean, optional: Defaults to `false`.  By
-   default, `/opt` and `/usr/local` are symlinks to subdirectories in `/
-   var`. This prevents the ability to compose with packages that install in
-   those directories. If enabled, RPMs with `/opt` and `/usr/local` content
-   are allowed; client-side, both paths are writable overlay directories on.
-   Requires libostree v2023.9+.
+ * `opt-usrlocal`: enum, optional: Defaults to `var`.  There are
+   two possible behaviors:
+   - `var`: `/opt` and `/usr/local` are symlinks to subdirectories in `/var`
+     and are purely machine-local state.
+   - `root`: These are plain directories; only use this with composefs enabled!
+

rpmostree-cxxrs.cxx

@@ -1312,6 +1312,7 @@ enum class RepoMetadataTarget : ::std::uint8_t;
 struct Refspec;
 enum class OverrideReplacementType : ::std::uint8_t;
 struct OverrideReplacement;
+enum class OptUsrLocal : ::std::uint8_t;
 struct Treefile;
 struct RepoPackage;
 struct LiveApplyState;
@@ -1708,6 +1709,16 @@ struct OverrideReplacement final
 };
 #endif // CXXBRIDGE1_STRUCT_rpmostreecxx$OverrideReplacement
 
+#ifndef CXXBRIDGE1_ENUM_rpmostreecxx$OptUsrLocal
+#define CXXBRIDGE1_ENUM_rpmostreecxx$OptUsrLocal
+enum class OptUsrLocal : ::std::uint8_t
+{
+  Var = 0,
+  Root = 1,
+  StateOverlay = 2,
+};
+#endif // CXXBRIDGE1_ENUM_rpmostreecxx$OptUsrLocal
+
 #ifndef CXXBRIDGE1_STRUCT_rpmostreecxx$Treefile
 #define CXXBRIDGE1_STRUCT_rpmostreecxx$Treefile
 struct Treefile final : public ::rust::Opaque
@@ -1773,7 +1784,7 @@ struct Treefile final : public ::rust::Opaque
   ::rpmostreecxx::RepoMetadataTarget get_repo_metadata_target () const noexcept;
   bool rpmdb_backend_is_target () const noexcept;
   bool should_normalize_rpmdb () const noexcept;
-  bool get_opt_usrlocal_overlays () const noexcept;
+  ::rpmostreecxx::OptUsrLocal get_opt_usrlocal () const noexcept;
   ::rust::Vec< ::rust::String> get_files_remove_regex (::rust::Str package) const noexcept;
   ::rust::String get_checksum (::rpmostreecxx::OstreeRepo const &repo) const;
   ::rust::String get_ostree_ref () const noexcept;
@@ -2201,8 +2212,10 @@ extern "C"
   ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$convert_var_to_tmpfiles_d (
       ::std::int32_t rootfs_dfd, ::rpmostreecxx::GCancellable const &cancellable) noexcept;
 
-  ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$rootfs_prepare_links (::std::int32_t rootfs_dfd,
-                                                                     bool skip_usrlocal) noexcept;
+  ::rust::repr::PtrLen
+  rpmostreecxx$cxxbridge1$rootfs_prepare_links (::std::int32_t rootfs_dfd,
+                                                ::rpmostreecxx::Treefile const &treefile,
+                                                bool skip_usrlocal) noexcept;
 
   ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$workaround_selinux_cross_labeling (
       ::std::int32_t rootfs_dfd, ::rpmostreecxx::GCancellable &cancellable) noexcept;
@@ -2647,8 +2660,8 @@ extern "C"
   bool rpmostreecxx$cxxbridge1$Treefile$should_normalize_rpmdb (
       ::rpmostreecxx::Treefile const &self) noexcept;
 
-  bool rpmostreecxx$cxxbridge1$Treefile$get_opt_usrlocal_overlays (
-      ::rpmostreecxx::Treefile const &self) noexcept;
+  ::rpmostreecxx::OptUsrLocal
+  rpmostreecxx$cxxbridge1$Treefile$get_opt_usrlocal (::rpmostreecxx::Treefile const &self) noexcept;
 
   void rpmostreecxx$cxxbridge1$Treefile$get_files_remove_regex (
       ::rpmostreecxx::Treefile const &self, ::rust::Str package,
@@ -4045,10 +4058,11 @@ convert_var_to_tmpfiles_d (::std::int32_t rootfs_dfd,
 }
 
 void
-rootfs_prepare_links (::std::int32_t rootfs_dfd, bool skip_usrlocal)
+rootfs_prepare_links (::std::int32_t rootfs_dfd, ::rpmostreecxx::Treefile const &treefile,
+                      bool skip_usrlocal)
 {
   ::rust::repr::PtrLen error$
-      = rpmostreecxx$cxxbridge1$rootfs_prepare_links (rootfs_dfd, skip_usrlocal);
+      = rpmostreecxx$cxxbridge1$rootfs_prepare_links (rootfs_dfd, treefile, skip_usrlocal);
   if (error$.ptr)
     {
       throw ::rust::impl< ::rust::Error>::error (error$);
@@ -5270,10 +5284,10 @@ Treefile::should_normalize_rpmdb () const noexcept
   return rpmostreecxx$cxxbridge1$Treefile$should_normalize_rpmdb (*this);
 }
 
-bool
-Treefile::get_opt_usrlocal_overlays () const noexcept
+::rpmostreecxx::OptUsrLocal
+Treefile::get_opt_usrlocal () const noexcept
 {
-  return rpmostreecxx$cxxbridge1$Treefile$get_opt_usrlocal_overlays (*this);
+  return rpmostreecxx$cxxbridge1$Treefile$get_opt_usrlocal (*this);
 }
 
 ::rust::Vec< ::rust::String>

rpmostree-cxxrs.h

@@ -1094,6 +1094,7 @@ enum class RepoMetadataTarget : ::std::uint8_t;
 struct Refspec;
 enum class OverrideReplacementType : ::std::uint8_t;
 struct OverrideReplacement;
+enum class OptUsrLocal : ::std::uint8_t;
 struct Treefile;
 struct RepoPackage;
 struct LiveApplyState;
@@ -1490,6 +1491,16 @@ struct OverrideReplacement final
 };
 #endif // CXXBRIDGE1_STRUCT_rpmostreecxx$OverrideReplacement
 
+#ifndef CXXBRIDGE1_ENUM_rpmostreecxx$OptUsrLocal
+#define CXXBRIDGE1_ENUM_rpmostreecxx$OptUsrLocal
+enum class OptUsrLocal : ::std::uint8_t
+{
+  Var = 0,
+  Root = 1,
+  StateOverlay = 2,
+};
+#endif // CXXBRIDGE1_ENUM_rpmostreecxx$OptUsrLocal
+
 #ifndef CXXBRIDGE1_STRUCT_rpmostreecxx$Treefile
 #define CXXBRIDGE1_STRUCT_rpmostreecxx$Treefile
 struct Treefile final : public ::rust::Opaque
@@ -1555,7 +1566,7 @@ struct Treefile final : public ::rust::Opaque
   ::rpmostreecxx::RepoMetadataTarget get_repo_metadata_target () const noexcept;
   bool rpmdb_backend_is_target () const noexcept;
   bool should_normalize_rpmdb () const noexcept;
-  bool get_opt_usrlocal_overlays () const noexcept;
+  ::rpmostreecxx::OptUsrLocal get_opt_usrlocal () const noexcept;
   ::rust::Vec< ::rust::String> get_files_remove_regex (::rust::Str package) const noexcept;
   ::rust::String get_checksum (::rpmostreecxx::OstreeRepo const &repo) const;
   ::rust::String get_ostree_ref () const noexcept;
@@ -1866,7 +1877,8 @@ void compose_postprocess_final (::std::int32_t rootfs_dfd,
 void convert_var_to_tmpfiles_d (::std::int32_t rootfs_dfd,
                                 ::rpmostreecxx::GCancellable const &cancellable);
 
-void rootfs_prepare_links (::std::int32_t rootfs_dfd, bool skip_usrlocal);
+void rootfs_prepare_links (::std::int32_t rootfs_dfd, ::rpmostreecxx::Treefile const &treefile,
+                           bool skip_usrlocal);
 
 void workaround_selinux_cross_labeling (::std::int32_t rootfs_dfd,
                                         ::rpmostreecxx::GCancellable &cancellable);

rust/src/composepost.rs

@@ -12,7 +12,7 @@ use crate::ffi::BubblewrapMutability;
 use crate::ffiutil::ffi_dirfd;
 use crate::normalization;
 use crate::passwd::PasswdDB;
-use crate::treefile::Treefile;
+use crate::treefile::{OptUsrLocal, Treefile};
 use crate::{bwrap, importer};
 use anyhow::{anyhow, bail, format_err, Context, Result};
 use camino::{Utf8Path, Utf8PathBuf};
@@ -148,31 +148,33 @@ fn compose_init_rootfs_transient(rootfs_dfd: &cap_std::fs::Dir) -> Result<()> {
 fn compose_init_rootfs_strict(
     rootfs_dfd: &cap_std::fs::Dir,
     tmp_is_dir: bool,
-    opt_state_overlay: bool,
+    opt_usrlocal: OptUsrLocal,
 ) -> Result<()> {
     println!("Initializing rootfs");
 
     compose_init_rootfs_base(rootfs_dfd, tmp_is_dir)?;
 
-    const OPT_SYMLINK_LEGACY: &str = "var/opt";
-    const OPT_SYMLINK_STATEOVERLAY: &str = "usr/lib/opt";
-    let opt_symlink = if opt_state_overlay {
-        OPT_SYMLINK_STATEOVERLAY
-    } else {
-        OPT_SYMLINK_LEGACY
+    let opt_symlink = match opt_usrlocal {
+        OptUsrLocal::Var => Some("var/opt"),
+        OptUsrLocal::Root => {
+            rootfs_dfd.create_dir_all("opt")?;
+            None
+        }
+        OptUsrLocal::StateOverlay => Some("usr/lib/opt"),
     };
 
     // This is used in the case where we don't have a transient rootfs; redirect
     // these toplevel directories underneath /var.
-    let ostree_strict_mode_symlinks: &[(&str, &str)] = &[
-        (opt_symlink, "opt"),
+    let ostree_strict_mode_symlinks = [
         ("var/srv", "srv"),
         ("var/mnt", "mnt"),
         ("run/media", "media"),
     ];
+
     ostree_strict_mode_symlinks
-        .par_iter()
-        .try_for_each(|&(dest, src)| {
+        .into_iter()
+        .chain(opt_symlink.map(|link| (link, "opt")))
+        .try_for_each(|(dest, src)| {
             rootfs_dfd
                 .symlink(dest, src)
                 .with_context(|| format!("Creating {src}"))
@@ -230,7 +232,8 @@ pub fn compose_prepare_rootfs(
         treefile
             .parsed
             .base
-            .opt_usrlocal_overlays
+            .opt_usrlocal
+            .clone()
             .unwrap_or_default(),
     )?;
 
@@ -679,12 +682,10 @@ pub fn compose_postprocess(
         compose_postprocess_default_target(rootfs, t)?;
     }
 
-    if treefile
-        .parsed
-        .base
-        .opt_usrlocal_overlays
-        .unwrap_or_default()
-    {
+    if matches!(
+        treefile.parsed.base.opt_usrlocal,
+        Some(OptUsrLocal::StateOverlay)
+    ) {
         compose_postprocess_state_overlays(rootfs)?;
     }
 
@@ -1034,13 +1035,18 @@ fn state_overlay_enabled(rootfs_dfd: &cap_std::fs::Dir, state_overlay: &str) ->
 ///  - If present, symlink /var/lib/alternatives -> /usr/lib/alternatives
 ///  - If present, symlink /var/lib/vagrant -> /usr/lib/vagrant
 #[context("Preparing symlinks in rootfs")]
-pub fn rootfs_prepare_links(rootfs_dfd: i32, skip_usrlocal: bool) -> CxxResult<()> {
+pub fn rootfs_prepare_links(
+    rootfs_dfd: i32,
+    treefile: &Treefile,
+    skip_usrlocal: bool,
+) -> CxxResult<()> {
     let rootfs = unsafe { &crate::ffiutil::ffi_dirfd(rootfs_dfd)? };
     let mut db = dirbuilder_from_mode(0o755);
     db.recursive(true);
 
     if !skip_usrlocal {
-        if state_overlay_enabled(rootfs, "usr-local")? {
+        let usrlocal_root = matches!(treefile.parsed.base.opt_usrlocal, Some(OptUsrLocal::Root));
+        if usrlocal_root || state_overlay_enabled(rootfs, "usr-local")? {
             // because of the filesystem lua issue (see
             // compose_init_rootfs_base()) we need to create this manually
             rootfs.ensure_dir_with("usr/local", &db)?;
@@ -1636,16 +1642,19 @@ OSTREE_VERSION='33.4'
     }
 
     #[test]
-    fn test_prepare_symlinks() {
-        let rootfs = cap_tempfile::tempdir(cap_std::ambient_authority()).unwrap();
+    fn test_prepare_symlinks() -> Result<()> {
+        let tempdir = tempfile::tempdir()?;
+        let tempdir: &Utf8Path = tempdir.path().try_into().unwrap();
+        let tf = crate::treefile::tests::new_test_treefile(tempdir, "", None)?;
+        let rootfs = Dir::open_ambient_dir(tempdir, cap_std::ambient_authority())?;
         let mut db = DirBuilder::new();
         db.recursive(true);
         db.mode(0o755);
         rootfs.ensure_dir_with("usr/local", &db).unwrap();
         rootfs.ensure_dir_with("var/lib/alternatives", &db).unwrap();
         rootfs.ensure_dir_with("var/lib/vagrant", &db).unwrap();
 
-        rootfs_prepare_links(rootfs.as_raw_fd(), false).unwrap();
+        rootfs_prepare_links(rootfs.as_raw_fd(), &tf, false).unwrap();
         {
             let usr_dir = rootfs.open_dir("usr").unwrap();
             let local_target = usr_dir.read_link("local").unwrap();
@@ -1663,6 +1672,7 @@ OSTREE_VERSION='33.4'
                 assert_eq!(target.unwrap().to_str(), Some(*content));
             }
         }
+        Ok(())
     }
 
     #[test]

rust/src/lib.rs

@@ -300,7 +300,11 @@ pub mod ffi {
         fn compose_postprocess_final_pre(rootfs_dfd: i32) -> Result<()>;
         fn compose_postprocess_final(rootfs_dfd: i32, treefile: &Treefile) -> Result<()>;
         fn convert_var_to_tmpfiles_d(rootfs_dfd: i32, cancellable: &GCancellable) -> Result<()>;
-        fn rootfs_prepare_links(rootfs_dfd: i32, skip_usrlocal: bool) -> Result<()>;
+        fn rootfs_prepare_links(
+            rootfs_dfd: i32,
+            treefile: &Treefile,
+            skip_usrlocal: bool,
+        ) -> Result<()>;
         fn workaround_selinux_cross_labeling(
             rootfs_dfd: i32,
             cancellable: Pin<&mut GCancellable>,
@@ -547,6 +551,14 @@ pub mod ffi {
         packages: Vec<String>,
     }
 
+    // treefile.rs
+    #[derive(Debug)]
+    enum OptUsrLocal {
+        Var,
+        Root,
+        StateOverlay,
+    }
+
     extern "Rust" {
         type Treefile;
 
@@ -627,7 +639,7 @@ pub mod ffi {
         fn get_repo_metadata_target(&self) -> RepoMetadataTarget;
         fn rpmdb_backend_is_target(&self) -> bool;
         fn should_normalize_rpmdb(&self) -> bool;
-        fn get_opt_usrlocal_overlays(&self) -> bool;
+        fn get_opt_usrlocal(&self) -> OptUsrLocal;
         fn get_files_remove_regex(&self, package: &str) -> Vec<String>;
         fn get_checksum(&self, repo: &OstreeRepo) -> Result<String>;
         fn get_ostree_ref(&self) -> String;

rust/src/treefile.rs

@@ -427,7 +427,7 @@ fn treefile_merge(dest: &mut TreeComposeConfig, src: &mut TreeComposeConfig) {
         documentation,
         boot_location,
         tmp_is_dir,
-        opt_usrlocal_overlays,
+        opt_usrlocal,
         default_target,
         machineid_compat,
         releasever,
@@ -1412,8 +1412,8 @@ impl Treefile {
         self.parsed.base.rpmdb_normalize.unwrap_or(false)
     }
 
-    pub(crate) fn get_opt_usrlocal_overlays(&self) -> bool {
-        self.parsed.base.opt_usrlocal_overlays.unwrap_or_default()
+    pub(crate) fn get_opt_usrlocal(&self) -> crate::ffi::OptUsrLocal {
+        self.parsed.base.opt_usrlocal.unwrap_or_default().into()
     }
 
     pub(crate) fn get_files_remove_regex(&self, package: &str) -> Vec<String> {
@@ -2405,6 +2405,31 @@ impl From<RepoMetadataTarget> for crate::ffi::RepoMetadataTarget {
     }
 }
 
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy)]
+#[serde(rename_all = "kebab-case")]
+pub(crate) enum OptUsrLocal {
+    Var,
+    Root,
+    #[serde(alias = "stateoverlay")]
+    StateOverlay,
+}
+
+impl Default for OptUsrLocal {
+    fn default() -> Self {
+        Self::Var
+    }
+}
+
+impl From<OptUsrLocal> for crate::ffi::OptUsrLocal {
+    fn from(target: OptUsrLocal) -> Self {
+        match target {
+            OptUsrLocal::Var => Self::Var,
+            OptUsrLocal::Root => Self::Root,
+            OptUsrLocal::StateOverlay => Self::StateOverlay,
+        }
+    }
+}
+
 #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)]
 #[serde(rename_all = "kebab-case")]
 /// The database backend; see https://github.com/coreos/fedora-coreos-tracker/issues/609
@@ -2537,7 +2562,7 @@ pub(crate) struct BaseComposeConfigFields {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub(crate) tmp_is_dir: Option<bool>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub(crate) opt_usrlocal_overlays: Option<bool>,
+    pub(crate) opt_usrlocal: Option<OptUsrLocal>,
 
     // systemd
     #[serde(skip_serializing_if = "Option::is_none")]

src/app/rpmostree-compose-builtin-tree.cxx

@@ -498,7 +498,7 @@ install_packages (RpmOstreeTreeComposeContext *self, gboolean *out_unmodified,
                           cancellable, error))
     return FALSE;
 
-  if ((*self->treefile_rs)->get_opt_usrlocal_overlays ())
+  if ((*self->treefile_rs)->get_opt_usrlocal () == rpmostreecxx::OptUsrLocal::StateOverlay)
     {
       if (!glnx_file_copy_at (
               pkglibdir_dfd, "rpm-ostree-0-integration-opt-usrlocal-compat.conf", NULL, rootfs_dfd,