Environment variable expansion in spec.config.service.telemetry.metrics.address doesn't work

[douglascamata]

Component(s)

collector

What happened?

Description

Looks like the shell environment variable expansion breaks config validation when used in spec.config.service.telemetry.metrics.address.

It doesn't break the config validation when used in other places, like spec.config.receivers.otlp.protocols.grpc.endpoint.

Steps to Reproduce

1. Install cert manager operator (optional)
2. Install OTEL operator v0.114.0
3. Try to deploy a collector using the manifest below:

apiVersion: opentelemetry.io/v1beta1
kind: OpenTelemetryCollector
metadata:
  name: simplest
spec:
  config:
    receivers:
      otlp:
        protocols:
          grpc:
            endpoint: ${env:POD_IP}:4317
          http:
            endpoint: ${env:POD_IP}:4318
    processors:
      memory_limiter:
        check_interval: 1s
        limit_percentage: 75
        spike_limit_percentage: 15
      batch:
        send_batch_size: 10000
        timeout: 10s
    exporters:
      debug: {}
    service:
      telemetry:
        metrics:
          address: ${env:POD_IP}:8888
      pipelines:
        traces:
          receivers: [otlp]
          processors: [memory_limiter, batch]
          exporters: [debug]

Expected Result

The Collector is created.

Actual Result

It returns the error Error from server (Forbidden): error when creating "collector_crd.yaml": admission webhook "mopentelemetrycollectorbeta.kb.io" denied the request: address ${env:POD_IP}:8888: too many colons in address

Kubernetes Version

1.31.2

Operator version

0.114.0

Collector version

0.114.0

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

Log output

No response

Additional context

According to https://github.com/open-telemetry/opentelemetry-helm-charts/blob/831603f1351049a65550ec3211d8e3a44a0c5a2a/charts/opentelemetry-collector/UPGRADING.md#0782-to-0783, it would be a good idea to update all the examples to use ${env:POD_IP} instead of listening on 0.0.0.0.

[PePoDev]

mine workaround

manager:
  env:
    ENABLE_WEBHOOKS: "false"

[frzifus]

I didnt test it yet, but did you try:

-          address: ${env:POD_IP}:8888
+          address: "${env:POD_IP}:8888"

[albertlagman]

Same problem. Adding double quotes does not solve the issue.

[swiatekm]

The webhook really shouldn't reject the Collector CR just because it can't parse the metrics port. This happens because manifest building fails here, again because we error if we fail to parse the endpoint, whereas we should emit a warning and proceed.

[douglascamata]

@swiatekm agreed with everything you said. I have a PR that I'm working on to fix the issue. Will add the warning log as you mentioned after I fix some linting issues and add a few more tests over there.

[swiatekm]

For reference, we ended up returning the error after all. Creating the metrics Service (and ServiceMonitor, if present) is controlled by the Collector CR attribute observability.metrics.enableMetrics. It's better to inform the user this won't work with a good error message, giving them the opportunity to disable this feature, than silently fail.