Skip to content

Commit

Permalink
store: If ostree >= 2024.3, retain content in /var
Browse files Browse the repository at this point in the history
This is intended to pair with ostreedev/ostree#3166

If we detect a new enough ostree version, then by default don't remap
content in `/var`, assuming that ostree itself will handle it.

In order to unit test this (without depending on the ostree version
that happens to be on the host) add an API to the importer
which allows overriding the version.
  • Loading branch information
cgwalters committed Feb 11, 2024
1 parent 660614d commit 4bcc315
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 4 deletions.
9 changes: 9 additions & 0 deletions lib/src/container/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ pub struct ImageImporter {
disable_gc: bool, // If true, don't prune unused image layers
/// If true, require the image has the bootable flag
require_bootable: bool,
/// If true, we have ostree v2024.3 or newer.
ostree_v2024_3: bool,
pub(crate) proxy_img: OpenedImage,

layer_progress: Option<Sender<ImportProgress>>,
Expand Down Expand Up @@ -471,6 +473,7 @@ impl ImageImporter {
proxy_img,
target_imgref: None,
no_imgref: false,
ostree_v2024_3: ostree::check_version(2024, 3),
disable_gc: false,
require_bootable: false,
imgref: imgref.clone(),
Expand All @@ -496,6 +499,11 @@ impl ImageImporter {
self.require_bootable = true;
}

/// Override the ostree version being targeted
pub fn set_ostree_version(&mut self, year: u32, v: u32) {
self.ostree_v2024_3 = (year > 2024) || (year == 2024 && v >= 3)
}

/// Do not prune image layers.
pub fn disable_gc(&mut self) {
self.disable_gc = true;
Expand Down Expand Up @@ -864,6 +872,7 @@ impl ImageImporter {
base: Some(base_commit.clone()),
selinux: true,
allow_nonusr: root_is_transient,
retain_var: self.ostree_v2024_3,
};
let r =
crate::tar::write_tar(&self.repo, blob, layer.ostree_ref.as_str(), Some(opts));
Expand Down
4 changes: 4 additions & 0 deletions lib/src/fixture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,10 @@ impl Fixture {
pub fn clear_destrepo(&self) -> Result<()> {
self.destrepo()
.set_ref_immediate(None, self.testref(), None, gio::Cancellable::NONE)?;
for (r, _) in self.destrepo().list_refs(None, gio::Cancellable::NONE)? {
self.destrepo()
.set_ref_immediate(None, &r, None, gio::Cancellable::NONE)?;
}
self.destrepo()
.prune(ostree::RepoPruneFlags::REFS_ONLY, 0, gio::Cancellable::NONE)?;
Ok(())
Expand Down
31 changes: 27 additions & 4 deletions lib/src/tar/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ pub struct WriteTarOptions {
pub selinux: bool,
/// Allow content not in /usr; this should be paired with ostree rootfs.transient = true
pub allow_nonusr: bool,
/// If true, do not move content in /var to /usr/share/factory/var. This should be used
/// with ostree v2024.3 or newer.
pub retain_var: bool,
}

/// The result of writing a tar stream.
Expand Down Expand Up @@ -112,6 +115,7 @@ enum NormalizedPathResult<'a> {
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub(crate) struct TarImportConfig {
allow_nonusr: bool,
remap_factory_var: bool,
}

fn normalize_validate_path<'a>(
Expand Down Expand Up @@ -150,9 +154,13 @@ fn normalize_validate_path<'a>(
"etc" => {
ret.push("usr/etc");
}
// Content in /var will get copied by a systemd tmpfiles.d unit
"var" => {
ret.push("usr/share/factory/var");
// Content in /var will get copied by a systemd tmpfiles.d unit
if config.remap_factory_var {
ret.push("usr/share/factory/var");
} else {
ret.push(part)
}
}
o if EXCLUDED_TOPLEVEL_PATHS.contains(&o) => {
return Ok(NormalizedPathResult::Filtered(part));
Expand Down Expand Up @@ -373,6 +381,7 @@ pub async fn write_tar(
// Copy the filtered tar stream to child stdin
let mut import_config = TarImportConfig::default();
import_config.allow_nonusr = options.allow_nonusr;
import_config.remap_factory_var = !options.retain_var;
let filtered_result = filter_tar_async(src, child_stdin, &import_config);
let output_copier = async move {
// Gather stdout/stderr to buffers
Expand Down Expand Up @@ -429,8 +438,18 @@ mod tests {

#[test]
fn test_normalize_path() {
let imp_default = &TarImportConfig::default();
let allow_nonusr = &TarImportConfig { allow_nonusr: true };
let imp_default = &TarImportConfig {
allow_nonusr: false,
remap_factory_var: true,
};
let allow_nonusr = &TarImportConfig {
allow_nonusr: true,
remap_factory_var: true,
};
let composefs_and_new_ostree = &TarImportConfig {
allow_nonusr: true,
remap_factory_var: false,
};
let valid_all = &[
("/usr/bin/blah", "./usr/bin/blah"),
("usr/bin/blah", "./usr/bin/blah"),
Expand Down Expand Up @@ -475,6 +494,10 @@ mod tests {
assert!(normalize_validate_path(k.into(), allow_nonusr).is_err());
assert!(normalize_validate_path(k.into(), imp_default).is_err());
}
assert!(matches!(
normalize_validate_path("var/lib/foo".into(), composefs_and_new_ostree).unwrap(),
NormalizedPathResult::Normal(_)
));
}

#[tokio::test]
Expand Down
24 changes: 24 additions & 0 deletions lib/tests/it/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,7 @@ async fn test_container_var_content() -> Result<()> {
};
let mut imp =
store::ImageImporter::new(fixture.destrepo(), &derived_imgref, Default::default()).await?;
imp.set_ostree_version(2023, 11);
let prep = match imp.prepare().await.unwrap() {
store::PrepareResult::AlreadyPresent(_) => panic!("should not be already imported"),
store::PrepareResult::Ready(r) => r,
Expand Down Expand Up @@ -963,6 +964,29 @@ async fn test_container_var_content() -> Result<()> {
.is_none()
);

// Reset things
fixture.clear_destrepo()?;

let mut imp =
store::ImageImporter::new(fixture.destrepo(), &derived_imgref, Default::default()).await?;
imp.set_ostree_version(2024, 3);
let prep = match imp.prepare().await.unwrap() {
store::PrepareResult::AlreadyPresent(_) => panic!("should not be already imported"),
store::PrepareResult::Ready(r) => r,
};
let import = imp.import(prep).await.unwrap();
let ostree_root = fixture
.destrepo()
.read_commit(&import.merge_commit, gio::Cancellable::NONE)?
.0;
let varfile = ostree_root
.child("usr/share/factory/var/lib/foo")
.downcast::<ostree::RepoFile>()
.unwrap();
assert!(!varfile.query_exists(gio::Cancellable::NONE));
assert!(ostree_root
.child("var/lib/foo")
.query_exists(gio::Cancellable::NONE));
Ok(())
}

Expand Down

0 comments on commit 4bcc315

Please sign in to comment.