-
Notifications
You must be signed in to change notification settings - Fork 211
Support old-school signed composefs with bootc when layering #5497
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 adds support for signing composefs ostree commits and propagating composefs digests when layering bootc images. The changes look good overall, introducing a new --sign-commit option and the necessary logic to handle commit signing and digest propagation. I've found one issue with duplicated code in the commit signing logic that should be addressed.
caa9a2a to
1795522
Compare
This will be needed by coreos/rpm-ostree#5497 as it picks up ostree-rs via ostree-ext.
This will be needed by coreos/rpm-ostree#5497 as it picks up ostree-rs via ostree-ext. Signed-off-by: Alexander Larsson <[email protected]>
This will be needed by coreos/rpm-ostree#5497 as it picks up ostree-rs via ostree-ext. Signed-off-by: Alexander Larsson <[email protected]>
1795522 to
d020921
Compare
|
Ok, I updated this now that we have a released rust commit we can use to pick up the new ostree dependencies. However, testing this failed because build-chunked-oci labeled files incorrectly, so I also added a new commit to fix that. On top of this, there is another issue with build-chunked-oci, which doesn't break the composefs usecase, but is non-ideal, and that is that the base commit files in the are deployed without full metadata (is it bare user?). This means that we loose information when we build the new commit from just the deployed files. In particular, I saw this with directory permissions being canonicalized, but I wonder if we not also lose file ownership info. |
Unclear why this is failing with a ENOSPC. I tried to run the test locally, and it worked. Also, unclear why e2e failed with:
I'll try to look into it. And i'll try: /test fcos-e2e |
d020921 to
6b7271f
Compare
|
I moved the chunked-oci relabeling commit to #550 to make it easier to find what the CI issue is comming from. |
6b7271f to
8f366b0
Compare
|
I broke out the remaining issues to #5503 |
8f366b0 to
cd8e639
Compare
|
I rebased this on top of #5502 because they are interdependent. That should be merged first. |
|
continuous-integration/jenkins/pr-merge seems to fail with out of diskspace: Whereas fcos-e2e fails with: |
This also updates the related packages only. We need this because there are some binding fixes in 0.20.5 that we need.
When chunking an image that has a ostree commit, check if that commit has a composefs digest, and if so, create one in the new commit as well. This allows using signed composefs images after rechunking.
This adds a new --sign-commit CLI option to `rpm-ostree experimental compose build-chunked-oci` which signs the embedded ostree commit. With this, it is possible (with some care) to layer a signed bootc image that uses old-school signed ostree/composefs support. Note, this relies on the double free fix for ostree_sign_set_sk() in ostreedev/ostree#3527 to work.
cd8e639 to
3dfcbad
Compare
|
Now that #5502 is merged, i rebased this on main. |
|
@alexlarsson: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
The only CI failure seems to be the classic: |
cgwalters
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.
Thanks! This all looks relatively straightforward.
This adds support for adding composefs digests and signing commits to
rpm-ostree experimental compose build-chunked-oci, which allows (with care) using signed composfs ostree commits in layered bootc images (if they are rechunked).Note, this depends on the changes in ostreedev/ostree#3527 and those need to land and be released before this is buildable. Also, the double-free fix in ostree it contains is needed for --sign-commit to work at runtime (without failing on the double free).