Add writev, shutdown, and chown to system-probe seccomp profile by vitkyrka · Pull Request #2634 · DataDog/helm-charts

vitkyrka · 2026-05-07T09:53:17Z

What this PR does / why we need it:

Adds writev, shutdown, and chown to the system-probe seccomp profile (default action SCMP_ACT_ERRNO). All three are required by system-probe-lite (Rust/hyper-based) but were tolerated or never invoked by the Go system-probe, so the existing profile rejected them with EPERM.

`writev` and `shutdown`

Hyper calls shutdown(SHUT_WR) at the end of every served HTTP/1 connection (proto/h1/dispatch.rs — Future::poll passes should_shutdown=true and the dispatcher invokes poll_shutdown once is_done()), and uses tokio's AsyncWrite::poll_write_vectored, which translates to writev(2). The Go-based system-probe didn't surface either as a problem because net/http buffers responses through bufio.Writer and writes them via plain write(2), and the HTTP/1 keep-alive happy path used by the agent's checks never triggers a shutdown(2) (Go only calls CloseWrite in edge cases like 431 oversized headers or partially-consumed request bodies).

Symptom: hyper's first writev returns EPERM, so the response body never reaches the agent. The agent's pkg/system-probe/api/client/check.go:ensureStarted() retries GET /debug/stats every 5s and only ever sees EOF, blocking the discovery check from starting at all:

SYS-PROBE-LITE | ERROR | Error serving connection: error writing a body to connection
                                                  ^-- BodyWrite, EPERM (Operation not permitted)

CORE | WARN | system-probe not started yet: Get "http://sysprobe/debug/stats": EOF

`chown`

After binding the socket, system-probe-lite calls std::os::unix::fs::chown(socket_path, uid, gid) to hand it off to dd-agent. On x86_64 this dispatches to the chown(2) syscall, which the existing profile rejected with EPERM:

SYS-PROBE-LITE | WARN | could not set socket ownership: Operation not permitted (os error 1)

Two reasons this didn't show up before:

Go's syscall.Chown on Linux always dispatches through fchownat(AT_FDCWD, ...) (src/syscall/syscall_linux.go), and fchownat was already in the allowlist — so the Go system-probe was unaffected.
aarch64 has no chown(2) syscall in the kernel; glibc's chown() wrapper falls through to fchownat(2). Original verification was done on aarch64, where the profile happened to allow what was actually called.

Failure is non-fatal in default helm deployments where every agent container runs as root (datadog.securityContext.runAsUser: 0) and can talk to the resulting root:root mode 0720 socket. But for setups overriding the agent user to dd-agent, the socket would be unreachable.

Which issue this PR fixes

N/A.

Special notes for your reviewer:

All three syscalls are basic socket-I/O / file primitives; they're already in the upstream Docker default seccomp profile.
The previous GPU-monitoring conditional around chown is removed — fchown/fchown32/fchownat were already unconditional, so the chown gate was inconsistent.
Verified on a kind cluster (aarch64 originally, x86_64 follow-up) with the agent built locally execing into system-probe-lite — no Error serving connection logs, no agent-side EOF warnings, no chown WARN, socket correctly owned dd-agent:root, check:discovery running cleanly.

Checklist

All commits are signed and show as "Verified" on GitHub
Chart Version semver bump label has been added (datadog/patch-version)
For datadog chart changes, update the test baselines (regenerated via make update-test-baselines-datadog-agent)
For datadog chart changes, received ✅ from a member of your team
CHANGELOG.md has been updated

Required by system-probe-lite (Rust/hyper-based) which uses vectored writes (`writev`) for HTTP responses and `shutdown(2)` for graceful connection close. The Go-based system-probe didn't surface these as a problem because net/http buffers responses through bufio.Writer and writes them via plain `write(2)`, and the HTTP/1 keep-alive happy path used by the agent's checks never triggers a `shutdown(2)` (Go only calls `CloseWrite` in edge cases like 431 oversized headers or partially-consumed request bodies). Without this fix, the seccomp profile (defaultAction SCMP_ACT_ERRNO) returns EPERM for `writev`/`shutdown`, surfacing as "error writing a body to connection" / "error shutting down connection" in system-probe-lite logs and "EOF" on the agent's `GET /debug/stats` startup probe, preventing the discovery check from ever starting.

Run helm-docs to refresh the README version badge to match the bumped chart version.

github-actions · 2026-05-07T09:53:27Z

⚠️ GKE Autopilot / GDC Baseline Manifests Changed

This PR modifies GKE Autopilot or GDC baseline manifest snapshots. Before merging, confirm:

GKE Autopilot/GKE GDC baseline manifest diffs have been reviewed and confirmed to be supported in GKE Autopilot and the latest Datadog WorkloadAllowlist.

If changes introduce constraints not yet covered by the Datadog WorkloadAllowlist CR, gate them with {{- if not (or .Values.providers.gke.autopilot .Values.providers.gke.gdc) }} until the WorkloadAllowlist is updated.
See gke-constraints-review-guide.md for the full constraint reference.

Required by system-probe-lite on x86_64. After socket creation, system-probe-lite calls `std::os::unix::fs::chown(path, uid, gid)` to hand the socket off to `dd-agent`; on x86_64 that resolves to the `chown(2)` syscall, which the existing profile rejects with EPERM (SCMP_ACT_ERRNO): SYS-PROBE-LITE | WARN | could not set socket ownership: Operation not permitted (os error 1) The Go-based system-probe didn't surface this because Go's `syscall.Chown` on Linux dispatches through `fchownat(AT_FDCWD, ...)` (`src/syscall/syscall_linux.go`), which is already in the allowlist. arm64 didn't surface it either, because glibc's `chown()` on aarch64 also wraps `fchownat(2)` (the kernel has no `chown` syscall there). Failure is non-fatal in default helm deployments where every agent container runs as root (`datadog.securityContext.runAsUser: 0`) and can talk to the resulting `root:root` mode 0720 socket regardless. But for setups that override the agent to run as `dd-agent`, the socket would be unreachable. Allow `chown` unconditionally to match the existing unconditional `fchown`/`fchownat` entries.

…er causes (#50538) ### What does this PR do? In \`pkg/discovery/module/rust/src/main.rs\`, three \`error!\`/\`warn!\` call sites now use the \`{:#}\` alternate format (via \`anyhow\`) so the full error cause chain is included in the log line. | Call site | Before | After | |---|---|---| | \`serve_connection\` failure | \`Error serving connection: error writing a body to connection\` | \`Error serving connection: error writing a body to connection: Operation not permitted (os error 1)\` | | \`handle_request\` failure | \`Request handling failed: <top-level>\` | \`Request handling failed: <top-level>: <cause>\` | | \`drop_capabilities\` failure | \`Failed to restrict capabilities: <top-level>; …\` | \`Failed to restrict capabilities: <top-level>: <cause>; …\` | For \`hyper::Error\` (the connection error), wrapping in \`anyhow::Error::new(err)\` is required because \`hyper::Error\`'s own \`Display\` impl does not traverse \`.source()\`. For the two \`anyhow::Error\` sites the change is simply adding \`#\` to the format specifier. ### Motivation While debugging [helm-charts#2634](DataDog/helm-charts#2634), which added \`writev\`, \`shutdown\`, and \`chown\` to the system-probe seccomp allowlist for SPL, the only log produced was: ``` SYS-PROBE-LITE | ERROR | Error serving connection: error writing a body to connection ``` The underlying cause (\`Operation not permitted (os error 1)\`) was invisible, making \`strace\` necessary to identify the blocked syscall. With this fix the EPERM — or any other root cause — appears directly in the log. ### Describe how you validated your changes - \`bazel build --config=lint //pkg/discovery/module/rust/...\` passes - \`bazel test //pkg/discovery/module/rust/...\` — all 7 tests pass - Manual test for the \`serve_connection\` path: ran SPL with an \`LD_PRELOAD\` shim that makes every \`writev(2)\` call return \`EPERM\`, then sent a GET request to the socket: - **Before:** \`Error serving connection: error writing a body to connection\` - **After:** \`Error serving connection: error writing a body to connection: Operation not permitted (os error 1)\` - Manual test for the \`handle_request\` path: ran SPL with a temporary chained-error injection (\`anyhow!("inner").context("outer")\`) at the top of \`handle_request\`: - **Before:** \`Request handling failed: outer\` - **After:** \`Request handling failed: outer: inner\` Co-authored-by: vincent.whitchurch <[email protected]>

### What does this PR do? Adds a Kubernetes E2E sanity test for the Datadog Agent's discovery feature, mirroring (in spirit) the existing Linux VM suite in `test/new-e2e/tests/discovery/linux_test.go`. The new suite at `test/new-e2e/tests/discovery/k8s_test.go`: - Provisions a Kind-on-EC2 cluster (`provkindvm`) with the agent installed via Helm and an `nginx` workload. - Runs `TestNginxDiscovered` in two sub-tests covering both discovery modes: - `system-probe-lite` — discovery enabled alone; the agent execs into `system-probe-lite` per `cmd/system-probe/subcommands/run/splite.go`. - `system-probe` — adds `datadog.systemProbe.enableOOMKill: true` so the agent's `shouldExecSPLite` keeps the full binary running. - Asserts on the matched nginx process: TCP port 80 listener, `ServiceDiscovery.GeneratedServiceName == {nginx, SOURCE_COMMAND_LINE}`, `ApmInstrumentation == false`, `InjectionState == NOT_INJECTED`, non-empty `ContainerId`. - Pins the Helm chart version to `3.213.0`; the framework default (`3.155.1`) predates both the discovery template (chart 3.205.0, DataDog/helm-charts#2598) and the `latest`/`7` → `7.78.x` fallback fix (chart 3.213.0, DataDog/helm-charts#2643). ### Motivation https://datadoghq.atlassian.net/browse/DSCVR-385 There is no k8s e2e for discovery today. DataDog/helm-charts#2634 fixed a bug in discovery on k8s related to the seccomp profile; the change went undetected before merge because nothing exercises the discovery flow on a Helm-installed agent. This suite closes that gap with a smoke-level check that the discovery payload reaches fakeintake in both modes. The k8s suite is narrower than `linux_test.go` (single workload, smoke-level assertions), at least for now. This is to avoid having to add new app definitions at this point for a basic test. The VM suite continues to be the exhaustive per-language source of truth. ### Describe how you validated your changes Ran locally against AWS twice with `E2E_DEV_MODE=true dda inv -- -e new-e2e-tests.run --targets ./tests/discovery --run TestK8sTestSuite --stack-name-suffix v2`: ``` --- PASS: TestK8sTestSuite (364.64s) --- PASS: TestK8sTestSuite/TestNginxDiscovered (...) --- PASS: TestK8sTestSuite/TestNginxDiscovered/system-probe-lite --- PASS: TestK8sTestSuite/TestNginxDiscovered/system-probe ``` Lint: `golangci-lint run ./test/new-e2e/tests/discovery/...` clean (0 issues). ### Additional Notes A few quirks worth flagging for reviewers: - **Forced DaemonSet rollout via pod annotation.** Toggling `datadog.systemProbe.enableOOMKill` between sub-tests only mutates the chart-rendered configmap; the chart's `daemonset.yaml` has no `checksum/system-probe-yaml` annotation tying the pod template to that configmap, so the existing pod stays alive across the change. The suite writes a `agents.podAnnotations.discovery-e2e-mode: "spl"/"sp"` that intentionally differs per sub-test to bump the pod-template hash and force a rollout. - **Tracer metadata / language detection out of scope.** Nginx is C/native so `language` and `serviceDiscovery.tracerMetadata` aren't populated. The two natural ways to extend this — switch to (or add) an OTel-instrumented Go server (`apps-calendar-go` — no `K8sAppDefinition` yet) or a `dd-trace-go`-instrumented app (`apps-tracegen` predates the memfd-tracer-metadata feature in `dd-trace-go.v1`) — both require cross-repo work in `test-infra-definitions`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: vincent.whitchurch <[email protected]>

…er causes (#50538) ### What does this PR do? In \`pkg/discovery/module/rust/src/main.rs\`, three \`error!\`/\`warn!\` call sites now use the \`{:#}\` alternate format (via \`anyhow\`) so the full error cause chain is included in the log line. | Call site | Before | After | |---|---|---| | \`serve_connection\` failure | \`Error serving connection: error writing a body to connection\` | \`Error serving connection: error writing a body to connection: Operation not permitted (os error 1)\` | | \`handle_request\` failure | \`Request handling failed: <top-level>\` | \`Request handling failed: <top-level>: <cause>\` | | \`drop_capabilities\` failure | \`Failed to restrict capabilities: <top-level>; …\` | \`Failed to restrict capabilities: <top-level>: <cause>; …\` | For \`hyper::Error\` (the connection error), wrapping in \`anyhow::Error::new(err)\` is required because \`hyper::Error\`'s own \`Display\` impl does not traverse \`.source()\`. For the two \`anyhow::Error\` sites the change is simply adding \`#\` to the format specifier. ### Motivation While debugging [helm-charts#2634](DataDog/helm-charts#2634), which added \`writev\`, \`shutdown\`, and \`chown\` to the system-probe seccomp allowlist for SPL, the only log produced was: ``` SYS-PROBE-LITE | ERROR | Error serving connection: error writing a body to connection ``` The underlying cause (\`Operation not permitted (os error 1)\`) was invisible, making \`strace\` necessary to identify the blocked syscall. With this fix the EPERM — or any other root cause — appears directly in the log. ### Describe how you validated your changes - \`bazel build --config=lint //pkg/discovery/module/rust/...\` passes - \`bazel test //pkg/discovery/module/rust/...\` — all 7 tests pass - Manual test for the \`serve_connection\` path: ran SPL with an \`LD_PRELOAD\` shim that makes every \`writev(2)\` call return \`EPERM\`, then sent a GET request to the socket: - **Before:** \`Error serving connection: error writing a body to connection\` - **After:** \`Error serving connection: error writing a body to connection: Operation not permitted (os error 1)\` - Manual test for the \`handle_request\` path: ran SPL with a temporary chained-error injection (\`anyhow!("inner").context("outer")\`) at the top of \`handle_request\`: - **Before:** \`Request handling failed: outer\` - **After:** \`Request handling failed: outer: inner\` Co-authored-by: vincent.whitchurch <[email protected]>

### What does this PR do? Adds a Kubernetes E2E sanity test for the Datadog Agent's discovery feature, mirroring (in spirit) the existing Linux VM suite in `test/new-e2e/tests/discovery/linux_test.go`. The new suite at `test/new-e2e/tests/discovery/k8s_test.go`: - Provisions a Kind-on-EC2 cluster (`provkindvm`) with the agent installed via Helm and an `nginx` workload. - Runs `TestNginxDiscovered` in two sub-tests covering both discovery modes: - `system-probe-lite` — discovery enabled alone; the agent execs into `system-probe-lite` per `cmd/system-probe/subcommands/run/splite.go`. - `system-probe` — adds `datadog.systemProbe.enableOOMKill: true` so the agent's `shouldExecSPLite` keeps the full binary running. - Asserts on the matched nginx process: TCP port 80 listener, `ServiceDiscovery.GeneratedServiceName == {nginx, SOURCE_COMMAND_LINE}`, `ApmInstrumentation == false`, `InjectionState == NOT_INJECTED`, non-empty `ContainerId`. - Pins the Helm chart version to `3.213.0`; the framework default (`3.155.1`) predates both the discovery template (chart 3.205.0, DataDog/helm-charts#2598) and the `latest`/`7` → `7.78.x` fallback fix (chart 3.213.0, DataDog/helm-charts#2643). ### Motivation https://datadoghq.atlassian.net/browse/DSCVR-385 There is no k8s e2e for discovery today. DataDog/helm-charts#2634 fixed a bug in discovery on k8s related to the seccomp profile; the change went undetected before merge because nothing exercises the discovery flow on a Helm-installed agent. This suite closes that gap with a smoke-level check that the discovery payload reaches fakeintake in both modes. The k8s suite is narrower than `linux_test.go` (single workload, smoke-level assertions), at least for now. This is to avoid having to add new app definitions at this point for a basic test. The VM suite continues to be the exhaustive per-language source of truth. ### Describe how you validated your changes Ran locally against AWS twice with `E2E_DEV_MODE=true dda inv -- -e new-e2e-tests.run --targets ./tests/discovery --run TestK8sTestSuite --stack-name-suffix v2`: ``` --- PASS: TestK8sTestSuite (364.64s) --- PASS: TestK8sTestSuite/TestNginxDiscovered (...) --- PASS: TestK8sTestSuite/TestNginxDiscovered/system-probe-lite --- PASS: TestK8sTestSuite/TestNginxDiscovered/system-probe ``` Lint: `golangci-lint run ./test/new-e2e/tests/discovery/...` clean (0 issues). ### Additional Notes A few quirks worth flagging for reviewers: - **Forced DaemonSet rollout via pod annotation.** Toggling `datadog.systemProbe.enableOOMKill` between sub-tests only mutates the chart-rendered configmap; the chart's `daemonset.yaml` has no `checksum/system-probe-yaml` annotation tying the pod template to that configmap, so the existing pod stays alive across the change. The suite writes a `agents.podAnnotations.discovery-e2e-mode: "spl"/"sp"` that intentionally differs per sub-test to bump the pod-template hash and force a rollout. - **Tracer metadata / language detection out of scope.** Nginx is C/native so `language` and `serviceDiscovery.tracerMetadata` aren't populated. The two natural ways to extend this — switch to (or add) an OTel-instrumented Go server (`apps-calendar-go` — no `K8sAppDefinition` yet) or a `dd-trace-go`-instrumented app (`apps-tracegen` predates the memfd-tracer-metadata feature in `dd-trace-go.v1`) — both require cross-repo work in `test-infra-definitions`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: vincent.whitchurch <[email protected]>

vitkyrka added 2 commits May 7, 2026 07:30

Update README version badge for 3.208.1

bd271fd

Run helm-docs to refresh the README version badge to match the bumped chart version.

github-actions Bot added the chart/datadog This issue or pull request is related to the datadog chart label May 7, 2026

vitkyrka marked this pull request as ready for review May 7, 2026 10:13

vitkyrka requested review from a team as code owners May 7, 2026 10:13

vitkyrka requested review from tedkahwaji and removed request for a team May 7, 2026 10:13

vitkyrka mentioned this pull request May 7, 2026

Add writev and shutdown to system-probe seccomp profile DataDog/datadog-operator#2986

Merged

3 tasks

vitkyrka marked this pull request as draft May 7, 2026 10:53

vitkyrka changed the title ~~Add writev and shutdown to system-probe seccomp profile~~ Add writev, shutdown, and chown to system-probe seccomp profile May 7, 2026

vitkyrka marked this pull request as ready for review May 7, 2026 11:06

swang392 approved these changes May 7, 2026

View reviewed changes

brycekahle approved these changes May 7, 2026

View reviewed changes

gh-worker-dd-devflow-36fce6 Bot added mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: error and removed mergequeue-status: queued mergequeue-status: in_progress labels May 7, 2026

Merge branch 'main' into vitkyrka/disco-seccomp

3e856f5

gh-worker-dd-devflow-36fce6 Bot added mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: rejected and removed mergequeue-status: error mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: rejected labels May 7, 2026

gh-worker-dd-devflow-36fce6 Bot added mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: rejected and removed mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: rejected labels May 7, 2026

gh-worker-dd-mergequeue-cf854d Bot merged commit 393a594 into main May 8, 2026
66 checks passed

gh-worker-dd-devflow-36fce6 Bot removed the mergequeue-status: in_progress label May 8, 2026

gh-worker-dd-mergequeue-cf854d Bot deleted the vitkyrka/disco-seccomp branch May 8, 2026 08:58

gh-worker-dd-devflow-36fce6 Bot added the mergequeue-status: done label May 8, 2026

vitkyrka mentioned this pull request May 12, 2026

test(e2e/discovery): add k8s test using Helm charts DataDog/datadog-agent#50666

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add writev, shutdown, and chown to system-probe seccomp profile#2634

Add writev, shutdown, and chown to system-probe seccomp profile#2634
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
mainfrom
vitkyrka/disco-seccomp

vitkyrka commented May 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

vitkyrka commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

writev and shutdown

chown

Which issue this PR fixes

Special notes for your reviewer:

Checklist

Uh oh!

github-actions Bot commented May 7, 2026

⚠️ GKE Autopilot / GDC Baseline Manifests Changed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vitkyrka commented May 7, 2026 •

edited

Loading

`writev` and `shutdown`

`chown`