From b6ca110db557cae511607c29c28ed7f658b64232 Mon Sep 17 00:00:00 2001 From: HuijingHei Date: Fri, 1 Dec 2023 18:51:06 +0800 Subject: [PATCH] utils.rs: remove duplicate tmpfiles entries rpm-ostree should scan the tmpfiles.d snippets at install time and skip synthesizing entries if they are already specified. As we may have confilicted tmpfile entries, like it will prevent to cleanup of `/var/tmp` by `systemd-tmpfiles-clean.service`: `pkg-filesystem.conf` has `d /var/tmp 1777 root root - -` `systemd's tmp.conf` has `q /var/tmp 1777 root root 30d` It is not as simple because the auto-conversion we do at package import time may be duplicating a tmpfiles.d dropin that lives in a separate package. With Jonathan's suggestion: - the importer code puts the generated tmpfiles.d dropins in e.g. `/usr/lib/rpm-ostree/tmpfiles.d` instead of the canonical place - `deduplicate_tmpfiles_entries()` builds one hashmap from `/usr/lib/rpm-ostree/tmpfiles.d` and another from `/usr/lib/tmpfiles.d` and generates `rpm-ostree-autovar.conf` from the difference - delete_package_from_root() is updated to remove from `/usr/lib/rpm-ostree/tmpfiles.d` See https://github.com/coreos/rpm-ostree/issues/26#issuecomment-1832828459 Fixes https://github.com/coreos/rpm-ostree/issues/26 --- rpmostree-cxxrs.cxx | 14 +++++ rpmostree-cxxrs.h | 2 + rust/src/lib.rs | 1 + rust/src/utils.rs | 83 ++++++++++++++++++++++++++++++ src/libpriv/rpmostree-core.cxx | 14 +++-- src/libpriv/rpmostree-importer.cxx | 6 ++- 6 files changed, 113 insertions(+), 7 deletions(-) diff --git a/rpmostree-cxxrs.cxx b/rpmostree-cxxrs.cxx index 4f4dd700a6..0c3c8c0cf3 100644 --- a/rpmostree-cxxrs.cxx +++ b/rpmostree-cxxrs.cxx @@ -2922,6 +2922,9 @@ extern "C" void rpmostreecxx$cxxbridge1$cache_branch_to_nevra (::rust::Str nevra, ::rust::String *return$) noexcept; + ::rust::repr::PtrLen + rpmostreecxx$cxxbridge1$deduplicate_tmpfiles_entries (::std::int32_t rootfs) noexcept; + ::std::uint32_t rpmostreecxx$cxxbridge1$CxxGObjectArray$length (::rpmostreecxx::CxxGObjectArray &self) noexcept { @@ -6059,6 +6062,16 @@ cache_branch_to_nevra (::rust::Str nevra) noexcept rpmostreecxx$cxxbridge1$cache_branch_to_nevra (nevra, &return$.value); return ::std::move (return$.value); } + +void +deduplicate_tmpfiles_entries (::std::int32_t rootfs) +{ + ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$deduplicate_tmpfiles_entries (rootfs); + if (error$.ptr) + { + throw ::rust::impl< ::rust::Error>::error (error$); + } +} } // namespace rpmostreecxx extern "C" @@ -6773,5 +6786,6 @@ Vec< ::rpmostreecxx::LockedPackage>::truncate (::std::size_t len) { return cxxbridge1$rust_vec$rpmostreecxx$LockedPackage$truncate (this, len); } + } // namespace cxxbridge1 } // namespace rust diff --git a/rpmostree-cxxrs.h b/rpmostree-cxxrs.h index 323e1d0f61..f66d8cda86 100644 --- a/rpmostree-cxxrs.h +++ b/rpmostree-cxxrs.h @@ -2043,4 +2043,6 @@ ::rpmostreecxx::GKeyFile *treefile_to_origin (::rpmostreecxx::Treefile const &tf void origin_validate_roundtrip (::rpmostreecxx::GKeyFile const &kf) noexcept; ::rust::String cache_branch_to_nevra (::rust::Str nevra) noexcept; + +void deduplicate_tmpfiles_entries (::std::int32_t rootfs); } // namespace rpmostreecxx diff --git a/rust/src/lib.rs b/rust/src/lib.rs index e0f6dd80d3..233dd0900d 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -710,6 +710,7 @@ pub mod ffi { extern "Rust" { fn prepare_rpm_layering(rootfs: i32, merge_passwd_dir: &str) -> Result; fn complete_rpm_layering(rootfs: i32) -> Result<()>; + fn deduplicate_tmpfiles_entries(rootfs: i32) -> Result<()>; fn passwd_cleanup(rootfs: i32) -> Result<()>; fn migrate_group_except_root(rootfs: i32, preserved_groups: &Vec) -> Result<()>; fn migrate_passwd_except_root(rootfs: i32) -> Result<()>; diff --git a/rust/src/utils.rs b/rust/src/utils.rs index 380527c038..dace64c442 100644 --- a/rust/src/utils.rs +++ b/rust/src/utils.rs @@ -8,10 +8,14 @@ */ use crate::cxxrsutil::*; +use crate::ffiutil; use crate::variant_utils; use anyhow::{bail, Context, Result}; use camino::Utf8Path; +use cap_std::fs::{Dir, Permissions}; use cap_std::io_lifetimes::AsFilelike; +use cap_std_ext::prelude::CapStdExtDirExt; +use fn_error_context::context; use glib::Variant; use once_cell::sync::Lazy; use ostree_ext::prelude::*; @@ -22,6 +26,7 @@ use std::collections::{HashMap, HashSet}; use std::io::prelude::*; use std::os::fd::OwnedFd; use std::os::unix::io::IntoRawFd; +use std::os::unix::prelude::PermissionsExt; use std::path::Path; use std::{fs, io}; @@ -304,6 +309,84 @@ pub(crate) fn translate_path_for_ostree_impl(path: &str) -> Option { None } +#[context("Deduplicate tmpfiles entries")] +pub fn deduplicate_tmpfiles_entries(tmprootfs_dfd: i32) -> CxxResult<()> { + let tmprootfs_dfd = unsafe { ffiutil::ffi_dirfd(tmprootfs_dfd)? }; + static TMPFILESD: &str = "usr/lib/tmpfiles.d"; + static RPMOSTREE_TMPFILESD: &str = "usr/lib/rpm-ostree/tmpfiles.d"; + static AUTOVAR_PATH: &str = "rpm-ostree-autovar.conf"; + + // save rpm-ostree auto generated entries to hashmaps + let tmpfiles_dir = tmprootfs_dfd + .open_dir(RPMOSTREE_TMPFILESD) + .context("readdir {RPMOSTREE_TMPFILESD}")?; + let mut rpmostree_tmpfiles_entries: HashMap = + save_tmpfile_entries(&tmpfiles_dir)? + .into_iter() + .map(|s| { + let parts: Vec<&str> = s.split_whitespace().collect(); + let entry = parts.get(1).unwrap(); + (entry.to_string(), s.to_string()) + }) + .collect::>(); + + // remove autovar.conf first, then save system entries to hashmaps + let tmpfiles_dir = tmprootfs_dfd + .open_dir(TMPFILESD) + .context("readdir {TMPFILESD}")?; + + if tmpfiles_dir.try_exists(AUTOVAR_PATH)? { + tmpfiles_dir.remove_file(AUTOVAR_PATH)?; + } + let system_tmpfiles_entries: HashSet = + save_tmpfile_entries(&tmpfiles_dir)? + .into_iter() + .map(|s| { + let parts: Vec<&str> = s.split_whitespace().collect(); + let entry = parts.get(1).unwrap(); + entry.to_string() + }) + .collect::>(); + + // remove duplicated entries in auto-generated tmpfiles.d, + // which are already in system tmpfiles + let _ = system_tmpfiles_entries.into_iter().map(|key| rpmostree_tmpfiles_entries.retain(|k, _| k != &key)); + + { + // save the noduplicated entries + let mut entries = String::from("# This file was generated by rpm-ostree.\n"); + for (_key, value) in rpmostree_tmpfiles_entries { + entries.push_str(&format!("{}\n", value)); + } + + let perms = Permissions::from_mode(0o644); + tmpfiles_dir.atomic_write_with_perms(&AUTOVAR_PATH, entries.as_bytes(), perms)?; + } + Ok(()) +} + +#[context("Save tmpfiles entries")] +pub(crate) fn save_tmpfile_entries(tmpfiles_dir: &Dir) -> Result> { + let mut entries: Vec = Vec::new(); + for entry in tmpfiles_dir.entries()? { + let entry = entry?; + let name = entry.file_name(); + if !name.to_string_lossy().ends_with(".conf") { + continue; + } + let mut tmp_entries = tmpfiles_dir + .read_to_string(name) + .unwrap() + .lines() + .map(|s| s.to_string()) + .filter(|s| !s.is_empty() && !s.starts_with('#')) + .collect::>(); + + entries.append(&mut tmp_entries); + } + Ok(entries) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/libpriv/rpmostree-core.cxx b/src/libpriv/rpmostree-core.cxx index 0c52c70e3f..2e2784e25e 100644 --- a/src/libpriv/rpmostree-core.cxx +++ b/src/libpriv/rpmostree-core.cxx @@ -3085,13 +3085,11 @@ delete_package_from_root (RpmOstreeContext *self, rpmte pkg, int rootfs_dfd, GHa } } - /* And finally, delete any automatically generated tmpfiles.d dropin. Though - * ideally we'd have a way to be sure that this was the rpm-ostree generated - * one. */ + /* And finally, delete any automatically generated tmpfiles.d dropin. */ glnx_autofd int tmpfiles_dfd = -1; - if (!glnx_opendirat (rootfs_dfd, "usr/lib/tmpfiles.d", TRUE, &tmpfiles_dfd, error)) + if (!glnx_opendirat (rootfs_dfd, "usr/lib/rpm-ostree/tmpfiles.d", TRUE, &tmpfiles_dfd, error)) return FALSE; - g_autofree char *dropin = g_strdup_printf ("pkg-%s.conf", rpmteN (pkg)); + g_autofree char *dropin = g_strdup_printf ("%s.conf", rpmteN (pkg)); if (!glnx_shutil_rm_rf_at (tmpfiles_dfd, dropin, cancellable, error)) return FALSE; @@ -4442,6 +4440,12 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G task->end (msg); } + /* Remove duplicated tmpfiles entries; + * see https://github.com/coreos/rpm-ostree/issues/26 + */ + if (overlays->len > 0 || overrides_remove->len > 0 || overrides_replace->len > 0) + ROSCXX_TRY (deduplicate_tmpfiles_entries (tmprootfs_dfd), error); + /* We want this to be the first error message if something went wrong * with a script; see https://github.com/projectatomic/rpm-ostree/pull/888 * (otherwise, on a script that did `rm -rf`, we'd fail first on the renameat below) diff --git a/src/libpriv/rpmostree-importer.cxx b/src/libpriv/rpmostree-importer.cxx index 4a85d316d3..41f2f9dbdc 100644 --- a/src/libpriv/rpmostree-importer.cxx +++ b/src/libpriv/rpmostree-importer.cxx @@ -627,10 +627,12 @@ import_rpm_to_repo (RpmOstreeImporter *self, char **out_csum, char **out_metadat if (!glnx_mkdtemp ("rpm-ostree-import.XXXXXX", 0700, &tmpdir, error)) return FALSE; - if (!glnx_shutil_mkdir_p_at (tmpdir.fd, "usr/lib/tmpfiles.d", 0755, cancellable, error)) + if (!glnx_shutil_mkdir_p_at (tmpdir.fd, "usr/lib/rpm-ostree/tmpfiles.d", 0755, cancellable, + error)) return FALSE; if (!glnx_file_replace_contents_at ( - tmpdir.fd, glnx_strjoina ("usr/lib/tmpfiles.d/", "pkg-", pkg_name.c_str (), ".conf"), + tmpdir.fd, + glnx_strjoina ("usr/lib/rpm-ostree/tmpfiles.d/", pkg_name.c_str (), ".conf"), (guint8 *)content.data (), content.size (), GLNX_FILE_REPLACE_NODATASYNC, cancellable, error)) return FALSE;