Skip to content

WIP: install: Handle mount points in rootfs operations #1727

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+283 −11
Open

WIP: install: Handle mount points in rootfs operations #1727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 146 additions & 11 deletions crates/lib/src/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1803,6 +1803,50 @@ pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> {
Ok(())
}

/// Require that a directory contains only mount points (or is empty), recursively.
/// Returns Ok(()) if all entries in the directory tree are either:
/// - Mount points (on different filesystems)
/// - Directories that themselves contain only mount points (recursively)
/// - The lost+found directory
///
/// Returns an error if any non-mount entry is found.
///
/// This handles cases like /var containing /var/lib (not a mount) which contains
/// /var/lib/containers (a mount point).
#[context("Requiring directory contains only mount points")]
fn require_dir_contains_only_mounts(parent_fd: &Dir, dir_name: &str) -> Result<()> {
let Some(dir_fd) = parent_fd.open_dir_noxdev(dir_name)? else {
// The directory itself is a mount point
return Ok(());
};

if dir_fd.entries()?.next().is_none() {
anyhow::bail!("Found empty directory: {dir_name}");
}

for entry in dir_fd.entries()? {
let entry = DirEntryUtf8::from_cap_std(entry?);
let entry_name = entry.file_name()?;

if entry_name == LOST_AND_FOUND {
continue;
}

if dir_name == BOOT && entry_name == crate::bootloader::EFI_DIR {
continue;
}

let etype = entry.file_type()?;
if etype == FileType::dir() && dir_fd.open_dir_noxdev(&entry_name)?.is_some() {
require_dir_contains_only_mounts(&dir_fd, &entry_name)?;
} else {
anyhow::bail!("Found entry in {dir_name}: {entry_name}");
}
}

Ok(())
}

#[context("Verifying empty rootfs")]
fn require_empty_rootdir(rootfs_fd: &Dir) -> Result<()> {
for e in rootfs_fd.entries()? {
Expand All @@ -1811,17 +1855,11 @@ fn require_empty_rootdir(rootfs_fd: &Dir) -> Result<()> {
if name == LOST_AND_FOUND {
continue;
}
// There must be a boot directory (that is empty)
if name == BOOT {
let mut entries = rootfs_fd.read_dir(BOOT)?;
if let Some(e) = entries.next() {
let e = DirEntryUtf8::from_cap_std(e?);
let name = e.file_name()?;
if matches!(name.as_str(), LOST_AND_FOUND | crate::bootloader::EFI_DIR) {
continue;
}
anyhow::bail!("Non-empty boot directory, found {name}");
}

// Check if this entry is a directory
let etype = e.file_type()?;
if etype == FileType::dir() {
require_dir_contains_only_mounts(rootfs_fd, &name)?;
} else {
anyhow::bail!("Non-empty root filesystem; found {name:?}");
}
Expand Down Expand Up @@ -2573,4 +2611,101 @@ UUID=boot-uuid /boot ext4 defaults 0 0

Ok(())
}

#[test]
fn test_require_dir_contains_only_mounts() -> Result<()> {
// Test 1: Empty directory should fail (not a mount point)
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir("empty")?;
assert!(require_dir_contains_only_mounts(&td, "empty").is_err());
}

// Test 2: Directory with only lost+found should succeed (lost+found is ignored)
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir_all("var/lost+found")?;
assert!(require_dir_contains_only_mounts(&td, "var").is_ok());
}

// Test 3: Directory with a regular file should fail
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir("var")?;
td.write("var/test.txt", b"content")?;
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
}

// Test 4: Nested directory structure with a file should fail
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir_all("var/lib/containers")?;
td.write("var/lib/containers/storage.db", b"data")?;
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
}

// Test 5: boot directory with efi subdirectory should succeed (efi is allowed in boot)
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir_all("boot/efi")?;
assert!(require_dir_contains_only_mounts(&td, "boot").is_ok());
}

// Test 6: boot directory with both efi and lost+found should succeed (both are allowed)
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir_all("boot/efi")?;
td.create_dir_all("boot/lost+found")?;
assert!(require_dir_contains_only_mounts(&td, "boot").is_ok());
}

// Test 7: boot directory with grub should fail (grub2 is not a mount and contains files)
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir_all("boot/grub2")?;
td.write("boot/grub2/grub.cfg", b"config")?;
assert!(require_dir_contains_only_mounts(&td, "boot").is_err());
}

// Test 8: Nested empty directories should fail (empty directories are not mount points)
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir_all("var/lib/containers")?;
td.create_dir_all("var/log/journal")?;
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
}

// Test 9: Directory with lost+found and a file should fail (lost+found is ignored, but file is not allowed)
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir_all("var/lost+found")?;
td.write("var/data.txt", b"content")?;
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
}

// Test 10: Directory with a symlink should fail
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir("var")?;
td.symlink_contents("../usr/lib", "var/lib")?;
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
}

// Test 11: Deeply nested directory with a file should fail
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir_all("var/lib/containers/storage/overlay")?;
td.write("var/lib/containers/storage/overlay/file.txt", b"data")?;
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
}

// Test 12: Non-boot directory with efi should fail (special handling only applies to boot/efi)
{
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
td.create_dir_all("var/efi")?;
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
}

Ok(())
}
}
137 changes: 137 additions & 0 deletions crates/tests-integration/src/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,143 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
cmd!(sh, "sudo {BASE_ARGS...} -v {tmpdisk}:/disk {image} bootc install to-disk --via-loopback /disk").run()?;
Ok(())
}),
Trial::test(
"install to-filesystem with separate /var mount",
move || {
let sh = &xshell::Shell::new()?;
reset_root(sh, image)?;

// Create work directory for the test
let tmpd = sh.create_temp_dir()?;
let work_dir = tmpd.path();

// Create a disk image with partitions for root and var
let disk_img = work_dir.join("disk.img");
let size = 12 * 1024 * 1024 * 1024;
let disk_file = std::fs::File::create(&disk_img)?;
disk_file.set_len(size)?;
drop(disk_file);

// Setup loop device
let loop_dev = cmd!(sh, "sudo losetup -f --show {disk_img}")
.read()?
.trim()
.to_string();

// Helper closure for cleanup
let cleanup = |sh: &Shell, loop_dev: &str, target: &str| {
// Unmount filesystems
let _ = cmd!(sh, "sudo umount -R {target}").ignore_status().run();
// Deactivate LVM
let _ = cmd!(sh, "sudo vgchange -an BL").ignore_status().run();
let _ = cmd!(sh, "sudo vgremove -f BL").ignore_status().run();
// Detach loop device
let _ = cmd!(sh, "sudo losetup -d {loop_dev}").ignore_status().run();
};

// Create partition table
if let Err(e) = (|| -> Result<()> {
cmd!(sh, "sudo parted -s {loop_dev} mklabel gpt").run()?;
// Create BIOS boot partition (for GRUB on GPT)
cmd!(sh, "sudo parted -s {loop_dev} mkpart primary 1MiB 2MiB").run()?;
cmd!(sh, "sudo parted -s {loop_dev} set 1 bios_grub on").run()?;
// Create EFI partition
cmd!(
sh,
"sudo parted -s {loop_dev} mkpart primary fat32 2MiB 202MiB"
)
.run()?;
cmd!(sh, "sudo parted -s {loop_dev} set 2 esp on").run()?;
// Create boot partition
cmd!(
sh,
"sudo parted -s {loop_dev} mkpart primary ext4 202MiB 1226MiB"
)
.run()?;
// Create LVM partition
cmd!(sh, "sudo parted -s {loop_dev} mkpart primary 1226MiB 100%").run()?;

// Reload partition table
cmd!(sh, "sudo partprobe {loop_dev}").run()?;
std::thread::sleep(std::time::Duration::from_secs(2));

let loop_part2 = format!("{}p2", loop_dev); // EFI
let loop_part3 = format!("{}p3", loop_dev); // Boot
let loop_part4 = format!("{}p4", loop_dev); // LVM

// Create filesystems on boot partitions
cmd!(sh, "sudo mkfs.vfat -F32 {loop_part2}").run()?;
cmd!(sh, "sudo mkfs.ext4 -F {loop_part3}").run()?;

// Setup LVM
cmd!(sh, "sudo pvcreate {loop_part4}").run()?;
cmd!(sh, "sudo vgcreate BL {loop_part4}").run()?;

// Create logical volumes
cmd!(sh, "sudo lvcreate -L 4G -n var02 BL").run()?;
cmd!(sh, "sudo lvcreate -L 5G -n root02 BL").run()?;

// Create filesystems on logical volumes
cmd!(sh, "sudo mkfs.ext4 -F /dev/BL/var02").run()?;
cmd!(sh, "sudo mkfs.ext4 -F /dev/BL/root02").run()?;

// Get UUIDs
let root_uuid = cmd!(sh, "sudo blkid -s UUID -o value /dev/BL/root02")
.read()?
.trim()
.to_string();
let boot_uuid = cmd!(sh, "sudo blkid -s UUID -o value {loop_part2}")
.read()?
.trim()
.to_string();

// Mount the partitions
let target_dir = work_dir.join("target");
std::fs::create_dir_all(&target_dir)?;
let target = target_dir.to_str().unwrap();

cmd!(sh, "sudo mount /dev/BL/root02 {target}").run()?;
cmd!(sh, "sudo mkdir -p {target}/boot").run()?;
cmd!(sh, "sudo mount {loop_part3} {target}/boot").run()?;
cmd!(sh, "sudo mkdir -p {target}/boot/efi").run()?;
cmd!(sh, "sudo mount {loop_part2} {target}/boot/efi").run()?;

// Critical: Mount /var as a separate partition
cmd!(sh, "sudo mkdir -p {target}/var").run()?;
cmd!(sh, "sudo mount /dev/BL/var02 {target}/var").run()?;

// Run bootc install to-filesystem
// This should succeed and handle the separate /var mount correctly
// Mount the target at /target inside the container for simplicity
cmd!(
sh,
"sudo {BASE_ARGS...} -v {target}:/target -v /dev:/dev {image} bootc install to-filesystem --karg=root=UUID={root_uuid} --root-mount-spec=UUID={root_uuid} --boot-mount-spec=UUID={boot_uuid} /target"
)
.run()?;

// Verify the installation succeeded
// Check that bootc created the necessary files
cmd!(sh, "sudo test -d {target}/ostree").run()?;
cmd!(sh, "sudo test -d {target}/ostree/repo").run()?;
// Verify bootloader was installed
cmd!(sh, "sudo test -d {target}/boot/grub2").run()?;

Ok(())
})() {
let target = work_dir.join("target");
let target_str = target.to_str().unwrap();
cleanup(sh, &loop_dev, target_str);
return Err(e.into());
}

// Clean up on success
let target = work_dir.join("target");
let target_str = target.to_str().unwrap();
cleanup(sh, &loop_dev, target_str);

Ok(())
},
),
Trial::test(
"replace=alongside with ssh keys and a karg, and SELinux disabled",
move || {
Expand Down