Skip to content

Support a trusted CA for API-key and no-credentials Connections - #546

Open
aldredb wants to merge 3 commits into
temporalio:mainfrom
aldredb:connection-ca-cert
Open

Support a trusted CA for API-key and no-credentials Connections#546
aldredb wants to merge 3 commits into
temporalio:mainfrom
aldredb:connection-ca-cert

Conversation

@aldredb

@aldredb aldredb commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Let API-key and no-credentials Connections trust a private CA, matching what mutual-TLS Connections already get via MutualTLSSecretRef's own ca.crt key.

  • add ConnectionTLSConfig.CACertSecretRef, applicable when the auth mode is API key or no credentials
  • extract the additive CA-pool logic already used for mTLS (fetchClientUsingMTLSSecret) into a shared TLSCertPool helper and reuse it across auth modes
  • error out (not silently fall back to system-only trust) when the referenced secret has no ca.crt key
  • include the new field in ClientPoolKey so the client cache invalidates correctly when the CA changes

Without this, the only way to reach a Temporal server on a privately-signed certificate over API-key auth is a process-wide SSL_CERT_FILE override on the controller — which replaces rather than extends the system trust store, and affects every Connection the controller manages, not just the one that needs it.

Only mutual-TLS Connections could trust a private CA, via MutualTLSSecretRef's
own ca.crt key. API-key and no-credentials Connections had no such option,
forcing self-hosted clusters with a private CA onto a process-wide
SSL_CERT_FILE override, which replaces rather than extends the system trust
store.

- add ConnectionTLSConfig.CACertSecretRef, independent of AuthMode
- extract mergeCACert from the existing mTLS logic and reuse it for API-key
  and no-credentials auth
- error out (not silently skip) when the referenced secret has no ca.crt key
- add the new field to ClientPoolKey so the client cache still invalidates
  correctly on a CA change
@aldredb
aldredb requested review from a team, eniko-dif and jlegrone as code owners August 24, 2026 03:37
@tomba7

tomba7 commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Hi @aldredb thank you for this PR. We'll get it reviewed as soon as we can. Do you have an issue open for this by any chance?

@jaypipes jaypipes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @aldredb! Mostly minor suggestions and renames, please see inline!

Comment thread api/v1alpha1/connection_types.go Outdated
Comment thread internal/controller/clientpool/clientpool.go Outdated
Comment thread api/v1alpha1/connection_types.go Outdated
Comment thread api/v1alpha1/connection_types.go
Comment thread internal/controller/clientpool/clientpool.go Outdated
Comment thread internal/controller/clientpool/clientpool.go Outdated
Comment thread api/v1alpha1/connection_types.go Outdated
…ents

- rename CACertSecretName to TLSCACertSecretName, matching TLSServerName
- rename mergeCACert to TLSCertPool
- say which auth modes CACertSecretRef applies to instead of "independent of AuthMode"
@aldredb

aldredb commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@jaypipes thanks for the review — addressed in a762022:

  • renamed CACertSecretNameTLSCACertSecretName and mergeCACertTLSCertPool for consistency with TLSServerName
  • replaced the "independent of AuthMode" wording with the applicable auth modes, in both the field doc and ParseClientSecret
  • applied the ca.crt doc suggestion

I left a question on the CEL validation thread. Ready for another look when you get a chance.

@jaypipes jaypipes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍 thanks @aldredb!

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.

3 participants