https://github.com/coreos/rpm-ostree/pull/5339 which pairs with the previous backport: https://github.com/coreos/rpm-ostree/pull/5341 Resolves: #RHEL-84351
This commit is contained in:
parent
a16b26a669
commit
725c09cf42
@ -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>
|
From: Colin Walters <walters@verbum.org>
|
||||||
Date: Thu, 20 Mar 2025 14:53:37 -0400
|
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
|
- First ensure we're always ignoring SELinux here, in this
|
||||||
use case we rely on e.g. bootc to do client side labeling.
|
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
|
But worse, having `user.` xattrs provokes backwards incompat
|
||||||
bugs from https://github.com/ostreedev/ostree/pull/3346
|
bugs from https://github.com/ostreedev/ostree/pull/3346
|
||||||
---
|
---
|
||||||
rust/src/compose.rs | 115 ++++++++++++++++++++-
|
rust/src/compose.rs | 124 ++++++++++++++++++++-
|
||||||
src/app/rpmostree-compose-builtin-tree.cxx | 23 +++--
|
src/app/rpmostree-compose-builtin-tree.cxx | 23 ++--
|
||||||
tests/compose-rootfs/Containerfile | 4 +
|
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
|
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
|
--- a/rust/src/compose.rs
|
||||||
+++ b/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
|
// SPDX-License-Identifier: Apache-2.0 OR MIT
|
||||||
|
|
||||||
use std::borrow::Cow;
|
use std::borrow::Cow;
|
||||||
@ -32,13 +157,12 @@ index 0bd77883..e5207308 100644
|
|||||||
use std::io::{BufRead, BufReader, BufWriter, Write};
|
use std::io::{BufRead, BufReader, BufWriter, Write};
|
||||||
use std::num::NonZeroU32;
|
use std::num::NonZeroU32;
|
||||||
use std::os::fd::{AsFd, AsRawFd};
|
use std::os::fd::{AsFd, AsRawFd};
|
||||||
-use std::path::Path;
|
|
||||||
+use std::os::unix::ffi::OsStrExt;
|
+use std::os::unix::ffi::OsStrExt;
|
||||||
+use std::path::{Path, PathBuf};
|
+use std::path::{Path, PathBuf};
|
||||||
use std::process::Command;
|
use std::process::Command;
|
||||||
|
|
||||||
use anyhow::{anyhow, Context, Result};
|
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(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -118,7 +242,24 @@ index 0bd77883..e5207308 100644
|
|||||||
impl RootfsOpts {
|
impl RootfsOpts {
|
||||||
// For bad legacy reasons "compose install" actually writes to a subdirectory named rootfs.
|
// 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/
|
// 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(
|
let repo = ostree::Repo::create_at(
|
||||||
libc::AT_FDCWD,
|
libc::AT_FDCWD,
|
||||||
repo_path.as_str(),
|
repo_path.as_str(),
|
||||||
@ -127,7 +268,7 @@ index 0bd77883..e5207308 100644
|
|||||||
None,
|
None,
|
||||||
gio::Cancellable::NONE,
|
gio::Cancellable::NONE,
|
||||||
)?;
|
)?;
|
||||||
@@ -553,6 +628,9 @@ impl RootfsOpts {
|
@@ -564,6 +650,9 @@ impl RootfsOpts {
|
||||||
.args([
|
.args([
|
||||||
"compose",
|
"compose",
|
||||||
"install",
|
"install",
|
||||||
@ -137,7 +278,7 @@ index 0bd77883..e5207308 100644
|
|||||||
"--unified-core",
|
"--unified-core",
|
||||||
"--postprocess",
|
"--postprocess",
|
||||||
"--repo",
|
"--repo",
|
||||||
@@ -571,6 +649,12 @@ impl RootfsOpts {
|
@@ -582,6 +671,12 @@ impl RootfsOpts {
|
||||||
.args([manifest.as_str(), self.dest.as_str()])
|
.args([manifest.as_str(), self.dest.as_str()])
|
||||||
.run()
|
.run()
|
||||||
.context("Executing compose install")?;
|
.context("Executing compose install")?;
|
||||||
@ -150,7 +291,7 @@ index 0bd77883..e5207308 100644
|
|||||||
// Undo the subdirectory "rootfs"
|
// Undo the subdirectory "rootfs"
|
||||||
{
|
{
|
||||||
let target = Dir::open_ambient_dir(&self.dest, cap_std::ambient_authority())?;
|
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::fs::PermissionsExt;
|
||||||
use cap_std_ext::cap_tempfile;
|
use cap_std_ext::cap_tempfile;
|
||||||
use gio::prelude::FileExt;
|
use gio::prelude::FileExt;
|
||||||
@ -250,11 +391,11 @@ index f05bae80..df446f0b 100644
|
|||||||
|
|
||||||
g_autoptr (OstreeRepo) layer_repo = NULL;
|
g_autoptr (OstreeRepo) layer_repo = NULL;
|
||||||
diff --git a/tests/compose-rootfs/Containerfile b/tests/compose-rootfs/Containerfile
|
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
|
--- a/tests/compose-rootfs/Containerfile
|
||||||
+++ b/tests/compose-rootfs/Containerfile
|
+++ b/tests/compose-rootfs/Containerfile
|
||||||
@@ -28,6 +28,10 @@ set -xeuo pipefail
|
@@ -30,6 +30,10 @@ test '!' -f /ostree/repo/config
|
||||||
test '!' -f /ostree/repo/config
|
test $(($(stat -c '0%a' /) % 2)) = 1
|
||||||
# Validate we have file caps
|
# Validate we have file caps
|
||||||
getfattr -d -m security.capability /usr/bin/newuidmap
|
getfattr -d -m security.capability /usr/bin/newuidmap
|
||||||
+# Validate we don't have user.ostreemeta
|
+# Validate we don't have user.ostreemeta
|
||||||
@ -265,5 +406,5 @@ index 91187cc9..d72577f0 100644
|
|||||||
EORUN
|
EORUN
|
||||||
LABEL containers.bootc 1
|
LABEL containers.bootc 1
|
||||||
--
|
--
|
||||||
2.49.0
|
2.48.1
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user