Fix Env/VolumeMount slice aliasing in auto-instrumentation, OpAMP Bridge, and Target Allocator

.chloggen/fix-volumemount-aliasing.yaml

@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
+component: auto-instrumentation, opamp, target allocator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix Env slice aliasing in Apache HTTPD, Nginx, OpAMP Bridge, and Target Allocator container builders when the spec slice has spare backing-array capacity
+
+# One or more tracking issues related to the change
+issues: [4954]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:

internal/instrumentation/apachehttpd.go

@@ -76,10 +76,10 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod
 			Args:    []string{"cp -r " + apacheConfDir + "/* " + apacheAgentConfDirFull},
 			Env:     container.Env,
 			EnvFrom: container.EnvFrom,
-			VolumeMounts: append(container.VolumeMounts, corev1.VolumeMount{
+			VolumeMounts: slices.Concat(container.VolumeMounts, []corev1.VolumeMount{{
 				Name:      apacheAgentConfigVolume,
 				MountPath: apacheAgentConfDirFull,
-			}),
+			}}),
 			Resources:       apacheSpec.Resources,
 			SecurityContext: container.SecurityContext,
 			ImagePullPolicy: container.ImagePullPolicy,

internal/instrumentation/apachehttpd_test.go

@@ -542,6 +542,29 @@ func TestInjectApacheHttpdagentUnknownNamespace(t *testing.T) {
 	}
 }
 
+// Regression test: when the container's VolumeMounts slice has spare capacity
+// (e.g. after Istio injection), append aliasing corrupts the clone init
+// container's mounts.
+func TestInjectApacheHttpdagentVolumemountAliasing(t *testing.T) {
+	// Spare capacity is the trigger — simulates a prior webhook leaving room.
+	mounts := make([]corev1.VolumeMount, 0, 4)
+	mounts = append(mounts,
+		corev1.VolumeMount{Name: "a", MountPath: "/a"},
+		corev1.VolumeMount{Name: "b", MountPath: "/b"},
+	)
+
+	pod := corev1.Pod{Spec: corev1.PodSpec{Containers: []corev1.Container{{VolumeMounts: mounts}}}}
+	result := injectApacheHttpdagent(logr.Discard(), v1alpha1.ApacheHttpd{Image: "foo/bar:1"},
+		pod, false, &pod.Spec.Containers[0], "http://otlp:4317",
+		map[string]string{string(semconv.K8SDeploymentNameKey): "svc"}, v1alpha1.InstrumentationSpec{})
+
+	clone := result.Spec.InitContainers[0]
+	lastMount := clone.VolumeMounts[len(clone.VolumeMounts)-1]
+	assert.Equal(t, apacheAgentConfigVolume, lastMount.Name,
+		"clone's config mount was corrupted by slice aliasing: %v", clone.VolumeMounts)
+	assert.Equal(t, apacheAgentConfDirFull, lastMount.MountPath)
+}
+
 func TestApacheInitContainerMissing(t *testing.T) {
 	tests := []struct {
 		name     string

internal/instrumentation/nginx.go

@@ -92,10 +92,10 @@ export %[4]s="$( { nginx -v ; } 2>&1 )" && echo ${%[4]s##*/} > %[3]s/version.txt
 			Args:    []string{nginxAgentCommands},
 			Env:     container.Env,
 			EnvFrom: container.EnvFrom,
-			VolumeMounts: append(container.VolumeMounts, corev1.VolumeMount{
+			VolumeMounts: slices.Concat(container.VolumeMounts, []corev1.VolumeMount{{
 				Name:      nginxAgentConfigVolume,
 				MountPath: nginxAgentConfDirFull,
-			}),
+			}}),
 			Resources:       nginxSpec.Resources,
 			SecurityContext: container.SecurityContext,
 			ImagePullPolicy: container.ImagePullPolicy,

internal/instrumentation/nginx_test.go

@@ -604,6 +604,27 @@ func TestInjectNginxUnknownNamespace(t *testing.T) {
 	}
 }
 
+// Regression test: append aliasing corrupts clone init container mounts when
+// the VolumeMounts slice has spare capacity.
+func TestInjectNginxSDKVolumemountAliasing(t *testing.T) {
+	mounts := make([]corev1.VolumeMount, 0, 4)
+	mounts = append(mounts,
+		corev1.VolumeMount{Name: "a", MountPath: "/a"},
+		corev1.VolumeMount{Name: "b", MountPath: "/b"},
+	)
+
+	pod := corev1.Pod{Spec: corev1.PodSpec{Containers: []corev1.Container{{VolumeMounts: mounts}}}}
+	result := injectNginxSDK(logr.Discard(), v1alpha1.Nginx{Image: "foo/bar:1"},
+		pod, false, &pod.Spec.Containers[0], "http://otlp:4317",
+		map[string]string{string(semconv.K8SDeploymentNameKey): "svc"}, v1alpha1.InstrumentationSpec{})
+
+	clone := result.Spec.InitContainers[0]
+	lastMount := clone.VolumeMounts[len(clone.VolumeMounts)-1]
+	assert.Equal(t, nginxAgentConfigVolume, lastMount.Name,
+		"clone's config mount was corrupted by slice aliasing: %v", clone.VolumeMounts)
+	assert.Equal(t, nginxAgentConfDirFull, lastMount.MountPath)
+}
+
 func TestNginxInitContainerMissing(t *testing.T) {
 	tests := []struct {
 		name     string

internal/manifests/opampbridge/container.go

@@ -4,6 +4,8 @@
 package opampbridge
 
 import (
+	"slices"
+
 	"github.com/go-logr/logr"
 	"github.com/operator-framework/operator-lib/proxy"
 	corev1 "k8s.io/api/core/v1"
@@ -30,7 +32,7 @@ func Container(cfg config.Config, _ logr.Logger, opampBridge v1alpha1.OpAMPBridg
 		volumeMounts = append(volumeMounts, opampBridge.Spec.VolumeMounts...)
 	}
 
-	envVars := opampBridge.Spec.Env
+	envVars := slices.Clone(opampBridge.Spec.Env)
 	if opampBridge.Spec.Env == nil {
 		envVars = []corev1.EnvVar{}
 	}

internal/manifests/opampbridge/container_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
@@ -65,3 +66,28 @@ func TestContainerVolumes(t *testing.T) {
 	assert.Len(t, c.VolumeMounts, 1)
 	assert.Equal(t, naming.OpAMPBridgeConfigMapVolume(), c.VolumeMounts[0].Name)
 }
+
+// Regression test: when Spec.Env has spare backing-array capacity,
+// the container's Env must not share the underlying array with the spec.
+func TestContainerEnvAliasing(t *testing.T) {
+	env := make([]corev1.EnvVar, 0, 10)
+	env = append(env, corev1.EnvVar{Name: "USER_VAR", Value: "val"})
+
+	opampBridge := v1alpha1.OpAMPBridge{
+		Spec: v1alpha1.OpAMPBridgeSpec{
+			Env: env,
+		},
+	}
+	cfg := config.New()
+
+	c := Container(cfg, logger, opampBridge)
+
+	// Mutate the original spec — container must not be affected.
+	opampBridge.Spec.Env = append(opampBridge.Spec.Env,
+		corev1.EnvVar{Name: "intruder", Value: "bad"})
+
+	for _, e := range c.Env {
+		assert.NotEqual(t, "intruder", e.Name,
+			"container Env shares backing array with spec")
+	}
+}

internal/manifests/targetallocator/container.go

@@ -48,7 +48,7 @@ func Container(cfg config.Config, _ logr.Logger, instance v1alpha1.TargetAllocat
 	}}
 	volumeMounts = append(volumeMounts, instance.Spec.VolumeMounts...)
 
-	envVars := instance.Spec.Env
+	envVars := slices.Clone(instance.Spec.Env)
 	if envVars == nil {
 		envVars = []corev1.EnvVar{}
 	}

internal/manifests/targetallocator/container_test.go

@@ -644,6 +644,33 @@ func TestContainerEnvFrom(t *testing.T) {
 	assert.Contains(t, c.EnvFrom, envFrom2)
 }
 
+// Regression test: when Spec.Env has spare backing-array capacity,
+// the container's Env must not share the underlying array with the spec.
+func TestContainerEnvAliasing(t *testing.T) {
+	env := make([]corev1.EnvVar, 0, 10)
+	env = append(env, corev1.EnvVar{Name: "USER_VAR", Value: "val"})
+
+	targetAllocator := v1alpha1.TargetAllocator{
+		Spec: v1alpha1.TargetAllocatorSpec{
+			OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{
+				Env: env,
+			},
+		},
+	}
+	cfg := config.New()
+
+	c := Container(cfg, logger, targetAllocator)
+
+	// Mutate the original spec — container must not be affected.
+	targetAllocator.Spec.Env = append(targetAllocator.Spec.Env,
+		corev1.EnvVar{Name: "intruder", Value: "bad"})
+
+	for _, e := range c.Env {
+		assert.NotEqual(t, "intruder", e.Name,
+			"container Env shares backing array with spec")
+	}
+}
+
 func TestContainerImagePullPolicy(t *testing.T) {
 	// prepare
 	targetAllocator := v1alpha1.TargetAllocator{