fix: add restricted-compliant securityContext to Java init container - #360
fix: add restricted-compliant securityContext to Java init container#360ab0utbla-k wants to merge 1 commit into
Conversation
|
Hi, we need this feature, kindly assign reviewer please. |
| // Set a minimal restricted-compliant securityContext without runAsNonRoot/runAsUser | ||
| // to avoid the runAsNonRoot conflict (https://github.com/open-telemetry/opentelemetry-operator/issues/2272) | ||
| // while still satisfying the restricted Pod Security Standard. | ||
| pod = i.setInitContainerRestrictedSecurityContext(pod, javaInitContainerName) |
There was a problem hiding this comment.
seems like this security context is missing in NGINX instrumentation as well - https://github.com/ab0utbla-k/amazon-cloudwatch-agent-operator/blob/3969c9f684e19dbd5fb5be421c6d72183b95036e/pkg/instrumentation/sdk.go#L231 mind adding there as well ?
There was a problem hiding this comment.
Thanks for the catch, but I think NGINX is already covered. It just sets the context inline in nginx.go rather than through the sdk.go helper, which is why it looks missing at the line linked:
otel-agent-attach-nginx:nginx.go#L169setsSecurityContext: pod.Spec.Containers[index].SecurityContextotel-agent-source-container-clone: created viacontainer.DeepCopy()atnginx.go#L78, so it inherits the instrumented container's context
Happy to move both to the shared helper for consistency if you'd prefer, but that felt out of scope for this fix. Java was the only language ending up with no context at all.
|
The other languages (Node.js, Python, .NET, Apache) use setInitContainerSecurityContext which copies the app container's full securityContext onto the init container. This PR introduces a separate Have you considered modifying the existing setInitContainerSecurityContext to copy the app container's securityContext but strip runAsNonRoot and runAsUser for the Java case? That would preserve any |
… container Copy the instrumented container's securityContext onto the Java init container and drop only runAsNonRoot/runAsUser, instead of applying a hardcoded minimal context. User-specified fields are preserved and the behavior is consistent with the other languages. Fields required by the restricted Pod Security Standard are defaulted when the container leaves them unset, so injection stays valid in namespaces enforcing "restricted". setInitContainerSecurityContext now deep-copies rather than sharing the pointer with the instrumented container, so per-language adjustments cannot leak back into it.
3969c9f to
8b90785
Compare
|
@mitali-salvi good call, done, the PR now does exactly that.
Two things worth flagging:
Added |
Fixes #361
Problem
The Java auto-instrumentation init container (
opentelemetry-auto-instrumentation-java) is created without anysecurityContext, causing pod creation to fail in namespaces enforcingpod-security.kubernetes.io/enforce: restricted.Other languages (NodeJS, Python, DotNet, Apache) are not affected, because they all call
setInitContainerSecurityContext, which copies the app container's security context onto the init container.Root Cause
In
pkg/instrumentation/sdk.go,setInitContainerSecurityContextwas commented out for Java to avoid arunAsNonRootconflict with the root-based Java agent image (opentelemetry-operator#2272). However, this left the init container with nosecurityContextat all, violating the restricted Pod Security Standard.Fix
Added
setJavaInitContainerSecurityContext, which derives the init container's context from the instrumented container instead of hardcoding one:securityContext, so user-specified fields (readOnlyRootFilesystem,seLinuxOptions,runAsGroup,capabilities.add, and so on) are preserved and behavior stays consistent with the other languages.runAsNonRootandrunAsUser, which are the fields that conflict with the root-based Java agent image (#2272).allowPrivilegeEscalation: false,capabilities.drop: ["ALL"],seccompProfile.type: RuntimeDefault), only when the instrumented container leaves them unset, so user values always win and a container with nosecurityContextat all still gets a compliant init container.setInitContainerSecurityContextnow deep-copies as well. It previously assigned the same pointer to both the init container and the instrumented container; stripping fields from a shared pointer would silently mutate the instrumented container. No behavior change today, since nothing mutated it before.Reproduction
pod-security.kubernetes.io/enforce: restrictedinstrumentation.opentelemetry.io/inject-java: "true"and a restricted-compliantsecurityContextFailedCreateevent on the ReplicaSetTesting
TestInjectJavaSecurityContextcovering three cases: nosecurityContexton the instrumented container (minimal restricted context applied), a full restricted context (runAsNonRoot/runAsUserdropped, everything else preserved), and a partial context (missing restricted fields defaulted). Each case also asserts the instrumented container'ssecurityContextis not modified.sdk_test.go, 4 inpodmutator_test.go) to include the resultingsecurityContext.go build ./...,go vet ./...andgo test ./pkg/instrumentation/...pass.