Add a config option for enabling target allocator mTLS

[swiatekm]

Component(s)

target allocator

Describe the issue you're reporting

Target Allocator has a feature where it can talk to the collector over mTLS, securing the connection and allowing metric endpoint credentials to be transferred over it. This feature is currently hidden behind an alpha feature flag operator.targetallocator.mtls and depends on cert-manager for provisioning the actual certificates.

It's a feature flag because the intent was to eventually enable it for everyone and be secure by default. However, the feature turned out to be quite complex and has resulted in several subtle bugs that were only uncovered after sustained production use. After discussing this with @jaronoff97 and @TylerHelmuth, we prefer to make it configurable via an attribute on the OpenTelemetryCollector and TargetAllocator CRDs, and disabled by default. Our reasons are as follows:

• The complexity of the mTLS setup is significant and can produce subtle bugs.
• Authenticated metrics endpoints are relatively niche, so we'd be foisting this complexity on the majority of users without much benefit to them.
• In terms of the default security of scrape targets, we're currently in line with prometheus-operator. The Prometheus web UI isn't secure by default either.

We don't want to completely close the door on enabling the option by default in some farther future, but it should definitely remain configurable.

@open-telemetry/operator-approvers @CharlieTLe please comment if you have a contrary opinion.

Tip

_{React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it. Learn more here.}

[CharlieTLe]

Thanks for making this issue, @swiatekm.

Authenticated metrics endpoints are relatively niche, so we'd be foisting this complexity on the majority of users without much benefit to them.

Regarding the use of authenticated metrics endpoints, it's needed for scraping the kubelet, cadvisor, and kubernetes-apiserver. Given that these are quite common to scrape for understanding the health of the cluster, I'd say that it's a common use-case.

Here are some examples of ScrapeConfig and ServiceMonitor that let the TA give the targets to the collector with the needed info to authenticate and scrape.

apiVersion: monitoring.coreos.com/v1alpha1
kind: ScrapeConfig
metadata:
  name: cadvisor
spec:
  scheme: HTTPS
  kubernetesSDConfigs:
    - role: Node
  authorization:
    type: Bearer
    credentials:
      key: "token"
      name: opentelemetry-collector
  tlsConfig:
    ca:
      secret:
        key: "ca.crt"
        name: opentelemetry-collector
  metricsPath: /metrics/cadvisor
  honorLabels: true
  sampleLimit: 300000
  relabelings: 
    - action: replace
      sourceLabels:
      - __metrics_path__
      targetLabel: metrics_path

apiVersion: monitoring.coreos.com/v1alpha1
kind: ScrapeConfig
metadata:
  name: kubelet
spec:
  scheme: HTTPS
  kubernetesSDConfigs:
    - role: Node
  authorization:
    type: Bearer
    credentials:
      key: "token"
      name: opentelemetry-collector
  tlsConfig:
    ca:
      secret:
        key: "ca.crt"
        name: opentelemetry-collector
  relabelings: 
    - action: labelmap
      regex: __meta_kubernetes_node_label_(.+)
      replacement: $1
      separator: ;
    - action: replace
      regex: (.*)
      replacement: kubernetes.default.svc:443
      separator: ;
      targetLabel: __address__
    - action: replace
      regex: (.+)
      replacement: /api/v1/nodes/$1/proxy/metrics
      separator: ;
      sourceLabels:
      - __meta_kubernetes_node_name
      targetLabel: __metrics_path__

apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  name: kubernetes-apiserver
spec:
  endpoints:
    - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
      honorLabels: true
      interval: 30s
      port: https
      scheme: https
      tlsConfig:
        caFile: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt
        serverName: kubernetes
  jobLabel: component
  namespaceSelector:
    matchNames:
      - default
  sampleLimit: 300000
  selector:
    matchLabels:
      component: apiserver
      provider: kubernetes

Of course, the service account used by the collector needs sufficient privileges to use its token to authenticate with the Kubernetes API server, so I'll include this for completeness.

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: opentelemetry-collector
rules:
  - apiGroups: [""]
    resources:
      - nodes/proxy
      - nodes/metrics
    verbs: ["get"]
  - nonResourceURLs:
      - /metrics
    verbs:
      - get

Enabling mTLS on the TA is actually required for the above scrape configurations to work correctly. When a collector asks for the scrape configuration related to any of the above examples, the TA will only send the scrape configuration with the secret data consisting of the certificate or service account token when mTLS is enabled. See:

opentelemetry-operator/cmd/otel-allocator/internal/server/server.go

Lines 243 to 246 in 8840b89

	result := s.scrapeConfigResponse
	if c.Request.TLS != nil {
	result = s.ScrapeConfigMarshalledSecretResponse
	}

When mTLS is disabled on the TA, there's a gap in functionality that a user would expect with these common scenarios like scraping the kubelet, Kubernetes API server, or cadvisor for information related to their cluster.

[swiatekm]

@CharlieTLe I believe you can get that to work using bearerTokenFile or the newer authorization.credentialsFile. These settings don't actually carry secrets, so they're propagated without mTLS, and it's sufficient to ensure the collector's ServiceAccount has the right permissions. In fact, this can be done for any credential in a Secret, it just requires manually ensuring that it's mounted at the right location in the collector Pods. But for K8s authentication tokens it's straightforward, as there's a standard, automatic mount point.

[CharlieTLe]

it just requires manually ensuring that it's mounted at the right location in the collector Pods.

I think this is what we wanted to avoid, the collector needing to mount secrets to do the scraping depending on the target.

In order for the collector to accomplish this, the TA would send these credentials to the collector to use to scrape the targets.

[swiatekm]

it just requires manually ensuring that it's mounted at the right location in the collector Pods.

I think this is what we wanted to avoid, the collector needing to mount secrets to do the scraping depending on the target.

In order for the collector to accomplish this, the TA would send these credentials to the collector to use to scrape the targets.

We do want to avoid this, as a rule, but for K8s API Bearer tokens specifically, you just need to ensure the respective ServiceAccount has the right permissions. This isn't much more complex than accomplishing the same thing using prometheus-operator - there you also need to set bearerTokenFile and set the right RBAC for the Prometheus ServiceAccount.

It's really for other types of authorization that the manual workarounds become onerous and much less pleasant than what prometheus-operator supports.

[Horiodino]

For mTLS, I think we should make it configurable in the CRD instead of depending solely on cert-manager.

targetAllocator:
  mtls:
    enabled: true
    certFile: ...
    keyFile: ...
    caFile: ...

This would allow users to provide and manage their own certificates required for mTLS, while keeping it optional.

What do you think? @swiatekm

[swiatekm]

targetAllocator:
  mtls:
    enabled: true

looks reasonable to me. I'd start with doing that and removing the feature flag we have currently. Then we can add manual certificate setup in a separate change and track it in a separate issue.

@CharlieTLe does this look reasonable to you?

[CharlieTLe]

LGTM

[Horiodino]

/assign