feat(tracing): add TLS CA cert configuration for OTLP exporter - #10628
feat(tracing): add TLS CA cert configuration for OTLP exporter#10628Vallabh2909 wants to merge 1 commit into
Conversation
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/kind feature |
|
@Vallabh2909 please follow the contributing guide and the pull request template here please |
|
Thanks @waveywaves , I've updated the PR description to match the template |
bdd9cab to
84f69f1
Compare
| if strings.Contains(cacert, "-----BEGIN") { | ||
| return cert.NewPoolFromBytes([]byte(cacert)) | ||
| } | ||
| return cert.NewPool(cacert) |
There was a problem hiding this comment.
The CA file is only read when the provider starts. Rotated CA bundles will not be picked up. Please reload the file or document that a restart is required.
There was a problem hiding this comment.
I have documented that when cacert is a file path, the bundle is read at tracing provider initialization (startup or config-tracing change). Rotated CA files at the same path are not picked up until config-tracing is updated or the controller is restarted. Added notes in docs/developers/tracing.md and config/config-tracing.yaml
| CACert: string(pem), | ||
| } | ||
|
|
||
| tp, err := createTracerProvider("test-service", cfg, "", "") |
There was a problem hiding this comment.
This only tests provider creation. Please use a TLS test server to verify export succeeds with the CA and fails without it.
There was a problem hiding this comment.
I have replaced the provider-creation-only test with TestCreateTracerProviderTLSExport, which uses an httptest TLS server to verify export succeeds with the correct CA and fails without it.
There was a problem hiding this comment.
Thanks for the review, @waveywaves! I've pushed updates addressing both comments. Please take another look.
Add an optional cacert field to config-tracing so the OTLP HTTP exporter can verify HTTPS collectors signed by custom CAs (e.g. OpenShift service-ca). The value may be a path to a mounted PEM bundle or inline PEM. When set for an https endpoint, the exporter uses the configured CA as RootCAs for TLS. Document that file-based cacert values are read at provider initialization and require a config-tracing update or controller restart to pick up rotated CA bundles at the same path. Add an httptest TLS server test that verifies trace export succeeds with the correct CA and fails without it. Co-authored-by: Cursor <cursoragent@cursor.com>
8e42a10 to
681678f
Compare
Fixes #9799
Part of the tracing improvements tracked in #9701.
Context
In-cluster OTLP collectors often use HTTPS with certificates signed by a custom CA (for example, OpenShift service-ca or cert-manager). The tracing exporter currently only trusts the system CA bundle, so TLS verification fails when connecting to those collectors over HTTPS.
This PR adds an optional
cacertfield to theconfig-tracingConfigMap. Operators can set it to a mounted PEM file path or inline PEM. When tracing is enabled with anhttpsendpoint, the OTLP HTTP exporter uses that CA viaotlptracehttp.WithTLSClientConfig.Downstream, the OpenShift operator is expected to mount the service-ca bundle and set
cacertinconfig-tracing.Changes
Add an optional
cacertfield toconfig-tracingso the OTLP HTTP exporter can verify HTTPS collectors signed by custom CAs (e.g. OpenShift service-ca).The value may be a path to a mounted PEM bundle or inline PEM. When set for an
httpsendpoint, the exporter uses the configured CA asRootCAsfor TLS.cacertinpkg/apis/config/tracing.gopkg/tracing/config/config-tracing.yamlanddocs/developers/tracing.mdSubmitter Checklist
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes