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..e3220ca86a 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<()>; @@ -986,6 +987,8 @@ mod rpmutils; pub(crate) use self::rpmutils::*; mod testutils; pub(crate) use self::testutils::*; +mod tmpfiles; +pub use self::tmpfiles::*; mod treefile; pub use self::treefile::*; pub mod utils; diff --git a/rust/src/tmpfiles.rs b/rust/src/tmpfiles.rs new file mode 100644 index 0000000000..0c7af2e3d6 --- /dev/null +++ b/rust/src/tmpfiles.rs @@ -0,0 +1,175 @@ +/* + * Copyright (C) 2023 Red Hat, Inc. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + */ +use crate::cxxrsutil::*; +use crate::ffiutil; + +use anyhow::{Context, Result}; +use cap_std::fs::{Dir, Permissions}; +use cap_std_ext::dirext::CapStdExtDirExt; +use fn_error_context::context; +use std::collections::{HashMap, HashSet}; +use std::fmt::Write; +use std::os::unix::prelude::PermissionsExt; +use std::path::Path; + +const TMPFILESD: &str = "usr/lib/tmpfiles.d"; +const RPMOSTREE_TMPFILESD: &str = "usr/lib/rpm-ostree/tmpfiles.d"; +const AUTOVAR_PATH: &str = "rpm-ostree-autovar.conf"; + +#[context("Deduplicate tmpfiles entries")] +pub fn deduplicate_tmpfiles_entries(tmprootfs_dfd: i32) -> CxxResult<()> { + let tmprootfs_dfd = unsafe { ffiutil::ffi_dirfd(tmprootfs_dfd)? }; + + // scan all rpm-ostree auto generated entries and save + let tmpfiles_dir = tmprootfs_dfd + .open_dir(RPMOSTREE_TMPFILESD) + .context("readdir {RPMOSTREE_TMPFILESD}")?; + let mut rpmostree_tmpfiles_entries = save_tmpfile_entries(&tmpfiles_dir)? + .map(|s| { + let entry = tmpfiles_entry_get_path(&s.as_str())?; + anyhow::Ok((entry.to_string(), s.to_string())) + }) + .collect::>>()?; + + // remove autovar.conf first, then scan all system entries and save + 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 = save_tmpfile_entries(&tmpfiles_dir)? + .map(|s| { + let entry = tmpfiles_entry_get_path(&s.as_str())?; + anyhow::Ok(entry.to_string()) + }) + .collect::>>()?; + + // remove duplicated entries in auto-generated tmpfiles.d, + // which are already in system tmpfiles + for key in system_tmpfiles_entries.into_iter() { + rpmostree_tmpfiles_entries.retain(|k, _value| 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 { + writeln!(entries, "{value}").unwrap(); + } + + let perms = Permissions::from_mode(0o644); + tmpfiles_dir.atomic_write_with_perms(&AUTOVAR_PATH, entries.as_bytes(), perms)?; + } + Ok(()) +} + +// #[context("Scan all tmpfiles conf and save entries")] +fn save_tmpfile_entries(tmpfiles_dir: &Dir) -> Result> { + let entries = tmpfiles_dir + .entries()? + .filter_map(|name| { + let name = name.unwrap().file_name(); + if let Some(extension) = Path::new(&name).extension() { + if extension != "conf" { + return None; + } + } else { + return None; + } + Some( + tmpfiles_dir + .read_to_string(name) + .ok()? + .lines() + .filter(|s| !s.is_empty() && !s.starts_with('#')) + .map(|s| s.to_string()) + .collect::>(), + ) + }) + .flatten() + .collect::>(); + + Ok(entries.into_iter()) +} + +#[context("Scan tmpfiles entries and get path")] +fn tmpfiles_entry_get_path(line: &str) -> Result<&str> { + line.split_whitespace() + .nth(1) + .ok_or_else(|| anyhow::anyhow!("Malformed tmpfiles.d entry ({line})")) +} + +#[cfg(test)] +mod tests { + use rustix::fd::AsRawFd; + + use super::*; + #[test] + fn test_tmpfiles_entry_get_path() { + let cases = [ + ("z /dev/kvm 0666 - kvm -", "/dev/kvm"), + ("d /run/lock/lvm 0700 root root -", "/run/lock/lvm"), + ("a+ /var/lib/tpm2-tss/system/keystore - - - - default:group:tss:rwx", "/var/lib/tpm2-tss/system/keystore"), + ]; + for (input, expected) in cases { + let path = tmpfiles_entry_get_path(input).unwrap(); + assert_eq!(path, expected, "Input: {input}"); + } + } + + fn newroot() -> Result { + let root = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + root.create_dir_all(RPMOSTREE_TMPFILESD)?; + root.create_dir_all(TMPFILESD)?; + Ok(root) + } + + #[test] + fn test_deduplicate_noop() -> Result<()> { + let root = &newroot()?; + deduplicate_tmpfiles_entries(root.as_raw_fd())?; + Ok(()) + } + + // The first and 3rd are duplicates + const PKG_FILESYSTEM_CONTENTS: &str = indoc::indoc! { r#" +d /var/cache 0755 root root - - +d /var/lib/games 0755 root root - - +d /var/tmp 1777 root root - - +d /var/spool/mail 0775 root mail - - +"# }; + const VAR_CONF: &str = indoc::indoc! { r#" +d /var/cache 0755 - - - +"# }; + const TMP_CONF: &str = indoc::indoc! { r#" +q /var/tmp 1777 root root 30d +"# }; + + #[test] + fn test_deduplicate() -> Result<()> { + let root = &newroot()?; + let rpmostree_tmpfiles_dir = root.open_dir(RPMOSTREE_TMPFILESD)?; + let tmpfiles_dir = root.open_dir(TMPFILESD)?; + rpmostree_tmpfiles_dir + .atomic_write(format!("pkg-filesystem.conf"), PKG_FILESYSTEM_CONTENTS)?; + tmpfiles_dir.atomic_write("var.conf", VAR_CONF)?; + tmpfiles_dir.atomic_write("tmp.conf", TMP_CONF)?; + assert!(!tmpfiles_dir.try_exists(AUTOVAR_PATH)?); + deduplicate_tmpfiles_entries(root.as_raw_fd())?; + let contents = tmpfiles_dir.read_to_string(AUTOVAR_PATH).unwrap(); + assert!(contents.contains("# This file was generated by rpm-ostree.")); + let entries = contents + .lines() + .filter(|l| !(l.is_empty() || l.starts_with('#'))) + .collect::>(); + assert_eq!(entries.len(), 2); + assert_eq!(entries[0], "d /var/lib/games 0755 root root - -"); + assert_eq!(entries[1], "d /var/spool/mail 0775 root mail - -"); + Ok(()) + } +} diff --git a/src/app/rpm-ostree-0-integration.conf b/src/app/rpm-ostree-0-integration.conf index c5fd009a0b..1e9822ef29 100644 --- a/src/app/rpm-ostree-0-integration.conf +++ b/src/app/rpm-ostree-0-integration.conf @@ -14,5 +14,4 @@ d /var/usrlocal/share 0755 root root - d /var/usrlocal/src 0755 root root - d /var/mnt 0755 root root - d /run/media 0755 root root - -d /var/lib 0755 root root - L /var/lib/rpm - - - - ../../usr/share/rpm diff --git a/src/libpriv/rpmostree-core.cxx b/src/libpriv/rpmostree-core.cxx index 0c52c70e3f..5ba6ca23b6 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; @@ -4464,6 +4462,12 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G return FALSE; } + /* 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); + /* Undo the /etc move above */ CXX_TRY (etc_guard->undo (), error); 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; diff --git a/src/libpriv/rpmostree-postprocess.cxx b/src/libpriv/rpmostree-postprocess.cxx index 0b0b33d4ce..4904f97e71 100644 --- a/src/libpriv/rpmostree-postprocess.cxx +++ b/src/libpriv/rpmostree-postprocess.cxx @@ -425,7 +425,20 @@ postprocess_final (int rootfs_dfd, rpmostreecxx::Treefile &treefile, gboolean un ROSCXX_TRY (rootfs_prepare_links (rootfs_dfd), error); - ROSCXX_TRY (convert_var_to_tmpfiles_d (rootfs_dfd, *cancellable), error); + if (!unified_core_mode) + ROSCXX_TRY (convert_var_to_tmpfiles_d (rootfs_dfd, *cancellable), error); + else + { + /* In unified core mode, /var entries are converted to tmpfiles.d at + * import time and scriptlets are prevented from writing to /var. What + * remains is just the compat symlinks that we created ourselves, which we + * should stop writing since it duplicates other tmpfiles.d entries. */ + if (!glnx_shutil_rm_rf_at (rootfs_dfd, "var", cancellable, error)) + return FALSE; + /* but we still want the mount point as part of the OSTree commit */ + if (mkdirat (rootfs_dfd, "var", 0755) < 0) + return glnx_throw_errno_prefix (error, "mkdirat(var)"); + } if (!rpmostree_rootfs_postprocess_common (rootfs_dfd, cancellable, error)) return FALSE; diff --git a/tests/compose/test-basic-unified.sh b/tests/compose/test-basic-unified.sh index 93740768ec..ea3b2d2003 100755 --- a/tests/compose/test-basic-unified.sh +++ b/tests/compose/test-basic-unified.sh @@ -119,20 +119,19 @@ echo "ok no cachedir" basic_test # This one is done by postprocessing /var -ostree --repo="${repo}" cat "${treeref}" /usr/lib/tmpfiles.d/pkg-filesystem.conf > autovar.txt +rpmostree_tmpfiles_path="/usr/lib/rpm-ostree/tmpfiles.d" +ostree --repo="${repo}" cat "${treeref}" ${rpmostree_tmpfiles_path}/filesystem.conf > autovar.txt # Picked this one at random as an example of something that won't likely be # converted to tmpfiles.d upstream. But if it is, we can change this test. assert_file_has_content_literal autovar.txt 'd /var/cache 0755 root root - -' -ostree --repo="${repo}" cat "${treeref}" /usr/lib/tmpfiles.d/pkg-chrony.conf > autovar.txt +ostree --repo="${repo}" cat "${treeref}" ${rpmostree_tmpfiles_path}/chrony.conf > autovar.txt # And this one has a non-root uid assert_file_has_content_literal autovar.txt 'd /var/lib/chrony 0750 chrony chrony - -' # see rpmostree-importer.c -if ostree --repo="${repo}" cat "${treeref}" /usr/lib/tmpfiles.d/pkg-rpm.conf > rpm.txt 2>/dev/null; then +if ostree --repo="${repo}" cat "${treeref}" ${rpmostree_tmpfiles_path}/rpm.conf > rpm.txt 2>/dev/null; then assert_not_file_has_content rpm.txt 'd /var/lib/rpm' fi -ostree --repo="${repo}" cat "${treeref}" /usr/lib/tmpfiles.d/pkg-pam.conf > autovar.txt -# Verify translating /var/run -> /run -assert_file_has_content_literal autovar.txt 'd /run/console' + echo "ok autovar" rpm-ostree db list --repo="${repo}" "${treeref}" --advisories > db-list-adv.txt diff --git a/tests/kolainst/nondestructive/misc.sh b/tests/kolainst/nondestructive/misc.sh index 2d4952cd9b..61f0d90c85 100755 --- a/tests/kolainst/nondestructive/misc.sh +++ b/tests/kolainst/nondestructive/misc.sh @@ -33,6 +33,28 @@ rm -f out.txt rpm-ostree -h usroverlay >out.txt assert_file_has_content out.txt "ostree admin unlock" +# Verify https://github.com/coreos/rpm-ostree/issues/26 +tmpfiles="/usr/lib/tmpfiles.d/rpm-ostree-autovar.conf" +# Duplication in tmp.conf +assert_not_file_has_content $tmpfiles 'd /var/tmp' +# Duplication in var.conf +assert_not_file_has_content $tmpfiles 'd /var/cache ' +assert_file_has_content_literal $tmpfiles 'd /var/lib/chrony 0750 chrony chrony - -' + +set +x # so our grepping doesn't get a hit on itself +if journalctl --grep 'Duplicate line'; then + fatal "Should not get logs (Duplicate line)" +fi +set -x # restore + +# Verify remove can trigger the generation of rpm-ostree-autovar.conf +rpm-ostree override remove chrony +deploy=$(rpm-ostree status --json | jq -r '.deployments[0]["id"]' | awk -F'-' '{print $3}') +osname=$(rpm-ostree status --json | jq -r '.deployments[0]["osname"]') +cat /ostree/deploy/${osname}/deploy/${deploy}/${tmpfiles} > autovar.txt +assert_not_file_has_content autovar.txt '/var/lib/chrony' +rpm-ostree cleanup -pr + # make sure that package-related entries are always present, # even when they're empty. # Validate there's no live state by default. diff --git a/tests/vmcheck/test-layering-basic-1.sh b/tests/vmcheck/test-layering-basic-1.sh index 29a9cf19bb..d35f7ad289 100755 --- a/tests/vmcheck/test-layering-basic-1.sh +++ b/tests/vmcheck/test-layering-basic-1.sh @@ -52,9 +52,9 @@ vm_reboot vm_cmd rpm -qlv test-opt root=$(vm_get_deployment_root 0) -vm_has_files $root/usr/lib/opt/app/bin/foo $root/usr/lib/tmpfiles.d/pkg-test-opt.conf -vm_cmd cat $root/usr/lib/tmpfiles.d/pkg-test-opt.conf -vm_cmd grep -q /usr/lib/opt/app $root/usr/lib/tmpfiles.d/pkg-test-opt.conf +vm_has_files $root/usr/lib/opt/app/bin/foo $root/usr/lib/rpm-ostree/tmpfiles.d/test-opt.conf +vm_cmd cat $root/usr/lib/rpm-ostree/tmpfiles.d/test-opt.conf +vm_cmd grep -q /usr/lib/opt/app $root/usr/lib/rpm-ostree/tmpfiles.d/test-opt.conf vm_cmd ls -al /var/opt/ /var/app vm_cmd test -d "'/opt/some lib/subdir'" vm_cmd test -d '/opt/quote\"ed/subdir' diff --git a/tests/vmcheck/test-override-replace-2.sh b/tests/vmcheck/test-override-replace-2.sh index 2c80e69066..4f1f0bc9d7 100755 --- a/tests/vmcheck/test-override-replace-2.sh +++ b/tests/vmcheck/test-override-replace-2.sh @@ -134,13 +134,15 @@ echo "ok override replace both" # And now verify https://github.com/coreos/rpm-ostree/pull/3228 prev_root=$(vm_get_deployment_root 0) -vm_cmd grep ' /var/pkg-with-var ' "${prev_root}/usr/lib/tmpfiles.d/pkg-pkg-with-var.conf" +vm_cmd grep ' /var/pkg-with-var ' "${prev_root}/usr/lib/rpm-ostree/tmpfiles.d/pkg-with-var.conf" vm_build_rpm pkg-with-var version 2 \ files "/var/pkg-with-different-var" \ install "mkdir -p '%{buildroot}/var/pkg-with-different-var'" vm_rpmostree override replace $YUMREPO/pkg-with-var-2-1.x86_64.rpm new_root=$(vm_get_deployment_root 0) -vm_cmd grep ' /var/pkg-with-different-var ' "${new_root}/usr/lib/tmpfiles.d/pkg-pkg-with-var.conf" +vm_cmd grep ' /var/pkg-with-different-var ' "${new_root}/usr/lib/rpm-ostree/tmpfiles.d/pkg-with-var.conf" +vm_cmd ! grep ' /var/pkg-with-var ' "${new_root}/usr/lib/rpm-ostree/tmpfiles.d/pkg-with-var.conf" +vm_cmd ! grep ' /var/pkg-with-var ' "${new_root}/usr/lib/tmpfiles.d/rpm-ostree-autovar.conf" vm_rpmostree cleanup -p echo "ok override replace deletes tmpfiles.d dropin"