tests: Add composefs-upgrade test for sealed UKI builds #2049
tests: Add composefs-upgrade test for sealed UKI builds #2049cgwalters wants to merge 2 commits intobootc-dev:mainfrom
Conversation
The upgrade test image (localhost/bootc-upgrade) was previously a simple one-layer addition on top of localhost/bootc that did not go through the sealing pipeline. This meant sealed composefs builds could not properly test upgrades, since the upgrade image lacked a signed UKI with the correct composefs digest. Rework Dockerfile.upgrade into a multi-stage build that mirrors the main Dockerfile sealing flow: when boot_type=uki, it computes the composefs digest of the upgrade rootfs, generates and optionally signs a UKI via seal-uki, and finalizes it with finalize-uki. For non-UKI builds, the extra stages are effectively no-ops and the image remains a simple derived layer. Update _build-upgrade-image in the Justfile to pass the required build arguments (boot_type, seal_state, filesystem) and build secrets (secureboot keys). Extra container capabilities (CAP_ALL, fuse device) are only added for UKI builds that need composefs support. Assisted-by: OpenCode (claude-opus-4)
The goal is ensuring we have upgrade coverage also for sealed UKIs; most of the other update code paths (because tmt doesn't make it easy to have a registry) do on-machine synthetic updates. Assisted-by: OpenCode (claude-opus-4) Signed-off-by: Colin Walters <walters@verbum.org>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a new test for composefs-upgrade with sealed UKI builds, involving updates to the Justfile, a new Dockerfile.upgrade, and related test scripts. However, it introduces several command injection vulnerabilities in the Justfile and Dockerfile.upgrade due to improper handling of environment variables and build arguments. These could allow arbitrary command execution and leakage of sensitive Secure Boot keys. The overall approach is solid, but robustness and portability of scripts can be improved, and the identified security vulnerabilities must be addressed by properly quoting variables and using shell variables directly.
| # deployment (upgrade) and one for the rollback (original) | ||
| let efi_files = (glob $"($boot_dir)/*.efi") | ||
| print $"EFI files: ($efi_files)" | ||
| assert ((($efi_files | length) >= 2)) $"expected at least 2 UKIs on ESP, found ($efi_files | length)" |
There was a problem hiding this comment.
The triple parentheses are redundant.
|
The failure should be fixed in PR #2047. And need a sign-off. |
| @@ -0,0 +1,111 @@ | |||
| # number: 39 | |||
There was a problem hiding this comment.
Could we not use the test-image-upgrade test here? Since it already has
if ($imgsrc | str ends-with "-local") {
bootc image copy-to-storage
# A simple derived container that adds a file
"FROM localhost/bootc
RUN touch /usr/share/testing-bootc-upgrade-apply
" | save Dockerfile
# Build it
podman build -t $imgsrc .
}It won't rebuild if we have the upgrade image built already
I think for now this is really good to have test, but imo we should have the same test (code) for all cases, so we don't end up missing something while special casing certain tests
3 participants
The goal is ensuring we have upgrade coverage also for sealed UKIs;
most of the other update code paths (because tmt doesn't make it easy
to have a registry) do on-machine synthetic updates.
Assisted-by: OpenCode (claude-opus-4)