Skip to content

Commit c59e749

Browse files
carlydfclaude
andauthored
Add unit tests for clientpool auth code paths (#236)
## Summary - Adds `clientpool_test.go` with 8 unit tests covering the auth code paths that had no test coverage - Two tests are explicit regression guards for the bugs fixed in #227 and #232 - Makes `dialFn` and `systemCertPoolFn` injectable on `ClientPool` (no behavior change in production) to enable testing without network I/O or OS trust store dependencies ## Regression tests **`TestFetchMTLS_CACertAppendsToSystemPool`** — guards against the PR #212 bug (fixed in #227): `fetchClientUsingMTLSSecret` used `x509.NewCertPool()` (empty) instead of `x509.SystemCertPool()`, silently dropping system root CAs and breaking Temporal Cloud connections. The test injects a fake system pool and verifies both the injected system CAs and the custom `ca.crt` are present in the returned pool. This test fails if the fix is reverted. **`TestDialAndUpsert_APIKeySkipsCheckHealth`** — guards against the PR #203 bug (fixed in #232): `DialAndUpsertClient` called `CheckHealth` unconditionally, which fails on Temporal Cloud with namespace-scoped API keys. The test uses an injected mock client and asserts `CheckHealth` is never called for `AuthModeAPIKey`. This test fails if the fix is reverted. ## Test plan - [x] `go test ./internal/controller/clientpool/... -v` — all 8 tests pass - [x] `go build ./...` — no compilation errors - [x] Manually revert the PR #227 fix → `TestFetchMTLS_CACertAppendsToSystemPool` fails - [x] Manually revert the PR #232 fix → `TestDialAndUpsert_APIKeySkipsCheckHealth` fails 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 62dbdb8 commit c59e749

2 files changed

Lines changed: 378 additions & 5 deletions

File tree

internal/controller/clientpool/clientpool.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,24 @@ type ClientPool struct {
5858
logger log.Logger
5959
clients map[ClientPoolKey]ClientInfo
6060
k8sClient runtimeclient.Client
61+
// dialFn establishes a Temporal SDK connection from the given options. In production
62+
// this is sdkclient.Dial; in tests it can be replaced with a function that returns a
63+
// mock client without making any network calls.
64+
dialFn func(sdkclient.Options) (sdkclient.Client, error)
65+
// systemCertPoolFn loads the host OS certificate pool used as the base when a custom
66+
// CA is appended via ca.crt. In production this is x509.SystemCertPool; in tests it
67+
// can be replaced to inject a known set of "system" root CAs without depending on the
68+
// OS trust store.
69+
systemCertPoolFn func() (*x509.CertPool, error)
6170
}
6271

6372
func New(l log.Logger, c runtimeclient.Client) *ClientPool {
6473
return &ClientPool{
65-
logger: l,
66-
clients: make(map[ClientPoolKey]ClientInfo),
67-
k8sClient: c,
74+
logger: l,
75+
clients: make(map[ClientPoolKey]ClientInfo),
76+
k8sClient: c,
77+
dialFn: sdkclient.Dial,
78+
systemCertPoolFn: x509.SystemCertPool,
6879
}
6980
}
7081

@@ -141,7 +152,7 @@ func (cp *ClientPool) fetchClientUsingMTLSSecret(secret corev1.Secret, opts NewC
141152
// Cloud. When ca.crt is absent, RootCAs remains unset and Go's TLS implementation
142153
// uses the system CA bundle by default.
143154
if caCert, ok := secret.Data["ca.crt"]; ok && len(caCert) > 0 {
144-
rootCAs, err := x509.SystemCertPool()
155+
rootCAs, err := cp.systemCertPoolFn()
145156
if err != nil {
146157
cp.logger.Warn("Failed to load system CA pool, falling back to empty pool", "error", err)
147158
rootCAs = x509.NewCertPool()
@@ -260,7 +271,7 @@ func (cp *ClientPool) ParseClientSecret(
260271
}
261272

262273
func (cp *ClientPool) DialAndUpsertClient(clientOpts sdkclient.Options, clientPoolKey ClientPoolKey, clientAuth ClientAuth) (sdkclient.Client, error) {
263-
c, err := sdkclient.Dial(clientOpts)
274+
c, err := cp.dialFn(clientOpts)
264275
if err != nil {
265276
return nil, err
266277
}

0 commit comments

Comments
 (0)