Skip to content
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

qm.spec: Fix packaing for other selinux policies #731

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

alexlarsson
Copy link
Collaborator

@alexlarsson alexlarsson commented Feb 27, 2025

The current package hardcodes that post-install it will install the module to the "targeted" policy (but the macro only does this is it is also the active policy). This means if the active policy is something else, such as "automotive", then the qm module is not installed at all, and qm doesn't work.

We fix this by always installing the module to the active selinux policy. This is how e.g. container-selinux does it.

In addition we remove the hard post-require on selinux-policy-target and replace it with selinux-policy-any, which all selinux policy packages provide. We also add a recommendataion of selinux-policy-targeted, so most people get it. This is also what container-selinux does.

Fixes #730

Summary by Sourcery

Fixes an issue where the qm module was not installed when the active SELinux policy was not 'targeted' by ensuring the module is installed to the active SELinux policy. Also updates the package dependencies to use selinux-policy-any instead of selinux-policy-targeted.

Bug Fixes:

  • Fixes an issue where the qm module was not installed when the active SELinux policy was not 'targeted'.

Build:

  • Replaces the hard post-require on selinux-policy-target with selinux-policy-any, which all SELinux policy packages provide.
  • Adds a recommendation for selinux-policy-targeted to ensure most users get it.

Copy link

sourcery-ai bot commented Feb 27, 2025

Reviewer's Guide by Sourcery

This pull request modifies the rpm spec file to correctly install the selinux module for qm, even when the active selinux policy is not 'targeted'. It achieves this by installing the module to the active selinux policy and updating the package dependencies.

Sequence diagram for SELinux module installation

sequenceDiagram
    participant RPM Package Manager
    participant SELinux Policy
    RPM Package Manager->>SELinux Policy: Installs qm module to active policy
    activate SELinux Policy
    SELinux Policy-->>RPM Package Manager: Acknowledges installation
    deactivate SELinux Policy
Loading

File-Level Changes

Change Details Files
Modified the SELinux policy handling to install modules to the active policy and updated dependencies.
  • Replaced the hardcoded selinuxtype with a dynamic check against the active SELinux policy.
  • Replaced the hard post-require on selinux-policy-targeted with selinux-policy-any and added a recommendation for selinux-policy-targeted.
rpm/qm.spec

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @alexlarsson - I've reviewed your changes - here's some feedback:

Overall Comments:

  • It might be good to add a comment explaining why we are sourcing the selinux config file in the post scripts.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

The current package hardcodes that post-install it will install the
module to the "targeted" policy (but the macro only does this is it is
also the active policy). This means if the active policy is something
else, such as "automotive", then the qm module is not installed at
all, and qm doesn't work.

We fix this by always installing the module to the active selinux policy.
This is how e.g. container-selinux does it.

In addition we remove the hard post-require on selinux-policy-target
and replace it with selinux-policy-any, which all selinux policy
packages provide. We also add a recommendataion of
selinux-policy-targeted, so most people get it. This is also what
container-selinux does.

Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson alexlarsson force-pushed the fix-selinux-packaging branch from 390734c to 70e6647 Compare February 27, 2025 13:45
@dougsland dougsland merged commit b7264e4 into main Feb 27, 2025
16 checks passed
engelmi added a commit to engelmi/bluechi that referenced this pull request Mar 3, 2025
Relates to: containers/qm#731

Change the bluechi-selinux module to be installed to the targeted
policy (which is hardcoded), simply install it to the active SELinux policy.

Signed-off-by: Michael Engel <[email protected]>
engelmi added a commit to engelmi/bluechi that referenced this pull request Mar 5, 2025
Relates to: containers/qm#731

Change the bluechi-selinux module to be installed to the targeted
policy (which is hardcoded), simply install it to the active SELinux policy.

Signed-off-by: Michael Engel <[email protected]>
engelmi added a commit to engelmi/bluechi that referenced this pull request Mar 6, 2025
Relates to: containers/qm#731

Change the bluechi-selinux module to be installed to the targeted
policy (which is hardcoded), simply install it to the active SELinux policy.

Signed-off-by: Michael Engel <[email protected]>
engelmi added a commit to eclipse-bluechi/bluechi that referenced this pull request Mar 6, 2025
Relates to: containers/qm#731

Change the bluechi-selinux module to be installed to the targeted
policy (which is hardcoded), simply install it to the active SELinux policy.

Signed-off-by: Michael Engel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selinux policy in QM rpm only works with the targeted policy
2 participants