-
Notifications
You must be signed in to change notification settings - Fork 168
build-sys: Enable CentOS Stream compose repos to avoid version skew #1926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a mechanism to use CentOS Stream compose repositories to prevent version skew issues during image builds by adding a new script enable-compose-repos and executing it in the Dockerfile. However, a high-severity security vulnerability was identified: the script disables GPG signature checking, which exposes the build process to potential package-tampering attacks. It is strongly recommended to enable GPG checks and provide the official CentOS GPG key to mitigate this risk.
150853e to
d3a4127
Compare
d3a4127 to
37f71d3
Compare
|
Heh, the centos gpg filenames changed because of the PQC work...it looked obviously right to me in a quick diff but...yeah. I did test this one now. |
It's funny because I tested it with gpg keys after gemini mentioned it, but i had copied it out of the other repo file and didn't notice that it didn't match with the version here. Then when you switched gpgcheck on I said "great i just tested that!" |
37f71d3 to
5ea1aec
Compare
|
OK so c9s is some kind of tmpfile leaking but I don't know from where, investigating. |
Ah no we actually need to roll #1927 into this |
5ea1aec to
0ade2c3
Compare
308acb9 to
0ade2c3
Compare
|
I see, can't do the unified storage test with containers-storage... But I think the bootupd desynchronization is fixed now, let's see |
The base image may be built from a compose that has newer packages than what's available on the public mirrors. This causes version skew where packages like bootupd have different versions between the base image and our built image. For example, bootupd 0.2.32 changed the EFI file layout from /usr/lib/bootupd/updates/EFI/ to /usr/lib/efi/, and if we build with an older bootupd from mirrors while the target image has the newer layout, bootloader installation fails. Enable the CentOS Stream compose repos with higher priority to ensure we get matching versions. xref https://gitlab.com/redhat/centos-stream/containers/bootc/-/issues/1174 Signed-off-by: Colin Walters <walters@verbum.org> Assisted-by: OpenCode (Opus 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
Use copy-to-storage to add the booted container to podman storage instead of pulling a remote image. This matches the pattern used by other TMT tests and ensures we test the actual bootc under test. Changes: - Use localhost/bootc from copy-to-storage instead of remote image - Disable LBIs via bind mount of /usr/share/empty - Remove unnecessary host modifications (usr-overlay, dnf install, etc.) - Use 100%FREE for root LV to ensure sufficient space for deployment Assisted-by: OpenCode (Opus 4.5) Signed-off-by: ckyrouac <ckyrouac@redhat.com> Signed-off-by: Colin Walters <walters@verbum.org>
|
Hmm I think there's race condition in something invoked during our build in c9s that is leaking files in |
Signed-off-by: Colin Walters <walters@verbum.org>
51351b1 to
56ada3d
Compare
In C9S there's something leaking files in `/tmp` so let's just enforce use of tmpfs for `/run` at build time too. But fix `RUN bootc container lint` to *not* have those mounts becuase otherwise we don't actually see the leaked content. Assisted-by: Cursor (Opus 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
56ada3d to
f674cf1
Compare
|
Something else I notice is the "don't rebuild the packages in tests" broke. Something to fix after this |
jeckersb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤞
|
Yeah looks like the updated bootupd wasn't pushed to Fedora 42 per https://bodhi.fedoraproject.org/updates/?search=rust-bootupd (only F43+). So bootc-f42 will fail to install any newer target containers. And yeah I didn't test that before pushing this but it's quite possible just that one fails. If it does my inclination is to drop f42 from gating here for now. But the fix would be to push an updated bootupd to f42. The only other thing we can easily do is change the tests to use an older image, but I'm not a big fan of that. The most comprehensive fix is #1816 but I am a bit leery of rolling that into this... |
Bootupd is too old, see coreos/bootupd#995 (comment) Signed-off-by: Colin Walters <walters@verbum.org>
|
Ah yes and the new bootupd was never built for c9s either, so all the "install outside container" tests that are targeting a newer image will also fail there. But hmm...we can change such tests to also target the c9s image instead. |
The newer bootupd hasn't been rolled out to this image yet, so this should avoid version skew problems. See coreos/bootupd#995 Signed-off-by: Colin Walters <walters@verbum.org>
jeckersb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
This relates to coreos/bootupd#995
The base image may be built from a compose that has newer packages than what's available on the public mirrors. This causes version skew where packages like bootupd have different versions between the base image and our built image.
For example, bootupd 0.2.32 changed the EFI file layout from /usr/lib/bootupd/updates/EFI/ to /usr/lib/efi/, and if we build with an older bootupd from mirrors while the target image has the newer layout, bootloader installation fails.
Enable the CentOS Stream compose repos with higher priority to ensure we get matching versions.
xref https://gitlab.com/redhat/centos-stream/containers/bootc/-/issues/1174
Assisted-by: OpenCode (Opus 4.5)