apache / apache/pulsar

[improve][broker] PIP-478: make DefaultBrokerTlsFactory's engine selection purpose-aware

Open
#26,375 0 comments 0 reactions 1 assignee Claimed by @lhotari View on GitHub
Dominant language
Java
Stars
15.3k
Forks
3.8k
Avg merge
1d 14h
Merged PRs (30d)
160

Description

`DefaultBrokerTlsFactory` serves four purposes from one `FileBasedTlsFactory` — `BROKER`, `PROXY`, `WEB`
and `BROKER_CLIENT` — but the factory-wide `FileBasedTlsFactorySettings.engineProvider` is resolved once,
from the server-side `tlsProvider`. Its `BROKER_CLIENT` purpose therefore cannot honour
`brokerClientSslProvider` on the engine axis, while its JSSE and JCA axes do honour the corresponding
`brokerClient*` keys. `ProxyTlsFactories` gets this right by building two factories, one per side.

This is **latent today**: no in-tree consumer acquires a Netty context for `BROKER_CLIENT` from the
broker's server factory — the broker's outbound clients (internal client, admin client, geo-replication,
cross-cluster admin) each compose their own client-side factory from a `ClientConfigurationData`. It also
only diverges when `brokerClientSslProvider` holds an *engine literal* (`JDK`, `OPENSSL`,
`OPENSSL_REFCNT`) that differs from `tlsProvider`; any other value is routed to the JSSE axis and is
already correct.

It is worth closing because the purpose map is internally inconsistent: a future consumer of that
factory's `BROKER_CLIENT` purpose would silently get the server's engine. Either make engine selection
purpose-aware, or split the broker factory the way the proxy does. Preserve custom-factory behaviour,
refresh ownership and today's defaults, and add a test with different server and broker-client engine
values.

Found while verifying #26326; deliberately not fixed there because it is latent and the fix is a
structural change to the factory rather than a propagation fix.

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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.