[PROF-15441][Host Profiler] OOB SELinux to spc_t - #3275
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: b119cb4 | Docs | Datadog PR Page | Give us feedback! |
e3507a2 to
ed9535b
Compare
ed9535b to
486bee3
Compare
theomagellan
changed the title
spc_tspc_t
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 486bee315e
ℹ️ 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".
| sc.SELinuxOptions = &corev1.SELinuxOptions{ | ||
| Type: o.selinuxType, | ||
| } |
There was a problem hiding this comment.
When users set pod-level spec.override.nodeAgent.securityContext.seLinuxOptions (as the OpenShift examples do with user, role, type, and level), this container-level SELinuxOptions object takes precedence for the host-profiler container, and override.PodTemplateSpec applies the pod override after feature hooks in controller_reconcile_agent.go. On SELinux/OpenShift installs that rely on those pod-level user/role/level labels, enabling host-profiler leaves this container with only the type label instead of inheriting the full pod label; preserve or merge the pod SELinux fields when setting the type.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think this comment is somewhat relevant
we might want to merge ?
There was a problem hiding this comment.
Pod-level securityContext has less precedence than container-level securityContext, and I think that's the correct choice and codex is getting the order wrong.
Nevertheless, I think we should always configure the Host Profiler independently of other containers in the pod given the blast radius it could have. If users want to change the SELinux profile, they can use the annotation
agent.datadoghq.com/host-profiler-selinux-type.
Additionally, I'm also not sure I understand all consequences of merging user/role/level while keeping the type override, I think this could be error-prone.
There was a problem hiding this comment.
Fair, I'm also not sure if this matters. The merge question is essentially what I was interested in. I was asking claude for examples:
- OpenShift with SCC: the SCC admission plugin injects user/role/level into the container's SELinuxOptions at admission time based on the SCC's seLinuxContext. So on OpenShift specifically, the container ends up with {type: spc_t, user/role/level from SCC}. Probably fine.
So let's stay consistent.
For now we can keep container-level fully replaces, annotations are available for overrides. If folks end up using overrides, or making requests to change this, we can look back into it.
Thanks
486bee3 to
b119cb4
Compare
2 participants
What does this PR do?
Defaults
securityContext.SELinuxOptionstotype: spc_tso the host profiler can work out of the box on nodes enforcing SELinux profiles.Adds an annotation to change the value.
Motivation
Without this, the default SELinux profile denies ptrace and the profiler only shows kernel frames.
Additional Notes
Based on https://datadoghq.atlassian.net/wiki/spaces/PROF/pages/6986172492/Accepted+Handling+of+LSMs+across+OSes.
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Describe your test plan
Setting
seLinuxOptions.type: spc_talongside apparmor was tested on:Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel