Add handling of environment variables in ConfigMaps

Currently when environment variable is defined in ConfigMap, it is like
being invisible to OpenTelemetry Operator. Improve this by adding a common
class handling the environment variables, which is also able to load
the referenced ConfigMaps.

Signed-off-by: Oldřich Jedlička <[email protected]>

.chloggen/enhancement-envs-from-configmap.yaml

@@ -0,0 +1,19 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# 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
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Add support for detecting and reading environment variables from POD's ConfigMap resources.
+
+# One or more tracking issues related to the change
+issues: [1393, 1814]
+
+# (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: |
+  Now the auto-instrumentation will see the environment variables defined via POD's `env.valueFrom.configMapKeyRef` and
+  `envFrom.configMapRef` fields, so the auto-instrumentation will be able to prevent overriding them, or it will be able
+  to extend the existing definition.

pkg/instrumentation/apachehttpd.go

@@ -59,17 +59,10 @@ const (
 	6) Inject mounting of volumes / files into appropriate directories in application container
 */
 
-func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod corev1.Pod, useLabelsForResourceAttributes bool, index int, otlpEndpoint string, resourceMap map[string]string) corev1.Pod {
-
-	// caller checks if there is at least one container
-	container := &pod.Spec.Containers[index]
-
+func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod corev1.Pod, useLabelsForResourceAttributes bool, container Container, otlpEndpoint string, resourceMap map[string]string) corev1.Pod {
 	// inject env vars
 	for _, env := range apacheSpec.Env {
-		idx := getIndexOfEnv(container.Env, env.Name)
-		if idx == -1 {
-			container.Env = append(container.Env, env)
-		}
+		container.appendEnvVarIfNotExists(&pod, env)
 	}
 
 	// First make a clone of the instrumented container to take the existing Apache configuration from
@@ -86,7 +79,7 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod
 
 		apacheConfDir := getApacheConfDir(apacheSpec.ConfigPath)
 
-		cloneContainer := container.DeepCopy()
+		cloneContainer := pod.Spec.Containers[container.index].DeepCopy()
 		cloneContainer.Name = apacheAgentCloneContainerName
 		cloneContainer.Command = []string{"/bin/sh", "-c"}
 		cloneContainer.Args = []string{"cp -r " + apacheConfDir + "/* " + apacheAgentConfDirFull}
@@ -107,24 +100,24 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod
 		// drop volume mount with volume-provided Apache config from original container
 		// since it could over-write configuration provided by the injection
 		idxFound := -1
-		for idx, volume := range container.VolumeMounts {
+		for idx, volume := range pod.Spec.Containers[container.index].VolumeMounts {
 			if strings.Contains(volume.MountPath, apacheConfDir) { // potentially passes config, which we want to pass to init copy only
 				idxFound = idx
 				break
 			}
 		}
 		if idxFound >= 0 {
-			volumeMounts := container.VolumeMounts
+			volumeMounts := pod.Spec.Containers[container.index].VolumeMounts
 			volumeMounts = append(volumeMounts[:idxFound], volumeMounts[idxFound+1:]...)
-			container.VolumeMounts = volumeMounts
+			pod.Spec.Containers[container.index].VolumeMounts = volumeMounts
 		}
 
 		// Inject volumes info instrumented container - Apache config dir + Apache agent
-		container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
+		pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{
 			Name:      apacheAgentVolume,
 			MountPath: apacheAgentDirFull,
 		})
-		container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
+		pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{
 			Name:      apacheAgentConfigVolume,
 			MountPath: apacheConfDir,
 		})
@@ -162,7 +155,7 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod
 			Env: []corev1.EnvVar{
 				{
 					Name:  apacheAttributesEnvVar,
-					Value: getApacheOtelConfig(pod, useLabelsForResourceAttributes, apacheSpec, index, otlpEndpoint, resourceMap),
+					Value: getApacheOtelConfig(pod, useLabelsForResourceAttributes, apacheSpec, container.index, otlpEndpoint, resourceMap),
 				},
 				{Name: apacheServiceInstanceIdEnvVar,
 					ValueFrom: &corev1.EnvVarSource{

pkg/instrumentation/apachehttpd_test.go

@@ -15,10 +15,12 @@
 package instrumentation
 
 import (
+	"context"
 	"testing"
 
 	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	semconv "go.opentelemetry.io/otel/semconv/v1.7.0"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -417,7 +419,10 @@ func TestInjectApacheHttpdagent(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			pod := injectApacheHttpdagent(logr.Discard(), test.ApacheHttpd, test.pod, false, 0, "http://otlp-endpoint:4317", resourceMap)
+			pod := test.pod
+			container, err := NewContainer(k8sClient, context.Background(), logr.Discard(), "req-namespace", &pod, 0)
+			require.NoError(t, err)
+			pod = injectApacheHttpdagent(logr.Discard(), test.ApacheHttpd, pod, false, container, "http://otlp-endpoint:4317", resourceMap)
 			assert.Equal(t, test.expected, pod)
 		})
 	}
@@ -527,7 +532,10 @@ func TestInjectApacheHttpdagentUnknownNamespace(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			pod := injectApacheHttpdagent(logr.Discard(), test.ApacheHttpd, test.pod, false, 0, "http://otlp-endpoint:4317", resourceMap)
+			pod := test.pod
+			container, err := NewContainer(k8sClient, context.Background(), logr.Discard(), "", &pod, 0)
+			require.NoError(t, err)
+			pod = injectApacheHttpdagent(logr.Discard(), test.ApacheHttpd, pod, false, container, "http://otlp-endpoint:4317", resourceMap)
 			assert.Equal(t, test.expected, pod)
 		})
 	}