[improve][client] PIP-478: let a CLIENT_OAUTH2-only PulsarTlsFactory build on a plaintext client
- Dominant language
- Java
- Stars
- 15.3k
- Forks
- 3.8k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 160
Description
PIP-478 removed `setUseTls(true)` from the v5 builder's `tlsFactory(...)`, so that a factory can supply
material for a non-transport purpose without switching the transport to TLS. The comment on that change
names the case it exists to serve: a client on a `pulsar://` URL adopting a custom factory purely to
serve `CLIENT_OAUTH2` — an HTTPS identity provider behind a private CA.
That case still fails to build. `ClientTlsFactorySupport.resolveClientTlsFactory` probes
`TlsPurpose.CLIENT_DEFAULT` unconditionally on the v5-builder path, and
`TlsContextAcquisition.acquireNettyContext` returns empty when the factory serves neither an `SslContext`
nor an `SSLContext` for the requested purpose, so `probe` throws *"supplied no Netty SslContext for
purpose CLIENT_DEFAULT"*. A strictly purpose-scoped factory — which the codebase treats as compliant
elsewhere, see the `wrapBrokerClientPurpose` rationale — therefore fails client construction on a
plaintext URL where the transport would never have requested `CLIENT_DEFAULT`.
Repro: v5 builder, `serviceUrl("pulsar://...")` plus `tlsFactory(oauth2OnlyFactory)` → build throws
`IllegalStateException` instead of producing a plaintext client whose OAuth2 IdP calls use the factory.
The probe semantics pre-date the `useTls` change (probing ran before it too, because `useTls` was
forced), and a factory can opt to serve a fallback context, so this may be an accepted contract rather
than a defect — but if it is, the comment on `tlsFactory()` overstates what the method enables. Suggested
resolution: probe only the purposes the configured transport and policies actually require, keeping
fail-fast behaviour for TLS broker connections, and add a plaintext-client test with a factory serving
only `CLIENT_OAUTH2`.
Found while verifying #26326.
Deferred out of #26326 as agreed in review; see [the re-review summary](https://github.com/apache/pulsar/pull/26326#issuecomment-5334709320) for where it sits relative to that PR.
Contributor guide
Research direction
Start with ClientTlsFactorySupport.resolveClientTlsFactory and TlsContextAcquisition.acquireNettyContext, then reproduce the v5 builder case using a plaintext pulsar:// URL and a CLIENT_OAUTH2-only factory. Add coverage for successful plaintext client construction and verify that TLS broker connections still fail fast when required context is unavailable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- authentication, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100