Repository navigation
Conversation
Member
Author
|
Needs a rebase when #1518 is merged |
|
Build action triggered: https://github.com/flatcar/scripts/actions/runs/7395050910 |
Member
Author
|
I've started a test run and when that passes I'll try again with flatcar/mantle#487 |
Member
Author
|
The |
Member
Author
|
We get a lot of denials - the image only works in permissive mode: |
Member
|
I think that if we want to start labeling the whole filesystem, we should at least start enabling selinux USE flag globally and pull in the missing |
tormath1
had a problem deploying
to
development
November 29, 2024 08:50 — with 
GitHub Actions
Failure
GitHub Actions
Failure
tormath1
had a problem deploying
to
development
September 8, 2026 12:43 — with
GitHub Actions
Failure
GitHub Actions
Failure
3 tasks done
tormath1
marked this pull request as draft
September 8, 2026 12:45
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of concrete correctness/consistency issues (notably setfiles path symlink handling in build_sysext, plus KEYWORDS consistency for riscv) that should be addressed before landing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the image build flow to generate SELinux policy earlier and apply file labeling to /usr and systemd-sysext contents, aiming to avoid previous Docker breakage while getting correct contexts into the immutable image artifacts.
Changes:
- Add
sec-policy/selinux-dockerto the base OS dependency set and portage-stable package list. - Split out
finish_image_common()to build SELinux policy before branching off the base squashfs, then extend labeling to/usrduringfinish_image. - Label sysext
/usrcontents directly inbuild_sysextafter the overlay unmount.
File summaries
| File | Description |
|---|---|
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/selinux-docker-9999.ebuild | New live ebuild for SELinux Docker policy. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/selinux-docker-2.20260616_p1.ebuild | New versioned ebuild for SELinux Docker policy. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/selinux-docker-2.20260312_p1.ebuild | New versioned ebuild for SELinux Docker policy. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/selinux-docker-2.20250618_p1.ebuild | New versioned ebuild for SELinux Docker policy. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/metadata.xml | Adds Gentoo SELinux team metadata for the new package. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/Manifest | Adds distfile entries required by the new policy package. |
| sdk_container/src/third_party/coreos-overlay/coreos-base/coreos/coreos-0.0.1.ebuild | Pulls selinux-docker policy into the base OS SELinux policy set. |
| build_sysext | Adds setfiles labeling for sysext /usr after overlay unmount. |
| build_library/prod_image_util.sh | Calls finish_image_common() prior to sysext branching in prod image builds. |
| build_library/dev_container_util.sh | Calls finish_image_common() prior to finish_image() in dev container builds. |
| build_library/build_image_util.sh | Introduces finish_image_common() and expands SELinux labeling to include /usr. |
| .github/workflows/portage-stable-packages-list | Ensures selinux-docker is included in the portage-stable package list. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
An active finding remains because the release-image change lacks the required user-facing changelog entry.
2 open issues
5 resolved since last review
~riscv, unlike the othersec-policy/selinux-*ebuilds in this repo…~riscv, unlike the othersec-policy/selinux-*ebuilds in this repo…~riscv, unlike the othersec-policy/selinux-*ebuilds in this repo…~riscv, unlike the othersec-policy/selinux-*ebuilds in this repo…finish_image_commondeclares a long list of positional parameters that are unused (only…
Review effort: Lite
| sec-policy/selinux-base-policy | ||
| sec-policy/selinux-container | ||
| sec-policy/selinux-dbus | ||
| sec-policy/selinux-docker |
So far we did not correctly label /usr because it broke certain things like Docker. With the sysext Docker and new policies we should try again. First generate the policy before branching off the base squashfs (which already misses a lot of things because they the most postprocessing is done late in finish_image!). Then label /usr and also the sysext contents in their folder - not in the overlay mount because this would operate on the whole image. Signed-off-by: James Le Cuirot <jlecuirot@microsoft.com>
Contributor
|
I rebased this following the gentoo merge and made some adjustments following feedback from Copilot. To be honest, so much has changed since Kai's version that I feel I should set myself as the author. I don't want him to be blamed for my bad work. 😅 And it is still bad because Kola couldn't even SSH to the nodes. |
Contributor
|
The sysext files still have the wrong label, so I must have messed up there, but there appear to be far more issues beyond that. |
Contributor
|
I've looked deeper into this. When I build the sysext on my own machine (Gentoo without SELinux), it gets labelled correctly. It appears to be an issue specific to the Jenkins worker. |
This branch is waiting to be deployed
This file contains hidden or 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
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.
So far we did not correctly label /usr because it broke certain things like Docker. With the sysext Docker and new policies we should try again.
First generate the policy before branching off the base squashfs (which already misses a lot of things because they the most postprocessing is done late in finish_image!). Then label /usr and also the sysext contents in their folder - not in the overlay mount because this would operate on the whole image.
How to use
Hope that setfiles is clever enough
Verify with flatcar/mantle#487
Testing done
The sysext contents have the right label:
changelog/directory (user-facing change, bug fix, security fix, update)/bootand/usrsize, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.