Skip to content

RUN-1121 | feat: run odiglet and data-collection as non-privileged by default - #5759

Merged
RonFed merged 4 commits into
odigos-io:mainfrom
RonFed:data_collection_un_piv
Sep 7, 2026
Merged

RUN-1121 | feat: run odiglet and data-collection as non-privileged by default#5759
RonFed merged 4 commits into
odigos-io:mainfrom
RonFed:data_collection_un_piv

Conversation

@RonFed

@RonFed RonFed commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:

The odiglet daemonset ran its containers as privileged by default, and there was no explicit flag controlling the data-collection container at all — its privilege level was implied by the collected signals. Running it non-privileged required --set signals="{traces}", which coupled an unrelated setting to a security decision and made logs/metrics collection
impossible without privileged mode.

This PR:

  • Runs the odiglet and data-collection containers non-privileged by default, using the capabilities, appArmorProfile and seLinuxOptions already defined per container.
  • Replaces odiglet.unPrivileged with per-container odiglet.odiglet.privileged and odiglet.dataCollection.privileged, both defaulting to false. The old key sat at the pod level but only ever controlled the odiglet and init containers, which was misleading.
  • Removes the signals-derived privilege decision, so data-collection can run non-privileged with metrics and logs enabled.
  • Adds odiglet.dataCollection.seLinuxOptions (default spc_t), the SELinux domain the container already receives when running privileged, so dropping privileged does not change its domain on SELinux-enforcing nodes.
  • Adds SYSLOG and SYS_RESOURCE to data-collection when profiling.enabled is true, only when not already present in the user-supplied capabilities list. The eBPF profiler reads /proc/kallsyms addresses (CAP_SYSLOG) and raises RLIMIT_MEMLOCK (CAP_SYS_RESOURCE).
  • for OBI NET_ADMIN and NET_RAW are added to the default capability set of the odiglet container - these can be removed if OBI is not used.
  • For community go instrumentation we add the KILL capability since the upstream go openteleemtry instrumentation use signal(0) to check for existance of a PID.

Changelog entry: Does this PR introduce a user-facing bug fix, feature, dependency update, or breaking change??

feat: [BREAKING CHANGE] the odiglet and data-collection containers now run as non-privileged by default. To restore the previous behavior set `odiglet.odiglet.privileged=true` and `odiglet.dataCollection.privileged=true`.
feat: [BREAKING CHANGE] `odiglet.unPrivileged` is replaced by the per-container `odiglet.odiglet.privileged` and `odiglet.dataCollection.privileged` flags, to make clear they apply to individual containers in the odiglet daemonset and not to the whole pod.
feat: [BREAKING CHANGE] the `k8s-csi-driver` mount method now requires `odiglet.odiglet.privileged=true`.
feat: data-collection can now run non-privileged with metrics and logs enabled; it is no longer implied by `signals`.

@RonFed RonFed changed the title feat: add dedicated flag for data-collection as non privileged RUN-1121 | feat: add dedicated flag for data-collection as non privileged Sep 6, 2026

@blumamir blumamir left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

added few nit comments

Comment thread helm/odigos/values.yaml
Comment thread helm/odigos/values.yaml Outdated
Comment on lines +893 to +897
# @schema
# description: |-
# Run the odiglet container, and its init container, as non-privileged containers.
# @schema
unPrivileged: false

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess if it's possible then all people would prefer to use it right, but then we should mention when it would not work and under which conditions it needs to be set?

{{- end }}
securityContext:
{{- if not .Values.odiglet.unPrivileged}}
{{- if not .Values.odiglet.odiglet.unPrivileged}}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

will it work for oss? should we check and fail the deployment if it's used where not supported?

Comment thread helm/odigos/values.yaml
Comment on lines +989 to +995
# @schema
# description: |-
# seLinuxOptions for the data-collection container when running as unPrivileged.
# "spc_t" is the SELinux domain the container already gets when running privileged.
# @schema
seLinuxOptions:
type: "spc_t"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When someone will need to use it, how can he know what to write here in order to make it work?
Should we mention when to use it and how?

Comment thread helm/odigos/values.yaml Outdated
Comment on lines +979 to +982
# uncomment when profiling.enabled is true - the eBPF profiler reads
# /proc/kallsyms addresses and raises RLIMIT_MEMLOCK
# - SYSLOG
# - SYS_RESOURCE

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we can write this into the template, right? it will be easy to miss. if we choose to keep it, should we also add a comment next to profiling.enabled to make it more discoverable?

@RonFed
RonFed force-pushed the data_collection_un_piv branch from 9cb77ce to aaf4ebb Compare September 7, 2026 11:39
@RonFed
RonFed marked this pull request as draft September 7, 2026 11:54
@RonFed
RonFed force-pushed the data_collection_un_piv branch from aaf4ebb to 353b434 Compare September 7, 2026 12:18
@RonFed
RonFed marked this pull request as ready for review September 7, 2026 12:20
@RonFed RonFed changed the title RUN-1121 | feat: add dedicated flag for data-collection as non privileged RUN-1121 | feat: run odiglet and data-collection as non-privileged by default Sep 7, 2026
@RonFed
RonFed force-pushed the data_collection_un_piv branch from 22f7337 to 05c6a49 Compare September 7, 2026 13:20
@RonFed
RonFed force-pushed the data_collection_un_piv branch from 05c6a49 to a47ec71 Compare September 7, 2026 14:43
@RonFed
RonFed merged commit 08a677a into odigos-io:main Sep 7, 2026
153 of 162 checks passed
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.

3 participants