Skip to content

Commit

Permalink
utils.rs: remove duplicate tmpfiles entries
Browse files Browse the repository at this point in the history
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 coreos#26 (comment)

Fixes coreos#26
  • Loading branch information
HuijingHei committed Dec 8, 2023
1 parent ce207c9 commit 36e450c
Show file tree
Hide file tree
Showing 10 changed files with 186 additions and 18 deletions.
14 changes: 14 additions & 0 deletions rpmostree-cxxrs.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
2 changes: 2 additions & 0 deletions rpmostree-cxxrs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 3 additions & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ pub mod ffi {
extern "Rust" {
fn prepare_rpm_layering(rootfs: i32, merge_passwd_dir: &str) -> Result<bool>;
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<String>) -> Result<()>;
fn migrate_passwd_except_root(rootfs: i32) -> Result<()>;
Expand Down Expand Up @@ -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;
Expand Down
120 changes: 120 additions & 0 deletions rust/src/tmpfiles.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/*
* 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::<Result<HashMap<String, String>>>()?;

// 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::<Result<HashSet<String>>>()?;

// 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<impl Iterator<Item = String>> {
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;
}
}
Some(
tmpfiles_dir
.read_to_string(name)
.unwrap()
.lines()
.filter(|s| !s.is_empty() && !s.starts_with('#'))
.map(|s| s.to_string())
.collect::<Vec<_>>(),
)
})
.flatten()
.collect::<Vec<_>>();

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 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}");
}
}
}
14 changes: 9 additions & 5 deletions src/libpriv/rpmostree-core.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);

Expand Down
6 changes: 4 additions & 2 deletions src/libpriv/rpmostree-importer.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 5 additions & 6 deletions tests/compose/test-basic-unified.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions tests/kolainst/nondestructive/misc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions tests/vmcheck/test-layering-basic-1.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
6 changes: 4 additions & 2 deletions tests/vmcheck/test-override-replace-2.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down

0 comments on commit 36e450c

Please sign in to comment.