Skip to content
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.

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

A few misc prep patches #1108

Merged
merged 4 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion hack/provision-derived.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,8 @@ mkdir -p ~/.config/nushell
echo '$env.config = { show_banner: false, }' > ~/.config/nushell/config.nu
touch ~/.config/nushell/env.nu
dnf -y install nu
dnf clean all && rm /var/log/* -rf
dnf clean all
# Stock extra cleaning of logs and caches in general (mostly dnf)
rm /var/log/* /var/cache /var/lib/dnf /var/lib/rpm-state -rf
# And clean root's homedir
rm /var/roothome/.config -rf
4 changes: 2 additions & 2 deletions lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ use crate::progress_jsonl::ProgressWriter;
use crate::spec::ImageReference;
use crate::store::Storage;
use crate::task::Task;
use crate::utils::{open_dir_noxdev, sigpolicy_from_opt};
use crate::utils::sigpolicy_from_opt;

/// The toplevel boot directory
const BOOT: &str = "boot";
Expand Down Expand Up @@ -1579,7 +1579,7 @@ fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> {
let name = entry.file_name();
let etype = entry.file_type()?;
if etype == FileType::dir() {
if let Some(subdir) = open_dir_noxdev(d, &name)? {
if let Some(subdir) = d.open_dir_noxdev(&name)? {
remove_all_in_dir_no_xdev(&subdir, mount_err)?;
d.remove_dir(&name)?;
} else if mount_err {
Expand Down
2 changes: 1 addition & 1 deletion lib/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ fn check_utf8(dir: &Dir) -> LintResult {
return lint_err(format!("/{strname}: Found non-utf8 symlink target"));
}
} else if ifmt.is_dir() {
let Some(subdir) = crate::utils::open_dir_noxdev(dir, entry.file_name())? else {
let Some(subdir) = dir.open_dir_noxdev(entry.file_name())? else {
continue;
};
if let Err(err) = check_utf8(&subdir)? {
Expand Down
63 changes: 1 addition & 62 deletions lib/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::future::Future;
use std::io::Write;
use std::os::fd::{AsFd, BorrowedFd, OwnedFd};
use std::path::Path;
use std::os::fd::BorrowedFd;
use std::process::Command;
use std::time::Duration;

Expand All @@ -17,7 +16,6 @@ use libsystemd::logging::journal_print;
use ostree::glib;
use ostree_ext::container::SignatureSource;
use ostree_ext::ostree;
use rustix::path::Arg;

/// Try to look for keys injected by e.g. rpm-ostree requesting machine-local
/// changes; if any are present, return `true`.
Expand Down Expand Up @@ -54,33 +52,6 @@ pub(crate) fn deployment_fd(
sysroot_dir.open_dir(&dirpath).map_err(Into::into)
}

/// A thin wrapper for [`openat2`] but that retries on interruption.
pub fn openat2_with_retry(
dirfd: impl AsFd,
path: impl AsRef<Path>,
oflags: rustix::fs::OFlags,
mode: rustix::fs::Mode,
resolve: rustix::fs::ResolveFlags,
) -> rustix::io::Result<OwnedFd> {
let dirfd = dirfd.as_fd();
let path = path.as_ref();
// We loop forever on EAGAIN right now. The cap-std version loops just 4 times,
// which seems really arbitrary.
path.into_with_c_str(|path_c_str| 'start: loop {
match rustix::fs::openat2(dirfd, path_c_str, oflags, mode, resolve) {
Ok(file) => {
return Ok(file);
}
Err(rustix::io::Errno::AGAIN | rustix::io::Errno::INTR) => {
continue 'start;
}
Err(e) => {
return Err(e);
}
}
})
}

/// 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.
Expand Down Expand Up @@ -110,25 +81,6 @@ pub(crate) fn open_dir_remount_rw(root: &Dir, target: &Utf8Path) -> Result<Dir>
root.open_dir(target).map_err(anyhow::Error::new)
}

/// Open the target directory, but return Ok(None) if this would cross a mount point.
pub fn open_dir_noxdev(
parent: &Dir,
path: impl AsRef<std::path::Path>,
) -> std::io::Result<Option<Dir>> {
use rustix::fs::{Mode, OFlags, ResolveFlags};
match openat2_with_retry(
parent,
path,
OFlags::CLOEXEC | OFlags::DIRECTORY | OFlags::NOFOLLOW,
Mode::empty(),
ResolveFlags::NO_XDEV | ResolveFlags::BENEATH,
) {
Ok(r) => Ok(Some(Dir::reopen_dir(&r)?)),
Err(e) if e == rustix::io::Errno::XDEV => Ok(None),
Err(e) => return Err(e.into()),
}
}

/// Given a target path, remove its immutability if present
#[context("Removing immutable flag from {target}")]
pub(crate) fn remove_immutability(root: &Dir, target: &Utf8Path) -> Result<()> {
Expand Down Expand Up @@ -236,8 +188,6 @@ pub(crate) fn digested_pullspec(image: &str, digest: &str) -> String {

#[cfg(test)]
mod tests {
use cap_std_ext::cap_std;

use super::*;

#[test]
Expand Down Expand Up @@ -273,15 +223,4 @@ mod tests {
SignatureSource::ContainerPolicyAllowInsecure
);
}

#[test]
fn test_open_noxdev() -> Result<()> {
let root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
// This hard requires the host setup to have /usr/bin on the same filesystem as /
let usr = Dir::open_ambient_dir("/usr", cap_std::ambient_authority())?;
assert!(open_dir_noxdev(&usr, "bin").unwrap().is_some());
// Requires a mounted /proc, but that also seems ane.
assert!(open_dir_noxdev(&root, "proc").unwrap().is_none());
Ok(())
}
}
2 changes: 1 addition & 1 deletion utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
tempfile = { workspace = true }
tracing = { workspace = true }
tokio = { workspace = true, features = ["process"] }
tokio = { workspace = true, features = ["process", "rt", "macros"] }
tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }

[dev-dependencies]
Expand Down
94 changes: 94 additions & 0 deletions utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,97 @@ mod command;
mod tracing_util;
pub use command::*;
pub use tracing_util::*;

/// Given an iterator that's clonable, split it into two iterators
/// at a given maximum number of elements.
pub fn iterator_split<I>(
it: I,
max: usize,
) -> (impl Iterator<Item = I::Item>, impl Iterator<Item = I::Item>)
where
I: Iterator + Clone,
{
let rest = it.clone();
(it.take(max), rest.skip(max))
}

/// Given an iterator that's clonable, split off the first N elements
/// at the pivot point. If the iterator would be empty, return None.
/// Return the count of the remainder.
pub fn iterator_split_nonempty_rest_count<I>(
it: I,
max: usize,
) -> Option<(impl Iterator<Item = I::Item>, usize)>
where
I: Iterator + Clone,
{
let rest = it.clone();
let mut it = it.peekable();
if it.peek().is_some() {
Some((it.take(max), rest.skip(max).count()))
} else {
None
}
}

#[cfg(test)]
mod tests {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

use super::*;

#[test]
fn test_it_split() {
let a: &[&str] = &[];
for v in [0, 1, 5] {
let (first, rest) = iterator_split(a.iter(), v);
assert_eq!(first.count(), 0);
assert_eq!(rest.count(), 0);
}
let a = &["foo"];
for v in [1, 5] {
let (first, rest) = iterator_split(a.iter(), v);
assert_eq!(first.count(), 1);
assert_eq!(rest.count(), 0);
}
let (first, rest) = iterator_split(a.iter(), 1);
assert_eq!(first.count(), 1);
assert_eq!(rest.count(), 0);
let a = &["foo", "bar", "baz", "blah", "other"];
let (first, rest) = iterator_split(a.iter(), 2);
assert_eq!(first.count(), 2);
assert_eq!(rest.count(), 3);
}

#[test]
fn test_split_empty_iterator() {
let a: &[&str] = &[];
for v in [0, 1, 5] {
assert!(iterator_split_nonempty_rest_count(a.iter(), v).is_none());
}
}

#[test]
fn test_split_nonempty_iterator() {
let a = &["foo"];

let Some((elts, 1)) = iterator_split_nonempty_rest_count(a.iter(), 0) else {
panic!()
};
assert_eq!(elts.count(), 0);

let Some((elts, 0)) = iterator_split_nonempty_rest_count(a.iter(), 1) else {
panic!()
};
assert_eq!(elts.count(), 1);

let Some((elts, 0)) = iterator_split_nonempty_rest_count(a.iter(), 5) else {
panic!()
};
assert_eq!(elts.count(), 1);

let a = &["foo", "bar", "baz", "blah", "other"];
let Some((elts, 3)) = iterator_split_nonempty_rest_count(a.iter(), 2) else {
panic!()
};
assert_eq!(elts.count(), 2);
}
}