-
Notifications
You must be signed in to change notification settings - Fork 107
Revert "container: label /run/user/*/crun as container_var_run_t" #408
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR reverts the previous SELinux policy update that labeled /run/user/*/crun as container_var_run_t by removing the associated file_context entries and type enforcement rules, restoring the policy to its prior state to prevent permission errors in rootless Podman. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Summary of Changes
Hello @sbrivio-rh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request provides a critical, temporary fix by reverting a recent SELinux policy update that inadvertently introduced a regression in Podman's rootless networking functionality. The original change caused permission denied errors for PID files, rendering pasta(1) unusable. By rolling back this specific labeling rule, the PR restores stability for users, allowing Podman to function correctly while a more robust, long-term solution is being prepared.
Highlights
- Revert of SELinux Policy Change: This pull request reverts a previous commit (ae3532b) that introduced a specific SELinux labeling rule for
/run/user/*/crun. - Resolution of Podman Rootless Issue: The reverted commit caused a 'Permission denied' error, breaking basic Podman operations when using
pasta(1)for rootless networking, specifically preventing the opening of PID files. - Temporary Fix for Widespread Breakage: This revert serves as an immediate measure to prevent widespread user breakage while a more comprehensive and permanent solution is actively being developed and reviewed.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
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 is a revert of a previous commit that was causing permission denied errors for rootless Podman operations. The change correctly removes the problematic userdom_user_tmp_filetrans rule from container.te. The reason for the revert is clearly explained in the pull request description. This is a necessary and correct temporary fix to prevent widespread issues for users while a more permanent solution is developed. The change is approved.
|
Stop spamming please? @sourcery-ai dismiss /gemini stop |
|
Hello! I noticed your comment regarding stopping bot activity. For |
|
/gemini help srsly? |
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
|
/gemini please stop this thing, I never opted in |
|
Hello again. I understand your frustration. The command As My presence in this repository is typically configured by the repository maintainers, which might explain why you didn't personally opt-in. If you'd like to see the list of valid commands, please use |
|
RE: AI bots, I think we should only have 1 bot. @sbrivio-rh please ignore bot comments. |
giuseppe
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.
LGTM
Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
This should make local and testing-farm CLI runs rather consistent with Packit runs by enforcing the main package installation. Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
This reverts commit ae3532b ("container: label /run/user/*/crun as container_var_run_t") as it breaks basic Podman operation with pasta(1) (default rootless back-end): Error: setting up Pasta: pasta failed with exit code 1: Couldn't open PID file /run/user/1000/containers/networks/rootless-netns/rootless-netns-conn.pid: Permission denied A solution is being worked on, but it's not quite ready yet, see: containers#405 in the meantime, revert this to avoid widespread breakage for users. Link: containers#405 Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
lsm5
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.
/lgtm
/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, lsm5, sbrivio-rh, sourcery-ai[bot] The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Tests failed. @containers/packit-build please check. |
Luap99
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.
Looks like tests are still failing, is it possible that the new policy is not installed right?
| # Run podman system tests | ||
| su - "$ROOTLESS_USER" -c "bats /usr/share/podman/test/system/410-selinux.bats" | ||
| su - "$ROOTLESS_USER" -c "bats /usr/share/podman/test/system/500-networking.bats" | ||
| su - "$ROOTLESS_USER" -c bats "/usr/share/podman/test/system/505-networking-pasta.bats" |
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.
That does not actually work I think
| su - "$ROOTLESS_USER" -c bats "/usr/share/podman/test/system/505-networking-pasta.bats" | |
| su - "$ROOTLESS_USER" -c "bats /usr/share/podman/test/system/505-networking-pasta.bats" |
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.
ugh my bad on that one. I'll leave it to @sbrivio-rh to accept the suggestion. But it shouldn't affect 500-networking.bats.
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.
Wait, sorry, what should I review / accept exactly?
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.
do you see the option to Sign off and commit suggestion in @Luap99's 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.
Well the "Sign off and commit suggestion" is just "garbage" commit message wise, it creates a new commit with a totally useless title. My recommendation is also apply the chnage locally squash it into the right commit and force push again
|
From the test log @lsm5 I assume that is the wrong rpm version, the cop builds |
That is actually weird. Wonder if the TMT changes are to blame. I'll work on this in a followup. I'll go ahead with merging this. |
This reverts commit ae3532b ("container: label /run/user/*/crun as container_var_run_t") as it breaks basic Podman operation with pasta(1) (default rootless back-end):
Error: setting up Pasta: pasta failed with exit code 1:
Couldn't open PID file /run/user/1000/containers/networks/rootless-netns/rootless-netns-conn.pid: Permission denied
A solution is being worked on, but it's not quite ready yet, see:
#405
in the meantime, revert this to avoid widespread breakage for users.
Link: #405
Summary by Sourcery
Bug Fixes: