Skip to content

Commit

Permalink
tar: Unconditionally use repo tmpdir
Browse files Browse the repository at this point in the history
The tar code had some fancy logic to lazily allocate a temporary
directory if it turned out we needed one, which only broke
in the rare case we needed one in an obscure circumstance (a bwrap
container in osbuild).

But we already always have a tmpdir allocated in the ostree repo,
so switch the code to use that.
  • Loading branch information
cgwalters committed Feb 29, 2024
1 parent 04fb812 commit fe8146d
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 18 deletions.
10 changes: 8 additions & 2 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,14 @@ async fn testing(opts: &TestingOpts) -> Result<()> {
TestingOpts::Run => crate::integrationtest::run_tests(),
TestingOpts::RunIMA => crate::integrationtest::test_ima(),
TestingOpts::FilterTar => {
crate::tar::filter_tar(std::io::stdin(), std::io::stdout(), &Default::default())
.map(|_| {})
let tmpdir = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
crate::tar::filter_tar(
std::io::stdin(),
std::io::stdout(),
&Default::default(),
&tmpdir,
)
.map(|_| {})
}
}
}
Expand Down
27 changes: 11 additions & 16 deletions lib/src/tar/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,16 @@ use anyhow::{anyhow, Context};
use camino::{Utf8Component, Utf8Path, Utf8PathBuf};

use cap_std::io_lifetimes;
use cap_std_ext::cap_std::fs::Dir;
use cap_std_ext::cmdext::CapStdExtCommandExt;
use cap_std_ext::{cap_std, cap_tempfile};
use fn_error_context::context;
use once_cell::unsync::OnceCell;
use ostree::gio;
use ostree::prelude::FileExt;
use std::collections::{BTreeMap, HashMap};
use std::io::{BufWriter, Seek, Write};
use std::path::Path;
use std::process::Stdio;

use std::sync::Arc;
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite};
use tracing::instrument;
Expand Down Expand Up @@ -196,6 +195,7 @@ pub(crate) fn filter_tar(
src: impl std::io::Read,
dest: impl std::io::Write,
config: &TarImportConfig,
tmpdir: &Dir,
) -> Result<BTreeMap<String, u32>> {
let src = std::io::BufReader::new(src);
let mut src = tar::Archive::new(src);
Expand All @@ -210,8 +210,6 @@ pub(crate) fn filter_tar(
// Lookaside data for dealing with hardlinked files into /sysroot; see below.
let mut changed_sysroot_objects = HashMap::new();
let mut new_sysroot_link_targets = HashMap::<Utf8PathBuf, Utf8PathBuf>::new();
// A temporary directory if needed
let tmpdir = OnceCell::new();

for entry in ents {
let mut entry = entry?;
Expand All @@ -228,15 +226,6 @@ pub(crate) fn filter_tar(
// file instead.
if is_modified && is_regular {
tracing::debug!("Processing modified sysroot file {path}");
// Lazily allocate a temporary directory
let tmpdir = tmpdir.get_or_try_init(|| -> anyhow::Result<_> {
let vartmp = &cap_std::fs::Dir::open_ambient_dir(
"/var/tmp",
cap_std::ambient_authority(),
)
.context("Allocating tmpdir")?;
cap_tempfile::tempdir_in(vartmp).map_err(anyhow::Error::msg)
})?;
// Create an O_TMPFILE (anonymous file) to use as a temporary store for the file data
let mut tmpf = cap_tempfile::TempFile::new_anonymous(tmpdir)
.map(BufWriter::new)
Expand Down Expand Up @@ -304,6 +293,7 @@ async fn filter_tar_async(
src: impl AsyncRead + Send + 'static,
mut dest: impl AsyncWrite + Send + Unpin,
config: &TarImportConfig,
repo_tmpdir: Dir,
) -> Result<BTreeMap<String, u32>> {
let (tx_buf, mut rx_buf) = tokio::io::duplex(8192);
// The source must be moved to the heap so we know it is stable for passing to the worker thread
Expand All @@ -312,7 +302,7 @@ async fn filter_tar_async(
let tar_transformer = tokio::task::spawn_blocking(move || {
let mut src = tokio_util::io::SyncIoBridge::new(src);
let dest = tokio_util::io::SyncIoBridge::new(tx_buf);
let r = filter_tar(&mut src, dest, &config);
let r = filter_tar(&mut src, dest, &config, &repo_tmpdir);
// Pass ownership of the input stream back to the caller - see below.
(r, src)
});
Expand Down Expand Up @@ -390,7 +380,10 @@ pub async fn write_tar(
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 repo_tmpdir = Dir::reopen_dir(&repo.dfd_borrow())?
.open_dir("tmp")
.context("Getting repo tmpdir")?;
let filtered_result = filter_tar_async(src, child_stdin, &import_config, repo_tmpdir);
let output_copier = async move {
// Gather stdout/stderr to buffers
let mut child_stdout_buf = String::new();
Expand Down Expand Up @@ -512,6 +505,7 @@ mod tests {
async fn tar_filter() -> Result<()> {
let tempd = tempfile::tempdir()?;
let rootfs = &tempd.path().join("rootfs");

std::fs::create_dir_all(rootfs.join("etc/systemd/system"))?;
std::fs::write(rootfs.join("etc/systemd/system/foo.service"), "fooservice")?;
std::fs::write(rootfs.join("blah"), "blah")?;
Expand All @@ -522,7 +516,8 @@ mod tests {
let _ = rootfs_tar.into_inner()?;
let mut dest = Vec::new();
let src = tokio::io::BufReader::new(tokio::fs::File::open(rootfs_tar_path).await?);
filter_tar_async(src, &mut dest, &Default::default()).await?;
let cap_tmpdir = Dir::open_ambient_dir(&tempd, cap_std::ambient_authority())?;
filter_tar_async(src, &mut dest, &Default::default(), cap_tmpdir).await?;
let dest = dest.as_slice();
let mut final_tar = tar::Archive::new(Cursor::new(dest));
let destdir = &tempd.path().join("destdir");
Expand Down

0 comments on commit fe8146d

Please sign in to comment.