stacklok / toolhive / 33640666112

70%

Build Type

push

github

Committed by web-flow

Commit Message

Bound and close HTTP client connection pools (#6480)

* Bound and close HTTP client connection pools

Every client from networking.HttpClientBuilder.Build had an unbounded,
undrainable connection pool: IdleConnTimeout was left at zero ("never
expire"), and CloseIdleConnections was a silent no-op because the
ValidatingTransport wrapper did not implement it. A client that made one
request and was dropped held a socket and its goroutine pair for the
lifetime of the process, with no remediation available.

On the auth-server upstream path this compounded — one transport per
configured upstream, released by nothing on shutdown, so every server
reconstruction in a long-lived process leaked the whole set.

Implements changes for issue #6479:
- Set IdleConnTimeout, MaxIdleConns and MaxIdleConnsPerHost in Build so
  every client's pool is bounded and self-draining
- Forward CloseIdleConnections through ValidatingTransport, and wrap the
  auth-token-file oauth2.Transport so the call reaches the pool there too
- Add upstream.IdleConnectionCloser as an optional capability, implemented
  by BaseOAuth2Provider; keeping it out of OAuth2Provider avoids breaking
  external implementations of that open interface
- Add Server.CloseIdleConnections, distinct from Close, so an embedder can
  retire a superseded server without closing storage a replacement server
  is still serving through; Close now drains upstreams first
- Add Config.UpstreamFactory (with exported UpstreamProviderFactory and
  DefaultUpstreamFactory) so callers can share HTTP clients across
  reconstructions and isolate per-upstream construction failures

* Address code review feedback

Fixed issues from code review:
- HIGH: OIDCProviderImpl satisfied upstream.IdleConnectionCloser only by
  promotion from its embedded *BaseOAuth2Provider, so losing the promotion
  would restore the leak silently — exactly the failure mode this change
  exists to remove. Pinned it with a var _ assertion and added a test th... (continued)

Coverage Stats

111 of 129 new or added lines in 8 files covered. (86.05%)

10 existing lines in 3 files now uncovered.

77216 of 110556 relevant lines covered (69.84%)

91.82 hits per line