repo: Add an API to init OstreeSePolicy from commit directly
#2447
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cgwalters
added a commit
to cgwalters/ostree-rs-ext
that referenced
this pull request
Sep 28, 2021
Using `--selinux-policy-from-base` is very convenient but wrong here because it means that each derived commit also has the whole base filesystem tree, which greatly obscures its logical content. Instead, we get the base when we dynamically union things at the end. As noted the API we really want here is ostreedev/ostree#2447 but it will take a bit for ostree to release with it. And even once it does, we need to do some other changes to switch over to parsing the tarball directly too.
lucab
reviewed
Sep 29, 2021
cgwalters
force-pushed
the
sepolicy-for-commit
branch
2 times, most recently
from
92770f7
to
41a8d6a
Compare
jlebon
approved these changes
Sep 29, 2021
| GCancellable *cancellable, | ||
| GError **error) | ||
| { | ||
| GLNX_AUTO_PREFIX_ERROR ("setting sepolicy from commit", error); |
There was a problem hiding this comment.
strjoina hack to include commit hash?
There was a problem hiding this comment.
I've been trying to be more careful about adding checksums into error messages, because we now have several codepaths that log the checksum twice just inside ostree, and if the caller does it too the error message can become hard to read.
cgwalters
force-pushed
the
sepolicy-for-commit
branch
from
41a8d6a
to
8907e13
Compare
cgwalters
force-pushed
the
sepolicy-for-commit
branch
from
8907e13
to
88375f7
Compare
|
Hooray for useful CI coverage, I had missed that we supported passing refs and not just commits into this API. |
|
Re-surfacing a comment I made somewhere else, I think |
lucab
approved these changes
Sep 30, 2021
Sure, done |
cgwalters
added a commit
to cgwalters/ostree
that referenced
this pull request
Sep 30, 2021
Came up in review ostreedev#2447 (comment)
cgwalters
force-pushed
the
sepolicy-for-commit
branch
from
88375f7
to
138ae85
Compare
cgwalters
added a commit
to cgwalters/ostree-rs-ext
that referenced
this pull request
Sep 30, 2021
Using `--selinux-policy-from-base` is very convenient but wrong here because it means that each derived commit also has the whole base filesystem tree, which greatly obscures its logical content. Instead, we get the base when we dynamically union things at the end. As noted the API we really want here is ostreedev/ostree#2447 but it will take a bit for ostree to release with it. And even once it does, we need to do some other changes to switch over to parsing the tarball directly too.
jlebon
requested changes
Sep 30, 2021
src/libostree/ostree-sepolicy.c
Outdated
| GLNX_AUTO_PREFIX_ERROR ("setting sepolicy from commit", error); | ||
| g_autoptr(GFile) root = NULL; | ||
| g_autofree char *commit = NULL; | ||
| if (!ostree_repo_read_commit (repo, commit, &root, &commit, cancellable, error)) |
There was a problem hiding this comment.
Suggested change
| if (!ostree_repo_read_commit (repo, commit, &root, &commit, cancellable, error)) | |
| if (!ostree_repo_read_commit (repo, rev, &root, &commit, cancellable, error)) |
This is part of `OstreeCommitModifier`, but I'm not using that in some of the ostree-ext Rust code. It just makes more sense as a direct policy API, where it should have been in the first place. There's already support for setting a policy object on a commit modifier, so that's all the old API needs to do now.
Came up in review ostreedev#2447 (comment)
cgwalters
force-pushed
the
sepolicy-for-commit
branch
from
138ae85
to
ddc0d54
Compare
jlebon
approved these changes
Sep 30, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is part of
OstreeCommitModifier, but I'm not usingthat in some of the ostree-ext Rust code.
It just makes more sense as a direct policy API, where it should
have been in the first place. There's already support for
setting a policy object on a commit modifier, so that's all the
old API needs to do now.