Skip to content

Add OTLP client certificate support#139

Draft
mfreeman451 wants to merge 2 commits into
GoogleCloudPlatform:mainfrom
mfreeman451:fix/telemetry-otlp-mtls
Draft

Add OTLP client certificate support#139
mfreeman451 wants to merge 2 commits into
GoogleCloudPlatform:mainfrom
mfreeman451:fix/telemetry-otlp-mtls

Conversation

@mfreeman451
Copy link
Copy Markdown
Contributor

Summary

  • add OTLP client certificate and key config support for telemetry
  • extend shared OTLP TLS option loading to handle optional client cert/key pairs in addition to CA bundles
  • wire the same TLS config into the OTel log bridge path and add focused tests

Why

This was part of the known-good Kubernetes integration stack. It fixes the mTLS client-auth gap for OTLP exporters so hosted agents can connect to an OTLP endpoint that requires client certificates.

Validation

  • go test ./pkg/sciontool/telemetry ./pkg/util/logging -count=1

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for OTLP mTLS by introducing client certificate and key configuration options across the telemetry and logging packages. It refactors TLS loading into a centralized utility and updates environment variable handling. Feedback suggests hardening the TLS configuration by explicitly requiring a minimum version of TLS 1.2.

var errOTLPMissingClientKeyPair = errors.New("OTLP client certificate and key must be provided together")

func LoadOTLPTLSConfig(caFile, certFile, keyFile string) (*tls.Config, error) {
tlsConfig := &tls.Config{}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

For better security, it is recommended to explicitly set the minimum TLS version to 1.2. This ensures that the client does not negotiate older, less secure versions of TLS, which is a best practice for modern OTLP communication.

	tlsConfig := &tls.Config{
		MinVersion: tls.VersionTLS12,
	}

@mfreeman451 mfreeman451 marked this pull request as draft April 13, 2026 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant