selinux: add pcp-import policy module for local context recording tools#2657
Conversation
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (26)
📝 WalkthroughWalkthroughThe change separates PCP import runtime and log directories, updates import and atop consumers, adds a dedicated SELinux import policy, and packages that policy in a new RPM subpackage. ChangesImport runtime and SELinux integration
Sequence Diagram(s)sequenceDiagram
participant configure
participant libpcp_import
participant pmda_pmimport
configure->>libpcp_import: Provide PCP_IMPORTRUN_DIR
libpcp_import->>libpcp_import: Create and remove status sidecars
pmda_pmimport->>libpcp_import: Scan PCP_IMPORTRUN_DIR
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pcp/atop/atop-daily.sh`:
- Line 66: Update the default LOGPATH assignment to quote the parameter
expansion, preventing word splitting and pathname expansion when LOGPATH or
PCP_IMPORTLOG_DIR contains whitespace or glob characters.
In `@src/selinux/import/pcp-import.if`:
- Around line 59-60: Add the pmimport PID-file transition alongside the existing
manage_files_pattern and manage_dirs_pattern declarations in
src/selinux/import/pcp-import.if lines 59-60, and add the corresponding
files_pid_filetrans declaration in src/selinux/import/pcp-import.te lines
143-144, covering both dir and file types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5531a53a-7317-46e1-81d7-9df92c340621
📒 Files selected for processing (26)
build/rpm/pcp.spec.inbuild/rpm/redhat.specconfigureconfigure.acman/man3/pmisetimportprogram.3qa/2005qa/2007qa/518src/include/builddefs.insrc/include/pcp.conf.insrc/libpcp_import/src/GNUmakefilesrc/libpcp_import/src/archive.csrc/libpcp_import/src/import.csrc/libpcp_import/src/libpcp_import.conf.insrc/libpcp_import/src/private.hsrc/pcp/atop/GNUmakefilesrc/pcp/atop/atop-daily.shsrc/pcp/atop/atop.confsrc/pcp/atop/atop.service.insrc/pmdas/pmimport/helpsrc/pmdas/pmimport/pmda_pmimport.csrc/selinux/GNUmakefilesrc/selinux/import/GNUmakefilesrc/selinux/import/pcp-import.fcsrc/selinux/import/pcp-import.ifsrc/selinux/import/pcp-import.te
576a3d7 to
fe643f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
build/rpm/redhat.spec (1)
2289-2304: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unnecessary
pcp-libsdependency to avoid a circular dependency.The
pcp-selinux-importpackage provides SELinux policy files and does not link against the C libraries. Requiringpcp-libscreates a cyclic dependency becausepcp-libsitself conditionally requirespcp-selinux-import.Removing it aligns with
pcp.spec.inand the existingpcp-selinuxsubpackage, which also omit this dependency.♻️ Proposed refactor
%package selinux-import License: GPL-2.0-or-later AND CC-BY-3.0 Summary: SELinux policy for PCP local context recording tools URL: https://pcp.io -Requires: pcp-libs = %{version}-%{release} Requires: policycoreutils selinux-policy-targeted🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build/rpm/redhat.spec` around lines 2289 - 2304, Remove the “Requires: pcp-libs = %{version}-%{release}” entry from the selinux-import package definition, while preserving its policycoreutils and selinux-policy-targeted dependencies and the surrounding package metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build/rpm/pcp.spec.in`:
- Around line 285-287: Guard the pcp-selinux-import boolean dependency in
build/rpm/pcp.spec.in lines 285-287 with the established 0%{?fedora} >= 36 ||
0%{?rhel} >= 9 check, retaining an unconditional Requires fallback for older
systems. Apply the corresponding 0%{?fedora} >= 35 || 0%{?rhel} >= 8 guard to
the same dependency in build/rpm/redhat.spec lines 467-469.
---
Nitpick comments:
In `@build/rpm/redhat.spec`:
- Around line 2289-2304: Remove the “Requires: pcp-libs = %{version}-%{release}”
entry from the selinux-import package definition, while preserving its
policycoreutils and selinux-policy-targeted dependencies and the surrounding
package metadata.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 68d3d528-25b0-4114-9343-d883fb2e9271
📒 Files selected for processing (26)
build/rpm/pcp.spec.inbuild/rpm/redhat.specconfigureconfigure.acman/man3/pmisetimportprogram.3qa/2005qa/2007qa/518src/include/builddefs.insrc/include/pcp.conf.insrc/libpcp_import/src/GNUmakefilesrc/libpcp_import/src/archive.csrc/libpcp_import/src/import.csrc/libpcp_import/src/libpcp_import.conf.insrc/libpcp_import/src/private.hsrc/pcp/atop/GNUmakefilesrc/pcp/atop/atop-daily.shsrc/pcp/atop/atop.confsrc/pcp/atop/atop.service.insrc/pmdas/pmimport/helpsrc/pmdas/pmimport/pmda_pmimport.csrc/selinux/GNUmakefilesrc/selinux/import/GNUmakefilesrc/selinux/import/pcp-import.fcsrc/selinux/import/pcp-import.ifsrc/selinux/import/pcp-import.te
🚧 Files skipped from review as they are similar to previous changes (20)
- src/selinux/import/pcp-import.fc
- src/include/pcp.conf.in
- src/libpcp_import/src/private.h
- src/pmdas/pmimport/help
- man/man3/pmisetimportprogram.3
- src/pcp/atop/atop.conf
- src/pmdas/pmimport/pmda_pmimport.c
- src/pcp/atop/atop-daily.sh
- src/include/builddefs.in
- src/selinux/import/pcp-import.if
- src/selinux/import/GNUmakefile
- qa/2005
- src/pcp/atop/GNUmakefile
- src/libpcp_import/src/archive.c
- qa/2007
- src/libpcp_import/src/import.c
- configure.ac
- src/selinux/import/pcp-import.te
- src/selinux/GNUmakefile
- configure
7f40b9b to
a6400b7
Compare
Add a new SELinux policy module (pcp-import) for PCP tools that operate in PM_CONTEXT_LOCAL mode without pmcd — loading DSO PMDAs directly and writing PCP archives via libpcp_import. This covers pcp-atop and sysstat's sadc (with -O pcp), and provides a reusable pcp_import_t domain for future import tools. The pcp-import module is independent of the existing pcp-selinux package (which covers pmcd/pmlogger/pmproxy/pmie) and works when only pcp-libs is installed. The sysstat_t domain from the base targeted policy is augmented via optional_policy when sysstat is installed. Rename PCP_IMPORT_DIR to PCP_IMPORTRUN_DIR for clarity (/run/pmimport status files) and add PCP_IMPORTLOG_DIR (/var/log, base directory for import tool archives). pcp-atop uses PCP_IMPORTLOG_DIR/atop for its daily archives. New pcp-selinux-import RPM subpackage, weak-required by pcp-libs when selinux-policy-targeted is installed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
a6400b7 to
327acb7
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
build/rpm/redhat.spec (1)
2498-2498: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse
%{_localstatedir}/run/pmimportfor compatibility.The
%{_rundir}macro may not be defined on older distributions (such as RHEL 6) that this spec file still supports, which will cause the build to fail. Furthermore, it is inconsistent with the%{_localstatedir}/run/...pattern used for existing runtime directories in this file (e.g.,%{_localstatedir}/run/pcp).🛠 Proposed fix
-echo %{_rundir}/pmimport >>pcp-libs-files +echo %{_localstatedir}/run/pmimport >>pcp-libs-files🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build/rpm/redhat.spec` at line 2498, Update the pcp-libs-files entry to use the %{_localstatedir}/run/pmimport path instead of %{_rundir}/pmimport, matching the existing runtime-directory pattern and preserving compatibility with older distributions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@build/rpm/redhat.spec`:
- Line 2498: Update the pcp-libs-files entry to use the
%{_localstatedir}/run/pmimport path instead of %{_rundir}/pmimport, matching the
existing runtime-directory pattern and preserving compatibility with older
distributions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: bfb3dd4e-2b40-4a98-b493-75f4d8dfb4b3
📒 Files selected for processing (26)
build/rpm/pcp.spec.inbuild/rpm/redhat.specconfigureconfigure.acman/man3/pmisetimportprogram.3qa/2005qa/2007qa/518src/include/builddefs.insrc/include/pcp.conf.insrc/libpcp_import/src/GNUmakefilesrc/libpcp_import/src/archive.csrc/libpcp_import/src/import.csrc/libpcp_import/src/libpcp_import.conf.insrc/libpcp_import/src/private.hsrc/pcp/atop/GNUmakefilesrc/pcp/atop/atop-daily.shsrc/pcp/atop/atop.confsrc/pcp/atop/atop.service.insrc/pmdas/pmimport/helpsrc/pmdas/pmimport/pmda_pmimport.csrc/selinux/GNUmakefilesrc/selinux/import/GNUmakefilesrc/selinux/import/pcp-import.fcsrc/selinux/import/pcp-import.ifsrc/selinux/import/pcp-import.te
🚧 Files skipped from review as they are similar to previous changes (20)
- src/selinux/import/pcp-import.fc
- src/include/builddefs.in
- qa/2007
- src/pcp/atop/atop.conf
- src/libpcp_import/src/private.h
- src/include/pcp.conf.in
- src/pcp/atop/GNUmakefile
- src/libpcp_import/src/GNUmakefile
- src/libpcp_import/src/import.c
- src/pcp/atop/atop.service.in
- src/pmdas/pmimport/pmda_pmimport.c
- src/libpcp_import/src/libpcp_import.conf.in
- src/libpcp_import/src/archive.c
- qa/2005
- configure.ac
- src/selinux/GNUmakefile
- src/pcp/atop/atop-daily.sh
- src/selinux/import/pcp-import.te
- configure
- src/selinux/import/GNUmakefile
|
@coderabbitai The All CI checks are now passing. The |
1 participant
Summary
pcp-importfor PCP tools operating inPM_CONTEXT_LOCALmode without pmcd (pcp-atop, sysstat sadc with-O pcp)pcp_import_tdomain — future import tools just add their binary path to the.fcfilesysstat_tdomain viaoptional_policywhen sysstat is installed (no sysstat packaging changes needed)pcp-selinux(which covers pmcd/pmlogger/pmproxy/pmie)pcp-selinux-importRPM subpackage, weak-required bypcp-libswhen SELinux is activePCP_IMPORT_DIR→PCP_IMPORTRUN_DIR(runtime status,/run/pmimport)PCP_IMPORTLOG_DIR(/var/log, base directory for import tool archives)$PCP_IMPORTLOG_DIR/atopfor daily archives🤖 Generated with Claude Code