Add host volume mounts and NET_RAW capability to PAR container#2799
Add host volume mounts and NET_RAW capability to PAR container#2799
Conversation
Mount /var/log, /etc/os-release, and /proc from the host into the PAR container under /host as read-only volumes. This enables the PAR to inspect host-level logs, OS information, and process data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1d03dfaee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Move host-varlog and host-osrelease volumes from the base volumesForAgent list into a conditional block gated on PrivateActionRunnerContainerName, mirroring the existing SystemProbe pattern. This prevents unused HostPath volumes from being added to every Agent pod, which can cause admission failures in environments enforcing HostPath allowlists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex conduct a comprehensive security and code review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The Private Action Runner container needs the NET_RAW capability to perform network operations on the host. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tbavelier
left a comment
There was a problem hiding this comment.
I ddin't review in depth but that's not the right approach. It should be done as part of the feature code (feature/par), e.g. you can see how it's done for logcollection feature and other features. They should be responsible to manage the volume and mounts
Volumes, mounts, and capabilities should be managed by the feature system, not hardcoded in component defaults. This moves host volume mounts (/proc, /etc/os-release, /var/log) and the NET_RAW capability from default.go into the PAR feature's ManageNodeAgent(), following the same pattern used by logcollection, npm, and other features. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The SystemProbeOSReleaseDirVolumeName constants are semantically tied to system-probe despite being general-purpose. Add generic aliases (HostOSReleaseVolumeName, HostOSReleaseHostPath, HostOSReleaseMountPath) and use them in PAR feature code and volume helpers so that privateactionrunner does not reference system-probe constants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Define HostOSRelease constants with their own literal values instead of aliasing the SystemProbe variants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Host volume constants (varlog, os-release, proc) are only used by the PAR feature, so they belong in the PAR package as unexported constants. Remove the now-unused GetVolumeForHostVarLog, GetVolumeMountForHostVarLog, GetVolumeForOSRelease, GetVolumeMountForOSRelease helpers and their corresponding exported constants from common. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex conduct a comprehensive security and code review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2799 +/- ##
==========================================
+ Coverage 38.69% 38.73% +0.04%
==========================================
Files 311 311
Lines 26971 27017 +46
==========================================
+ Hits 10437 10466 +29
- Misses 15756 15773 +17
Partials 778 778
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
tbavelier
left a comment
There was a problem hiding this comment.
Apart from the comments, lgtm
internal/controller/datadogagent/feature/privateactionrunner/const.go
Outdated
Show resolved
Hide resolved
internal/controller/datadogagent/feature/privateactionrunner/const.go
Outdated
Show resolved
Hide resolved
internal/controller/datadogagent/feature/privateactionrunner/feature.go
Outdated
Show resolved
Hide resolved
The procdir and os-release volume constants already exist in common/const.go. Remove the duplicates from PAR's const.go and reference the common ones directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace loop-based volume addition with individual volume.GetVolumes() calls per volume, matching the pattern used by npm and other features for better readability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
This pull request was merged directly. |
* Add host volume mounts to Private Action Runner container Mount /var/log, /etc/os-release, and /proc from the host into the PAR container under /host as read-only volumes. This enables the PAR to inspect host-level logs, OS information, and process data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add PAR host volumes only when PAR container is required Move host-varlog and host-osrelease volumes from the base volumesForAgent list into a conditional block gated on PrivateActionRunnerContainerName, mirroring the existing SystemProbe pattern. This prevents unused HostPath volumes from being added to every Agent pod, which can cause admission failures in environments enforcing HostPath allowlists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: format files * Add NET_RAW capability to PAR container The Private Action Runner container needs the NET_RAW capability to perform network operations on the host. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Move PAR host volumes, mounts, and NET_RAW to feature code Volumes, mounts, and capabilities should be managed by the feature system, not hardcoded in component defaults. This moves host volume mounts (/proc, /etc/os-release, /var/log) and the NET_RAW capability from default.go into the PAR feature's ManageNodeAgent(), following the same pattern used by logcollection, npm, and other features. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add generic HostOSRelease aliases for os-release volume constants The SystemProbeOSReleaseDirVolumeName constants are semantically tied to system-probe despite being general-purpose. Add generic aliases (HostOSReleaseVolumeName, HostOSReleaseHostPath, HostOSReleaseMountPath) and use them in PAR feature code and volume helpers so that privateactionrunner does not reference system-probe constants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use standalone values for HostOSRelease constants Define HostOSRelease constants with their own literal values instead of aliasing the SystemProbe variants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Move host volume constants to PAR package, remove unused common helpers Host volume constants (varlog, os-release, proc) are only used by the PAR feature, so they belong in the PAR package as unexported constants. Remove the now-unused GetVolumeForHostVarLog, GetVolumeMountForHostVarLog, GetVolumeForOSRelease, GetVolumeMountForOSRelease helpers and their corresponding exported constants from common. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert const.go * Reuse existing common constants for procdir and os-release volumes The procdir and os-release volume constants already exist in common/const.go. Remove the duplicates from PAR's const.go and reference the common ones directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add host volumes individually instead of loop Replace loop-based volume addition with individual volume.GetVolumes() calls per volume, matching the pattern used by npm and other features for better readability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Timothée Bavelier <97530782+tbavelier@users.noreply.github.com> (cherry picked from commit 8c060c4)
#2828) * Add host volume mounts to Private Action Runner container Mount /var/log, /etc/os-release, and /proc from the host into the PAR container under /host as read-only volumes. This enables the PAR to inspect host-level logs, OS information, and process data. * Add PAR host volumes only when PAR container is required Move host-varlog and host-osrelease volumes from the base volumesForAgent list into a conditional block gated on PrivateActionRunnerContainerName, mirroring the existing SystemProbe pattern. This prevents unused HostPath volumes from being added to every Agent pod, which can cause admission failures in environments enforcing HostPath allowlists. * style: format files * Add NET_RAW capability to PAR container The Private Action Runner container needs the NET_RAW capability to perform network operations on the host. * Move PAR host volumes, mounts, and NET_RAW to feature code Volumes, mounts, and capabilities should be managed by the feature system, not hardcoded in component defaults. This moves host volume mounts (/proc, /etc/os-release, /var/log) and the NET_RAW capability from default.go into the PAR feature's ManageNodeAgent(), following the same pattern used by logcollection, npm, and other features. * Add generic HostOSRelease aliases for os-release volume constants The SystemProbeOSReleaseDirVolumeName constants are semantically tied to system-probe despite being general-purpose. Add generic aliases (HostOSReleaseVolumeName, HostOSReleaseHostPath, HostOSReleaseMountPath) and use them in PAR feature code and volume helpers so that privateactionrunner does not reference system-probe constants. * Use standalone values for HostOSRelease constants Define HostOSRelease constants with their own literal values instead of aliasing the SystemProbe variants. * Move host volume constants to PAR package, remove unused common helpers Host volume constants (varlog, os-release, proc) are only used by the PAR feature, so they belong in the PAR package as unexported constants. Remove the now-unused GetVolumeForHostVarLog, GetVolumeMountForHostVarLog, GetVolumeForOSRelease, GetVolumeMountForOSRelease helpers and their corresponding exported constants from common. * revert const.go * Reuse existing common constants for procdir and os-release volumes The procdir and os-release volume constants already exist in common/const.go. Remove the duplicates from PAR's const.go and reference the common ones directly. * Add host volumes individually instead of loop Replace loop-based volume addition with individual volume.GetVolumes() calls per volume, matching the pattern used by npm and other features for better readability. --------- (cherry picked from commit 8c060c4) Co-authored-by: Matthew DeGuzman <91019033+matt-dz@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Timothée Bavelier <97530782+tbavelier@users.noreply.github.com> Co-authored-by: levan-m <116471169+levan-m@users.noreply.github.com>
Summary
/var/log,/etc/os-release, and/procfrom the host into the Private Action Runner container under/hostas read-only volumesNET_RAWcapability to the PAR container SecurityContext for network operationsManageNodeAgent(), following the established feature patternTest plan
TestVolumesForAgent— verifies base agent volumes are unchanged (no PAR leakage)TestPrivateActionRunnerContainer— verifies default container definitionTest_privateActionRunnerFeature_ManageNodeAgent— verifies host volumes, mounts (read-only), and NET_RAW capability are added via feature managers🤖 Generated with Claude Code