rpms/rpm-ostree
6
0
Fork 0
Code Issues Pull Requests Releases Activity

backport: https://github.com/coreos/rpm-ostree/pull/5322

Browse Source 
and https://github.com/coreos/rpm-ostree/pull/5339
which pairs with the previous backport:
https://github.com/coreos/rpm-ostree/pull/5341

Resolves: #RHEL-84466
This commit is contained in:
Joseph Marrero Corchado 2025-03-21 15:05:29 -04:00
parent 55ebf69623
commit 6eddd3fe57
2 changed files with 165 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

175
0001-compose-rootfs-Ensure-we-don-t-emit-user.ostreemeta.patch
View File

@ -1,7 +1,132 @@
From 13773d16f0c94b19c73bf321fcc4b9dc87e57843 Mon Sep 17 00:00:00 2001
From 303a691757317df8cb9d4b576c3b8dd73d8e53df Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Tue, 4 Mar 2025 17:07:40 -0500
Subject: [PATCH 1/3] compose-rootfs: Fix perms for /
The trick we did here in working around `compose install`
generating a `rootfs` subdirectory by moving its contents into
the parent meant we lost the mode bits.
Somehow, only when targeting this for CentOS we hit on the
mode being 0700 (I think this may relate to umask? We probably
need to audit our whole codebase to avoid umask issues; but
really in the longer term switch to trusting the `filesystem`
rpm instead of manually extracting the root).
Signed-off-by: Colin Walters <walters@verbum.org>
---
rust/src/compose.rs | 17 +++++++++++++----
tests/compose-rootfs/Containerfile | 2 ++
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/rust/src/compose.rs b/rust/src/compose.rs
index 0bd77883..34dfb2db 100644
--- a/rust/src/compose.rs
+++ b/rust/src/compose.rs
@@ -8,7 +8,6 @@ use std::fs::File;
use std::io::{BufRead, BufReader, BufWriter, Write};
use std::num::NonZeroU32;
use std::os::fd::{AsFd, AsRawFd};
-use std::path::Path;
use std::process::Command;
use anyhow::{anyhow, Context, Result};
@@ -493,15 +492,25 @@ impl RootfsOpts {
}
d.remove_all_optional(&name)?;
}
- for ent in d.read_dir(rootfs_name).context("Reading rootfs")? {
+ let rootfs = d.open_dir(rootfs_name)?;
+ for ent in rootfs.entries().context("Reading rootfs")? {
let ent = ent?;
let name = ent.file_name();
- let origpath = Path::new(rootfs_name).join(&name);
- d.rename(origpath, d, &name)
+ rootfs
+ .rename(&name, d, &name)
.with_context(|| format!("Renaming rootfs/{name:?}"))?;
}
// Clean up the now empty dir
d.remove_dir(rootfs_name).context("Removing rootfs")?;
+
+ // Propagate mode bits from source to target
+ {
+ let perms = rootfs.dir_metadata()?.permissions();
+ d.set_permissions(".", perms)
+ .context("Setting target permissions")?;
+ tracing::debug!("rootfs fixup complete");
+ }
+
Ok(())
}
diff --git a/tests/compose-rootfs/Containerfile b/tests/compose-rootfs/Containerfile
index 91187cc9..00b70f7e 100644
--- a/tests/compose-rootfs/Containerfile
+++ b/tests/compose-rootfs/Containerfile
@@ -26,6 +26,8 @@ RUN <<EORUN
set -xeuo pipefail
# Validate we aren't using ostree-container format
test '!' -f /ostree/repo/config
+# Validate executable bit for others on /
+test $(($(stat -c '0%a' /) % 2)) = 1
# Validate we have file caps
getfattr -d -m security.capability /usr/bin/newuidmap
bootc container lint
--
2.48.1
From 5962ce2978e5f0c60bb7506b707572a0779a74b6 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Mon, 17 Mar 2025 19:05:21 -0400
Subject: [PATCH 2/3] compose: Ensure tempdir is dropped after commit
The tempdir holds the ostree repo.
I'm looking at an issue reported in a downstream build that failed
like this:
```
+ rpm-ostree experimental compose build-chunked-oci --bootc --format-version=1 --rootfs /target-rootfs --output oci-archive:/buildcontext/out.ociarchive
Generating commit...
error: Generating commit from rootfs: syncfs: Not a directory
subprocess exited with status 1
subprocess exited with status 1
```
Which frankly has me just confused. The `syncfs` here is
almost certainly coming from libostree's sync of the target repo.
And "Not a directory" is especially weird, implying
that `repo/tmp` got swapped with something else?
Signed-off-by: Colin Walters <walters@verbum.org>
---
rust/src/compose.rs | 2 ++
1 file changed, 2 insertions(+)
diff --git a/rust/src/compose.rs b/rust/src/compose.rs
index 34dfb2db..ef63446c 100644
--- a/rust/src/compose.rs
+++ b/rust/src/compose.rs
@@ -352,6 +352,8 @@ impl BuildChunkedOCIOpts {
.context("Invoking compose container-encapsulate")?;
drop(rootfs);
+ // Ensure our tempdir is only dropped now
+ drop(td);
match rootfs_source {
FileSource::Rootfs(_) => {}
FileSource::Podman(mnt) => {
--
2.48.1
From 96c90c9d3bf6461fc1c96f37825445dd0afa6d7b Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Thu, 20 Mar 2025 14:53:37 -0400
Subject: [PATCH] compose-rootfs: Ensure we don't emit user.ostreemeta
Subject: [PATCH 3/3] compose-rootfs: Ensure we don't emit user.ostreemeta
- First ensure we're always ignoring SELinux here, in this
use case we rely on e.g. bootc to do client side labeling.
@ -12,16 +137,16 @@ Subject: [PATCH] compose-rootfs: Ensure we don't emit user.ostreemeta
But worse, having `user.` xattrs provokes backwards incompat
bugs from https://github.com/ostreedev/ostree/pull/3346
---
rust/src/compose.rs | 115 ++++++++++++++++++++-
src/app/rpmostree-compose-builtin-tree.cxx | 23 +++--
rust/src/compose.rs | 124 ++++++++++++++++++++-
src/app/rpmostree-compose-builtin-tree.cxx | 23 ++--
tests/compose-rootfs/Containerfile | 4 +
3 files changed, 132 insertions(+), 10 deletions(-)
3 files changed, 142 insertions(+), 9 deletions(-)
diff --git a/rust/src/compose.rs b/rust/src/compose.rs
index 0bd77883..e5207308 100644
index ef63446c..67fef8cf 100644
--- a/rust/src/compose.rs
+++ b/rust/src/compose.rs
@@ -3,12 +3,14 @@
@@ -3,11 +3,14 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT
use std::borrow::Cow;
@ -32,13 +157,12 @@ index 0bd77883..e5207308 100644
use std::io::{BufRead, BufReader, BufWriter, Write};
use std::num::NonZeroU32;
use std::os::fd::{AsFd, AsRawFd};
-use std::path::Path;
+use std::os::unix::ffi::OsStrExt;
+use std::path::{Path, PathBuf};
use std::process::Command;
use anyhow::{anyhow, Context, Result};
@@ -477,6 +479,79 @@ fn mutate_source_root(exec_root: &Dir, source_root: &Utf8Path) -> Result<()> {
@@ -478,6 +481,79 @@ fn mutate_source_root(exec_root: &Dir, source_root: &Utf8Path) -> Result<()> {
Ok(())
}
@ -118,7 +242,24 @@ index 0bd77883..e5207308 100644
impl RootfsOpts {
// For bad legacy reasons "compose install" actually writes to a subdirectory named rootfs.
// Clean that up by deleting everything except rootfs/ and moving the contents of rootfs/
@@ -540,7 +615,7 @@ impl RootfsOpts {
@@ -513,6 +589,16 @@ impl RootfsOpts {
tracing::debug!("rootfs fixup complete");
}
+ // And finally, clean up the ostree.usermeta xattr
+ let mut info = XattrRemovalInfo::default();
+ strip_usermeta(d, &mut info)?;
+ if info.count > 0 {
+ eprintln!("Found unhandled xattrs in files: {}", info.count);
+ for attr in info.names {
+ eprintln!(" {attr:?}");
+ }
+ }
+
Ok(())
}
@@ -551,7 +637,7 @@ impl RootfsOpts {
let repo = ostree::Repo::create_at(
libc::AT_FDCWD,
repo_path.as_str(),
@ -127,7 +268,7 @@ index 0bd77883..e5207308 100644
None,
gio::Cancellable::NONE,
)?;
@@ -553,6 +628,9 @@ impl RootfsOpts {
@@ -564,6 +650,9 @@ impl RootfsOpts {
.args([
"compose",
"install",
@ -137,7 +278,7 @@ index 0bd77883..e5207308 100644
"--unified-core",
"--postprocess",
"--repo",
@@ -571,6 +649,12 @@ impl RootfsOpts {
@@ -582,6 +671,12 @@ impl RootfsOpts {
.args([manifest.as_str(), self.dest.as_str()])
.run()
.context("Executing compose install")?;
@ -150,7 +291,7 @@ index 0bd77883..e5207308 100644
// Undo the subdirectory "rootfs"
{
let target = Dir::open_ambient_dir(&self.dest, cap_std::ambient_authority())?;
@@ -1136,9 +1220,34 @@ mod tests {
@@ -1147,9 +1242,34 @@ mod tests {
use cap_std::fs::PermissionsExt;
use cap_std_ext::cap_tempfile;
use gio::prelude::FileExt;
@ -250,11 +391,11 @@ index f05bae80..df446f0b 100644
g_autoptr (OstreeRepo) layer_repo = NULL;
diff --git a/tests/compose-rootfs/Containerfile b/tests/compose-rootfs/Containerfile
index 91187cc9..d72577f0 100644
index 00b70f7e..e711e2c4 100644
--- a/tests/compose-rootfs/Containerfile
+++ b/tests/compose-rootfs/Containerfile
@@ -28,6 +28,10 @@ set -xeuo pipefail
test '!' -f /ostree/repo/config
@@ -30,6 +30,10 @@ test '!' -f /ostree/repo/config
test $(($(stat -c '0%a' /) % 2)) = 1
# Validate we have file caps
getfattr -d -m security.capability /usr/bin/newuidmap
+# Validate we don't have user.ostreemeta
@ -265,5 +406,5 @@ index 91187cc9..d72577f0 100644
EORUN
LABEL containers.bootc 1
--
2.49.0
2.48.1

8
rpm-ostree.spec
View File

@ -4,7 +4,7 @@
Summary: Hybrid image/package system
Name: rpm-ostree
Version: 2025.6
Release: 3%{?dist}
Release: 4%{?dist}
License: LGPL-2.0-or-later
URL: https://github.com/coreos/rpm-ostree
# This tarball is generated via "cd packaging && make -f Makefile.dist-packaging dist-snapshot"
@ -303,6 +303,12 @@ fi
%files devel -f files.devel
%changelog
* Fri Mar 21 2025 Joseph Marrero <jmarrero@fedoraproject.org> - 2025.6-4
- Backport: https://github.com/coreos/rpm-ostree/pull/5322
Backport: https://github.com/coreos/rpm-ostree/pull/5339
Resolves: #RHEL-84466
* Fri Mar 21 2025 Joseph Marrero <jmarrero@fedoraproject.org> - 2025.6-3
- Backport https://github.com/coreos/rpm-ostree/pull/5341
Resolves: #RHEL-84466