Skip to content

Commit fe8146d

Browse files
committed
tar: Unconditionally use repo tmpdir
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.
1 parent 04fb812 commit fe8146d

File tree

2 files changed

+19
-18
lines changed

2 files changed

+19
-18
lines changed

lib/src/cli.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -849,8 +849,14 @@ async fn testing(opts: &TestingOpts) -> Result<()> {
849849
TestingOpts::Run => crate::integrationtest::run_tests(),
850850
TestingOpts::RunIMA => crate::integrationtest::test_ima(),
851851
TestingOpts::FilterTar => {
852-
crate::tar::filter_tar(std::io::stdin(), std::io::stdout(), &Default::default())
853-
.map(|_| {})
852+
let tmpdir = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
853+
crate::tar::filter_tar(
854+
std::io::stdin(),
855+
std::io::stdout(),
856+
&Default::default(),
857+
&tmpdir,
858+
)
859+
.map(|_| {})
854860
}
855861
}
856862
}

lib/src/tar/write.rs

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,16 @@ use anyhow::{anyhow, Context};
1212
use camino::{Utf8Component, Utf8Path, Utf8PathBuf};
1313

1414
use cap_std::io_lifetimes;
15+
use cap_std_ext::cap_std::fs::Dir;
1516
use cap_std_ext::cmdext::CapStdExtCommandExt;
1617
use cap_std_ext::{cap_std, cap_tempfile};
1718
use fn_error_context::context;
18-
use once_cell::unsync::OnceCell;
1919
use ostree::gio;
2020
use ostree::prelude::FileExt;
2121
use std::collections::{BTreeMap, HashMap};
2222
use std::io::{BufWriter, Seek, Write};
2323
use std::path::Path;
2424
use std::process::Stdio;
25-
2625
use std::sync::Arc;
2726
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite};
2827
use tracing::instrument;
@@ -196,6 +195,7 @@ pub(crate) fn filter_tar(
196195
src: impl std::io::Read,
197196
dest: impl std::io::Write,
198197
config: &TarImportConfig,
198+
tmpdir: &Dir,
199199
) -> Result<BTreeMap<String, u32>> {
200200
let src = std::io::BufReader::new(src);
201201
let mut src = tar::Archive::new(src);
@@ -210,8 +210,6 @@ pub(crate) fn filter_tar(
210210
// Lookaside data for dealing with hardlinked files into /sysroot; see below.
211211
let mut changed_sysroot_objects = HashMap::new();
212212
let mut new_sysroot_link_targets = HashMap::<Utf8PathBuf, Utf8PathBuf>::new();
213-
// A temporary directory if needed
214-
let tmpdir = OnceCell::new();
215213

216214
for entry in ents {
217215
let mut entry = entry?;
@@ -228,15 +226,6 @@ pub(crate) fn filter_tar(
228226
// file instead.
229227
if is_modified && is_regular {
230228
tracing::debug!("Processing modified sysroot file {path}");
231-
// Lazily allocate a temporary directory
232-
let tmpdir = tmpdir.get_or_try_init(|| -> anyhow::Result<_> {
233-
let vartmp = &cap_std::fs::Dir::open_ambient_dir(
234-
"/var/tmp",
235-
cap_std::ambient_authority(),
236-
)
237-
.context("Allocating tmpdir")?;
238-
cap_tempfile::tempdir_in(vartmp).map_err(anyhow::Error::msg)
239-
})?;
240229
// Create an O_TMPFILE (anonymous file) to use as a temporary store for the file data
241230
let mut tmpf = cap_tempfile::TempFile::new_anonymous(tmpdir)
242231
.map(BufWriter::new)
@@ -304,6 +293,7 @@ async fn filter_tar_async(
304293
src: impl AsyncRead + Send + 'static,
305294
mut dest: impl AsyncWrite + Send + Unpin,
306295
config: &TarImportConfig,
296+
repo_tmpdir: Dir,
307297
) -> Result<BTreeMap<String, u32>> {
308298
let (tx_buf, mut rx_buf) = tokio::io::duplex(8192);
309299
// The source must be moved to the heap so we know it is stable for passing to the worker thread
@@ -312,7 +302,7 @@ async fn filter_tar_async(
312302
let tar_transformer = tokio::task::spawn_blocking(move || {
313303
let mut src = tokio_util::io::SyncIoBridge::new(src);
314304
let dest = tokio_util::io::SyncIoBridge::new(tx_buf);
315-
let r = filter_tar(&mut src, dest, &config);
305+
let r = filter_tar(&mut src, dest, &config, &repo_tmpdir);
316306
// Pass ownership of the input stream back to the caller - see below.
317307
(r, src)
318308
});
@@ -390,7 +380,10 @@ pub async fn write_tar(
390380
let mut import_config = TarImportConfig::default();
391381
import_config.allow_nonusr = options.allow_nonusr;
392382
import_config.remap_factory_var = !options.retain_var;
393-
let filtered_result = filter_tar_async(src, child_stdin, &import_config);
383+
let repo_tmpdir = Dir::reopen_dir(&repo.dfd_borrow())?
384+
.open_dir("tmp")
385+
.context("Getting repo tmpdir")?;
386+
let filtered_result = filter_tar_async(src, child_stdin, &import_config, repo_tmpdir);
394387
let output_copier = async move {
395388
// Gather stdout/stderr to buffers
396389
let mut child_stdout_buf = String::new();
@@ -512,6 +505,7 @@ mod tests {
512505
async fn tar_filter() -> Result<()> {
513506
let tempd = tempfile::tempdir()?;
514507
let rootfs = &tempd.path().join("rootfs");
508+
515509
std::fs::create_dir_all(rootfs.join("etc/systemd/system"))?;
516510
std::fs::write(rootfs.join("etc/systemd/system/foo.service"), "fooservice")?;
517511
std::fs::write(rootfs.join("blah"), "blah")?;
@@ -522,7 +516,8 @@ mod tests {
522516
let _ = rootfs_tar.into_inner()?;
523517
let mut dest = Vec::new();
524518
let src = tokio::io::BufReader::new(tokio::fs::File::open(rootfs_tar_path).await?);
525-
filter_tar_async(src, &mut dest, &Default::default()).await?;
519+
let cap_tmpdir = Dir::open_ambient_dir(&tempd, cap_std::ambient_authority())?;
520+
filter_tar_async(src, &mut dest, &Default::default(), cap_tmpdir).await?;
526521
let dest = dest.as_slice();
527522
let mut final_tar = tar::Archive::new(Cursor::new(dest));
528523
let destdir = &tempd.path().join("destdir");

0 commit comments

Comments
 (0)