Skip to content

Commit

Permalink
compose: change opt_usrlocal_overlays to be an enum
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cgwalters committed Mar 14, 2024
1 parent 6be42a6 commit 9bde7ca
Show file tree
Hide file tree
Showing 16 changed files with 140 additions and 57 deletions.
4 changes: 2 additions & 2 deletions ci/prow/fcos-e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions docs/treefile.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!

34 changes: 24 additions & 10 deletions rpmostree-cxxrs.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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$);
Expand Down Expand Up @@ -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>
Expand Down
16 changes: 14 additions & 2 deletions rpmostree-cxxrs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
58 changes: 34 additions & 24 deletions rust/src/composepost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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}"))
Expand Down Expand Up @@ -230,7 +232,8 @@ pub fn compose_prepare_rootfs(
treefile
.parsed
.base
.opt_usrlocal_overlays
.opt_usrlocal
.clone()
.unwrap_or_default(),
)?;

Expand Down Expand Up @@ -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)?;
}

Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -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();
Expand All @@ -1663,6 +1672,7 @@ OSTREE_VERSION='33.4'
assert_eq!(target.unwrap().to_str(), Some(*content));
}
}
Ok(())
}

#[test]
Expand Down
16 changes: 14 additions & 2 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -547,6 +551,14 @@ pub mod ffi {
packages: Vec<String>,
}

// treefile.rs
#[derive(Debug)]
enum OptUsrLocal {
Var,
Root,
StateOverlay,
}

extern "Rust" {
type Treefile;

Expand Down Expand Up @@ -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;
Expand Down
33 changes: 29 additions & 4 deletions rust/src/treefile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")]
Expand Down
2 changes: 1 addition & 1 deletion src/app/rpmostree-compose-builtin-tree.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 9bde7ca

Please sign in to comment.