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,