Merge pull request #1101 from cgwalters/tmpfiles

lints: Add var-tmpfiles

Author: cgwalters

Cargo.lock

@@ -7,7 +7,8 @@ members = [
     "utils",
     "blockdev",
     "xtask",
-    "tests-integration"
+    "tests-integration",
+    "tmpfiles"
 ]
 resolver = "2"
 
@@ -59,6 +60,7 @@ similar-asserts = "1.5.0"
 static_assertions = "1.1.0"
 tempfile = "3.10.1"
 tracing = "0.1.40"
+thiserror = "2.0.11"
 tokio = ">= 1.37.0"
 tokio-util = { features = ["io-util"], version = "0.7.10" }
 

Cargo.toml

@@ -34,4 +34,4 @@ RUN tar -C / --zstd -xvf /tmp/bootc.tar.zst && rm -vrf /tmp/*
 # Also copy over arbitrary bits from the target root
 COPY --from=build /build/target/dev-rootfs/ /
 # Test our own linting
-RUN bootc container lint
+RUN bootc container lint --fatal-warnings

hack/Containerfile

@@ -16,6 +16,24 @@ touch ~/.config/nushell/env.nu
 dnf -y install nu
 dnf clean all
 # Stock extra cleaning of logs and caches in general (mostly dnf)
-rm /var/log/* /var/cache /var/lib/dnf /var/lib/rpm-state -rf
+rm /var/log/* /var/cache /var/lib/{dnf,rpm-state,rhsm} -rf
 # And clean root's homedir
 rm /var/roothome/.config -rf
+
+# Fast track tmpfiles.d content from the base image, xref
+# https://gitlab.com/fedora/bootc/base-images/-/merge_requests/92
+if test '!' -f /usr/lib/tmpfiles.d/bootc-base-rpmstate.conf; then
+  cat >/usr/lib/tmpfiles.d/bootc-base-rpmstate.conf <<'EOF'
+# Workaround for https://bugzilla.redhat.com/show_bug.cgi?id=771713
+d /var/lib/rpm-state 0755 - - -
+EOF
+fi
+if ! grep -q -r var/roothome/buildinfo /usr/lib/tmpfiles.d; then
+  cat > /usr/lib/tmpfiles.d/bootc-contentsets.conf <<'EOF'
+# Workaround for https://github.com/konflux-ci/build-tasks-dockerfiles/pull/243
+d /var/roothome/buildinfo 0755 - - -
+d /var/roothome/buildinfo/content_manifests 0755 - - -
+# Note we don't actually try to recreate the content; this just makes the linter ignore it
+f /var/roothome/buildinfo/content_manifests/content-sets.json 0644 - - -
+EOF
+fi

hack/provision-derived.sh

@@ -18,6 +18,7 @@ anstyle = "1.0.6"
 anyhow = { workspace = true }
 bootc-utils = { path = "../utils" }
 bootc-blockdev = { path = "../blockdev" }
+bootc-tmpfiles = { path = "../tmpfiles" }
 camino = { workspace = true, features = ["serde1"] }
 ostree-ext = { path = "../ostree-ext", features = ["bootc"] }
 chrono = { workspace = true, features = ["serde"] }
@@ -51,7 +52,7 @@ xshell = { version = "0.2.6", optional = true }
 uuid = { version = "1.8.0", features = ["v4"] }
 tini = "1.3.0"
 comfy-table = "7.1.1"
-thiserror = "2.0.11"
+thiserror = { workspace = true }
 
 [dev-dependencies]
 similar-asserts = { workspace = true }

lib/Cargo.toml

@@ -7,6 +7,7 @@
 
 use std::collections::BTreeSet;
 use std::env::consts::ARCH;
+use std::fmt::Write as WriteFmt;
 use std::os::unix::ffi::OsStrExt;
 
 use anyhow::Result;
@@ -467,6 +468,53 @@ fn check_varlog(root: &Dir) -> LintResult {
     lint_err(format!("Found non-empty logfile: {first}{others}"))
 }
 
+#[distributed_slice(LINTS)]
+static LINT_VAR_TMPFILES: Lint = Lint {
+    name: "var-tmpfiles",
+    ty: LintType::Warning,
+    description: indoc! { r#"
+Check for content in /var that does not have corresponding systemd tmpfiles.d entries.
+This can cause a problem across upgrades because content in /var from the container
+image will only be applied on the initial provisioning.
+
+Instead, it's recommended to have /var effectively empty in the container image,
+and use systemd tmpfiles.d to generate empty directories and compatibility symbolic links
+as part of each boot.
+"#},
+    f: check_var_tmpfiles,
+    root_type: Some(RootType::Running),
+};
+fn check_var_tmpfiles(_root: &Dir) -> LintResult {
+    let r = bootc_tmpfiles::find_missing_tmpfiles_current_root()?;
+    if r.tmpfiles.is_empty() && r.unsupported.is_empty() {
+        return lint_ok();
+    }
+    let mut msg = String::new();
+    if let Some((samples, rest)) =
+        bootc_utils::iterator_split_nonempty_rest_count(r.tmpfiles.iter(), 5)
+    {
+        msg.push_str("Found content in /var missing systemd tmpfiles.d entries:\n");
+        for elt in samples {
+            writeln!(msg, "  {elt}")?;
+        }
+        if rest > 0 {
+            writeln!(msg, "  ...and {} more", rest)?;
+        }
+    }
+    if let Some((samples, rest)) =
+        bootc_utils::iterator_split_nonempty_rest_count(r.unsupported.iter(), 5)
+    {
+        msg.push_str("Found non-directory/non-symlink files in /var:\n");
+        for elt in samples {
+            writeln!(msg, "  {elt:?}")?;
+        }
+        if rest > 0 {
+            writeln!(msg, "  ...and {} more", rest)?;
+        }
+    }
+    lint_err(msg)
+}
+
 #[distributed_slice(LINTS)]
 static LINT_NONEMPTY_BOOT: Lint = Lint::new_warning(
     "nonempty-boot",
@@ -498,8 +546,17 @@ fn check_boot(root: &Dir) -> LintResult {
 
 #[cfg(test)]
 mod tests {
+    use std::sync::LazyLock;
+
     use super::*;
 
+    static ALTROOT_LINTS: LazyLock<usize> = LazyLock::new(|| {
+        LINTS
+            .iter()
+            .filter(|lint| lint.root_type != Some(RootType::Running))
+            .count()
+    });
+
     fn fixture() -> Result<cap_std_ext::cap_tempfile::TempDir> {
         let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?;
         Ok(tempdir)
@@ -557,26 +614,27 @@ mod tests {
         let mut out = Vec::new();
         let root_type = RootType::Alternative;
         let r = lint_inner(root, root_type, [], &mut out).unwrap();
-        assert_eq!(r.passed, LINTS.len());
+        let running_only_lints = LINTS.len().checked_sub(*ALTROOT_LINTS).unwrap();
+        assert_eq!(r.passed, *ALTROOT_LINTS);
         assert_eq!(r.fatal, 0);
-        assert_eq!(r.skipped, 0);
+        assert_eq!(r.skipped, running_only_lints);
         assert_eq!(r.warnings, 0);
 
         let r = lint_inner(root, root_type, ["var-log"], &mut out).unwrap();
         // Trigger a failure in var-log
         root.create_dir_all("var/log/dnf")?;
         root.write("var/log/dnf/dnf.log", b"dummy dnf log")?;
-        assert_eq!(r.passed, LINTS.len().checked_sub(1).unwrap());
+        assert_eq!(r.passed, ALTROOT_LINTS.checked_sub(1).unwrap());
         assert_eq!(r.fatal, 0);
-        assert_eq!(r.skipped, 1);
+        assert_eq!(r.skipped, running_only_lints + 1);
         assert_eq!(r.warnings, 0);
 
         // But verify that not skipping it results in a warning
         let mut out = Vec::new();
         let r = lint_inner(root, root_type, [], &mut out).unwrap();
-        assert_eq!(r.passed, LINTS.len().checked_sub(1).unwrap());
+        assert_eq!(r.passed, ALTROOT_LINTS.checked_sub(1).unwrap());
         assert_eq!(r.fatal, 0);
-        assert_eq!(r.skipped, 0);
+        assert_eq!(r.skipped, running_only_lints);
         assert_eq!(r.warnings, 1);
         Ok(())
     }

lib/src/lints.rs

@@ -0,0 +1,24 @@
+[package]
+name = "bootc-tmpfiles"
+version = "0.1.0"
+license = "MIT OR Apache-2.0"
+edition = "2021"
+publish = false
+
+[dependencies]
+camino = { workspace = true }
+fn-error-context = { workspace = true }
+cap-std-ext = { version = "4" }
+thiserror = { workspace = true }
+tempfile = { workspace = true }
+bootc-utils = { path = "../utils" }
+rustix = { workspace = true }
+uzers = "0.12"
+
+[dev-dependencies]
+anyhow = { workspace = true }
+indoc = { workspace = true }
+similar-asserts = { workspace = true }
+
+[lints]
+workspace = true