Skip to content

Commit fd6f356

Browse files
Shivs11claude
andcommitted
fix: append custom CA to system cert pool instead of replacing it (#227)
## Summary - PR #212 introduced `ca.crt` support for server certificate verification but used `x509.NewCertPool()`, which creates an **empty** CA pool — replacing the system CA bundle entirely - This breaks connections to Temporal Cloud (public CA) when the mTLS secret contains a `ca.crt` key from cert-manager (the CA that signed the **client** cert, not the server cert) - This fix uses `x509.SystemCertPool()` instead, so the custom CA is **appended** to the system bundle rather than replacing it ## Why this broke cert-manager always includes `ca.crt` in TLS secrets (the issuing CA). When connecting to Temporal Cloud: 1. The controller sees `ca.crt` in the secret (the self-signed client CA) 2. `NewCertPool()` creates an empty pool with **only** that CA 3. Temporal Cloud's server cert is signed by a public CA (e.g., DigiCert) 4. The public CA is no longer trusted → `x509: certificate signed by unknown authority` ## What this fixes - `SystemCertPool()` loads the system CA bundle first, then appends the custom CA - Both public CAs (Temporal Cloud) and private CAs (self-hosted) are trusted simultaneously - Falls back to `NewCertPool()` with a warning log if the system pool can't be loaded ## Affected versions - v1.2.1, v1.2.2, v1.2.3 — all contain the regression from PR #212 - Closes #223 ## Test plan - [ ] Deploy against Temporal Cloud with cert-manager mTLS secret (has `ca.crt`) — verify connection succeeds - [ ] Deploy against self-hosted Temporal with private CA — verify connection succeeds - [ ] Deploy with mTLS secret without `ca.crt` — verify fallback to system bundle works 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 94bc3b8 commit fd6f356

1 file changed

Lines changed: 11 additions & 6 deletions

File tree

internal/controller/clientpool/clientpool.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,13 +133,18 @@ func (cp *ClientPool) fetchClientUsingMTLSSecret(secret corev1.Secret, opts NewC
133133
tlsCfg := &tls.Config{
134134
Certificates: []tls.Certificate{cert},
135135
}
136-
// If the secret contains a CA certificate, use it as the trusted root for server
137-
// certificate verification. This enables connecting to Temporal servers whose TLS
138-
// certificates are signed by private or internal CAs (e.g. cert-manager in a test
139-
// cluster). When ca.crt is absent, Go falls back to the system CA bundle, which is
140-
// the correct behaviour for Temporal Cloud and other publicly-signed endpoints.
136+
// If the secret contains a CA certificate, append it to the system CA pool for
137+
// server certificate verification. This enables connecting to Temporal servers whose
138+
// TLS certificates are signed by private or internal CAs (e.g. cert-manager in a
139+
// self-hosted cluster) while still trusting publicly-signed endpoints like Temporal
140+
// Cloud. When ca.crt is absent, RootCAs remains unset and Go's TLS implementation
141+
// uses the system CA bundle by default.
141142
if caCert, ok := secret.Data["ca.crt"]; ok && len(caCert) > 0 {
142-
rootCAs := x509.NewCertPool()
143+
rootCAs, err := x509.SystemCertPool()
144+
if err != nil {
145+
cp.logger.Warn("Failed to load system CA pool, falling back to empty pool", "error", err)
146+
rootCAs = x509.NewCertPool()
147+
}
143148
if !rootCAs.AppendCertsFromPEM(caCert) {
144149
return nil, errors.New("failed to parse CA certificate from secret")
145150
}

0 commit comments

Comments
 (0)