ostree-ext/store: Relabel layer commits to avoid duplicate objects#2088
ostree-ext/store: Relabel layer commits to avoid duplicate objects#2088jlebon wants to merge 3 commits intobootc-dev:mainfrom
Conversation
jlebon
force-pushed
the
pr/relabel-layers
branch
from
764ec83 to
3e846b3
Compare
![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 introduces a new mechanism for handling SELinux relabeling for non-OSTree container images. The ImageImporter has been updated to apply SELinux policies to individual layer commits, ensuring correct labeling and efficient object sharing. A new CI test has been added to validate this functionality, however, a bug was identified in the test's awk command for extracting checksums, which could lead to incorrect test results.
jlebon
force-pushed
the
pr/relabel-layers
branch
from
3e846b3 to
4426ba1
Compare
|
|
||
| for (layer_ref, old_commit) in layer_commits { | ||
| let td = cap_std_ext::cap_tempfile::TempDir::new_in(&repo_tmp)?; | ||
| repo.checkout_at( |
There was a problem hiding this comment.
Not a blocker but note: The composefs labeling logic always operates on an in-memory VFS tree for this stuff and it's obviously WAY more efficient.
A case where we have the right fix in the next-gen backend, and definitely some risk in trying to do bigger changes to this ostree backend.
That said it may be relatively simple to have "recursive relabel of OstreeMutableTree" which would avoid all the hardlink + unlink traffic.
(Or build a bridge from ostree commit -> composefs generic_tree -> ostree commit)
There was a problem hiding this comment.
Hmm yeah, I was following the checkout and commit pattern that exists for the merge commit, but doing this at the mutable tree level would indeed make this a lot cheaper.
There was a problem hiding this comment.
I tried the ostree_repo_write_directory_to_mtree approach, passing in the commit root, but that actually has way way worse performance because we're not hitting the dfd local path in libostree, but the GIO path.
I didn't investigate the composefs path.
Anyway, relabeling the layers is now down to 2.3s locally with the latest changes, which is still unfortunate but much better.
jlebon
force-pushed
the
pr/relabel-layers
branch
from
4426ba1 to
02d4da6
Compare
We already GC layer refs ourselves so avoid the importer doing it. Prep for making the importer GC also a repo prune (which we already do as well). Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
When importing layers from a plain OCI image (i.e. without `/ostree`), right now we don't do any initial labeling. So all the real labeling happens during the merge commit. This causes a lot of file duplication. We'll fix that more categorically in a later patch, but as a first pass let's at least do the initial import with _an_ SELinux policy; a natural choice is to use the one from the booted deployment. In the common case where we're upgrading, the policies are likely similar enough and so this significantly reduces file duplication in the first place. (There's also the case at install time where we're not yet in a booted commit but may have an SELinux policy lying around; I didn't bother trying to support that because it seems fine to be a bit less efficient there for simpler code.) See also bootc-dev#1637. Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
jlebon
force-pushed
the
pr/relabel-layers
branch
from
02d4da6 to
c4e5e9e
Compare
|
OK, updated this now. See individual commits. |
When importing a non-ostree container (i.e. without `/ostree`), layer commits may use the SELinux policy of the booted commit if available. We need to handle the case where there isn't one, and the case where there is one, but the booted policy has drifted from the target policy. In either case, layer commits may reference objects which don't exist in the merge commit, effectively inflating (up to doubling) the number of commit objects in the repo associated with that import. Fix this by relabeling each layer commit after the merge commit is written. Each layer is checked out, recommitted with the same SELinux policy used for the merge commit, and its ref updated to the new commit. Since the correctly-labeled objects already exist in the repo from the merge commit, most writes are no-ops. A subsequent prune removes the orphaned pre-relabel objects. Closes: bootc-dev#1637 Assisted-by: OpenCode (Claude Opus 4.6) Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
jlebon
force-pushed
the
pr/relabel-layers
branch
from
c4e5e9e to
bfb004a
Compare
2 participants
When importing a non-ostree container (i.e. without /ostree), layer commits are created without SELinux labels. The merge commit then applies the correct labels but the layer commits still reference the unlabeled version of every object. We do get reflinks, but it still effectively doubles the number of commit objects in the repo associated with that import.
Fix this by relabeling each layer commit after the merge commit is written. Each layer is checked out, recommitted with the same SELinux policy used for the merge commit, and its ref updated to the new commit. Since the correctly-labeled objects already exist in the repo from the merge commit, most writes are no-ops. A subsequent prune removes the orphaned pre-relabel objects.
Closes: #1637
Assisted-by: OpenCode (Claude Opus 4.6)