From ff4776d97f5a2c25b4eee2ebf64064d202fb901f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Old=C5=99ich=20Jedli=C4=8Dka?= Date: Wed, 9 Oct 2024 00:48:02 +0200 Subject: [PATCH 1/2] Add handling of environment variables in ConfigMaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../enhancement-envs-from-configmap.yaml | 19 + pkg/instrumentation/apachehttpd.go | 24 +- pkg/instrumentation/apachehttpd_test.go | 12 +- pkg/instrumentation/container.go | 336 ++++ pkg/instrumentation/container_test.go | 1397 +++++++++++++++++ pkg/instrumentation/dotnet.go | 61 +- pkg/instrumentation/dotnet_test.go | 8 +- pkg/instrumentation/exporter.go | 38 +- pkg/instrumentation/exporter_test.go | 6 +- pkg/instrumentation/golang.go | 3 +- pkg/instrumentation/javaagent.go | 29 +- pkg/instrumentation/javaagent_test.go | 10 +- pkg/instrumentation/nginx.go | 43 +- pkg/instrumentation/nginx_test.go | 14 +- pkg/instrumentation/nodejs.go | 27 +- pkg/instrumentation/nodejs_test.go | 12 +- pkg/instrumentation/python.go | 61 +- pkg/instrumentation/python_test.go | 8 +- pkg/instrumentation/sdk.go | 416 ++--- pkg/instrumentation/sdk_test.go | 11 +- .../00-install-collector.yaml | 22 + .../00-install-instrumentation.yaml | 34 + .../01-assert.yaml | 78 + .../01-install-app.yaml | 39 + .../chainsaw-test.yaml | 40 + .../01-assert.yaml | 4 +- .../02-assert.yaml | 2 +- .../03-assert.yaml | 2 +- .../instrumentation-java-tls/01-assert.yaml | 2 +- .../00-install-collector.yaml | 22 + .../00-install-instrumentation.yaml | 34 + .../01-assert.yaml | 80 + .../01-install-app.yaml | 47 + .../chainsaw-test.yaml | 40 + .../instrumentation-java/01-assert.yaml | 2 +- .../01-assert.yaml | 4 +- .../02-assert.yaml | 2 +- .../01-assert.yaml | 2 +- .../instrumentation-nodejs/01-assert.yaml | 2 +- .../01-assert.yaml | 4 +- .../01-assert.yaml | 2 +- 41 files changed, 2570 insertions(+), 429 deletions(-) create mode 100644 .chloggen/enhancement-envs-from-configmap.yaml create mode 100644 pkg/instrumentation/container.go create mode 100644 pkg/instrumentation/container_test.go create mode 100644 tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/00-install-collector.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/00-install-instrumentation.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/01-assert.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/01-install-app.yaml create mode 100755 tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/chainsaw-test.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/00-install-collector.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/00-install-instrumentation.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/01-assert.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/01-install-app.yaml create mode 100755 tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/chainsaw-test.yaml diff --git a/.chloggen/enhancement-envs-from-configmap.yaml b/.chloggen/enhancement-envs-from-configmap.yaml new file mode 100644 index 0000000000..80dc5a9b2b --- /dev/null +++ b/.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. diff --git a/pkg/instrumentation/apachehttpd.go b/pkg/instrumentation/apachehttpd.go index 87c1b744b4..656c1212ca 100644 --- a/pkg/instrumentation/apachehttpd.go +++ b/pkg/instrumentation/apachehttpd.go @@ -59,19 +59,13 @@ 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 { +func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod corev1.Pod, useLabelsForResourceAttributes bool, container Container, otlpEndpoint string, resourceMap map[string]string) corev1.Pod { volume := instrVolume(apacheSpec.VolumeClaimTemplate, apacheAgentVolume, apacheSpec.VolumeSizeLimit) - // caller checks if there is at least one container - container := &pod.Spec.Containers[index] - // 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 @@ -88,7 +82,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} @@ -109,24 +103,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, }) @@ -157,7 +151,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{ diff --git a/pkg/instrumentation/apachehttpd_test.go b/pkg/instrumentation/apachehttpd_test.go index 3a93d7418d..c9b4c0510b 100644 --- a/pkg/instrumentation/apachehttpd_test.go +++ b/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) }) } diff --git a/pkg/instrumentation/container.go b/pkg/instrumentation/container.go new file mode 100644 index 0000000000..75b9366079 --- /dev/null +++ b/pkg/instrumentation/container.go @@ -0,0 +1,336 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package instrumentation + +import ( + "context" + "fmt" + "reflect" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type Container struct { + client client.Reader + ctx context.Context + logger logr.Logger + namespace string + index int + inheritedEnv map[string]string + configMaps map[string]*corev1.ConfigMap +} + +func NewContainer(client client.Reader, ctx context.Context, logger logr.Logger, namespace string, pod *corev1.Pod, index int) (Container, error) { + if pod.Namespace != "" { + namespace = pod.Namespace + } + container := &pod.Spec.Containers[index] + + configMaps := make(map[string]*corev1.ConfigMap) + inheritedEnv := make(map[string]string) + for _, envsFrom := range container.EnvFrom { + if envsFrom.ConfigMapRef != nil { + prefix := envsFrom.Prefix + name := envsFrom.ConfigMapRef.Name + if cm, err := getOrLoadResource(client, ctx, namespace, configMaps, name); err == nil { + for k, v := range cm.Data { + // Safely overwrite the value, last one from EnvFrom wins in Kubernetes, with the direct value + // from the container itself taking precedence + inheritedEnv[prefix+k] = v + } + } else if envsFrom.ConfigMapRef.Optional == nil || !*envsFrom.ConfigMapRef.Optional { + return Container{}, fmt.Errorf("failed to load environment variables: %w", err) + } + } else if envsFrom.SecretRef != nil { + logger.V(2).Info("ignoring SecretRef in EnvFrom", "container", container.Name, "secret", envsFrom.SecretRef.Name) + } + } + + if len(inheritedEnv) == 0 { + inheritedEnv = nil + } + + return Container{ + client: client, + ctx: ctx, + logger: logger, + namespace: namespace, + index: index, + inheritedEnv: inheritedEnv, + configMaps: configMaps, + }, nil +} + +func getOrLoadResource[T any, PT interface { + client.Object + *T +}](client client.Reader, ctx context.Context, namespace string, cache map[string]*T, name string) (*T, error) { + var obj T + if cached, ok := cache[name]; ok { + if cached != nil { + return cached, nil + } else { + return nil, fmt.Errorf("failed to get %s %s/%s", reflect.TypeOf(obj).Name(), namespace, name) + } + } + + if client == nil || ctx == nil { + // Cache error value + cache[name] = nil + return nil, fmt.Errorf("client or context is nil, cannot load %s %s/%s", reflect.TypeOf(obj).Name(), namespace, name) + } + + err := client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, PT(&obj)) + if err != nil { + // Cache error value + cache[name] = nil + return nil, fmt.Errorf("failed to get %s %s/%s: %w", reflect.TypeOf(obj).Name(), namespace, name, err) + } + + cache[name] = &obj + return &obj, nil +} + +func (c *Container) validate(pod *corev1.Pod, envsToBeValidated ...string) error { + // Try if the value is resolvable + for _, envToBeValidated := range envsToBeValidated { + for _, envVar := range pod.Spec.Containers[c.index].Env { + if envVar.Name == envToBeValidated { + if _, err := c.resolveEnvVar(envVar); err != nil { + return err + } + break + } + } + } + return nil +} + +func getContainerIndex(pod *corev1.Pod, containerName string) int { + // We search for specific container to inject variables and if no one is found + // We fall back to the first container + var index = 0 + for idx, container := range pod.Spec.Containers { + if container.Name == containerName { + index = idx + } + } + + return index +} + +func (c *Container) exists(pod *corev1.Pod, name string) bool { + if found := existsEnvVarInEnv(pod.Spec.Containers[c.index].Env, name); found { + return found + } + if _, found := c.inheritedEnv[name]; found { + return found + } + return false +} + +func (c *Container) setOrAppendEnvVar(pod *corev1.Pod, envVar corev1.EnvVar) { + if idx, found := findEnvVarInEnv(pod.Spec.Containers[c.index].Env, envVar.Name); found { + pod.Spec.Containers[c.index].Env[idx] = envVar + } else { + c.appendEnvVar(pod, envVar) + } +} + +func (c *Container) getOrMakeEnvVar(pod *corev1.Pod, name string) (corev1.EnvVar, error) { + var envVar corev1.EnvVar + var idx int + var found bool + if idx, found = findEnvVarInEnv(pod.Spec.Containers[c.index].Env, name); found { + envVar = pod.Spec.Containers[c.index].Env[idx] + } else if envVar, found = getEnvVarFromMap(c.inheritedEnv, name); found { + // do nothing + } else { + envVar = corev1.EnvVar{Name: name} + } + return c.resolveEnvVar(envVar) +} + +func (c *Container) resolveEnvVar(envVar corev1.EnvVar) (corev1.EnvVar, error) { + if envVar.Value == "" && envVar.ValueFrom != nil { + if envVar.ValueFrom.ConfigMapKeyRef != nil { + configMapName := envVar.ValueFrom.ConfigMapKeyRef.Name + configMapKey := envVar.ValueFrom.ConfigMapKeyRef.Key + if cm, err := getOrLoadResource(c.client, c.ctx, c.namespace, c.configMaps, configMapName); err == nil { + if value, ok := cm.Data[configMapKey]; ok { + return corev1.EnvVar{Name: envVar.Name, Value: value}, nil + } else if envVar.ValueFrom.ConfigMapKeyRef.Optional == nil || !*envVar.ValueFrom.ConfigMapKeyRef.Optional { + return corev1.EnvVar{}, fmt.Errorf("failed to resolve environment variable %s, key %s not found in ConfigMap %s/%s", envVar.Name, configMapKey, c.namespace, configMapName) + } else { + return corev1.EnvVar{Name: envVar.Name, Value: ""}, nil + } + } else if envVar.ValueFrom.ConfigMapKeyRef.Optional == nil || !*envVar.ValueFrom.ConfigMapKeyRef.Optional { + return corev1.EnvVar{}, fmt.Errorf("failed to resolve environment variable %s: %w", envVar.Name, err) + } else { + return corev1.EnvVar{Name: envVar.Name, Value: ""}, nil + } + } else { + v := reflect.ValueOf(*envVar.ValueFrom) + for i := 0; i < v.NumField(); i++ { + if v.Field(i).Kind() == reflect.Ptr && !v.Field(i).IsNil() { + return corev1.EnvVar{}, fmt.Errorf("the container defines env var value via ValueFrom.%s, envVar: %s", v.Type().Field(i).Name, envVar.Name) + } + } + return corev1.EnvVar{}, fmt.Errorf("the container defines env var value via ValueFrom, envVar: %s", envVar.Name) + } + } + return envVar, nil +} + +func existsEnvVarInEnv(env []corev1.EnvVar, name string) bool { + for i := range env { + if env[i].Name == name { + return true + } + } + return false +} + +func findEnvVarInEnv(env []corev1.EnvVar, name string) (int, bool) { + for i := range env { + if env[i].Name == name { + return i, true + } + } + return -1, false +} + +func getEnvVarFromMap(env map[string]string, name string) (corev1.EnvVar, bool) { + if value, ok := env[name]; ok { + return corev1.EnvVar{Name: name, Value: value}, true + } + return corev1.EnvVar{}, false +} + +func (c *Container) prependEnvVar(pod *corev1.Pod, envVar corev1.EnvVar) { + pod.Spec.Containers[c.index].Env = append([]corev1.EnvVar{envVar}, pod.Spec.Containers[c.index].Env...) +} + +func (c *Container) prepend(pod *corev1.Pod, name string, value string) { + c.prependEnvVar(pod, corev1.EnvVar{Name: name, Value: value}) +} + +func (c *Container) appendEnvVar(pod *corev1.Pod, envVar corev1.EnvVar) { + pod.Spec.Containers[c.index].Env = append(pod.Spec.Containers[c.index].Env, envVar) +} + +func (c *Container) append(pod *corev1.Pod, name string, value string) { + c.appendEnvVar(pod, corev1.EnvVar{Name: name, Value: value}) +} + +func (c *Container) prependIfNotExists(pod *corev1.Pod, name string, value string) { + if !c.exists(pod, name) { + c.prepend(pod, name, value) + } +} + +func (c *Container) prependEnvVarIfNotExists(pod *corev1.Pod, envVar corev1.EnvVar) { + if !c.exists(pod, envVar.Name) { + c.prependEnvVar(pod, envVar) + } +} + +func (c *Container) appendIfNotExists(pod *corev1.Pod, name string, value string) { + if !c.exists(pod, name) { + c.append(pod, name, value) + } +} + +func (c *Container) appendEnvVarIfNotExists(pod *corev1.Pod, envVar corev1.EnvVar) { + if !c.exists(pod, envVar.Name) { + c.appendEnvVar(pod, envVar) + } +} + +//goland:noinspection SpellCheckingInspection +type Concatter interface { + Concat(vals ...string) string +} + +type ConcatFunc func(vals ...string) string + +func (f ConcatFunc) Concat(vals ...string) string { + return f(vals...) +} + +func (c *Container) appendOrConcat(pod *corev1.Pod, name string, value string, concatter Concatter) error { + if concatter == nil { + return fmt.Errorf("concatter is nil") + } + + if envVar, err := c.getOrMakeEnvVar(pod, name); err == nil { + envVar.Value = concatter.Concat(envVar.Value, value) + c.setOrAppendEnvVar(pod, envVar) + return nil + } else { + return err + } +} + +func (c *Container) moveToListEnd(pod *corev1.Pod, name string) { + if idx, ok := findEnvVarInEnv(pod.Spec.Containers[c.index].Env, name); ok { + envToMove := pod.Spec.Containers[c.index].Env[idx] + envs := append(pod.Spec.Containers[c.index].Env[:idx], pod.Spec.Containers[c.index].Env[idx+1:]...) + pod.Spec.Containers[c.index].Env = append(envs, envToMove) + } +} + +func concatWithCharacterChecked(val1, val2, char string) string { + if val1 != "" { + if val2 != "" { + if val1[len(val1)-1:] == char { + if val2[:1] == char { + return val1 + val2[1:] + } else { + return val1 + val2 + } + } else { + return val1 + char + val2 + } + } else { + return val1 + } + } else { + return val2 + } +} + +func concatWithCharacter(char string, vals ...string) string { + result := "" + for _, val := range vals { + result = concatWithCharacterChecked(result, val, char) + } + return result +} + +func concatWithSpace(vals ...string) string { + return concatWithCharacter(" ", vals...) +} + +func concatWithComma(vals ...string) string { + return concatWithCharacter(",", vals...) +} + +func concatWithColon(vals ...string) string { + return concatWithCharacter(":", vals...) +} diff --git a/pkg/instrumentation/container_test.go b/pkg/instrumentation/container_test.go new file mode 100644 index 0000000000..f512dbe8f5 --- /dev/null +++ b/pkg/instrumentation/container_test.go @@ -0,0 +1,1397 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package instrumentation + +import ( + "bytes" + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/go-logr/logr/funcr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func NewConfigMapRef(name string, prefix string, optional *bool) corev1.EnvFromSource { + return corev1.EnvFromSource{ + Prefix: prefix, + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: name, + }, + Optional: optional, + }, + } +} + +func NewSecretRef(name string, prefix string, optional *bool) corev1.EnvFromSource { + return corev1.EnvFromSource{ + Prefix: prefix, + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: name, + }, + Optional: optional, + }, + } +} + +func NewConfigMapKeyRef(name string, key string, optional *bool) *corev1.EnvVarSource { + return &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: name, + }, + Key: key, + Optional: optional, + }, + } +} + +func NewSecretKeyRef(name string, key string, optional *bool) *corev1.EnvVarSource { + return &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: name, + }, + Key: key, + Optional: optional, + }, + } +} + +func NewFieldRef(fieldPath string) *corev1.EnvVarSource { + return &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: fieldPath, + }, + } +} + +func NewResourceFieldRef(containerName string, resource string) *corev1.EnvVarSource { + return &corev1.EnvVarSource{ + ResourceFieldRef: &corev1.ResourceFieldSelector{ + ContainerName: containerName, + Resource: resource, + }, + } +} + +func NewStringLogger() (logr.Logger, *bytes.Buffer) { + var buf bytes.Buffer + logFunc := func(prefix, args string) { buf.WriteString(prefix + args + "\n") } + options := funcr.Options{Verbosity: 4} + logger := funcr.New(logFunc, options) + return logger, &buf +} + +func TestFindContainerByName(t *testing.T) { + tests := []struct { + name string + container string + pod corev1.Pod + expected int + }{ + { + name: "Container found by name", + container: "my-container", + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "my-container"}, + }, + }, + }, + expected: 0, + }, + { + name: "Containter found by name between multiple containers", + container: "my-container2", + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "my-container1"}, + {Name: "my-container2"}, + }, + }, + }, + expected: 1, + }, + { + name: "Default container returned", + container: "", + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "my-container1"}, + {Name: "my-container2"}, + }, + }, + }, + expected: 0, + }, + { + name: "No matching container found, default returned", + container: "no-match", + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "my-container1"}, + {Name: "my-container2"}, + }, + }, + }, + expected: 0, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + pod := test.pod + index := getContainerIndex(&pod, test.container) + assert.Equal(t, test.expected, index) + }) + } +} + +func TestInheritedEnv(t *testing.T) { + true := true + false := false + + tests := []struct { + name string + ns corev1.Namespace + cm []corev1.ConfigMap + secret []corev1.Secret + pod corev1.Pod + err string + log string + expected map[string]string + }{ + { + name: "No ConfigMap usage", + ns: corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "inheritedenv-noconfigmap", + }, + }, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-pod", + Namespace: "inheritedenv-noconfigmapv", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "app"}}, + }, + }, + }, + { + name: "Simple ConfigMap usage", + ns: corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "inheritedenv-simple", + }, + }, + cm: []corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-config", + Namespace: "inheritedenv-simple", + }, + Data: map[string]string{ + "OTEL_SERVICE_NAME": "my-service", + "OTEL_TRACES_SAMPLER_ARG": "0.85", + }, + }, + }, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-pod", + Namespace: "inheritedenv-simple", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + EnvFrom: []corev1.EnvFromSource{NewConfigMapRef("my-config", "", nil)}, + }, + }, + }, + }, + expected: map[string]string{ + "OTEL_SERVICE_NAME": "my-service", + "OTEL_TRACES_SAMPLER_ARG": "0.85", + }, + }, + { + name: "Multiple ConfigMap usage with overriding", + ns: corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "inheritedenv-multiple", + }, + }, + cm: []corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-config", + Namespace: "inheritedenv-multiple", + }, + Data: map[string]string{ + "OTEL_SERVICE_NAME": "my-service", + "OTEL_TRACES_SAMPLER": "parentbased_traceidratio", + "OTEL_TRACES_SAMPLER_ARG": "0.85", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-another-config", + Namespace: "inheritedenv-multiple", + }, + Data: map[string]string{ + "OTEL_TRACES_SAMPLER_ARG": "0.95", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-prefixed-config", + Namespace: "inheritedenv-multiple", + }, + Data: map[string]string{ + "EXPORTER_OTLP_TIMEOUT": "20", + }, + }, + }, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-pod", + Namespace: "inheritedenv-multiple", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + EnvFrom: []corev1.EnvFromSource{ + NewConfigMapRef("my-config", "", nil), + NewConfigMapRef("my-another-config", "", nil), + NewConfigMapRef("my-prefixed-config", "OTEL_", nil), + }, + }, + }, + }, + }, + expected: map[string]string{ + "OTEL_SERVICE_NAME": "my-service", + "OTEL_TRACES_SAMPLER": "parentbased_traceidratio", + "OTEL_TRACES_SAMPLER_ARG": "0.95", + "OTEL_EXPORTER_OTLP_TIMEOUT": "20", + }, + }, + { + name: "Optional ConfigMap not found", + ns: corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "inheritedenv-notfoundoptional", + }, + }, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-pod", + Namespace: "inheritedenv-notfoundoptional", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + EnvFrom: []corev1.EnvFromSource{ + NewConfigMapRef("my-config", "", &true), + }, + }, + }, + }, + }, + }, + { + name: "Implicitly mandatory ConfigMap not found", + ns: corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "inheritedenv-notfoundimplicit", + }, + }, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-pod", + Namespace: "inheritedenv-notfoundimplicit", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + EnvFrom: []corev1.EnvFromSource{ + NewConfigMapRef("my-config", "", nil), + }, + }, + }, + }, + }, + err: "failed to get ConfigMap inheritedenv-notfoundimplicit/my-config", + }, + { + name: "Explicitly mandatory ConfigMap not found", + ns: corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "inheritedenv-notfoundexplicit", + }, + }, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-pod", + Namespace: "inheritedenv-notfoundexplicit", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + EnvFrom: []corev1.EnvFromSource{ + NewConfigMapRef("my-config", "", &false), + }, + }, + }, + }, + }, + err: "failed to get ConfigMap inheritedenv-notfoundexplicit/my-config", + }, + { + name: "SecretRef not supported", + ns: corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "inheritedenv-secretref", + }, + }, + secret: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-secret", + Namespace: "inheritedenv-secretref", + }, + Data: map[string][]byte{ + "OTEL_ENVFROM_SECRET_VALUE1": []byte("my-secret-value1"), + }, + }, + }, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-pod", + Namespace: "inheritedenv-secretref", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + EnvFrom: []corev1.EnvFromSource{ + NewSecretRef("my-secret", "", nil), + }, + }, + }, + }, + }, + log: "ignoring SecretRef in EnvFrom", + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + err := k8sClient.Create(context.Background(), &test.ns) + require.NoError(t, err) + defer func() { + _ = k8sClient.Delete(context.Background(), &test.ns) + }() + + for _, cm := range test.cm { + cm := cm + err = k8sClient.Create(context.Background(), &cm) + require.NoError(t, err) + //goland:noinspection GoDeferInLoop + defer func() { + _ = k8sClient.Delete(context.Background(), &cm) + }() + } + for _, secret := range test.secret { + secret := secret + err = k8sClient.Create(context.Background(), &secret) + require.NoError(t, err) + //goland:noinspection GoDeferInLoop + defer func() { + _ = k8sClient.Delete(context.Background(), &secret) + }() + } + + pod := test.pod + logger, buf := NewStringLogger() + container, err := NewContainer(k8sClient, context.Background(), logger, test.ns.Name, &pod, 0) + if test.err == "" { + assert.NoError(t, err) + assert.Equal(t, test.expected, container.inheritedEnv) + } else { + require.Error(t, err) + assert.Contains(t, err.Error(), test.err) + } + if test.log != "" { + assert.Contains(t, buf.String(), test.log) + } + }) + } +} + +type ModificationTester interface { + Test(pod *corev1.Pod, c Container) +} + +type ModificationTestFunc func(pod *corev1.Pod, c Container) + +func (f ModificationTestFunc) Test(pod *corev1.Pod, c Container) { + f(pod, c) +} + +var _ ModificationTester = ModificationTestFunc(nil) + +func TestModifications(t *testing.T) { + testNs := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "modifications", + }, + } + testCm := []corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-config", + Namespace: "modifications", + }, + Data: map[string]string{ + "OTEL_ENVFROM_VALUE1": "my-envfrom-value1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-refconfig", + Namespace: "modifications", + }, + Data: map[string]string{ + "ref-value1": "my-valuefrom-value1", + }, + }, + } + testSecret := []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-secret", + Namespace: "modifications", + }, + Data: map[string][]byte{ + "OTEL_ENVFROM_SECRET_VALUE1": []byte("my-secret-value1"), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-refsecret", + Namespace: "modifications", + }, + Data: map[string][]byte{ + "secret-ref-value1": []byte("my-valuefrom-value1"), + }, + }, + } + testPod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-pod", + Namespace: "modifications", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + EnvFrom: []corev1.EnvFromSource{ + { + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "my-config", + }, + }, + }, + }, + Env: []corev1.EnvVar{ + { + Name: "OTEL_ENV_VALUE1", + Value: "my-env-value1", + }, + { + Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", + ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil), + }, + { + Name: "OTEL_ENV_VALUEFROM_SECRET1", + ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil), + }, + }, + }, + }, + }, + } + + tests := []struct { + name string + pod *corev1.Pod + tester ModificationTester + expected []corev1.EnvVar + }{ + { + name: "Test prepend", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.prepend(pod, "OTEL_ENV_VALUE2", "my-env-value2") + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}, + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test prependEnvVar", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.prependEnvVar(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}, + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test append", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.append(pod, "OTEL_ENV_VALUE2", "my-env-value2") + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + {Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}, + }, + }, + { + name: "Test appendEnvVar", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.appendEnvVar(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + {Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}, + }, + }, + { + name: "Test prependIfNotExists when env var does not exist", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.prependIfNotExists(pod, "OTEL_ENV_VALUE2", "my-env-value2") + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}, + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test prependIfNotExists when env var exists", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.prependIfNotExists(pod, "OTEL_ENV_VALUE1", "my-overridden-value1") + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test prependEnvVarIfNotExists when env var does not exist", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.prependEnvVarIfNotExists(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}, + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test prependEnvVarIfNotExists when env var exists", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.prependEnvVarIfNotExists(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUE1", Value: "my-overridden-value1"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test appendIfNotExists when env var does not exist", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.appendIfNotExists(pod, "OTEL_ENV_VALUE2", "my-env-value2") + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + {Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}, + }, + }, + { + name: "Test appendIfNotExists when env var exists", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.appendIfNotExists(pod, "OTEL_ENV_VALUE1", "my-overridden-value1") + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test appendEnvVarIfNotExists when env var does not exist", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.appendEnvVarIfNotExists(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + {Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}, + }, + }, + { + name: "Test appendEnvVarIfNotExists when env var exists", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.appendEnvVarIfNotExists(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUE1", Value: "my-overridden-value1"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test setOrAppendEnvVar when env var does not exist", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.setOrAppendEnvVar(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + {Name: "OTEL_ENV_VALUE2", Value: "my-env-value2"}, + }, + }, + { + name: "Test setOrAppendEnvVar when env var exists in Env", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.setOrAppendEnvVar(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUE1", Value: "my-overridden-value1"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-overridden-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test setOrAppendEnvVar when env var exists as ConfigMapKeyRef", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.setOrAppendEnvVar(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", Value: "my-overridden-value1"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", Value: "my-overridden-value1"}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + { + name: "Test setOrAppendEnvVar on existing unsupported env var", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.setOrAppendEnvVar(pod, corev1.EnvVar{Name: "OTEL_ENV_VALUEFROM_SECRET1", Value: "my-overridden-value1"}) + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", Value: "my-overridden-value1"}, + }, + }, + { + name: "Test moveToListEnd when env var exists", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.moveToListEnd(pod, "OTEL_ENV_VALUE1") + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + }, + }, + { + name: "Test moveToListEnd when env var does not exist", + pod: &testPod, + tester: ModificationTestFunc(func(pod *corev1.Pod, c Container) { + c.moveToListEnd(pod, "OTEL_ENV_VALUE2") + }), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE1", Value: "my-env-value1"}, + {Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil)}, + {Name: "OTEL_ENV_VALUEFROM_SECRET1", ValueFrom: NewSecretKeyRef("my-secret-refconfig", "secret-ref-value1", nil)}, + }, + }, + } + + err := k8sClient.Create(context.Background(), &testNs) + require.NoError(t, err) + defer func() { + _ = k8sClient.Delete(context.Background(), &testNs) + }() + + for _, cm := range testCm { + cm := cm + err = k8sClient.Create(context.Background(), &cm) + require.NoError(t, err) + //goland:noinspection GoDeferInLoop + defer func() { + _ = k8sClient.Delete(context.Background(), &cm) + }() + } + for _, secret := range testSecret { + secret := secret + err = k8sClient.Create(context.Background(), &secret) + require.NoError(t, err) + //goland:noinspection GoDeferInLoop + defer func() { + _ = k8sClient.Delete(context.Background(), &secret) + }() + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + pod := test.pod.DeepCopy() + + container, err := NewContainer(k8sClient, context.Background(), logr.Discard(), testNs.Name, pod, 0) + require.NoError(t, err) + test.tester.Test(pod, container) + assert.Equal(t, test.expected, pod.Spec.Containers[0].Env) + }) + } +} + +type LoadConfigMapTester interface { + Test(client client.Reader, ctx context.Context, configMaps map[string]*corev1.ConfigMap) (*corev1.ConfigMap, error) +} + +type LoadConfigMapTestFunc func(client client.Reader, ctx context.Context, configMaps map[string]*corev1.ConfigMap) (*corev1.ConfigMap, error) + +func (f LoadConfigMapTestFunc) Test(client client.Reader, ctx context.Context, configMaps map[string]*corev1.ConfigMap) (*corev1.ConfigMap, error) { + return f(client, ctx, configMaps) +} + +var _ LoadConfigMapTester = LoadConfigMapTestFunc(nil) + +func TestLoadConfigMap(t *testing.T) { + testNs := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "loadconfigmap", + }, + } + testCm := []corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-config", + Namespace: "loadconfigmap", + }, + Data: map[string]string{ + "OTEL_ENVFROM_VALUE1": "my-envfrom-value1", + }, + }, + } + + tests := []struct { + name string + tester LoadConfigMapTester + err string + expected map[string]map[string]string + }{ + { + name: "Test loading of ConfigMap", + tester: LoadConfigMapTestFunc(func(client client.Reader, ctx context.Context, configMaps map[string]*corev1.ConfigMap) (*corev1.ConfigMap, error) { + return getOrLoadResource(client, ctx, "loadconfigmap", configMaps, "my-config") + }), + expected: map[string]map[string]string{ + "my-config": {"OTEL_ENVFROM_VALUE1": "my-envfrom-value1"}, + }, + }, + { + name: "Test cached loading of ConfigMap", + tester: LoadConfigMapTestFunc(func(client client.Reader, ctx context.Context, configMaps map[string]*corev1.ConfigMap) (*corev1.ConfigMap, error) { + _, _ = getOrLoadResource(client, ctx, "loadconfigmap", configMaps, "my-config") + return getOrLoadResource(nil, nil, "loadconfigmap", configMaps, "my-config") + }), + expected: map[string]map[string]string{ + "my-config": {"OTEL_ENVFROM_VALUE1": "my-envfrom-value1"}, + }, + }, + { + name: "Test missing ConfigMap", + tester: LoadConfigMapTestFunc(func(client client.Reader, ctx context.Context, configMaps map[string]*corev1.ConfigMap) (*corev1.ConfigMap, error) { + return getOrLoadResource(client, ctx, "loadconfigmap", configMaps, "nonexisting-config") + }), + err: "failed to get ConfigMap loadconfigmap/nonexisting-config: ", + expected: map[string]map[string]string{ + "nonexisting-config": nil, + }, + }, + { + name: "Test cached missing ConfigMap", + tester: LoadConfigMapTestFunc(func(client client.Reader, ctx context.Context, configMaps map[string]*corev1.ConfigMap) (*corev1.ConfigMap, error) { + _, _ = getOrLoadResource(client, ctx, "loadconfigmap", configMaps, "nonexisting-config") + return getOrLoadResource(nil, nil, "loadconfigmap", configMaps, "nonexisting-config") + }), + err: "failed to get ConfigMap loadconfigmap/nonexisting-config", + expected: map[string]map[string]string{ + "nonexisting-config": nil, + }, + }, + { + name: "Test unconfigured", + tester: LoadConfigMapTestFunc(func(client client.Reader, ctx context.Context, configMaps map[string]*corev1.ConfigMap) (*corev1.ConfigMap, error) { + return getOrLoadResource(nil, nil, "loadconfigmap", configMaps, "my-config") + }), + err: "cannot load ConfigMap loadconfigmap/my-config", + expected: map[string]map[string]string{ + "my-config": nil, + }, + }, + } + + err := k8sClient.Create(context.Background(), &testNs) + require.NoError(t, err) + defer func() { + _ = k8sClient.Delete(context.Background(), &testNs) + }() + + for _, cm := range testCm { + cm := cm + err = k8sClient.Create(context.Background(), &cm) + require.NoError(t, err) + //goland:noinspection GoDeferInLoop + defer func() { + _ = k8sClient.Delete(context.Background(), &cm) + }() + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + maps := map[string]*corev1.ConfigMap{} + cm, err := test.tester.Test(k8sClient, context.Background(), maps) + + if test.err == "" { + assert.NoError(t, err) + assert.Equal(t, testCm[0].Data, cm.Data) + } else { + if assert.Error(t, err) { + assert.Contains(t, err.Error(), test.err) + } + } + + if test.expected == nil { + assert.Nil(t, cm) + } else { + for key, value := range test.expected { + cm, ok := maps[key] + assert.True(t, ok) + if value == nil { + assert.Nil(t, cm) + } else { + if assert.NotNil(t, cm) { + assert.Equal(t, value, cm.Data) + } + } + } + assert.Len(t, maps, len(test.expected)) + } + }) + } +} + +func TestResolving(t *testing.T) { + true := true + false := false + + testNs := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "validations", + }, + } + testCm := []corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-config", + Namespace: "validations", + }, + Data: map[string]string{ + "OTEL_ENVFROM_VALUE1": "my-envfrom-value1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-refconfig", + Namespace: "validations", + }, + Data: map[string]string{ + "ref-value1": "my-valuefrom-value1", + }, + }, + } + testSecret := []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-secret", + Namespace: "validations", + }, + Data: map[string][]byte{ + "OTEL_ENVFROM_SECRET_VALUE1": []byte("my-secret-value1"), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "my-refsecret", + Namespace: "validations", + }, + Data: map[string][]byte{ + "secret-ref-value1": []byte("my-valuefrom-value1"), + }, + }, + } + testPod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-pod", + Namespace: "validations", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + EnvFrom: []corev1.EnvFromSource{ + NewConfigMapRef("my-config", "", nil), + NewSecretRef("my-secret", "", nil), + }, + Env: []corev1.EnvVar{ + { + Name: "OTEL_ENV_VALUE1", + Value: "my-env-value1", + }, + { + Name: "OTEL_ENV_VALUEFROM_CONFIGMAP1", + ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value1", nil), + }, + { + Name: "OTEL_ENV_VALUEFROM_CONFIGMAP2", + ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value-nonexisting", nil), + }, + { + Name: "OTEL_ENV_VALUEFROM_CONFIGMAP3", + ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value-nonexisting", &false), + }, + { + Name: "OTEL_ENV_VALUEFROM_CONFIGMAP4", + ValueFrom: NewConfigMapKeyRef("my-refconfig", "ref-value-nonexisting", &true), + }, + { + Name: "OTEL_ENV_VALUEFROM_CONFIGMAP5", + ValueFrom: NewConfigMapKeyRef("my-refconfig-nonexisting", "ref-value1", nil), + }, + { + Name: "OTEL_ENV_VALUEFROM_CONFIGMAP6", + ValueFrom: NewConfigMapKeyRef("my-refconfig-nonexisting", "ref-value1", &false), + }, + { + Name: "OTEL_ENV_VALUEFROM_CONFIGMAP7", + ValueFrom: NewConfigMapKeyRef("my-refconfig-nonexisting", "ref-value1", &true), + }, + { + Name: "OTEL_ENV_VALUEFROM_SECRET1", + ValueFrom: NewSecretKeyRef("my-refsecret", "secret-ref-value1", nil), + }, + { + Name: "OTEL_ENV_VALUEFROM_FIELD1", + ValueFrom: NewFieldRef("spec.nodeName"), + }, + { + Name: "OTEL_ENV_VALUEFROM_RESOURCEFIELD1", + ValueFrom: NewResourceFieldRef("app", "limits.cpu"), + }, + { + Name: "OTEL_ENV_VALUEFROM_INVALID", + ValueFrom: &corev1.EnvVarSource{}, + }, + }, + }, + }, + }, + } + + tests := []struct { + name string + variable string + err string + expectedExists bool + expectedResolve string + }{ + { + name: "Test existing variable", + variable: "OTEL_ENV_VALUE1", + expectedExists: true, + expectedResolve: "my-env-value1", + }, + { + name: "Test non-existing variable", + variable: "OTEL_ENV_VALUE_NONEXISTING", + expectedExists: false, + expectedResolve: "", + }, + { + name: "Test existing ConfigMap variable", + variable: "OTEL_ENVFROM_VALUE1", + expectedExists: true, + expectedResolve: "my-envfrom-value1", + }, + { + name: "Test existing ConfigMapKeyRef variable", + variable: "OTEL_ENV_VALUEFROM_CONFIGMAP1", + expectedExists: true, + expectedResolve: "my-valuefrom-value1", + }, + { + name: "Test implicitly mandatory non-existing ConfigMapKeyRef variable", + variable: "OTEL_ENV_VALUEFROM_CONFIGMAP2", + expectedExists: true, + err: "failed to resolve environment variable OTEL_ENV_VALUEFROM_CONFIGMAP2, key ref-value-nonexisting not found in ConfigMap validations/my-refconfig", + }, + { + name: "Test explicitly mandatory non-existing ConfigMapKeyRef variable", + variable: "OTEL_ENV_VALUEFROM_CONFIGMAP3", + expectedExists: true, + err: "failed to resolve environment variable OTEL_ENV_VALUEFROM_CONFIGMAP3, key ref-value-nonexisting not found in ConfigMap validations/my-refconfig", + }, + { + name: "Test optional non-existing ConfigMapKeyRef variable", + variable: "OTEL_ENV_VALUEFROM_CONFIGMAP4", + expectedExists: true, + expectedResolve: "", + }, + { + name: "Test implicitly mandatory variable of non-existing ConfigMap", + variable: "OTEL_ENV_VALUEFROM_CONFIGMAP5", + expectedExists: true, + err: "failed to resolve environment variable OTEL_ENV_VALUEFROM_CONFIGMAP5: ", + }, + { + name: "Test explicitly mandatory variable of non-existing ConfigMap", + variable: "OTEL_ENV_VALUEFROM_CONFIGMAP6", + expectedExists: true, + err: "failed to resolve environment variable OTEL_ENV_VALUEFROM_CONFIGMAP6: ", + }, + { + name: "Test optional variable of non-existing ConfigMap", + variable: "OTEL_ENV_VALUEFROM_CONFIGMAP7", + expectedExists: true, + expectedResolve: "", + }, + { + name: "Test unsupported existing Secret variable", + variable: "OTEL_ENV_VALUEFROM_SECRET1", + expectedExists: true, + err: "the container defines env var value via ValueFrom.SecretKeyRef, envVar: OTEL_ENV_VALUEFROM_SECRET1", + }, + { + name: "Test unsupported existing FieldRef variable", + variable: "OTEL_ENV_VALUEFROM_FIELD1", + expectedExists: true, + err: "the container defines env var value via ValueFrom.FieldRef, envVar: OTEL_ENV_VALUEFROM_FIELD1", + }, + { + name: "Test unsupported existing ResourceFieldRef variable", + variable: "OTEL_ENV_VALUEFROM_RESOURCEFIELD1", + expectedExists: true, + err: "the container defines env var value via ValueFrom.ResourceFieldRef, envVar: OTEL_ENV_VALUEFROM_RESOURCEFIELD1", + }, + { + name: "Test invalid variable", + variable: "OTEL_ENV_VALUEFROM_INVALID", + expectedExists: true, + err: "the container defines env var value via ValueFrom, envVar: OTEL_ENV_VALUEFROM_INVALID", + }, + } + + err := k8sClient.Create(context.Background(), &testNs) + require.NoError(t, err) + defer func() { + _ = k8sClient.Delete(context.Background(), &testNs) + }() + + for _, cm := range testCm { + cm := cm + err = k8sClient.Create(context.Background(), &cm) + require.NoError(t, err) + //goland:noinspection GoDeferInLoop + defer func() { + _ = k8sClient.Delete(context.Background(), &cm) + }() + } + for _, secret := range testSecret { + secret := secret + err = k8sClient.Create(context.Background(), &secret) + require.NoError(t, err) + //goland:noinspection GoDeferInLoop + defer func() { + _ = k8sClient.Delete(context.Background(), &secret) + }() + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + c, err := NewContainer(k8sClient, context.Background(), logr.Discard(), testNs.Name, &testPod, 0) + require.NoError(t, err) + + exists := c.exists(&testPod, test.variable) + errValidate := c.validate(&testPod, test.variable) + resolved, errGet := c.getOrMakeEnvVar(&testPod, test.variable) + + assert.Equal(t, test.expectedExists, exists) + + if test.err == "" { + assert.NoError(t, errValidate) + assert.NoError(t, errGet) + assert.Equal(t, test.variable, resolved.Name) + assert.Equal(t, test.expectedResolve, resolved.Value) + } else { + if assert.Error(t, errValidate) { + assert.Contains(t, errValidate.Error(), test.err) + } + if assert.Error(t, errGet) { + assert.Contains(t, errGet.Error(), test.err) + } + } + }) + } +} + +func TestConcatenations(t *testing.T) { + testNs := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "concatenations", + }, + } + testPod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-pod", + Namespace: "concatenations", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + Env: []corev1.EnvVar{ + { + Name: "OTEL_ENV_VALUE", + Value: "my-env-value", + }, + { + Name: "OTEL_ENV_COLON", + Value: "my-env-value:", + }, + { + Name: "OTEL_ENV_INVALID", + ValueFrom: NewConfigMapKeyRef("non-existing", "some-key", nil), + }, + }, + }, + }, + }, + } + + //goland:noinspection SpellCheckingInspection + tests := []struct { + name string + variable string + value string + concatter Concatter + err string + expected []corev1.EnvVar + }{ + { + name: "Test concatenation on non-existing variable", + variable: "OTEL_ENV_NEW", + value: "added", + concatter: ConcatFunc(concatWithColon), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE", Value: "my-env-value"}, + {Name: "OTEL_ENV_COLON", Value: "my-env-value:"}, + {Name: "OTEL_ENV_INVALID", ValueFrom: NewConfigMapKeyRef("non-existing", "some-key", nil)}, + {Name: "OTEL_ENV_NEW", Value: "added"}, + }, + }, + { + name: "Test concatenation with colon", + variable: "OTEL_ENV_VALUE", + value: "added", + concatter: ConcatFunc(concatWithColon), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE", Value: "my-env-value:added"}, + {Name: "OTEL_ENV_COLON", Value: "my-env-value:"}, + {Name: "OTEL_ENV_INVALID", ValueFrom: NewConfigMapKeyRef("non-existing", "some-key", nil)}, + }, + }, + { + name: "Test concatenation of empty value", + variable: "OTEL_ENV_VALUE", + value: "", + concatter: ConcatFunc(concatWithColon), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE", Value: "my-env-value"}, + {Name: "OTEL_ENV_COLON", Value: "my-env-value:"}, + {Name: "OTEL_ENV_INVALID", ValueFrom: NewConfigMapKeyRef("non-existing", "some-key", nil)}, + }, + }, + { + name: "Test concatenation not adding redundant colon", + variable: "OTEL_ENV_COLON", + value: "added", + concatter: ConcatFunc(concatWithColon), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE", Value: "my-env-value"}, + {Name: "OTEL_ENV_COLON", Value: "my-env-value:added"}, + {Name: "OTEL_ENV_INVALID", ValueFrom: NewConfigMapKeyRef("non-existing", "some-key", nil)}, + }, + }, + { + name: "Test concatenation not adding redundant colon on both sides", + variable: "OTEL_ENV_COLON", + value: ":added", + concatter: ConcatFunc(concatWithColon), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE", Value: "my-env-value"}, + {Name: "OTEL_ENV_COLON", Value: "my-env-value:added"}, + {Name: "OTEL_ENV_INVALID", ValueFrom: NewConfigMapKeyRef("non-existing", "some-key", nil)}, + }, + }, + { + name: "Test concatenation with comma", + variable: "OTEL_ENV_VALUE", + value: "added", + concatter: ConcatFunc(concatWithComma), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE", Value: "my-env-value,added"}, + {Name: "OTEL_ENV_COLON", Value: "my-env-value:"}, + {Name: "OTEL_ENV_INVALID", ValueFrom: NewConfigMapKeyRef("non-existing", "some-key", nil)}, + }, + }, + { + name: "Test concatenation with space", + variable: "OTEL_ENV_VALUE", + value: "added", + concatter: ConcatFunc(concatWithSpace), + expected: []corev1.EnvVar{ + {Name: "OTEL_ENV_VALUE", Value: "my-env-value added"}, + {Name: "OTEL_ENV_COLON", Value: "my-env-value:"}, + {Name: "OTEL_ENV_INVALID", ValueFrom: NewConfigMapKeyRef("non-existing", "some-key", nil)}, + }, + }, + { + name: "Test concatenation with non-resolvable variable", + variable: "OTEL_ENV_INVALID", + value: "added", + concatter: ConcatFunc(concatWithColon), + err: "failed to resolve environment variable OTEL_ENV_INVALID: ", + }, + { + name: "Test concatenation with nil concatter", + variable: "OTEL_ENV_VALUE", + value: "added", + concatter: nil, + err: "concatter is nil", + }, + } + + err := k8sClient.Create(context.Background(), &testNs) + require.NoError(t, err) + defer func() { + _ = k8sClient.Delete(context.Background(), &testNs) + }() + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + pod := testPod.DeepCopy() + c, err := NewContainer(k8sClient, context.Background(), logr.Discard(), testNs.Name, pod, 0) + require.NoError(t, err) + + err = c.appendOrConcat(pod, test.variable, test.value, test.concatter) + + if test.err == "" { + assert.NoError(t, err) + assert.Equal(t, test.expected, pod.Spec.Containers[0].Env) + } else { + if assert.Error(t, err) { + assert.Contains(t, err.Error(), test.err) + } + } + }) + } +} diff --git a/pkg/instrumentation/dotnet.go b/pkg/instrumentation/dotnet.go index 74f68744ac..01ede135a2 100644 --- a/pkg/instrumentation/dotnet.go +++ b/pkg/instrumentation/dotnet.go @@ -50,27 +50,24 @@ const ( dotNetRuntimeLinuxMusl = "linux-musl-x64" ) -func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, index int, runtime string) (corev1.Pod, error) { +func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, container Container, runtime string) (corev1.Pod, error) { volume := instrVolume(dotNetSpec.VolumeClaimTemplate, dotnetVolumeName, dotNetSpec.VolumeSizeLimit) - // caller checks if there is at least one container. - container := &pod.Spec.Containers[index] - - err := validateContainerEnv(container.Env, envDotNetStartupHook, envDotNetAdditionalDeps, envDotNetSharedStore) + err := container.validate(&pod, envDotNetStartupHook, envDotNetAdditionalDeps, envDotNetSharedStore) if err != nil { return pod, err } // check if OTEL_DOTNET_AUTO_HOME env var is already set in the container // if it is already set, then we assume that .NET Auto-instrumentation is already configured for this container - if getIndexOfEnv(container.Env, envDotNetOTelAutoHome) > -1 { + if container.exists(&pod, envDotNetOTelAutoHome) { return pod, errors.New("OTEL_DOTNET_AUTO_HOME environment variable is already set in the container") } // check if OTEL_DOTNET_AUTO_HOME env var is already set in the .NET instrumentation spec // if it is already set, then we assume that .NET Auto-instrumentation is already configured for this container - if getIndexOfEnv(dotNetSpec.Env, envDotNetOTelAutoHome) > -1 { + if existsEnvVarInEnv(dotNetSpec.Env, envDotNetOTelAutoHome) { return pod, errors.New("OTEL_DOTNET_AUTO_HOME environment variable is already set in the .NET instrumentation spec") } @@ -86,10 +83,7 @@ func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, index int, runt // inject .NET instrumentation spec env vars. for _, env := range dotNetSpec.Env { - idx := getIndexOfEnv(container.Env, env.Name) - if idx == -1 { - container.Env = append(container.Env, env) - } + container.appendEnvVarIfNotExists(&pod, env) } const ( @@ -97,21 +91,35 @@ func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, index int, runt concatEnvValues = true ) - setDotNetEnvVar(container, envDotNetCoreClrEnableProfiling, dotNetCoreClrEnableProfilingEnabled, doNotConcatEnvValues) + if err := setDotNetEnvVar(pod, container, envDotNetCoreClrEnableProfiling, dotNetCoreClrEnableProfilingEnabled, doNotConcatEnvValues); err != nil { + return pod, err + } - setDotNetEnvVar(container, envDotNetCoreClrProfiler, dotNetCoreClrProfilerID, doNotConcatEnvValues) + if err := setDotNetEnvVar(pod, container, envDotNetCoreClrProfiler, dotNetCoreClrProfilerID, doNotConcatEnvValues); err != nil { + return pod, err + } - setDotNetEnvVar(container, envDotNetCoreClrProfilerPath, coreClrProfilerPath, doNotConcatEnvValues) + if err := setDotNetEnvVar(pod, container, envDotNetCoreClrProfilerPath, coreClrProfilerPath, doNotConcatEnvValues); err != nil { + return pod, err + } - setDotNetEnvVar(container, envDotNetStartupHook, dotNetStartupHookPath, concatEnvValues) + if err := setDotNetEnvVar(pod, container, envDotNetStartupHook, dotNetStartupHookPath, concatEnvValues); err != nil { + return pod, err + } - setDotNetEnvVar(container, envDotNetAdditionalDeps, dotNetAdditionalDepsPath, concatEnvValues) + if err := setDotNetEnvVar(pod, container, envDotNetAdditionalDeps, dotNetAdditionalDepsPath, concatEnvValues); err != nil { + return pod, err + } - setDotNetEnvVar(container, envDotNetOTelAutoHome, dotNetOTelAutoHomePath, doNotConcatEnvValues) + if err := setDotNetEnvVar(pod, container, envDotNetOTelAutoHome, dotNetOTelAutoHomePath, doNotConcatEnvValues); err != nil { + return pod, err + } - setDotNetEnvVar(container, envDotNetSharedStore, dotNetSharedStorePath, concatEnvValues) + if err := setDotNetEnvVar(pod, container, envDotNetSharedStore, dotNetSharedStorePath, concatEnvValues); err != nil { + return pod, err + } - container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{ Name: volume.Name, MountPath: dotnetInstrMountPath, }) @@ -136,16 +144,11 @@ func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, index int, runt // setDotNetEnvVar function sets env var to the container if not exist already. // value of concatValues should be set to true if the env var supports multiple values separated by :. // If it is set to false, the original container's env var value has priority. -func setDotNetEnvVar(container *corev1.Container, envVarName string, envVarValue string, concatValues bool) { - idx := getIndexOfEnv(container.Env, envVarName) - if idx < 0 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: envVarName, - Value: envVarValue, - }) - return - } +func setDotNetEnvVar(pod corev1.Pod, container Container, envVarName string, envVarValue string, concatValues bool) error { if concatValues { - container.Env[idx].Value = fmt.Sprintf("%s:%s", container.Env[idx].Value, envVarValue) + return container.appendOrConcat(&pod, envVarName, envVarValue, ConcatFunc(concatWithColon)) + } else { + container.appendIfNotExists(&pod, envVarName, envVarValue) + return nil } } diff --git a/pkg/instrumentation/dotnet_test.go b/pkg/instrumentation/dotnet_test.go index 23d2924208..2a212d20b1 100644 --- a/pkg/instrumentation/dotnet_test.go +++ b/pkg/instrumentation/dotnet_test.go @@ -15,10 +15,13 @@ package instrumentation import ( + "context" "fmt" "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -550,7 +553,10 @@ func TestInjectDotNetSDK(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - pod, err := injectDotNetSDK(test.DotNet, test.pod, 0, test.runtime) + pod := test.pod + container, err := NewContainer(k8sClient, context.Background(), logr.Discard(), "", &pod, 0) + require.NoError(t, err) + pod, err = injectDotNetSDK(test.DotNet, pod, container, test.runtime) assert.Equal(t, test.expected, pod) assert.Equal(t, test.err, err) }) diff --git a/pkg/instrumentation/exporter.go b/pkg/instrumentation/exporter.go index 5598de24cf..f2eaff5b07 100644 --- a/pkg/instrumentation/exporter.go +++ b/pkg/instrumentation/exporter.go @@ -25,14 +25,9 @@ import ( "github.com/open-telemetry/opentelemetry-operator/pkg/constants" ) -func configureExporter(exporter v1alpha1.Exporter, pod *corev1.Pod, container *corev1.Container) { +func configureExporter(exporter v1alpha1.Exporter, pod *corev1.Pod, container Container) { if exporter.Endpoint != "" { - if getIndexOfEnv(container.Env, constants.EnvOTELExporterOTLPEndpoint) == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: constants.EnvOTELExporterOTLPEndpoint, - Value: exporter.Endpoint, - }) - } + container.appendIfNotExists(pod, constants.EnvOTELExporterOTLPEndpoint, exporter.Endpoint) } if exporter.TLS == nil { return @@ -52,36 +47,21 @@ func configureExporter(exporter v1alpha1.Exporter, pod *corev1.Pod, container *c if filepath.IsAbs(exporter.TLS.CA) { envVarVal = exporter.TLS.CA } - if getIndexOfEnv(container.Env, constants.EnvOTELExporterCertificate) == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: constants.EnvOTELExporterCertificate, - Value: envVarVal, - }) - } + container.appendIfNotExists(pod, constants.EnvOTELExporterCertificate, envVarVal) } if exporter.TLS.Cert != "" { envVarVal := fmt.Sprintf("%s/%s", secretMountPath, exporter.TLS.Cert) if filepath.IsAbs(exporter.TLS.Cert) { envVarVal = exporter.TLS.Cert } - if getIndexOfEnv(container.Env, constants.EnvOTELExporterClientCertificate) == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: constants.EnvOTELExporterClientCertificate, - Value: envVarVal, - }) - } + container.appendIfNotExists(pod, constants.EnvOTELExporterClientCertificate, envVarVal) } if exporter.TLS.Key != "" { envVarVar := fmt.Sprintf("%s/%s", secretMountPath, exporter.TLS.Key) if filepath.IsAbs(exporter.TLS.Key) { envVarVar = exporter.TLS.Key } - if getIndexOfEnv(container.Env, constants.EnvOTELExporterClientKey) == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: constants.EnvOTELExporterClientKey, - Value: envVarVar, - }) - } + container.appendIfNotExists(pod, constants.EnvOTELExporterClientKey, envVarVar) } if exporter.TLS.SecretName != "" { @@ -101,13 +81,13 @@ func configureExporter(exporter v1alpha1.Exporter, pod *corev1.Pod, container *c }}) } addVolumeMount := true - for _, vol := range container.VolumeMounts { + for _, vol := range pod.Spec.Containers[container.index].VolumeMounts { if vol.Name == secretVolumeName { addVolumeMount = false } } if addVolumeMount { - container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{ Name: secretVolumeName, MountPath: secretMountPath, ReadOnly: true, @@ -134,13 +114,13 @@ func configureExporter(exporter v1alpha1.Exporter, pod *corev1.Pod, container *c }}) } addVolumeMount := true - for _, vol := range container.VolumeMounts { + for _, vol := range pod.Spec.Containers[container.index].VolumeMounts { if vol.Name == configMapVolumeName { addVolumeMount = false } } if addVolumeMount { - container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{ Name: configMapVolumeName, MountPath: configMapMountPath, ReadOnly: true, diff --git a/pkg/instrumentation/exporter_test.go b/pkg/instrumentation/exporter_test.go index 2fddf1264a..0186bfcd99 100644 --- a/pkg/instrumentation/exporter_test.go +++ b/pkg/instrumentation/exporter_test.go @@ -15,8 +15,10 @@ package instrumentation import ( + "context" "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" @@ -202,7 +204,9 @@ func TestExporter(t *testing.T) { }, }, } - configureExporter(test.exporter, &pod, &pod.Spec.Containers[0]) + container, err := NewContainer(k8sClient, context.Background(), logr.Discard(), "", &pod, 0) + assert.NoError(t, err) + configureExporter(test.exporter, &pod, container) assert.Equal(t, test.expected, pod) }) } diff --git a/pkg/instrumentation/golang.go b/pkg/instrumentation/golang.go index 793d45b3f8..dc6d687016 100644 --- a/pkg/instrumentation/golang.go +++ b/pkg/instrumentation/golang.go @@ -82,8 +82,7 @@ func injectGoSDK(goSpec v1alpha1.Go, pod corev1.Pod, cfg config.Config) (corev1. // Inject Go instrumentation spec env vars. // For Go, env vars must be added to the agent contain for _, env := range goSpec.Env { - idx := getIndexOfEnv(goAgent.Env, env.Name) - if idx == -1 { + if !existsEnvVarInEnv(goAgent.Env, env.Name) { goAgent.Env = append(goAgent.Env, env) } } diff --git a/pkg/instrumentation/javaagent.go b/pkg/instrumentation/javaagent.go index ef91d296d8..b7982884dc 100644 --- a/pkg/instrumentation/javaagent.go +++ b/pkg/instrumentation/javaagent.go @@ -24,29 +24,22 @@ import ( const ( envJavaToolsOptions = "JAVA_TOOL_OPTIONS" - javaAgent = " -javaagent:/otel-auto-instrumentation-java/javaagent.jar" + javaAgent = "-javaagent:/otel-auto-instrumentation-java/javaagent.jar" javaInitContainerName = initContainerName + "-java" javaVolumeName = volumeName + "-java" javaInstrMountPath = "/otel-auto-instrumentation-java" ) -func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.Pod, error) { +func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, container Container) (corev1.Pod, error) { volume := instrVolume(javaSpec.VolumeClaimTemplate, javaVolumeName, javaSpec.VolumeSizeLimit) - // caller checks if there is at least one container. - container := &pod.Spec.Containers[index] - - err := validateContainerEnv(container.Env, envJavaToolsOptions) - if err != nil { + if err := container.validate(&pod, envJavaToolsOptions); err != nil { return pod, err } // inject Java instrumentation spec env vars. for _, env := range javaSpec.Env { - idx := getIndexOfEnv(container.Env, env.Name) - if idx == -1 { - container.Env = append(container.Env, env) - } + container.appendEnvVarIfNotExists(&pod, env) } javaJVMArgument := javaAgent @@ -54,17 +47,11 @@ func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1. javaJVMArgument = javaAgent + fmt.Sprintf(" -Dotel.javaagent.extensions=%s/extensions", javaInstrMountPath) } - idx := getIndexOfEnv(container.Env, envJavaToolsOptions) - if idx == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: envJavaToolsOptions, - Value: javaJVMArgument, - }) - } else { - container.Env[idx].Value = container.Env[idx].Value + javaJVMArgument + if err := container.appendOrConcat(&pod, envJavaToolsOptions, javaJVMArgument, ConcatFunc(concatWithSpace)); err != nil { + return pod, err } - container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{ Name: volume.Name, MountPath: javaInstrMountPath, }) @@ -97,5 +84,5 @@ func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1. } } - return pod, err + return pod, nil } diff --git a/pkg/instrumentation/javaagent_test.go b/pkg/instrumentation/javaagent_test.go index ea8d81305d..abf42a7cdf 100644 --- a/pkg/instrumentation/javaagent_test.go +++ b/pkg/instrumentation/javaagent_test.go @@ -15,10 +15,13 @@ package instrumentation import ( + "context" "fmt" "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -211,7 +214,7 @@ func TestInjectJavaagent(t *testing.T) { Env: []corev1.EnvVar{ { Name: "JAVA_TOOL_OPTIONS", - Value: "-Dbaz=bar" + javaAgent, + Value: "-Dbaz=bar " + javaAgent, }, }, }, @@ -257,7 +260,10 @@ func TestInjectJavaagent(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - pod, err := injectJavaagent(test.Java, test.pod, 0) + pod := test.pod + container, err := NewContainer(k8sClient, context.Background(), logr.Discard(), "", &pod, 0) + require.NoError(t, err) + pod, err = injectJavaagent(test.Java, pod, container) assert.Equal(t, test.expected, pod) assert.Equal(t, test.err, err) }) diff --git a/pkg/instrumentation/nginx.go b/pkg/instrumentation/nginx.go index 7bcc39d611..3bec33f18e 100644 --- a/pkg/instrumentation/nginx.go +++ b/pkg/instrumentation/nginx.go @@ -62,17 +62,11 @@ const ( 6) Inject mounting of volumes / files into appropriate directories in the application container */ -func injectNginxSDK(_ logr.Logger, nginxSpec v1alpha1.Nginx, 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 injectNginxSDK(_ logr.Logger, nginxSpec v1alpha1.Nginx, pod corev1.Pod, useLabelsForResourceAttributes bool, container Container, otlpEndpoint string, resourceMap map[string]string) (corev1.Pod, error) { // inject env vars for _, env := range nginxSpec.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 Nginx configuration from @@ -105,7 +99,7 @@ export %[4]s=$( { nginx -v ; } 2>&1 ) && echo ${%[4]s##*/} > %[3]s/version.txt nginxVersionEnvVar, ) - cloneContainer := container.DeepCopy() + cloneContainer := pod.Spec.Containers[container.index].DeepCopy() cloneContainer.Name = nginxAgentCloneContainerName cloneContainer.Command = []string{"/bin/sh", "-c"} cloneContainer.Args = []string{nginxAgentCommands} @@ -128,24 +122,24 @@ export %[4]s=$( { nginx -v ; } 2>&1 ) && echo ${%[4]s##*/} > %[3]s/version.txt // drop volume mount with volume-provided Nginx 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, nginxConfDir) { // 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 - Nginx config dir + Nginx agent - container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{ Name: nginxAgentVolume, MountPath: nginxAgentDirFull, }) - container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{ Name: nginxAgentConfigVolume, MountPath: nginxConfDir, }) @@ -217,7 +211,7 @@ mv ${NGINX_AGENT_CONF_DIR_FULL}/opentelemetry_agent.conf ${NGINX_AGENT_CONF_DIR Env: []corev1.EnvVar{ { Name: nginxAttributesEnvVar, - Value: getNginxOtelConfig(pod, useLabelsForResourceAttributes, nginxSpec, index, otlpEndpoint, resourceMap), + Value: getNginxOtelConfig(pod, useLabelsForResourceAttributes, nginxSpec, container.index, otlpEndpoint, resourceMap), }, { Name: "OTEL_NGINX_I13N_SCRIPT", @@ -243,26 +237,15 @@ mv ${NGINX_AGENT_CONF_DIR_FULL}/opentelemetry_agent.conf ${NGINX_AGENT_CONF_DIR MountPath: nginxAgentConfDirFull, }, }, - SecurityContext: pod.Spec.Containers[index].SecurityContext, + SecurityContext: pod.Spec.Containers[container.index].SecurityContext, }) - found := false - for i, e := range container.Env { - if e.Name == nginxLibraryPathEnv { - container.Env[i].Value = e.Value + ":" + nginxAgentDirFull + "/sdk_lib/lib" - found = true - break - } - } - if !found { - container.Env = append(container.Env, corev1.EnvVar{ - Name: nginxLibraryPathEnv, - Value: nginxAgentDirFull + "/sdk_lib/lib", - }) + if err := container.appendOrConcat(&pod, nginxLibraryPathEnv, nginxAgentDirFull+"/sdk_lib/lib", ConcatFunc(concatWithColon)); err != nil { + return pod, err } } - return pod + return pod, nil } // Calculate if we already inject InitContainers. diff --git a/pkg/instrumentation/nginx_test.go b/pkg/instrumentation/nginx_test.go index b483c38cf4..a78903e39a 100644 --- a/pkg/instrumentation/nginx_test.go +++ b/pkg/instrumentation/nginx_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" @@ -477,7 +479,11 @@ func TestInjectNginxSDK(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - pod := injectNginxSDK(logr.Discard(), test.Nginx, 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, err = injectNginxSDK(logr.Discard(), test.Nginx, pod, false, container, "http://otlp-endpoint:4317", resourceMap) + assert.NoError(t, err) assert.Equal(t, test.expected, pod) }) } @@ -600,7 +606,11 @@ func TestInjectNginxUnknownNamespace(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - pod := injectNginxSDK(logr.Discard(), test.Nginx, 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, err = injectNginxSDK(logr.Discard(), test.Nginx, pod, false, container, "http://otlp-endpoint:4317", resourceMap) + assert.NoError(t, err) assert.Equal(t, test.expected, pod) }) } diff --git a/pkg/instrumentation/nodejs.go b/pkg/instrumentation/nodejs.go index a3d02ea53d..9fd9d84fa9 100644 --- a/pkg/instrumentation/nodejs.go +++ b/pkg/instrumentation/nodejs.go @@ -22,42 +22,29 @@ import ( const ( envNodeOptions = "NODE_OPTIONS" - nodeRequireArgument = " --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js" + nodeRequireArgument = "--require /otel-auto-instrumentation-nodejs/autoinstrumentation.js" nodejsInitContainerName = initContainerName + "-nodejs" nodejsVolumeName = volumeName + "-nodejs" nodejsInstrMountPath = "/otel-auto-instrumentation-nodejs" ) -func injectNodeJSSDK(nodeJSSpec v1alpha1.NodeJS, pod corev1.Pod, index int) (corev1.Pod, error) { +func injectNodeJSSDK(nodeJSSpec v1alpha1.NodeJS, pod corev1.Pod, container Container) (corev1.Pod, error) { volume := instrVolume(nodeJSSpec.VolumeClaimTemplate, nodejsVolumeName, nodeJSSpec.VolumeSizeLimit) - // caller checks if there is at least one container. - container := &pod.Spec.Containers[index] - - err := validateContainerEnv(container.Env, envNodeOptions) - if err != nil { + if err := container.validate(&pod, envNodeOptions); err != nil { return pod, err } // inject NodeJS instrumentation spec env vars. for _, env := range nodeJSSpec.Env { - idx := getIndexOfEnv(container.Env, env.Name) - if idx == -1 { - container.Env = append(container.Env, env) - } + container.appendEnvVarIfNotExists(&pod, env) } - idx := getIndexOfEnv(container.Env, envNodeOptions) - if idx == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: envNodeOptions, - Value: nodeRequireArgument, - }) - } else if idx > -1 { - container.Env[idx].Value = container.Env[idx].Value + nodeRequireArgument + if err := container.appendOrConcat(&pod, envNodeOptions, nodeRequireArgument, ConcatFunc(concatWithSpace)); err != nil { + return pod, err } - container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{ Name: volume.Name, MountPath: nodejsInstrMountPath, }) diff --git a/pkg/instrumentation/nodejs_test.go b/pkg/instrumentation/nodejs_test.go index 7ed4fcd6d3..9ebb9daf76 100644 --- a/pkg/instrumentation/nodejs_test.go +++ b/pkg/instrumentation/nodejs_test.go @@ -15,10 +15,13 @@ package instrumentation import ( + "context" "fmt" "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -76,7 +79,7 @@ func TestInjectNodeJSSDK(t *testing.T) { Env: []corev1.EnvVar{ { Name: "NODE_OPTIONS", - Value: " --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js", + Value: "--require /otel-auto-instrumentation-nodejs/autoinstrumentation.js", }, }, }, @@ -137,7 +140,7 @@ func TestInjectNodeJSSDK(t *testing.T) { Env: []corev1.EnvVar{ { Name: "NODE_OPTIONS", - Value: "-Dbaz=bar" + " --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js", + Value: "-Dbaz=bar --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js", }, }, }, @@ -183,7 +186,10 @@ func TestInjectNodeJSSDK(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - pod, err := injectNodeJSSDK(test.NodeJS, test.pod, 0) + pod := test.pod + container, err := NewContainer(k8sClient, context.Background(), logr.Discard(), "", &pod, 0) + require.NoError(t, err) + pod, err = injectNodeJSSDK(test.NodeJS, pod, container) assert.Equal(t, test.expected, pod) assert.Equal(t, test.err, err) }) diff --git a/pkg/instrumentation/python.go b/pkg/instrumentation/python.go index db1c9d6a2d..f75b60199f 100644 --- a/pkg/instrumentation/python.go +++ b/pkg/instrumentation/python.go @@ -15,8 +15,6 @@ package instrumentation import ( - "fmt" - corev1 "k8s.io/api/core/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -35,72 +33,39 @@ const ( pythonInitContainerName = initContainerName + "-python" ) -func injectPythonSDK(pythonSpec v1alpha1.Python, pod corev1.Pod, index int) (corev1.Pod, error) { +func injectPythonSDK(pythonSpec v1alpha1.Python, pod corev1.Pod, container Container) (corev1.Pod, error) { volume := instrVolume(pythonSpec.VolumeClaimTemplate, pythonVolumeName, pythonSpec.VolumeSizeLimit) - // caller checks if there is at least one container. - container := &pod.Spec.Containers[index] - - err := validateContainerEnv(container.Env, envPythonPath) + err := container.validate(&pod, envPythonPath) if err != nil { return pod, err } // inject Python instrumentation spec env vars. for _, env := range pythonSpec.Env { - idx := getIndexOfEnv(container.Env, env.Name) - if idx == -1 { - container.Env = append(container.Env, env) - } + container.appendEnvVarIfNotExists(&pod, env) } - idx := getIndexOfEnv(container.Env, envPythonPath) - if idx == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: envPythonPath, - Value: fmt.Sprintf("%s:%s", pythonPathPrefix, pythonPathSuffix), - }) - } else if idx > -1 { - container.Env[idx].Value = fmt.Sprintf("%s:%s:%s", pythonPathPrefix, container.Env[idx].Value, pythonPathSuffix) + envPythonPathVar, err := container.getOrMakeEnvVar(&pod, envPythonPath) + if err != nil { + return pod, err } + envPythonPathVar.Value = concatWithColon(pythonPathPrefix, envPythonPathVar.Value, pythonPathSuffix) + container.setOrAppendEnvVar(&pod, envPythonPathVar) // Set OTEL_EXPORTER_OTLP_PROTOCOL to http/protobuf if not set by user because it is what our autoinstrumentation supports. - idx = getIndexOfEnv(container.Env, envOtelExporterOTLPProtocol) - if idx == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: envOtelExporterOTLPProtocol, - Value: "http/protobuf", - }) - } + container.appendIfNotExists(&pod, envOtelExporterOTLPProtocol, "http/protobuf") // Set OTEL_TRACES_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports. - idx = getIndexOfEnv(container.Env, envOtelTracesExporter) - if idx == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: envOtelTracesExporter, - Value: "otlp", - }) - } + container.appendIfNotExists(&pod, envOtelTracesExporter, "otlp") // Set OTEL_METRICS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports. - idx = getIndexOfEnv(container.Env, envOtelMetricsExporter) - if idx == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: envOtelMetricsExporter, - Value: "otlp", - }) - } + container.appendIfNotExists(&pod, envOtelMetricsExporter, "otlp") // Set OTEL_LOGS_EXPORTER to otlp exporter if not set by user because it is what our autoinstrumentation supports. - idx = getIndexOfEnv(container.Env, envOtelLogsExporter) - if idx == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: envOtelLogsExporter, - Value: "otlp", - }) - } + container.appendIfNotExists(&pod, envOtelLogsExporter, "otlp") - container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + pod.Spec.Containers[container.index].VolumeMounts = append(pod.Spec.Containers[container.index].VolumeMounts, corev1.VolumeMount{ Name: volume.Name, MountPath: pythonInstrMountPath, }) diff --git a/pkg/instrumentation/python_test.go b/pkg/instrumentation/python_test.go index 01fe9b1665..efffcf8fe9 100644 --- a/pkg/instrumentation/python_test.go +++ b/pkg/instrumentation/python_test.go @@ -15,10 +15,13 @@ package instrumentation import ( + "context" "fmt" "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -519,7 +522,10 @@ func TestInjectPythonSDK(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - pod, err := injectPythonSDK(test.Python, test.pod, 0) + pod := test.pod + container, err := NewContainer(k8sClient, context.Background(), logr.Discard(), "", &pod, 0) + require.NoError(t, err) + pod, err = injectPythonSDK(test.Python, pod, container) assert.Equal(t, test.expected, pod) assert.Equal(t, test.err, err) }) diff --git a/pkg/instrumentation/sdk.go b/pkg/instrumentation/sdk.go index 0033f70566..971501d4df 100644 --- a/pkg/instrumentation/sdk.go +++ b/pkg/instrumentation/sdk.go @@ -59,90 +59,123 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations } if insts.Java.Instrumentation != nil { otelinst := *insts.Java.Instrumentation - var err error i.logger.V(1).Info("injecting Java instrumentation into pod", "otelinst-namespace", otelinst.Namespace, "otelinst-name", otelinst.Name) if len(insts.Java.Containers) == 0 { insts.Java.Containers = []string{pod.Spec.Containers[0].Name} } - for _, container := range insts.Java.Containers { - index := getContainerIndex(container, pod) - pod, err = injectJavaagent(otelinst.Spec.Java, pod, index) + for _, containerName := range insts.Java.Containers { + origPod := pod.DeepCopy() + + index := getContainerIndex(&pod, containerName) + container, err := NewContainer(i.client, ctx, i.logger, ns.Name, &pod, index) + if err == nil { + pod, err = injectJavaagent(otelinst.Spec.Java, pod, container) + } + if err == nil { + pod = i.injectCommonEnvVar(otelinst, pod, container) + pod, err = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, container, container) + } + if err == nil { + pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[container.index].SecurityContext, javaInitContainerName) + } if err != nil { - i.logger.Info("Skipping javaagent injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name) - } else { - pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) - pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, javaInitContainerName) + i.logger.Info("Skipping javaagent injection", "reason", err.Error(), "container", origPod.Spec.Containers[index].Name) + pod = *origPod } } } if insts.NodeJS.Instrumentation != nil { otelinst := *insts.NodeJS.Instrumentation - var err error i.logger.V(1).Info("injecting NodeJS instrumentation into pod", "otelinst-namespace", otelinst.Namespace, "otelinst-name", otelinst.Name) if len(insts.NodeJS.Containers) == 0 { insts.NodeJS.Containers = []string{pod.Spec.Containers[0].Name} } - for _, container := range insts.NodeJS.Containers { - index := getContainerIndex(container, pod) - pod, err = injectNodeJSSDK(otelinst.Spec.NodeJS, pod, index) + for _, containerName := range insts.NodeJS.Containers { + origPod := pod.DeepCopy() + + index := getContainerIndex(&pod, containerName) + container, err := NewContainer(i.client, ctx, i.logger, ns.Name, &pod, index) + if err == nil { + pod, err = injectNodeJSSDK(otelinst.Spec.NodeJS, pod, container) + } + if err == nil { + pod = i.injectCommonEnvVar(otelinst, pod, container) + pod, err = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, container, container) + } + if err == nil { + pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[container.index].SecurityContext, nodejsInitContainerName) + } if err != nil { - i.logger.Info("Skipping NodeJS SDK injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name) - } else { - pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) - pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, nodejsInitContainerName) + i.logger.Info("Skipping NodeJS SDK injection", "reason", err.Error(), "container", origPod.Spec.Containers[index].Name) + pod = *origPod } } } if insts.Python.Instrumentation != nil { otelinst := *insts.Python.Instrumentation - var err error i.logger.V(1).Info("injecting Python instrumentation into pod", "otelinst-namespace", otelinst.Namespace, "otelinst-name", otelinst.Name) if len(insts.Python.Containers) == 0 { insts.Python.Containers = []string{pod.Spec.Containers[0].Name} } - for _, container := range insts.Python.Containers { - index := getContainerIndex(container, pod) - pod, err = injectPythonSDK(otelinst.Spec.Python, pod, index) + for _, containerName := range insts.Python.Containers { + origPod := pod.DeepCopy() + + index := getContainerIndex(&pod, containerName) + container, err := NewContainer(i.client, ctx, i.logger, ns.Name, &pod, index) + if err == nil { + pod, err = injectPythonSDK(otelinst.Spec.Python, pod, container) + } + if err == nil { + pod = i.injectCommonEnvVar(otelinst, pod, container) + pod, err = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, container, container) + } + if err == nil { + pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[container.index].SecurityContext, pythonInitContainerName) + } if err != nil { - i.logger.Info("Skipping Python SDK injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name) - } else { - pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) - pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, pythonInitContainerName) + i.logger.Info("Skipping Python SDK injection", "reason", err.Error(), "container", origPod.Spec.Containers[index].Name) + pod = *origPod } } } if insts.DotNet.Instrumentation != nil { otelinst := *insts.DotNet.Instrumentation - var err error i.logger.V(1).Info("injecting DotNet instrumentation into pod", "otelinst-namespace", otelinst.Namespace, "otelinst-name", otelinst.Name) if len(insts.DotNet.Containers) == 0 { insts.DotNet.Containers = []string{pod.Spec.Containers[0].Name} } - for _, container := range insts.DotNet.Containers { - index := getContainerIndex(container, pod) - pod, err = injectDotNetSDK(otelinst.Spec.DotNet, pod, index, insts.DotNet.AdditionalAnnotations[annotationDotNetRuntime]) + for _, containerName := range insts.DotNet.Containers { + origPod := pod.DeepCopy() + + index := getContainerIndex(&pod, containerName) + container, err := NewContainer(i.client, ctx, i.logger, ns.Name, &pod, index) + if err == nil { + pod, err = injectDotNetSDK(otelinst.Spec.DotNet, pod, container, insts.DotNet.AdditionalAnnotations[annotationDotNetRuntime]) + } + if err == nil { + pod = i.injectCommonEnvVar(otelinst, pod, container) + pod, err = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, container, container) + } + if err == nil { + pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[container.index].SecurityContext, dotnetInitContainerName) + } if err != nil { - i.logger.Info("Skipping DotNet SDK injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name) - } else { - pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) - pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, dotnetInitContainerName) + i.logger.Info("Skipping DotNet SDK injection", "reason", err.Error(), "container", origPod.Spec.Containers[index].Name) + pod = *origPod } } } if insts.Go.Instrumentation != nil { - origPod := pod + origPod := pod.DeepCopy() + otelinst := *insts.Go.Instrumentation var err error i.logger.V(1).Info("injecting Go instrumentation into pod", "otelinst-namespace", otelinst.Namespace, "otelinst-name", otelinst.Name) @@ -152,21 +185,27 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations } // Go instrumentation supports only single container instrumentation. - index := getContainerIndex(insts.Go.Containers[0], pod) - pod, err = injectGoSDK(otelinst.Spec.Go, pod, cfg) - if err != nil { - i.logger.Info("Skipping Go SDK injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name) - } else { - // Common env vars and config need to be applied to the agent contain. - pod = i.injectCommonEnvVar(otelinst, pod, len(pod.Spec.Containers)-1) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, len(pod.Spec.Containers)-1, 0) - - // Ensure that after all the env var coalescing we have a value for OTEL_GO_AUTO_TARGET_EXE - idx := getIndexOfEnv(pod.Spec.Containers[len(pod.Spec.Containers)-1].Env, envOtelTargetExe) - if idx == -1 { - i.logger.Info("Skipping Go SDK injection", "reason", "OTEL_GO_AUTO_TARGET_EXE not set", "container", pod.Spec.Containers[index].Name) - pod = origPod + var appContainer, agentContainer Container + index := getContainerIndex(&pod, insts.Go.Containers[0]) + appContainer, err = NewContainer(i.client, ctx, i.logger, ns.Name, &pod, index) + if err == nil { + pod, err = injectGoSDK(otelinst.Spec.Go, pod, cfg) + } + if err == nil { + agentContainer, err = NewContainer(i.client, ctx, i.logger, ns.Name, &pod, len(pod.Spec.Containers)-1) + } + if err == nil { + // Common env vars and config need to be applied to the agent container. + pod = i.injectCommonEnvVar(otelinst, pod, agentContainer) + pod, err = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, agentContainer, appContainer) + } + if err != nil || !agentContainer.exists(&pod, envOtelTargetExe) { + if err != nil { + i.logger.Info("Skipping Go SDK injection", "reason", err.Error(), "container", origPod.Spec.Containers[index].Name) + } else { + i.logger.Info("Skipping Go SDK injection", "reason", "OTEL_GO_AUTO_TARGET_EXE not set", "container", origPod.Spec.Containers[index].Name) } + pod = *origPod } } if insts.ApacheHttpd.Instrumentation != nil { @@ -177,16 +216,31 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations insts.ApacheHttpd.Containers = []string{pod.Spec.Containers[0].Name} } - for _, container := range insts.ApacheHttpd.Containers { - index := getContainerIndex(container, pod) + for _, containerName := range insts.ApacheHttpd.Containers { + origPod := pod.DeepCopy() + + index := getContainerIndex(&pod, containerName) + container, err := NewContainer(i.client, ctx, i.logger, ns.Name, &pod, index) // Apache agent is configured via config files rather than env vars. // Therefore, service name, otlp endpoint and other attributes are passed to the agent injection method - useLabelsForResourceAttributes := otelinst.Spec.Defaults.UseLabelsForResourceAttributes - pod = injectApacheHttpdagent(i.logger, otelinst.Spec.ApacheHttpd, pod, useLabelsForResourceAttributes, index, otelinst.Spec.Endpoint, i.createResourceMap(ctx, otelinst, ns, pod, index)) - pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) - pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, apacheAgentInitContainerName) - pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, apacheAgentCloneContainerName) + var resourceMap map[string]string + if err == nil { + resourceMap, err = i.createResourceMap(ctx, otelinst, ns, pod, container) + } + if err == nil { + useLabelsForResourceAttributes := otelinst.Spec.Defaults.UseLabelsForResourceAttributes + pod = injectApacheHttpdagent(i.logger, otelinst.Spec.ApacheHttpd, pod, useLabelsForResourceAttributes, container, otelinst.Spec.Endpoint, resourceMap) + pod = i.injectCommonEnvVar(otelinst, pod, container) + pod, err = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, container, container) + } + if err == nil { + pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[container.index].SecurityContext, apacheAgentInitContainerName) + pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[container.index].SecurityContext, apacheAgentCloneContainerName) + } + if err != nil { + i.logger.Info("Skipping Apache Httpd SDK injection", "reason", err.Error(), "container", origPod.Spec.Containers[index].Name) + pod = *origPod + } } } @@ -198,14 +252,29 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations insts.Nginx.Containers = []string{pod.Spec.Containers[0].Name} } - for _, container := range insts.Nginx.Containers { - index := getContainerIndex(container, pod) + for _, containerName := range insts.Nginx.Containers { + origPod := pod.DeepCopy() + + index := getContainerIndex(&pod, containerName) + container, err := NewContainer(i.client, ctx, i.logger, ns.Name, &pod, index) // Nginx agent is configured via config files rather than env vars. // Therefore, service name, otlp endpoint and other attributes are passed to the agent injection method - useLabelsForResourceAttributes := otelinst.Spec.Defaults.UseLabelsForResourceAttributes - pod = injectNginxSDK(i.logger, otelinst.Spec.Nginx, pod, useLabelsForResourceAttributes, index, otelinst.Spec.Endpoint, i.createResourceMap(ctx, otelinst, ns, pod, index)) - pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) + var resourceMap map[string]string + if err == nil { + resourceMap, err = i.createResourceMap(ctx, otelinst, ns, pod, container) + } + if err == nil { + useLabelsForResourceAttributes := otelinst.Spec.Defaults.UseLabelsForResourceAttributes + pod, err = injectNginxSDK(i.logger, otelinst.Spec.Nginx, pod, useLabelsForResourceAttributes, container, otelinst.Spec.Endpoint, resourceMap) + } + if err == nil { + pod = i.injectCommonEnvVar(otelinst, pod, container) + pod, err = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, container, container) + } + if err != nil { + i.logger.Info("Skipping Nginx SDK injection", "reason", err.Error(), "container", origPod.Spec.Containers[index].Name) + pod = *origPod + } } } @@ -217,10 +286,19 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations insts.Sdk.Containers = []string{pod.Spec.Containers[0].Name} } - for _, container := range insts.Sdk.Containers { - index := getContainerIndex(container, pod) - pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) + for _, containerName := range insts.Sdk.Containers { + origPod := pod.DeepCopy() + + index := getContainerIndex(&pod, containerName) + container, err := NewContainer(i.client, ctx, i.logger, ns.Name, &pod, index) + if err == nil { + pod = i.injectCommonEnvVar(otelinst, pod, container) + pod, err = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, container, container) + } + if err != nil { + i.logger.Info("Skipping sdk-only injection", "reason", err.Error(), "container", origPod.Spec.Containers[index].Name) + pod = *origPod + } } } @@ -237,51 +315,27 @@ func (i *sdkInjector) setInitContainerSecurityContext(pod corev1.Pod, securityCo return pod } -func getContainerIndex(containerName string, pod corev1.Pod) int { - // We search for specific container to inject variables and if no one is found - // We fallback to first container - var index = 0 - for idx, ctnair := range pod.Spec.Containers { - if ctnair.Name == containerName { - index = idx - } - } - - return index -} - -func (i *sdkInjector) injectCommonEnvVar(otelinst v1alpha1.Instrumentation, pod corev1.Pod, index int) corev1.Pod { - container := &pod.Spec.Containers[index] - - idx := getIndexOfEnv(container.Env, constants.EnvPodIP) - if idx == -1 { - container.Env = append([]corev1.EnvVar{{ - Name: constants.EnvPodIP, - ValueFrom: &corev1.EnvVarSource{ - FieldRef: &corev1.ObjectFieldSelector{ - FieldPath: "status.podIP", - }, +func (i *sdkInjector) injectCommonEnvVar(otelinst v1alpha1.Instrumentation, pod corev1.Pod, container Container) corev1.Pod { + container.prependEnvVarIfNotExists(&pod, corev1.EnvVar{ + Name: constants.EnvPodIP, + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "status.podIP", }, - }}, container.Env...) - } + }, + }) - idx = getIndexOfEnv(container.Env, constants.EnvNodeIP) - if idx == -1 { - container.Env = append([]corev1.EnvVar{{ - Name: constants.EnvNodeIP, - ValueFrom: &corev1.EnvVarSource{ - FieldRef: &corev1.ObjectFieldSelector{ - FieldPath: "status.hostIP", - }, + container.prependEnvVarIfNotExists(&pod, corev1.EnvVar{ + Name: constants.EnvNodeIP, + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "status.hostIP", }, - }}, container.Env...) - } + }, + }) for _, env := range otelinst.Spec.Env { - idx := getIndexOfEnv(container.Env, env.Name) - if idx == -1 { - container.Env = append(container.Env, env) - } + container.appendEnvVarIfNotExists(&pod, env) } return pod } @@ -293,21 +347,17 @@ func (i *sdkInjector) injectCommonEnvVar(otelinst v1alpha1.Instrumentation, pod // and appIndex should be the same value. This is true for dotnet, java, nodejs, and python instrumentations. // Go requires the agent to be a different container in the pod, so the agentIndex should represent this new sidecar // and appIndex should represent the application being instrumented. -func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alpha1.Instrumentation, ns corev1.Namespace, pod corev1.Pod, agentIndex int, appIndex int) corev1.Pod { - container := &pod.Spec.Containers[agentIndex] +func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alpha1.Instrumentation, ns corev1.Namespace, pod corev1.Pod, agentContainer Container, appContainer Container) (corev1.Pod, error) { useLabelsForResourceAttributes := otelinst.Spec.Defaults.UseLabelsForResourceAttributes - resourceMap := i.createResourceMap(ctx, otelinst, ns, pod, appIndex) - idx := getIndexOfEnv(container.Env, constants.EnvOTELServiceName) - if idx == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: constants.EnvOTELServiceName, - Value: chooseServiceName(pod, useLabelsForResourceAttributes, resourceMap, appIndex), - }) + resourceMap, err := i.createResourceMap(ctx, otelinst, ns, pod, appContainer) + if err != nil { + return pod, err } - configureExporter(otelinst.Spec.Exporter, &pod, container) + agentContainer.appendIfNotExists(&pod, constants.EnvOTELServiceName, chooseServiceName(pod, useLabelsForResourceAttributes, resourceMap, appContainer.index)) + configureExporter(otelinst.Spec.Exporter, &pod, agentContainer) // Always retrieve the pod name from the Downward API. Ensure that the OTEL_RESOURCE_ATTRIBUTES_POD_NAME env exists. - container.Env = append(container.Env, corev1.EnvVar{ + agentContainer.appendEnvVar(&pod, corev1.EnvVar{ Name: constants.EnvPodName, ValueFrom: &corev1.EnvVarSource{ FieldRef: &corev1.ObjectFieldSelector{ @@ -320,7 +370,7 @@ func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alph // Some attributes might be empty, we should get them via k8s downward API if otelinst.Spec.Resource.AddK8sUIDAttributes { if resourceMap[string(semconv.K8SPodUIDKey)] == "" { - container.Env = append(container.Env, corev1.EnvVar{ + agentContainer.appendEnvVar(&pod, corev1.EnvVar{ Name: constants.EnvPodUID, ValueFrom: &corev1.EnvVarSource{ FieldRef: &corev1.ObjectFieldSelector{ @@ -332,16 +382,19 @@ func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alph } } - idx = getIndexOfEnv(container.Env, constants.EnvOTELResourceAttrs) - if idx == -1 || !strings.Contains(container.Env[idx].Value, string(semconv.ServiceVersionKey)) { - vsn := chooseServiceVersion(pod, useLabelsForResourceAttributes, appIndex) + envResourceAttrsVar, err := agentContainer.getOrMakeEnvVar(&pod, constants.EnvOTELResourceAttrs) + if err != nil { + return pod, err + } + if !strings.Contains(envResourceAttrsVar.Value, string(semconv.ServiceVersionKey)) { + vsn := chooseServiceVersion(pod, useLabelsForResourceAttributes, appContainer.index) if vsn != "" { resourceMap[string(semconv.ServiceVersionKey)] = vsn } } if resourceMap[string(semconv.K8SNodeNameKey)] == "" { - container.Env = append(container.Env, corev1.EnvVar{ + agentContainer.appendEnvVar(&pod, corev1.EnvVar{ Name: constants.EnvNodeName, ValueFrom: &corev1.EnvVarSource{ FieldRef: &corev1.ObjectFieldSelector{ @@ -352,43 +405,26 @@ func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alph resourceMap[string(semconv.K8SNodeNameKey)] = fmt.Sprintf("$(%s)", constants.EnvNodeName) } - idx = getIndexOfEnv(container.Env, constants.EnvOTELResourceAttrs) - resStr := resourceMapToStr(resourceMap) - if idx == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: constants.EnvOTELResourceAttrs, - Value: resStr, - }) - } else { - if !strings.HasSuffix(container.Env[idx].Value, ",") { - resStr = "," + resStr - } - container.Env[idx].Value += resStr - } + envResourceAttrsVar.Value = concatWithComma(envResourceAttrsVar.Value, resourceMapToStr(resourceMap)) + agentContainer.setOrAppendEnvVar(&pod, envResourceAttrsVar) - idx = getIndexOfEnv(container.Env, constants.EnvOTELPropagators) - if idx == -1 && len(otelinst.Spec.Propagators) > 0 { + if len(otelinst.Spec.Propagators) > 0 { propagators := *(*[]string)((unsafe.Pointer(&otelinst.Spec.Propagators))) - container.Env = append(container.Env, corev1.EnvVar{ - Name: constants.EnvOTELPropagators, - Value: strings.Join(propagators, ","), - }) + agentContainer.appendIfNotExists(&pod, constants.EnvOTELPropagators, strings.Join(propagators, ",")) } - idx = getIndexOfEnv(container.Env, constants.EnvOTELTracesSampler) // configure sampler only if it is configured in the CR - if idx == -1 && otelinst.Spec.Sampler.Type != "" { - idxSamplerArg := getIndexOfEnv(container.Env, constants.EnvOTELTracesSamplerArg) - if idxSamplerArg == -1 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: constants.EnvOTELTracesSampler, - Value: string(otelinst.Spec.Sampler.Type), - }) + if !agentContainer.exists(&pod, constants.EnvOTELTracesSampler) && otelinst.Spec.Sampler.Type != "" { + if !agentContainer.exists(&pod, constants.EnvOTELTracesSamplerArg) { + agentContainer.append(&pod, + constants.EnvOTELTracesSampler, + string(otelinst.Spec.Sampler.Type), + ) if otelinst.Spec.Sampler.Argument != "" { - container.Env = append(container.Env, corev1.EnvVar{ - Name: constants.EnvOTELTracesSamplerArg, - Value: otelinst.Spec.Sampler.Argument, - }) + agentContainer.append(&pod, + constants.EnvOTELTracesSamplerArg, + otelinst.Spec.Sampler.Argument, + ) } } } @@ -398,11 +434,9 @@ func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alph // as attributes value they have to be configured before. // It is mandatory to set right order to avoid attributes with value // pointing to the name of used environment variable instead of its value. - idx = getIndexOfEnv(container.Env, constants.EnvOTELResourceAttrs) - envs := moveEnvToListEnd(container.Env, idx) - container.Env = envs + agentContainer.moveToListEnd(&pod, constants.EnvOTELResourceAttrs) - return pod + return pod, nil } // chooseServiceName returns the service name to be used in the instrumentation. @@ -492,18 +526,21 @@ func createServiceInstanceId(pod corev1.Pod, useLabelsForResourceAttributes bool // createResourceMap creates resource attribute map. // User defined attributes (in explicitly set env var) have higher precedence. -func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.Instrumentation, ns corev1.Namespace, pod corev1.Pod, index int) map[string]string { +func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.Instrumentation, ns corev1.Namespace, pod corev1.Pod, container Container) (map[string]string, error) { // get existing resources env var and parse it into a map existingRes := map[string]bool{} - existingResourceEnvIdx := getIndexOfEnv(pod.Spec.Containers[index].Env, constants.EnvOTELResourceAttrs) - if existingResourceEnvIdx > -1 { - existingResArr := strings.Split(pod.Spec.Containers[index].Env[existingResourceEnvIdx].Value, ",") - for _, kv := range existingResArr { - keyValueArr := strings.Split(strings.TrimSpace(kv), "=") - if len(keyValueArr) != 2 { - continue - } - existingRes[keyValueArr[0]] = true + if container.exists(&pod, constants.EnvOTELResourceAttrs) { + if envVar, err := container.getOrMakeEnvVar(&pod, constants.EnvOTELResourceAttrs); err == nil { + existingResArr := strings.Split(envVar.Value, ",") + for _, kv := range existingResArr { + keyValueArr := strings.Split(strings.TrimSpace(kv), "=") + if len(keyValueArr) != 2 { + continue + } + existingRes[keyValueArr[0]] = true + } + } else { + return nil, err } } @@ -521,13 +558,13 @@ func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.I // k8s resources have a higher precedence than CRD entries k8sResources := map[attribute.Key]string{} k8sResources[semconv.K8SNamespaceNameKey] = ns.Name - k8sResources[semconv.K8SContainerNameKey] = pod.Spec.Containers[index].Name + k8sResources[semconv.K8SContainerNameKey] = pod.Spec.Containers[container.index].Name // Some fields might be empty - node name, pod name // The pod name might be empty if the pod is created form deployment template k8sResources[semconv.K8SPodNameKey] = pod.Name k8sResources[semconv.K8SPodUIDKey] = string(pod.UID) k8sResources[semconv.K8SNodeNameKey] = pod.Spec.NodeName - k8sResources[semconv.ServiceInstanceIDKey] = createServiceInstanceId(pod, useLabelsForResourceAttributes, ns.Name, fmt.Sprintf("$(%s)", constants.EnvPodName), pod.Spec.Containers[index].Name) + k8sResources[semconv.ServiceInstanceIDKey] = createServiceInstanceId(pod, useLabelsForResourceAttributes, ns.Name, fmt.Sprintf("$(%s)", constants.EnvPodName), pod.Spec.Containers[container.index].Name) i.addParentResourceLabels(ctx, otelinst.Spec.Resource.AddK8sUIDAttributes, ns, pod.ObjectMeta, k8sResources) for k, v := range k8sResources { @@ -550,7 +587,7 @@ func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.I res[string(semconv.ServiceNamespaceKey)] = partOf } - return res + return res, nil } func (i *sdkInjector) addParentResourceLabels(ctx context.Context, uid bool, ns corev1.Namespace, objectMeta metav1.ObjectMeta, resources map[attribute.Key]string) { @@ -646,36 +683,3 @@ func resourceMapToStr(res map[string]string) string { return str } - -func getIndexOfEnv(envs []corev1.EnvVar, name string) int { - for i := range envs { - if envs[i].Name == name { - return i - } - } - return -1 -} - -func moveEnvToListEnd(envs []corev1.EnvVar, idx int) []corev1.EnvVar { - if idx >= 0 && idx < len(envs) { - envToMove := envs[idx] - envs = append(envs[:idx], envs[idx+1:]...) - envs = append(envs, envToMove) - } - - return envs -} - -func validateContainerEnv(envs []corev1.EnvVar, envsToBeValidated ...string) error { - for _, envToBeValidated := range envsToBeValidated { - for _, containerEnv := range envs { - if containerEnv.Name == envToBeValidated { - if containerEnv.ValueFrom != nil { - return fmt.Errorf("the container defines env var value via ValueFrom, envVar: %s", containerEnv.Name) - } - break - } - } - } - return nil -} diff --git a/pkg/instrumentation/sdk_test.go b/pkg/instrumentation/sdk_test.go index 38013ed3ac..fdf83e5515 100644 --- a/pkg/instrumentation/sdk_test.go +++ b/pkg/instrumentation/sdk_test.go @@ -905,7 +905,11 @@ func TestSDKInjection(t *testing.T) { inj := sdkInjector{ client: k8sClient, } - pod := inj.injectCommonSDKConfig(context.Background(), test.inst, corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: test.pod.Namespace}}, test.pod, 0, 0) + pod := test.pod + container, err := NewContainer(k8sClient, context.Background(), logr.Discard(), "", &pod, 0) + require.NoError(t, err) + pod, err = inj.injectCommonSDKConfig(context.Background(), test.inst, corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: pod.Namespace}}, pod, container, container) + assert.NoError(t, err) _, err = json.MarshalIndent(pod, "", " ") assert.NoError(t, err) assert.Equal(t, test.expected, pod) @@ -2548,7 +2552,7 @@ func TestChooseServiceName(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - serviceName := chooseServiceName(corev1.Pod{ + pod := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ "app.kubernetes.io/name": test.labelValue, @@ -2563,7 +2567,8 @@ func TestChooseServiceName(t *testing.T) { {Name: "2nd"}, }, }, - }, test.useLabelsForResourceAttributes, test.resources, test.index) + } + serviceName := chooseServiceName(pod, test.useLabelsForResourceAttributes, test.resources, test.index) assert.Equal(t, test.expectedServiceName, serviceName) }) diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/00-install-collector.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/00-install-collector.yaml new file mode 100644 index 0000000000..34a26ebb2c --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/00-install-collector.yaml @@ -0,0 +1,22 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: sidecar +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + exporters: [debug] + mode: sidecar diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/00-install-instrumentation.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/00-install-instrumentation.yaml new file mode 100644 index 0000000000..f77a3f835f --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/00-install-instrumentation.yaml @@ -0,0 +1,34 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: Instrumentation +metadata: + name: java +spec: + env: + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + exporter: + endpoint: http://localhost:4317 + propagators: + - jaeger + - b3 + sampler: + type: parentbased_traceidratio + argument: "0.25" + java: + env: + - name: OTEL_JAVAAGENT_DEBUG + value: "true" + - name: OTEL_INSTRUMENTATION_JDBC_ENABLED + value: "false" + - name: SPLUNK_PROFILER_ENABLED + value: "false" diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/01-assert.yaml new file mode 100644 index 0000000000..708849cf73 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/01-assert.yaml @@ -0,0 +1,78 @@ +apiVersion: v1 +kind: Pod +metadata: + annotations: + instrumentation.opentelemetry.io/inject-java: "true" + sidecar.opentelemetry.io/inject: "true" + labels: + app: my-java +spec: + containers: + - env: + - name: OTEL_NODE_IP + valueFrom: + fieldRef: + fieldPath: status.hostIP + - name: OTEL_POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP + - name: OTEL_JAVAAGENT_DEBUG + value: "true" + - name: OTEL_INSTRUMENTATION_JDBC_ENABLED + value: "false" + - name: SPLUNK_PROFILER_ENABLED + value: "false" + - name: JAVA_TOOL_OPTIONS + value: '-XX:+UseContainerSupport -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + - name: OTEL_RESOURCE_ATTRIBUTES_POD_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: metadata.name + - name: OTEL_RESOURCE_ATTRIBUTES_NODE_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: spec.nodeName + - name: OTEL_PROPAGATORS + value: jaeger,b3 + - name: OTEL_RESOURCE_ATTRIBUTES + envFrom: + - configMapRef: + name: my-conf + name: myapp + volumeMounts: + - mountPath: /var/run/secrets/kubernetes.io/serviceaccount + readOnly: true + - mountPath: /otel-auto-instrumentation-java + name: opentelemetry-auto-instrumentation-java + - args: + - --config=env:OTEL_CONFIG + name: otc-container + initContainers: + - name: opentelemetry-auto-instrumentation-java +status: + containerStatuses: + - name: myapp + ready: true + started: true + - name: otc-container + ready: true + started: true + initContainerStatuses: + - name: opentelemetry-auto-instrumentation-java + ready: true + phase: Running diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/01-install-app.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/01-install-app.yaml new file mode 100644 index 0000000000..19d8a040fa --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/01-install-app.yaml @@ -0,0 +1,39 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: my-conf +data: + OTEL_SERVICE_NAME: my-app + JAVA_TOOL_OPTIONS: -XX:+UseContainerSupport +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-java +spec: + selector: + matchLabels: + app: my-java + replicas: 1 + template: + metadata: + labels: + app: my-java + annotations: + sidecar.opentelemetry.io/inject: "true" + instrumentation.opentelemetry.io/inject-java: "true" + spec: + securityContext: + runAsUser: 1000 + runAsGroup: 3000 + fsGroup: 3000 + containers: + - name: myapp + image: ghcr.io/open-telemetry/opentelemetry-operator/e2e-test-app-java:main + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: ["ALL"] + envFrom: + - configMapRef: + name: my-conf diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/chainsaw-test.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/chainsaw-test.yaml new file mode 100755 index 0000000000..96657b1331 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-configmap/chainsaw-test.yaml @@ -0,0 +1,40 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: instrumentation-java-envfrom-configmap +spec: + steps: + - name: step-00 + try: + # In OpenShift, when a namespace is created, all necessary SCC annotations are automatically added. However, if a namespace is created using a resource file with only selected SCCs, the other auto-added SCCs are not included. Therefore, the UID-range and supplemental groups SCC annotations must be set after the namespace is created. + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.uid-range=1000/1000 + - --overwrite + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.supplemental-groups=3000/3000 + - --overwrite + - apply: + file: 00-install-collector.yaml + - apply: + file: 00-install-instrumentation.yaml + - name: step-01 + try: + - apply: + file: 01-install-app.yaml + - assert: + file: 01-assert.yaml + catch: + - podLogs: + selector: app=my-java \ No newline at end of file diff --git a/tests/e2e-instrumentation/instrumentation-java-multicontainer/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-multicontainer/01-assert.yaml index a4dca94976..09b2a5687b 100644 --- a/tests/e2e-instrumentation/instrumentation-java-multicontainer/01-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-java-multicontainer/01-assert.yaml @@ -25,7 +25,7 @@ spec: - name: SPLUNK_PROFILER_ENABLED value: "false" - name: JAVA_TOOL_OPTIONS - value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar' - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT @@ -75,7 +75,7 @@ spec: - name: SPLUNK_PROFILER_ENABLED value: "false" - name: JAVA_TOOL_OPTIONS - value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar' - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT diff --git a/tests/e2e-instrumentation/instrumentation-java-multicontainer/02-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-multicontainer/02-assert.yaml index 03c002d2d8..5bfa1ceff3 100644 --- a/tests/e2e-instrumentation/instrumentation-java-multicontainer/02-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-java-multicontainer/02-assert.yaml @@ -36,7 +36,7 @@ spec: - name: SPLUNK_PROFILER_ENABLED value: "false" - name: JAVA_TOOL_OPTIONS - value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar' - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT diff --git a/tests/e2e-instrumentation/instrumentation-java-other-ns/03-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-other-ns/03-assert.yaml index 0b6ea1db84..ef36aa4c46 100644 --- a/tests/e2e-instrumentation/instrumentation-java-other-ns/03-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-java-other-ns/03-assert.yaml @@ -24,7 +24,7 @@ spec: - name: SPLUNK_PROFILER_ENABLED value: "false" - name: JAVA_TOOL_OPTIONS - value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar' - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT diff --git a/tests/e2e-instrumentation/instrumentation-java-tls/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-tls/01-assert.yaml index 7ddecadb47..6cb4d2d206 100644 --- a/tests/e2e-instrumentation/instrumentation-java-tls/01-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-java-tls/01-assert.yaml @@ -17,7 +17,7 @@ spec: fieldRef: fieldPath: status.podIP - name: JAVA_TOOL_OPTIONS - value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar' - name: OTEL_SERVICE_NAME value: my-java - name: OTEL_EXPORTER_OTLP_ENDPOINT diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/00-install-collector.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/00-install-collector.yaml new file mode 100644 index 0000000000..34a26ebb2c --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/00-install-collector.yaml @@ -0,0 +1,22 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: sidecar +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + exporters: [debug] + mode: sidecar diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/00-install-instrumentation.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/00-install-instrumentation.yaml new file mode 100644 index 0000000000..f77a3f835f --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/00-install-instrumentation.yaml @@ -0,0 +1,34 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: Instrumentation +metadata: + name: java +spec: + env: + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + exporter: + endpoint: http://localhost:4317 + propagators: + - jaeger + - b3 + sampler: + type: parentbased_traceidratio + argument: "0.25" + java: + env: + - name: OTEL_JAVAAGENT_DEBUG + value: "true" + - name: OTEL_INSTRUMENTATION_JDBC_ENABLED + value: "false" + - name: SPLUNK_PROFILER_ENABLED + value: "false" diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/01-assert.yaml new file mode 100644 index 0000000000..3766b5a7aa --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/01-assert.yaml @@ -0,0 +1,80 @@ +apiVersion: v1 +kind: Pod +metadata: + annotations: + instrumentation.opentelemetry.io/inject-java: "true" + sidecar.opentelemetry.io/inject: "true" + labels: + app: my-java +spec: + containers: + - env: + - name: OTEL_NODE_IP + valueFrom: + fieldRef: + fieldPath: status.hostIP + - name: OTEL_POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP + - name: OTEL_SERVICE_NAME + valueFrom: + configMapKeyRef: + name: my-conf + key: otel-service-name + - name: JAVA_TOOL_OPTIONS + value: '-XX:+UseContainerSupport -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + - name: OTEL_JAVAAGENT_DEBUG + value: "true" + - name: OTEL_INSTRUMENTATION_JDBC_ENABLED + value: "false" + - name: SPLUNK_PROFILER_ENABLED + value: "false" + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + - name: OTEL_RESOURCE_ATTRIBUTES_POD_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: metadata.name + - name: OTEL_RESOURCE_ATTRIBUTES_NODE_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: spec.nodeName + - name: OTEL_PROPAGATORS + value: jaeger,b3 + - name: OTEL_RESOURCE_ATTRIBUTES + name: myapp + volumeMounts: + - mountPath: /var/run/secrets/kubernetes.io/serviceaccount + readOnly: true + - mountPath: /otel-auto-instrumentation-java + name: opentelemetry-auto-instrumentation-java + - args: + - --config=env:OTEL_CONFIG + name: otc-container + initContainers: + - name: opentelemetry-auto-instrumentation-java +status: + containerStatuses: + - name: myapp + ready: true + started: true + - name: otc-container + ready: true + started: true + initContainerStatuses: + - name: opentelemetry-auto-instrumentation-java + ready: true + phase: Running diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/01-install-app.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/01-install-app.yaml new file mode 100644 index 0000000000..dad37faa8a --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/01-install-app.yaml @@ -0,0 +1,47 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: my-conf +data: + otel-service-name: my-app + java-tool-options: -XX:+UseContainerSupport +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-java +spec: + selector: + matchLabels: + app: my-java + replicas: 1 + template: + metadata: + labels: + app: my-java + annotations: + sidecar.opentelemetry.io/inject: "true" + instrumentation.opentelemetry.io/inject-java: "true" + spec: + securityContext: + runAsUser: 1000 + runAsGroup: 3000 + fsGroup: 3000 + containers: + - name: myapp + image: ghcr.io/open-telemetry/opentelemetry-operator/e2e-test-app-java:main + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: ["ALL"] + env: + - name: OTEL_SERVICE_NAME + valueFrom: + configMapKeyRef: + name: my-conf + key: otel-service-name + - name: JAVA_TOOL_OPTIONS + valueFrom: + configMapKeyRef: + name: my-conf + key: java-tool-options diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/chainsaw-test.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/chainsaw-test.yaml new file mode 100755 index 0000000000..14d625ca96 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-configmap/chainsaw-test.yaml @@ -0,0 +1,40 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: instrumentation-java-valuefrom-configmap +spec: + steps: + - name: step-00 + try: + # In OpenShift, when a namespace is created, all necessary SCC annotations are automatically added. However, if a namespace is created using a resource file with only selected SCCs, the other auto-added SCCs are not included. Therefore, the UID-range and supplemental groups SCC annotations must be set after the namespace is created. + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.uid-range=1000/1000 + - --overwrite + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.supplemental-groups=3000/3000 + - --overwrite + - apply: + file: 00-install-collector.yaml + - apply: + file: 00-install-instrumentation.yaml + - name: step-01 + try: + - apply: + file: 01-install-app.yaml + - assert: + file: 01-assert.yaml + catch: + - podLogs: + selector: app=my-java \ No newline at end of file diff --git a/tests/e2e-instrumentation/instrumentation-java/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java/01-assert.yaml index cd8a8a37fe..57aa726e6f 100644 --- a/tests/e2e-instrumentation/instrumentation-java/01-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-java/01-assert.yaml @@ -24,7 +24,7 @@ spec: - name: SPLUNK_PROFILER_ENABLED value: "false" - name: JAVA_TOOL_OPTIONS - value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar' - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT diff --git a/tests/e2e-instrumentation/instrumentation-nodejs-multicontainer/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-nodejs-multicontainer/01-assert.yaml index 7da5c15637..74e9e409d2 100644 --- a/tests/e2e-instrumentation/instrumentation-nodejs-multicontainer/01-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-nodejs-multicontainer/01-assert.yaml @@ -21,7 +21,7 @@ spec: - name: NODE_PATH value: /usr/local/lib/node_modules - name: NODE_OPTIONS - value: ' --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' + value: '--require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT @@ -65,7 +65,7 @@ spec: fieldRef: fieldPath: status.podIP - name: NODE_OPTIONS - value: ' --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' + value: '--require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT diff --git a/tests/e2e-instrumentation/instrumentation-nodejs-multicontainer/02-assert.yaml b/tests/e2e-instrumentation/instrumentation-nodejs-multicontainer/02-assert.yaml index 0738786abd..459269061a 100644 --- a/tests/e2e-instrumentation/instrumentation-nodejs-multicontainer/02-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-nodejs-multicontainer/02-assert.yaml @@ -32,7 +32,7 @@ spec: - name: NODE_PATH value: /usr/local/lib/node_modules - name: NODE_OPTIONS - value: ' --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' + value: '--require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT diff --git a/tests/e2e-instrumentation/instrumentation-nodejs-volume/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-nodejs-volume/01-assert.yaml index 227a175150..f98e8c35f6 100644 --- a/tests/e2e-instrumentation/instrumentation-nodejs-volume/01-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-nodejs-volume/01-assert.yaml @@ -22,7 +22,7 @@ spec: - name: OTEL_NODEJS_DEBUG value: "true" - name: NODE_OPTIONS - value: " --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js" + value: "--require /otel-auto-instrumentation-nodejs/autoinstrumentation.js" - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT diff --git a/tests/e2e-instrumentation/instrumentation-nodejs/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-nodejs/01-assert.yaml index 719727ca68..3d02c0cecc 100644 --- a/tests/e2e-instrumentation/instrumentation-nodejs/01-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-nodejs/01-assert.yaml @@ -22,7 +22,7 @@ spec: - name: OTEL_NODEJS_DEBUG value: "true" - name: NODE_OPTIONS - value: ' --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' + value: '--require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' - name: OTEL_TRACES_EXPORTER value: otlp - name: OTEL_EXPORTER_OTLP_ENDPOINT diff --git a/tests/e2e-multi-instrumentation/instrumentation-multi-multicontainer/01-assert.yaml b/tests/e2e-multi-instrumentation/instrumentation-multi-multicontainer/01-assert.yaml index 3ba921ada1..aea4d311fb 100644 --- a/tests/e2e-multi-instrumentation/instrumentation-multi-multicontainer/01-assert.yaml +++ b/tests/e2e-multi-instrumentation/instrumentation-multi-multicontainer/01-assert.yaml @@ -89,7 +89,7 @@ spec: - name: OTEL_SERVICE_NAME value: javaapp - name: JAVA_TOOL_OPTIONS - value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar' - name: OTEL_TRACES_SAMPLER value: parentbased_traceidratio - name: OTEL_TRACES_SAMPLER_ARG @@ -131,7 +131,7 @@ spec: - name: OTEL_SERVICE_NAME value: nodejsapp - name: NODE_OPTIONS - value: ' --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' + value: '--require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' - name: OTEL_TRACES_SAMPLER value: parentbased_traceidratio - name: OTEL_TRACES_SAMPLER_ARG diff --git a/tests/e2e-multi-instrumentation/instrumentation-single-instr-first-container/01-assert.yaml b/tests/e2e-multi-instrumentation/instrumentation-single-instr-first-container/01-assert.yaml index 704e8b12a7..c865cb40bd 100644 --- a/tests/e2e-multi-instrumentation/instrumentation-single-instr-first-container/01-assert.yaml +++ b/tests/e2e-multi-instrumentation/instrumentation-single-instr-first-container/01-assert.yaml @@ -22,7 +22,7 @@ spec: - name: OTEL_SERVICE_NAME value: nodejsapp - name: NODE_OPTIONS - value: ' --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' + value: '--require /otel-auto-instrumentation-nodejs/autoinstrumentation.js' - name: OTEL_TRACES_SAMPLER value: parentbased_traceidratio - name: OTEL_TRACES_SAMPLER_ARG From 3880a4db66563577b392b539dddb5fdf90336981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Old=C5=99ich=20Jedli=C4=8Dka?= Date: Fri, 18 Oct 2024 22:46:40 +0200 Subject: [PATCH 2/2] Add handling of environment variables in Secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Load also environment variables defined in Secret to make them visible to OpenTelemetry Operator. Signed-off-by: Oldřich Jedlička --- .chloggen/enhancement-envs-from-secret.yaml | 19 +++ pkg/instrumentation/container.go | 159 ++++++++++++------ pkg/instrumentation/container_test.go | 16 +- .../00-install-collector.yaml | 22 +++ .../00-install-instrumentation.yaml | 34 ++++ .../01-assert.yaml | 78 +++++++++ .../01-install-app.yaml | 39 +++++ .../chainsaw-test.yaml | 40 +++++ .../00-install-collector.yaml | 22 +++ .../00-install-instrumentation.yaml | 34 ++++ .../01-assert.yaml | 80 +++++++++ .../01-install-app.yaml | 47 ++++++ .../chainsaw-test.yaml | 40 +++++ 13 files changed, 567 insertions(+), 63 deletions(-) create mode 100644 .chloggen/enhancement-envs-from-secret.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-envfrom-secret/00-install-collector.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-envfrom-secret/00-install-instrumentation.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-envfrom-secret/01-assert.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-envfrom-secret/01-install-app.yaml create mode 100755 tests/e2e-instrumentation/instrumentation-java-envfrom-secret/chainsaw-test.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/00-install-collector.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/00-install-instrumentation.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/01-assert.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/01-install-app.yaml create mode 100755 tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/chainsaw-test.yaml diff --git a/.chloggen/enhancement-envs-from-secret.yaml b/.chloggen/enhancement-envs-from-secret.yaml new file mode 100644 index 0000000000..9ee6dbe8d4 --- /dev/null +++ b/.chloggen/enhancement-envs-from-secret.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 Secret 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.secretKeyRef` and + `envFrom.secretRef` fields, so the auto-instrumentation will be able to prevent overriding them, or it will be able to + extend the existing definition. diff --git a/pkg/instrumentation/container.go b/pkg/instrumentation/container.go index 75b9366079..434d48c1d1 100644 --- a/pkg/instrumentation/container.go +++ b/pkg/instrumentation/container.go @@ -33,6 +33,7 @@ type Container struct { index int inheritedEnv map[string]string configMaps map[string]*corev1.ConfigMap + secrets map[string]*corev1.Secret } func NewContainer(client client.Reader, ctx context.Context, logger logr.Logger, namespace string, pod *corev1.Pod, index int) (Container, error) { @@ -42,22 +43,17 @@ func NewContainer(client client.Reader, ctx context.Context, logger logr.Logger, container := &pod.Spec.Containers[index] configMaps := make(map[string]*corev1.ConfigMap) + secrets := make(map[string]*corev1.Secret) inheritedEnv := make(map[string]string) for _, envsFrom := range container.EnvFrom { if envsFrom.ConfigMapRef != nil { - prefix := envsFrom.Prefix - name := envsFrom.ConfigMapRef.Name - if cm, err := getOrLoadResource(client, ctx, namespace, configMaps, name); err == nil { - for k, v := range cm.Data { - // Safely overwrite the value, last one from EnvFrom wins in Kubernetes, with the direct value - // from the container itself taking precedence - inheritedEnv[prefix+k] = v - } - } else if envsFrom.ConfigMapRef.Optional == nil || !*envsFrom.ConfigMapRef.Optional { - return Container{}, fmt.Errorf("failed to load environment variables: %w", err) + if err := loadAllEnvVars(client, ctx, namespace, configMaps, envsFrom.ConfigMapRef.Name, envsFrom.Prefix, envsFrom.ConfigMapRef.Optional, inheritedEnv); err != nil { + return Container{}, err } } else if envsFrom.SecretRef != nil { - logger.V(2).Info("ignoring SecretRef in EnvFrom", "container", container.Name, "secret", envsFrom.SecretRef.Name) + if err := loadAllEnvVars(client, ctx, namespace, secrets, envsFrom.SecretRef.Name, envsFrom.Prefix, envsFrom.SecretRef.Optional, inheritedEnv); err != nil { + return Container{}, err + } } } @@ -73,39 +69,10 @@ func NewContainer(client client.Reader, ctx context.Context, logger logr.Logger, index: index, inheritedEnv: inheritedEnv, configMaps: configMaps, + secrets: secrets, }, nil } -func getOrLoadResource[T any, PT interface { - client.Object - *T -}](client client.Reader, ctx context.Context, namespace string, cache map[string]*T, name string) (*T, error) { - var obj T - if cached, ok := cache[name]; ok { - if cached != nil { - return cached, nil - } else { - return nil, fmt.Errorf("failed to get %s %s/%s", reflect.TypeOf(obj).Name(), namespace, name) - } - } - - if client == nil || ctx == nil { - // Cache error value - cache[name] = nil - return nil, fmt.Errorf("client or context is nil, cannot load %s %s/%s", reflect.TypeOf(obj).Name(), namespace, name) - } - - err := client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, PT(&obj)) - if err != nil { - // Cache error value - cache[name] = nil - return nil, fmt.Errorf("failed to get %s %s/%s: %w", reflect.TypeOf(obj).Name(), namespace, name, err) - } - - cache[name] = &obj - return &obj, nil -} - func (c *Container) validate(pod *corev1.Pod, envsToBeValidated ...string) error { // Try if the value is resolvable for _, envToBeValidated := range envsToBeValidated { @@ -169,21 +136,9 @@ func (c *Container) getOrMakeEnvVar(pod *corev1.Pod, name string) (corev1.EnvVar func (c *Container) resolveEnvVar(envVar corev1.EnvVar) (corev1.EnvVar, error) { if envVar.Value == "" && envVar.ValueFrom != nil { if envVar.ValueFrom.ConfigMapKeyRef != nil { - configMapName := envVar.ValueFrom.ConfigMapKeyRef.Name - configMapKey := envVar.ValueFrom.ConfigMapKeyRef.Key - if cm, err := getOrLoadResource(c.client, c.ctx, c.namespace, c.configMaps, configMapName); err == nil { - if value, ok := cm.Data[configMapKey]; ok { - return corev1.EnvVar{Name: envVar.Name, Value: value}, nil - } else if envVar.ValueFrom.ConfigMapKeyRef.Optional == nil || !*envVar.ValueFrom.ConfigMapKeyRef.Optional { - return corev1.EnvVar{}, fmt.Errorf("failed to resolve environment variable %s, key %s not found in ConfigMap %s/%s", envVar.Name, configMapKey, c.namespace, configMapName) - } else { - return corev1.EnvVar{Name: envVar.Name, Value: ""}, nil - } - } else if envVar.ValueFrom.ConfigMapKeyRef.Optional == nil || !*envVar.ValueFrom.ConfigMapKeyRef.Optional { - return corev1.EnvVar{}, fmt.Errorf("failed to resolve environment variable %s: %w", envVar.Name, err) - } else { - return corev1.EnvVar{Name: envVar.Name, Value: ""}, nil - } + return loadEnvVar(c.client, c.ctx, c.namespace, c.configMaps, envVar.Name, envVar.ValueFrom.ConfigMapKeyRef.Name, envVar.ValueFrom.ConfigMapKeyRef.Key, envVar.ValueFrom.ConfigMapKeyRef.Optional) + } else if envVar.ValueFrom.SecretKeyRef != nil { + return loadEnvVar(c.client, c.ctx, c.namespace, c.secrets, envVar.Name, envVar.ValueFrom.SecretKeyRef.Name, envVar.ValueFrom.SecretKeyRef.Key, envVar.ValueFrom.SecretKeyRef.Optional) } else { v := reflect.ValueOf(*envVar.ValueFrom) for i := 0; i < v.NumField(); i++ { @@ -197,6 +152,98 @@ func (c *Container) resolveEnvVar(envVar corev1.EnvVar) (corev1.EnvVar, error) { return envVar, nil } +func getOrLoadResource[T any, PT interface { + client.Object + *T +}](client client.Reader, ctx context.Context, namespace string, cache map[string]*T, name string) (*T, error) { + var obj T + if cached, ok := cache[name]; ok { + if cached != nil { + return cached, nil + } else { + return nil, fmt.Errorf("failed to get %s %s/%s", reflect.TypeOf(obj).Name(), namespace, name) + } + } + + if client == nil || ctx == nil { + // Cache error value + cache[name] = nil + return nil, fmt.Errorf("client or context is nil, cannot load %s %s/%s", reflect.TypeOf(obj).Name(), namespace, name) + } + + err := client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, PT(&obj)) + if err != nil { + // Cache error value + cache[name] = nil + return nil, fmt.Errorf("failed to get %s %s/%s: %w", reflect.TypeOf(obj).Name(), namespace, name, err) + } + + cache[name] = &obj + return &obj, nil +} + +func loadAllEnvVars[T any, PT interface { + client.Object + *T +}](client client.Reader, ctx context.Context, namespace string, cache map[string]*T, name string, prefix string, optional *bool, inheritedEnv map[string]string) error { + if obj, err := getOrLoadResource[T, PT](client, ctx, namespace, cache, name); err == nil { + return fillEnvVars(obj, prefix, inheritedEnv) + } else if optional == nil || !*optional { + return fmt.Errorf("failed to load environment variables: %w", err) + } else { + return nil + } +} + +func loadEnvVar[T any, PT interface { + client.Object + *T +}](client client.Reader, ctx context.Context, namespace string, cache map[string]*T, variable string, name string, key string, optional *bool) (corev1.EnvVar, error) { + if resource, err := getOrLoadResource[T, PT](client, ctx, namespace, cache, name); err == nil { + if value, ok := getResourceValue(resource, key); ok { + return corev1.EnvVar{Name: variable, Value: value}, nil + } else if optional == nil || !*optional { + return corev1.EnvVar{}, fmt.Errorf("failed to resolve environment variable %s, key %s not found in %s %s/%s", variable, key, reflect.TypeOf(*resource).Name(), namespace, name) + } else { + return corev1.EnvVar{Name: variable, Value: ""}, nil + } + } else if optional == nil || !*optional { + return corev1.EnvVar{}, fmt.Errorf("failed to resolve environment variable %s: %w", variable, err) + } else { + return corev1.EnvVar{Name: variable, Value: ""}, nil + } +} + +func fillEnvVars(obj any, prefix string, inheritedEnv map[string]string) error { + switch o := obj.(type) { + case *corev1.ConfigMap: + for k, v := range o.Data { + inheritedEnv[prefix+k] = v + } + return nil + case *corev1.Secret: + for k, v := range o.Data { + inheritedEnv[prefix+k] = string(v) + } + return nil + default: + return fmt.Errorf("unsupported type %T", obj) + } +} + +func getResourceValue(obj interface{}, key string) (string, bool) { + switch o := obj.(type) { + case *corev1.ConfigMap: + val, ok := o.Data[key] + return val, ok + case *corev1.Secret: + val, ok := o.Data[key] + return string(val), ok + default: + return "", false + } +} + func existsEnvVarInEnv(env []corev1.EnvVar, name string) bool { for i := range env { if env[i].Name == name { diff --git a/pkg/instrumentation/container_test.go b/pkg/instrumentation/container_test.go index f512dbe8f5..cd5e014f6b 100644 --- a/pkg/instrumentation/container_test.go +++ b/pkg/instrumentation/container_test.go @@ -377,7 +377,7 @@ func TestInheritedEnv(t *testing.T) { err: "failed to get ConfigMap inheritedenv-notfoundexplicit/my-config", }, { - name: "SecretRef not supported", + name: "Simple SecretRef usage", ns: corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "inheritedenv-secretref", @@ -410,7 +410,9 @@ func TestInheritedEnv(t *testing.T) { }, }, }, - log: "ignoring SecretRef in EnvFrom", + expected: map[string]string{ + "OTEL_ENVFROM_SECRET_VALUE1": "my-secret-value1", + }, }, } @@ -1011,7 +1013,7 @@ func TestResolving(t *testing.T) { Namespace: "validations", }, Data: map[string][]byte{ - "secret-ref-value1": []byte("my-valuefrom-value1"), + "secret-ref-value1": []byte("my-secret-valuefrom-value1"), }, }, } @@ -1151,10 +1153,10 @@ func TestResolving(t *testing.T) { expectedResolve: "", }, { - name: "Test unsupported existing Secret variable", - variable: "OTEL_ENV_VALUEFROM_SECRET1", - expectedExists: true, - err: "the container defines env var value via ValueFrom.SecretKeyRef, envVar: OTEL_ENV_VALUEFROM_SECRET1", + name: "Test unsupported existing Secret variable", + variable: "OTEL_ENV_VALUEFROM_SECRET1", + expectedExists: true, + expectedResolve: "my-secret-valuefrom-value1", }, { name: "Test unsupported existing FieldRef variable", diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/00-install-collector.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/00-install-collector.yaml new file mode 100644 index 0000000000..34a26ebb2c --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/00-install-collector.yaml @@ -0,0 +1,22 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: sidecar +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + exporters: [debug] + mode: sidecar diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/00-install-instrumentation.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/00-install-instrumentation.yaml new file mode 100644 index 0000000000..f77a3f835f --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/00-install-instrumentation.yaml @@ -0,0 +1,34 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: Instrumentation +metadata: + name: java +spec: + env: + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + exporter: + endpoint: http://localhost:4317 + propagators: + - jaeger + - b3 + sampler: + type: parentbased_traceidratio + argument: "0.25" + java: + env: + - name: OTEL_JAVAAGENT_DEBUG + value: "true" + - name: OTEL_INSTRUMENTATION_JDBC_ENABLED + value: "false" + - name: SPLUNK_PROFILER_ENABLED + value: "false" diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/01-assert.yaml new file mode 100644 index 0000000000..895ddf9d90 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/01-assert.yaml @@ -0,0 +1,78 @@ +apiVersion: v1 +kind: Pod +metadata: + annotations: + instrumentation.opentelemetry.io/inject-java: "true" + sidecar.opentelemetry.io/inject: "true" + labels: + app: my-java +spec: + containers: + - env: + - name: OTEL_NODE_IP + valueFrom: + fieldRef: + fieldPath: status.hostIP + - name: OTEL_POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP + - name: OTEL_JAVAAGENT_DEBUG + value: "true" + - name: OTEL_INSTRUMENTATION_JDBC_ENABLED + value: "false" + - name: SPLUNK_PROFILER_ENABLED + value: "false" + - name: JAVA_TOOL_OPTIONS + value: '-XX:+UseContainerSupport -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + - name: OTEL_RESOURCE_ATTRIBUTES_POD_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: metadata.name + - name: OTEL_RESOURCE_ATTRIBUTES_NODE_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: spec.nodeName + - name: OTEL_PROPAGATORS + value: jaeger,b3 + - name: OTEL_RESOURCE_ATTRIBUTES + envFrom: + - secretRef: + name: my-conf + name: myapp + volumeMounts: + - mountPath: /var/run/secrets/kubernetes.io/serviceaccount + readOnly: true + - mountPath: /otel-auto-instrumentation-java + name: opentelemetry-auto-instrumentation-java + - args: + - --config=env:OTEL_CONFIG + name: otc-container + initContainers: + - name: opentelemetry-auto-instrumentation-java +status: + containerStatuses: + - name: myapp + ready: true + started: true + - name: otc-container + ready: true + started: true + initContainerStatuses: + - name: opentelemetry-auto-instrumentation-java + ready: true + phase: Running diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/01-install-app.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/01-install-app.yaml new file mode 100644 index 0000000000..e6b7378fd2 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/01-install-app.yaml @@ -0,0 +1,39 @@ +apiVersion: v1 +kind: Secret +metadata: + name: my-conf +data: + OTEL_SERVICE_NAME: bXktYXBw + JAVA_TOOL_OPTIONS: LVhYOitVc2VDb250YWluZXJTdXBwb3J0 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-java +spec: + selector: + matchLabels: + app: my-java + replicas: 1 + template: + metadata: + labels: + app: my-java + annotations: + sidecar.opentelemetry.io/inject: "true" + instrumentation.opentelemetry.io/inject-java: "true" + spec: + securityContext: + runAsUser: 1000 + runAsGroup: 3000 + fsGroup: 3000 + containers: + - name: myapp + image: ghcr.io/open-telemetry/opentelemetry-operator/e2e-test-app-java:main + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: ["ALL"] + envFrom: + - secretRef: + name: my-conf diff --git a/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/chainsaw-test.yaml b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/chainsaw-test.yaml new file mode 100755 index 0000000000..87e2579c95 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-envfrom-secret/chainsaw-test.yaml @@ -0,0 +1,40 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: instrumentation-java-envfrom-secret +spec: + steps: + - name: step-00 + try: + # In OpenShift, when a namespace is created, all necessary SCC annotations are automatically added. However, if a namespace is created using a resource file with only selected SCCs, the other auto-added SCCs are not included. Therefore, the UID-range and supplemental groups SCC annotations must be set after the namespace is created. + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.uid-range=1000/1000 + - --overwrite + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.supplemental-groups=3000/3000 + - --overwrite + - apply: + file: 00-install-collector.yaml + - apply: + file: 00-install-instrumentation.yaml + - name: step-01 + try: + - apply: + file: 01-install-app.yaml + - assert: + file: 01-assert.yaml + catch: + - podLogs: + selector: app=my-java \ No newline at end of file diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/00-install-collector.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/00-install-collector.yaml new file mode 100644 index 0000000000..34a26ebb2c --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/00-install-collector.yaml @@ -0,0 +1,22 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: sidecar +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + exporters: [debug] + mode: sidecar diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/00-install-instrumentation.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/00-install-instrumentation.yaml new file mode 100644 index 0000000000..f77a3f835f --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/00-install-instrumentation.yaml @@ -0,0 +1,34 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: Instrumentation +metadata: + name: java +spec: + env: + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + exporter: + endpoint: http://localhost:4317 + propagators: + - jaeger + - b3 + sampler: + type: parentbased_traceidratio + argument: "0.25" + java: + env: + - name: OTEL_JAVAAGENT_DEBUG + value: "true" + - name: OTEL_INSTRUMENTATION_JDBC_ENABLED + value: "false" + - name: SPLUNK_PROFILER_ENABLED + value: "false" diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/01-assert.yaml new file mode 100644 index 0000000000..7619b83e1a --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/01-assert.yaml @@ -0,0 +1,80 @@ +apiVersion: v1 +kind: Pod +metadata: + annotations: + instrumentation.opentelemetry.io/inject-java: "true" + sidecar.opentelemetry.io/inject: "true" + labels: + app: my-java +spec: + containers: + - env: + - name: OTEL_NODE_IP + valueFrom: + fieldRef: + fieldPath: status.hostIP + - name: OTEL_POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP + - name: OTEL_SERVICE_NAME + valueFrom: + secretKeyRef: + name: my-conf + key: otel-service-name + - name: JAVA_TOOL_OPTIONS + value: '-XX:+UseContainerSupport -javaagent:/otel-auto-instrumentation-java/javaagent.jar' + - name: OTEL_JAVAAGENT_DEBUG + value: "true" + - name: OTEL_INSTRUMENTATION_JDBC_ENABLED + value: "false" + - name: SPLUNK_PROFILER_ENABLED + value: "false" + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + - name: OTEL_RESOURCE_ATTRIBUTES_POD_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: metadata.name + - name: OTEL_RESOURCE_ATTRIBUTES_NODE_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: spec.nodeName + - name: OTEL_PROPAGATORS + value: jaeger,b3 + - name: OTEL_RESOURCE_ATTRIBUTES + name: myapp + volumeMounts: + - mountPath: /var/run/secrets/kubernetes.io/serviceaccount + readOnly: true + - mountPath: /otel-auto-instrumentation-java + name: opentelemetry-auto-instrumentation-java + - args: + - --config=env:OTEL_CONFIG + name: otc-container + initContainers: + - name: opentelemetry-auto-instrumentation-java +status: + containerStatuses: + - name: myapp + ready: true + started: true + - name: otc-container + ready: true + started: true + initContainerStatuses: + - name: opentelemetry-auto-instrumentation-java + ready: true + phase: Running diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/01-install-app.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/01-install-app.yaml new file mode 100644 index 0000000000..916d58e049 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/01-install-app.yaml @@ -0,0 +1,47 @@ +apiVersion: v1 +kind: Secret +metadata: + name: my-conf +data: + otel-service-name: bXktYXBw + java-tool-options: LVhYOitVc2VDb250YWluZXJTdXBwb3J0 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-java +spec: + selector: + matchLabels: + app: my-java + replicas: 1 + template: + metadata: + labels: + app: my-java + annotations: + sidecar.opentelemetry.io/inject: "true" + instrumentation.opentelemetry.io/inject-java: "true" + spec: + securityContext: + runAsUser: 1000 + runAsGroup: 3000 + fsGroup: 3000 + containers: + - name: myapp + image: ghcr.io/open-telemetry/opentelemetry-operator/e2e-test-app-java:main + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: ["ALL"] + env: + - name: OTEL_SERVICE_NAME + valueFrom: + secretKeyRef: + name: my-conf + key: otel-service-name + - name: JAVA_TOOL_OPTIONS + valueFrom: + secretKeyRef: + name: my-conf + key: java-tool-options diff --git a/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/chainsaw-test.yaml b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/chainsaw-test.yaml new file mode 100755 index 0000000000..0fc9729539 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-java-valuefrom-secret/chainsaw-test.yaml @@ -0,0 +1,40 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: instrumentation-java-valuefrom-secret +spec: + steps: + - name: step-00 + try: + # In OpenShift, when a namespace is created, all necessary SCC annotations are automatically added. However, if a namespace is created using a resource file with only selected SCCs, the other auto-added SCCs are not included. Therefore, the UID-range and supplemental groups SCC annotations must be set after the namespace is created. + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.uid-range=1000/1000 + - --overwrite + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.supplemental-groups=3000/3000 + - --overwrite + - apply: + file: 00-install-collector.yaml + - apply: + file: 00-install-instrumentation.yaml + - name: step-01 + try: + - apply: + file: 01-install-app.yaml + - assert: + file: 01-assert.yaml + catch: + - podLogs: + selector: app=my-java \ No newline at end of file