Conversation
|
OK I think the chain here should be:
|
3b90847 to
f92013b
Compare
f92013b to
69757fc
Compare
There was a problem hiding this comment.
lgtm
just needs a rebase I think.
The only nit is that
config.erofs_formats = composefs_ctl::composefs::erofs::format::FormatConfig {
default: composefs_ctl::composefs::erofs::format::FormatVersion::V1,
extra: [composefs_ctl::composefs::erofs::format::FormatVersion::V2].into(),
};
Is duplicated 3 times here. So if we add a new version in the future we need to care about 3 places. Maybe it should be a helper function?
Johan-Liebert1
left a comment
There was a problem hiding this comment.
Lots of .clone() that I believe shouldn't be needed, I might be wrong though. Supporting both V1 and V2 are great, but it is a bit messy, not sure if there's a better way to do this. Also, some inconsistencies here and there (esp in comments) regarding whether V1 is the default or V2
| seal_state=$1 | ||
| shift | ||
| # EROFS format version to pass to bootc container ukify (optional, default: v2) | ||
| erofs_version=${1:-v2} |
There was a problem hiding this comment.
Kind of conflicts with composefs/composefs-rs#330. We'd probably want to have the same defaults everywhere
| os_id: Option<String>, | ||
| boot_digest: String, | ||
| /// The composefs image digest parsed from (and validated against) the UKI's | ||
| /// own cmdline. This is the authoritative deployment key for UKI boots: |
There was a problem hiding this comment.
This is for every boot right, not just UKIs?
| let composefs_info = BootComposefsCmdline::<Sha512HashValue>::from_cmdline(&cmdline) | ||
| .context("Parsing composefs=")? | ||
| .ok_or_else(|| anyhow::anyhow!("No composefs= or composefs.digest.v1= karg found in UKI cmdline"))?; | ||
| let composefs_cmdline = composefs_info.digest().clone(); |
There was a problem hiding this comment.
This name is a bit confusing. afaiu this is only the digest and not the entire cmdline?
|
|
||
| if test "${boot_type}" = "uki"; then | ||
| /run/packaging/seal-uki /run/target /out /run/secrets "${allow_missing_verity}" "${seal_state}" | ||
| /run/packaging/seal-uki /run/target /out /run/secrets "${allow_missing_verity}" "${seal_state}" "${erofs_version}" |
There was a problem hiding this comment.
We also need this in tmt/tests/booted/test-install-to-filesystem-var-mount.sh
| // (see setup_composefs_boot for the full rationale). Provisional value for | ||
| // BLS (where bootc writes the karg from this same id); overridden for UKI by | ||
| // the digest the UKI cmdline actually carries. | ||
| let provisional_deploy_id = boot_id_v2.clone().unwrap_or_else(|| id.clone()); |
There was a problem hiding this comment.
id here is confusing especially with both boot_id_v1/v2 defined. I believe it's the erofs digest corresponding to the erofs version that the repo is currently using?
| // Authoritative collision check against the final deploy key. For UKI this | ||
| // may differ from the provisional checked above (the UKI may carry a | ||
| // non-default digest), so this is the load-bearing guarantee. | ||
| ensure_no_deploy_collision(host, &deploy_id)?; |
There was a problem hiding this comment.
why do we need to do this again?
| // setup-root opens `state/deploy/<this>` using that same karg, so we must | ||
| // key the deployment off exactly this value -- whether the UKI was sealed | ||
| // with the V2 (default) or V1 EROFS digest. | ||
| let deploy_id = uki_info.composefs_cmdline.clone(); |
There was a problem hiding this comment.
we shouldn't need to clone this?
6ec1bf4 to
cbff4b5
Compare
|
In the general case we may need to add support for "older bootc version" which includes not just the v2 digest but the xattr filtering logic too? See composefs/composefs-rs#337 |
cbff4b5 to
8e7417f
Compare
4bf145c to
45e92e4
Compare
|
OK, this one wants #2290 to land first which fixes our composefs mounts on c9s. |
45e92e4 to
f839490
Compare
OK! Finally that landed. I rebased this, but it's still somewhat raw, especially the last commit. |
| repo_requires_fsverity: bool, | ||
| missing_fsverity_allowed: bool, | ||
| uki_allows_missing_fsverity: bool, |
There was a problem hiding this comment.
Too many bool here...I think we may need an enum
| Ok(()) | ||
| } | ||
|
|
||
| /// Validate every primary UKI before any bootloader or ESP operation. The |
There was a problem hiding this comment.
Hmmm I'm not sure, I think we need to be robust to corrupted state and allow recovery. Bailing early hurts that.
This seems more like a bootc internals fsck style thing.
| .file_path | ||
| .file_name() | ||
| .ok_or_else(|| anyhow!("Could not get UKI file name"))? | ||
| .to_string_lossy() |
There was a problem hiding this comment.
No. We should require UTF-8.
| const COMPOSEFS_DIGEST_V1_FEATURE: &str = "/usr/lib/bootc/initramfs-features/composefs-digest-v1"; | ||
| const COMPOSEFS_DIGEST_V1_FEATURE_CONTENT: &[u8] = b"composefs-digest-v1 state-v1\n"; | ||
|
|
||
| /// Query `lsinitrd` without unpacking or executing any initramfs contents. |
There was a problem hiding this comment.
No, this is awful. I don't want ukify to be parsing the initramfs.
Among other things I don't want to hard depend on dracut.
I think we should just default to injecting both EROFS kargs right?
There was a problem hiding this comment.
If we just want the cmdline, we have a function in composefs-rs for that. get_uki_cmdline_buffered
|
The f44 UKI failures seem related to https://bugzilla.redhat.com/show_bug.cgi?id=2507393 I see the following in audit logs and the service fails with |
CentOS 9 cannot consume sealed host-built UKI upgrades because shared storage is unavailable and its guest-local builder produces unsigned images. Record that limitation while retaining installation, readonly, other upgrade variants, and newer-system sealed coverage. With V1 EROFS as the default, sealed UKIs are viable on CentOS 9; exclude only the BLS and unsealed modes that still require newer dracut/systemd features. Resolve both the runtime base and buildroot from each matrix OS. Otherwise CentOS 9 jobs silently build EL10 RPMs and binaries that cannot run against its older glibc. Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
The `--bind-storage-ro` host container-storage passthrough relies on a libvirt-managed virtiofsd, which cannot run in some environments such as nested user namespaces or cloud/non-qemu setups. Plans that normally request bind-storage previously had no way to opt out short of editing plan metadata. Add a `--skip-bind-storage` flag (and matching `BOOTC_skip_bind_storage` env var) that forces those plans to run without the host container- storage mount. Default behavior is unchanged: bind-storage is still used wherever it is requested and supported. Plans that depend on a locally built upgrade image reaching the VM via bind-storage will be unable to perform the upgrade/switch step when this is set. Assisted-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
composefs-rs landed support for V1 EROFS, which we need to enable composefs on RHEL9. Make new installs produce both V1 and V2 EROFS images for committed composefs images, and make V1 the default wherever a single format must be chosen: the repository's default EROFS format, the `--erofs-version` flag on `bootc container ukify` and `compute-composefs-digest`, and the provisional BLS deploy key computed at install time. V2 remains available via `--erofs-version=v2` and is always generated alongside V1, so a deployment can still be booted via the legacy `composefs=` karg. This keeps the install path consistent with the upgrade and GC paths, which already prefer V1. Critically, a V1 digest must be written as a `composefs.digest=v1-...` karg, not the legacy `composefs=` shorthand (which upstream reserves for V2). Add `build_composefs_karg`, which selects the correct form via composefs-boot's own `ComposefsCmdline::new_v1`/`new_v2` and `to_cmdline_arg`, and use it everywhere bootc writes a new karg (install, upgrade, `container ukify`, soft-reboot) instead of the version-unaware helper that only ever emitted `composefs=`. Assisted-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
Older and newer composefs-rs tooling can differ in xattr filtering and EROFS defaults, which otherwise breaks UKI upgrades across bootc versions. Search supported combinations for the digest embedded in the UKI so a newer client can adapt to an older target. Keep the missing-deployment resilience fixture syntactically valid so typed argument parsing reaches the warning path it is intended to exercise. Assisted-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
2df476c to
c30d873
Compare
|
Thinking this one should be a reason to bump to 1.17.x! |
All variants of this are stored in the splitstream and discoverable. |
| /// replacement is generated by composefs-rs itself, so format and feature | ||
| /// settings cannot drift from the normal initialization path. | ||
| pub(crate) fn finalize_fresh_repository_policy( | ||
| rootfs: &std::path::Path, |
There was a problem hiding this comment.
Ideally we use fd-relative APIs in general
There was a problem hiding this comment.
Yes, also we're doing a bunch of stuff here related to the composefs repo itself, which I think should belong in composefs-rs as helpers on the Repository object
c30d873 to
d986021
Compare
|
I tested upgrading a sealed composefs RHEL 10.2 system (stock bootc 1.16.4, UKI + systemd-boot + Secure Boot, the rhel-bootc-examples sealing example) to this PR's c10s RPMs, with the new bootc in the compose so the initramfs is regenerated. The upgrade chain works: 1.16.4 But there's a problem with bare (The on-disk image header also has
Suggested fix: treat bare Tested with the Generated-by: https://github.com/cgwalters/#llms |
We want to support upgrades from older bootc. bootc 1.16.0 stagers only understand the bare `composefs=` argument and create only V2 images and state. A UKI built with a newly regenerated initramfs carries the V1 digest followed by the V2 fallback, so its first boot selects the V2 image and state. A subsequent upgrade by a current client can then create and select the V1 image. Image builders must regenerate the initramfs when updating bootc; this change does not claim compatibility for stale initramfs artifacts. Explicit V2 remains available as a format control. Always emit both digests without probing initramfs contents. Preserve strict repository requirements when recovering a non-default boot image; changing serialization must not relax integrity. Assisted-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
Composefs installation must preserve kernel argument policy, validate the target repository before writing boot artifacts, and inspect external images from a target-backed staged copy. Keep the source identity and initial UKI fs-verity policy authoritative across the entire install path. A bare `composefs=<hex>` UKI argument does not identify an EROFS format, even though composefs-rs documents it as V2 shorthand: bootc 1.16.3 sealed a V2 digest there, but 1.16.4 through 1.16.13 seal V1. Accept a bare digest matching a boot image of either format and only enforce the format for the self-describing `composefs.digest=vN-...` form; otherwise switching to any image sealed by those releases fails. Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
The compatibility claim depends on regenerated current initramfs artifacts and on keeping strict and missing-verity fixtures distinct. Make the opt-in tests assert those boundaries and inspect the installed target rather than the running installer. Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
The previous text read like an internal test log (test plan numbers, TMT variables, evidence lists) and repeated the same compatibility caveat several times. Describe what users need instead: the V1/V2 kernel arguments, what the bare composefs= argument has meant across releases, how to upgrade from 1.16, and what is known not to work yet. The only tested upgrade path remains bootc 1.16.0. Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
The separate-/var install test should exercise the format selected by the test plan rather than silently forcing V1. Forward the environment selection while retaining V1 as the standalone default. Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
There's a very complex dependency chain between `systemd-tmpfiles-setup.service`, `systemd-random-seed.service` and `systemd-tpm2-setup.service` among others. Basically we need `/var/lib` to be created, which will be done by the tmpfiles setup, but random-seed will normally do it too, but it can't because Fedora SELinux denies it. A surprising discovery is that this happens to be papered over for the OSTree backend by `ostree-remount.service`. Add a test case that no services fail. However this check needs to currently skip due to https://bugzilla.redhat.com/show_bug.cgi?id=2507393. Assisted-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
The manifest tracks the v0.20.0 tag, but the stale revision-based lock entry makes locked Cargo operations fail. Regenerate the entry so dependency resolution agrees with the declared source. Assisted-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
d986021 to
51ae138
Compare
how would the older client parse V1 cmdline arg? |
It just parses |
|
In the copilot review
okay, kinda makes sense then |
Johan-Liebert1
left a comment
There was a problem hiding this comment.
Looks good to me now. @cgwalters I've disabled auto merge in case you want to change something here
This adapts bootc to build on top of the work in composefs/composefs-rs#297
A toplevel goal here is supporting both the v1 and v2 EROFS formats, which means we'll work with RHEL9 era systems.
Right now
bootc container ukifystill generatescomposefs=i.e. v2, but I'd like to change that to do both - it's a pretty cheap thing (the main cost is generating the fsverity digests).