From 8b90785da23d4d0a411fa7c65bc11ad30bd65cfe Mon Sep 17 00:00:00 2001 From: Ihar Statkevich Date: Tue, 10 Mar 2026 15:56:29 +0100 Subject: [PATCH] fix: derive Java init container securityContext from the instrumented 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. --- pkg/instrumentation/podmutator_test.go | 8 +- pkg/instrumentation/sdk.go | 47 +++++++++- pkg/instrumentation/sdk_test.go | 119 ++++++++++++++++++++++++- 3 files changed, 165 insertions(+), 9 deletions(-) diff --git a/pkg/instrumentation/podmutator_test.go b/pkg/instrumentation/podmutator_test.go index 0f9c7c6e7..091d60cc1 100644 --- a/pkg/instrumentation/podmutator_test.go +++ b/pkg/instrumentation/podmutator_test.go @@ -220,7 +220,8 @@ func TestMutatePod(t *testing.T) { Name: javaVolumeName, MountPath: javaInstrMountPath, }}, - Resources: testResourceRequirements, + Resources: testResourceRequirements, + SecurityContext: restrictedSecurityContext, }, }, Containers: []corev1.Container{ @@ -408,7 +409,8 @@ func TestMutatePod(t *testing.T) { Name: javaVolumeName, MountPath: javaInstrMountPath, }}, - Resources: testResourceRequirements, + Resources: testResourceRequirements, + SecurityContext: restrictedSecurityContext, }, }, Containers: []corev1.Container{ @@ -3403,6 +3405,7 @@ func TestMutatePod(t *testing.T) { Name: javaVolumeName, MountPath: javaInstrMountPath, }}, + SecurityContext: restrictedSecurityContext, }, { Name: nodejsInitContainerName, @@ -4061,6 +4064,7 @@ func TestMutatePod(t *testing.T) { Name: javaVolumeName, MountPath: javaInstrMountPath, }}, + SecurityContext: restrictedSecurityContext, }, { Name: nodejsInitContainerName, diff --git a/pkg/instrumentation/sdk.go b/pkg/instrumentation/sdk.go index 2fbe589b3..719155031 100644 --- a/pkg/instrumentation/sdk.go +++ b/pkg/instrumentation/sdk.go @@ -99,9 +99,8 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations } else { pod = i.injectCommonEnvVar(otelinst, pod, index) pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) - //disable setting security context in init container due to issue with runAsNonRoot conflict - //https://github.com/open-telemetry/opentelemetry-operator/issues/2272 - //pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, javaInitContainerName) + + pod = i.setJavaInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, javaInitContainerName) } } } @@ -293,7 +292,47 @@ func isOtcContainer(container corev1.Container) bool { func (i *sdkInjector) setInitContainerSecurityContext(pod corev1.Pod, securityContext *corev1.SecurityContext, instrInitContainerName string) corev1.Pod { for i, initContainer := range pod.Spec.InitContainers { if initContainer.Name == instrInitContainerName { - pod.Spec.InitContainers[i].SecurityContext = securityContext + // Copy, so later edits to the init container's context cannot leak into the + // instrumented container through a shared pointer. + pod.Spec.InitContainers[i].SecurityContext = securityContext.DeepCopy() + } + } + + return pod +} + +// setJavaInitContainerSecurityContext derives the Java init container's securityContext from the +// instrumented container, keeping user-specified fields such as readOnlyRootFilesystem or +// seLinuxOptions, but dropping runAsNonRoot/runAsUser: the Java agent image runs as root and would +// otherwise conflict with them (https://github.com/open-telemetry/opentelemetry-operator/issues/2272). +// Fields required by the restricted Pod Security Standard are defaulted when the instrumented +// container leaves them unset, so injection stays valid in namespaces enforcing "restricted". +func (i *sdkInjector) setJavaInitContainerSecurityContext(pod corev1.Pod, securityContext *corev1.SecurityContext, instrInitContainerName string) corev1.Pod { + initSecurityContext := securityContext.DeepCopy() + if initSecurityContext == nil { + initSecurityContext = &corev1.SecurityContext{} + } + + initSecurityContext.RunAsNonRoot = nil + initSecurityContext.RunAsUser = nil + + if initSecurityContext.AllowPrivilegeEscalation == nil { + initSecurityContext.AllowPrivilegeEscalation = new(false) + } + if initSecurityContext.Capabilities == nil { + initSecurityContext.Capabilities = &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + } + } + if initSecurityContext.SeccompProfile == nil { + initSecurityContext.SeccompProfile = &corev1.SeccompProfile{ + Type: corev1.SeccompProfileTypeRuntimeDefault, + } + } + + for idx, initContainer := range pod.Spec.InitContainers { + if initContainer.Name == instrInitContainerName { + pod.Spec.InitContainers[idx].SecurityContext = initSecurityContext.DeepCopy() } } diff --git a/pkg/instrumentation/sdk_test.go b/pkg/instrumentation/sdk_test.go index 054d8c835..ad9e1f036 100644 --- a/pkg/instrumentation/sdk_test.go +++ b/pkg/instrumentation/sdk_test.go @@ -34,6 +34,16 @@ var testResourceRequirements = corev1.ResourceRequirements{ }, } +var restrictedSecurityContext = &corev1.SecurityContext{ + AllowPrivilegeEscalation: new(false), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + SeccompProfile: &corev1.SeccompProfile{ + Type: corev1.SeccompProfileTypeRuntimeDefault, + }, +} + func TestSDKInjection(t *testing.T) { ns := corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -529,7 +539,8 @@ func TestInjectJava(t *testing.T) { Name: javaVolumeName, MountPath: javaInstrMountPath, }}, - Resources: testResourceRequirements, + Resources: testResourceRequirements, + SecurityContext: restrictedSecurityContext, }, }, Containers: []corev1.Container{ @@ -582,6 +593,106 @@ func TestInjectJava(t *testing.T) { }, pod) } +func TestInjectJavaSecurityContext(t *testing.T) { + inst := v1alpha1.Instrumentation{ + Spec: v1alpha1.InstrumentationSpec{ + Java: v1alpha1.Java{ + Image: "img:1", + Resources: testResourceRequirements, + }, + Exporter: v1alpha1.Exporter{ + Endpoint: "https://collector:4317", + }, + }, + } + insts := languageInstrumentations{ + Java: instrumentationWithContainers{Instrumentation: &inst, Containers: ""}, + } + + for _, tt := range []struct { + name string + containerSC *corev1.SecurityContext + expected *corev1.SecurityContext + }{ + { + name: "no securityContext on the instrumented container", + containerSC: nil, + expected: restrictedSecurityContext, + }, + { + name: "runAsNonRoot and runAsUser are dropped, other fields are kept", + containerSC: &corev1.SecurityContext{ + RunAsNonRoot: new(true), + RunAsUser: new(int64(1000)), + RunAsGroup: new(int64(3000)), + AllowPrivilegeEscalation: new(false), + ReadOnlyRootFilesystem: new(true), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + SeccompProfile: &corev1.SeccompProfile{ + Type: corev1.SeccompProfileTypeRuntimeDefault, + }, + SELinuxOptions: &corev1.SELinuxOptions{Level: "s0:c123,c456"}, + }, + expected: &corev1.SecurityContext{ + RunAsGroup: new(int64(3000)), + AllowPrivilegeEscalation: new(false), + ReadOnlyRootFilesystem: new(true), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + SeccompProfile: &corev1.SeccompProfile{ + Type: corev1.SeccompProfileTypeRuntimeDefault, + }, + SELinuxOptions: &corev1.SELinuxOptions{Level: "s0:c123,c456"}, + }, + }, + { + name: "restricted fields are defaulted when the container omits them", + containerSC: &corev1.SecurityContext{ + ReadOnlyRootFilesystem: new(true), + }, + expected: &corev1.SecurityContext{ + ReadOnlyRootFilesystem: new(true), + AllowPrivilegeEscalation: new(false), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + SeccompProfile: &corev1.SeccompProfile{ + Type: corev1.SeccompProfileTypeRuntimeDefault, + }, + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + inj := sdkInjector{ + logger: logr.Discard(), + } + originalSC := tt.containerSC.DeepCopy() + pod := inj.inject(context.Background(), insts, + corev1.Namespace{}, + corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + Image: "app:latest", + SecurityContext: tt.containerSC, + }, + }, + }, + }) + + require.Len(t, pod.Spec.InitContainers, 1) + assert.Equal(t, tt.expected, pod.Spec.InitContainers[0].SecurityContext) + // The instrumented container's securityContext must not be modified, which + // a pointer shared with the init container would allow. + assert.Equal(t, originalSC, pod.Spec.Containers[0].SecurityContext) + }) + } +} + func TestInjectNodeJS(t *testing.T) { inst := v1alpha1.Instrumentation{ Spec: v1alpha1.InstrumentationSpec{ @@ -876,7 +987,8 @@ func TestInjectJavaAndPython(t *testing.T) { Name: javaVolumeName, MountPath: javaInstrMountPath, }}, - Resources: testResourceRequirements, + Resources: testResourceRequirements, + SecurityContext: restrictedSecurityContext, }, { Name: pythonInitContainerName, @@ -1178,7 +1290,8 @@ func TestInjectJavaPythonAndDotNet(t *testing.T) { Name: javaVolumeName, MountPath: javaInstrMountPath, }}, - Resources: testResourceRequirements, + Resources: testResourceRequirements, + SecurityContext: restrictedSecurityContext, }, { Name: pythonInitContainerName,