[fix][misc] PIP-478: close out the migration follow-ups and reconcile the PIP - #26326
Open
lhotari wants to merge 14 commits into
Open
[fix][misc] PIP-478: close out the migration follow-ups and reconcile the PIP#26326lhotari wants to merge 14 commits into
lhotari wants to merge 14 commits into
Conversation
lhotari
force-pushed
the
lh-pip-478-followups
branch
from
August 13, 2026 11:04
e45fb47 to
7b14fe9
Compare
14 tasks
lhotari
force-pushed
the
lh-pip-478-followups
branch
from
August 14, 2026 11:39
2c86fdf to
6b806bd
Compare
14 tasks
lhotari
force-pushed
the
lh-pip-478-followups
branch
from
August 14, 2026 11:53
6b806bd to
d464a65
Compare
lhotari
force-pushed
the
lh-pip-478-followups
branch
from
August 14, 2026 14:06
d464a65 to
d10c458
Compare
…lic API module AsyncAuthenticationDriver lived in pulsar-client-api as a "stable internal" marker, because it existed for a carve-out: a v4 Authentication could additionally implement it and ClientCnx would then drive that plugin asynchronously instead of synchronously. Since the client drives the v5 model natively there is no such choice — the client resolves the authentication it drives and wraps it itself, and nothing outside pulsar-client implements or observes the type. Move it to org.apache.pulsar.client.impl.auth.v5 and name it for what it now is, BinaryAuthenticationDriver. That removes an internal type from the API module's surface, and its javadoc no longer has to describe a synchronous alternative that no longer exists. Also make the client's own blocking auth executor queue rather than reject. It was a SynchronousQueue with the default abort policy, so the seventeenth concurrent credential call failed outright — and every caller is a connection attempt or an authenticated request, so a reconnect storm against a slow identity provider produced failed connections rather than slow ones. This is the same fix the shared fallback pool already received, and it is what pip-478.md already claims the executor does. Assisted-by: Claude Code (Opus 5)
…ter carried Deleting V5ToV4AuthenticationAdapter took two loud failures with it, and neither is exotic. A plugin that exposes no BinaryAuthDataProvider cannot authenticate a binary connection — PIP-478 binary routing rule 1. The adapter refused it in start(), so a misconfiguration failed the client build. Without that, the client builds fine and every connection attempt fails for its lifetime with the reason buried in a connection error. The client now checks it where the adapter did, on the path the adapter served: a plugin configured through the v5 builder is initialized at build time (which PIP-478 already specifies for that path) and the capability is checked once initialization has run, since capabilities are only meaningful after it. Serializing a configuration that carries a v5 plugin used to fail with an actionable NotSerializableException naming authPluginClassName. The v5 slot is transient, so it became a silent drop — and a configuration whose authentication was dropped authenticates as nobody, discovered as a broker rejection far from its cause. ClientConfigurationData now refuses, unless the string form is also configured, which does survive the round trip and is what a remote or forked context should carry. Both are pinned by ClientAuthenticationFailsLoudlyTest, including the case that must keep working: a configuration using authPluginClassName + authParams still serializes. Assisted-by: Claude Code (Opus 5)
… moved BrokerService carried two stacked javadoc blocks before warnOnStalePip337ClusterFactory: the first documents resolveBrokerClientTlsFactory, which has its own copy further down, so the helper's real javadoc was preceded by a description of a different method. Remove the orphan. PulsarAdminImpl's binding said the scheduler and blocking executor are "unused on the HTTP-only admin path". That stopped being true when SASL gained a v5 body: its SASL-over-HTTP challenge rounds off-load their GSSAPI work through exactly that executor. Leaving it unbound is still the right choice — an unbound executor now falls back to the shared pool rather than running on the caller thread, and lending the admin's own request threads would let a slow KDC consume them — but the comment should say that rather than claim nothing uses it. Assisted-by: Claude Code (Opus 5)
…a null executor Four defects the PIP-vs-code consistency sweep turned up, each a case of the code not doing what the design document says it does. The broker ignored its own jsseProvider and brokerClientJsseProvider settings. DefaultBrokerTlsFactory passed null as the explicit JSSE provider at all three sites, so a key shipped in broker.conf and standalone.conf did nothing — while the proxy, websocket service and functions worker all honour theirs. Wired. An adopted PulsarTlsFactory was never initialized on a plaintext client. needsClientTlsFactory() omitted the adopted-factory arm, so a factory handed in through the v5 builder on a pulsar:// URL with no policy map was closed on shutdown without ever having been initialized. The admin path has that arm and cites the PIP for it. AuthenticationInitContext handed out a null blocking executor when no client bound services. The SPI tells a plugin to off-load its blocking work onto that executor, so null makes every third-party plugin either fail with an NPE or defensively do the one thing the contract forbids. It now returns the shared pool, which is what the built-ins already got by funnelling through supplyBlocking. TlsHandle#get()'s javadoc said only that its value after dispose() is "unspecified", where the PIP says an implementation must not return a released native context. A consumer handed a released ReferenceCountedOpenSslContext has a use-after-free, so the stronger statement is the one that belongs in the contract. Also removes org.apache.pulsar.client.api.internal, left empty in a shipped public artifact when the binary auth driver moved, with a package javadoc still linking the type that left. Assisted-by: Claude Code (Opus 5)
The design document and the code drifted apart over the series, most of all where the client's authentication changed direction: the PIP still specified V5ToV4AuthenticationAdapter as permanent machinery, described the built-in shims as additionally exposing an async driver with the no-credential-I/O ones staying on a verbatim synchronous path, and listed the framework-services marker as new public API. None of that survives the inversion — the client resolves one v5 Authentication and drives every binary connection from it, and the only bridge left runs v4 to v5. Reconciled across four axes — bridging direction, API inventory, threading and off-loading, and the TLS SPI — as 46 edits derived from reading each passage against the code it describes. Beyond the direction rewrite, the notable corrections are: - The inventories were incomplete. Four deleted public classes were never listed (JettySslContextFactory, PulsarHttpAsyncSslEngineFactory, the PIP-466 AuthenticationData stub, the CN-matching helpers), and neither was ClientCnx's protected-surface change — the authentication field replaced by a driver whose type is not in a public API module, plus a new overridable authMethodName(). An out-of-tree subclass notices both. - The threading text claimed a re-entrancy-aware executor the code does not implement, and described a "degraded inline" fallback that no longer exists now that credential work always off-loads. - The built-in mapping was described as a closed class-name allow-list; it is interface-based, which means a third-party plugin implementing the seam *is* picked up — the opposite of what the PIP promised. Passages that describe removed machinery as history are deliberately untouched: the PIP-337 critique in Motivation, the JettySslContextFactory and TrustManagerProxy analyses in the appendices, and the migration tables telling operators what to move away from. What this does NOT do is paper over the one place the code is behind the design: the PIP specifies a two-axis provider model and only the JSSE axis is implemented. That is a missing port, not documentation drift, so the JCA column stays and the decision is recorded as a follow-up rather than resolved by quietly deleting the text. Assisted-by: Claude Code (Opus 5)
A review of the cumulative final state of every file the series touched — 320 paths, 286 surviving — rather than of any one PR's diff, run with Codex gpt-5.6-sol against pip/pip-478.md as the spec. Nine defects, each verified against the code before being accepted. The three that would bite an operator: tlsFactory(...) forced transport TLS on a plaintext broker. PulsarClientBuilderV5.tlsFactory called setUseTls(true), so adopting a factory to serve CLIENT_OAUTH2 — an HTTPS identity provider behind a private CA, the case the SPI exists for — made a pulsar:// client attempt a TLS handshake against the plaintext broker port. The PIP states the rule for this method by name, and the sibling tlsPolicy(...) already implements it with a comment describing this exact failure; tlsFactory was simply missed. The factory is still composed: needsClientTlsFactory() has its own adopted-factory arm. The PIP-337 deprecation warning copied secrets into the log. warnOnStalePip337ClusterFactory attached the whole brokerClientSslFactoryPluginParams value as a log attribute. PIP-337 defined it as an opaque plugin-interpreted string, so it routinely carries keystore passwords or KMS credentials. It now reports only whether the value was set. The functions worker had no outbound provider keys at all. The PIP promises the worker propagates its brokerClient* provider keys onto its own PulsarClient and PulsarAdmin independently of brokerClientTlsFactoryClassName, and that an embedded worker inherits them from the broker. WorkerConfig declared only the listener-side tlsProvider/jsseProvider, and it ignores unknown YAML — so an operator pinning BCJSSE for FIPS got silence, not an error. Adds brokerClientSslProvider and brokerClientJsseProvider (the two implemented axes; the JCA axis waits on its own decision), wires both into the client and admin builders, and inherits them — plus the two web-listener keys — for an embedded worker. The rest: - A failed post-connect auth challenge left the channel open. The three failure paths in ClientCnx.completeAuthChallenge only failed connectionFuture, which on a broker-pushed REFRESH is already completed and so a no-op: no auth response was sent, the channel stayed open, and the client neither reauthenticated nor reconnected until the broker timed it out. PIP-478's own off-event-loop resolution timeout made this newly reachable. Now closes the channel, as the challenge-round cap in the same file already did. - A failed TLS probe leaked the initialized factory, with its metrics registration, file watcher and refresh task. PulsarClientImpl assigns conf.setTlsFactory(...) only to the returned value, so a throw left no owner. - TlsHandle.get() could return a released native context. Both handle kinds returned the instance after dispose() released it — a use-after-free on a ReferenceCountedOpenSslContext. They now throw IllegalStateException, which the contract explicitly permits. - Disabling hostname verification produced no WARN, though the PIP promises one for any insecure setting on first use; only allowInsecureConnection was covered. - Two comments that describe code as it is not: ClientTlsFactorySupport claiming brokerClientTlsFactoryConfig params are "not yet plumbed" when all five callers plumb them, and a test citing SimpleAuthInitContext, a class this series deleted. - BasicAuthenticationV5 resolves inline on the event loop, unlike the four sibling bodies. Left inline deliberately — the v4 shim supplies two field reads and a thread hop per connection would cost more than it buys — but the constructor now documents that suppliers must not block. Tests: the builder's useTls behaviour both ways, the dispose contract on both handle kinds, and the embedded-worker inheritance. Assisted-by: Claude Code (Opus 5), Codex (gpt-5.6-sol)
…d v5→v4 adapter The inversion deleted V5ToV4AuthenticationAdapter, but three javadocs still described the design built around it as current — and one of them was wrong about threading, not just about a name: - PulsarClientBuilderV5.applyAuthentication carried a paragraph routing every credential-fetching plugin "through V5ToV4AuthenticationAdapter so its getAuthData off-loads", which is no longer how off-loading happens: the client resolves one v5 body and the driver it builds is what keeps credential work off the event loop. Its resolveAuthenticationForTest @return likewise promised "a raw v4 engine, or a wrapping V5ToV4AuthenticationAdapter"; the slot now holds the raw v4 plugin or nothing. - BinaryAuthenticationExchange described itself as shared by two drivers that must not drift. There is one, V5BinaryAuthenticationDriver. - ClientAuthenticationServicesAware listed the deleted bridge as an implementor, and said an unbound driver "runs inline on the caller thread rather than being off-loaded". That stopped being true when the shared fallback pool landed — which matters, because the paths that skip binding are the proxy's broker connections, where the caller thread is a Netty event loop. The fourth surviving mention, in ClientAuthenticationFailsLoudlyTest, is correct: it names the adapter as the deleted thing whose invariants the test re-homes. Assisted-by: Claude Code (Opus 5)
…e scoping, SASL token
An independent second reviewer (Claude Fable) covered the same cumulative final state and found what the
first pass did not. The three that change behaviour:
A closed client's TLS factory was left in the caller's builder, breaking builder reuse.
ClientBuilderImpl.build() hands the builder's own ClientConfigurationData to the client without cloning
it, so the factory PulsarClientImpl composes during construction is written into an object the
application still holds. shutdown() closed that factory but left the reference. The next build() from the
same builder then took the dead factory for one adopted through the v5 builder, re-initialized it —
violating the SPI's initialize-exactly-once rule — and failed the build with "FileBasedTlsFactory is
closed". Reusing a builder is ordinary v4 usage that worked before this series, and the same hazard is
already documented and avoided for the v5 authentication slot a few lines away. A composed factory is now
cleared on shutdown; an adopted one is left alone, since the application supplied that instance.
tlsPolicy(purpose, ...) enabled the binary transport for every client-role purpose. The guard excluded
only CLIENT_OAUTH2, so BROKER_CLIENT and any plugin-minted purpose from TlsPurpose.client("...") still
flipped useTls — the same defect the guard was added to fix, for the purposes it did not name. pip-478.md
calls transport enablement "one narrow addition" belonging to CLIENT_DEFAULT alone, which is now the
condition.
The SASL HTTP driver rebuild discarded a validated Kerberos role token. The driver is rebuilt when the
bound services change, on the stated ground that it "holds no cross-request state" — but the rebuild
minted a fresh SaslAuthenticationV5, and that body holds the cached role token whose whole purpose is to
spare a full renegotiation per request. One plugin shared between a PulsarClient and a PulsarAdmin — the
scenario the comment itself cites — is exactly when rebinds happen. The body is now kept across driver
rebuilds and dropped with the plugin on close.
Also, dead code and comments that assert the opposite of the code:
- PulsarClientBuilderV5.resolveBridgedV4 returned a raw-vs-stay-wrapped decision that its only caller
discards, and warned that a bridged plugin with both TLS material and a fetched credential "runs inline
(no credential off-load)" with advice to set tlsPolicy(...) to fix it. Since the inversion the client
always re-wraps through LegacyV4AuthenticationAdapter, which always off-loads, so the warning told
users to act on a problem they did not have. Both removed; the probe is now skipped entirely when there
is no policy to fold into, which also avoids an eager credential fetch at build time.
- TlsFactorySupport said the PIP-337 removed-key validation "arrives with the server-side migration"; it
ships (PulsarConfigurationLoader.rejectRemovedPip337TlsFactoryKeys).
- SaslAuthenticationV5 and AthenzAuthenticationV5 described a null executor as "degraded inline
computation"; supplyBlocking substitutes the shared pool and never runs inline.
- The pulsar.tls.reload metric described itself as counting attempts; it counts events, as the PIP says.
- ServiceChannelInitializer.tlsEnabledWithKeyStore was written and never read, left by the deleted
KeyStoreSSLContext branch.
- conf/websocket.conf omitted jsseProvider, the one component whose provider pin was undocumented while
its config class declares it and its server reads it.
Tests: builder reuse after close, and that the composed factory is cleared from the configuration.
Mutation-verified.
Assisted-by: Claude Code (Opus 5), Claude Fable
…wing the final state Reviewing the whole series against the PIP surfaced five places where the document, not the code, is the wrong side. Two of them would mis-steer someone building against it: - The normative "HTTP challenge routing" rules describe a 401-reactive driver: single-pass headers on the initial request, the plugin's challenge handler invoked on the server's 401, the response returned unhandled when no handler exists. The shipped driver does none of that. It runs the exchange as a pre-flight side band of bodiless GETs to the original URI, engages only when the plugin exposes BOTH the challenge handler and the headers provider, and never sees the real request's response at all — it cannot resubmit. The adjacent prose already described this correctly, so the rule list contradicted its own section. Rewritten to the shipped behaviour, with a note on why reactive was not chosen. - The LegacyV4CredentialAdapter section was stale on four counts: it omitted BinaryAuthChallengeHandler from the adapter's interface list, said the post-start has*() probe decides the binary capabilities (they are advertised unconditionally, which is what preserves v4 parity for third-party multi-round plugins), described the credential as rendered via getCommandData() rather than authenticate(INIT_AUTH_DATA), and carried a "known limitation" — that a non-"sasl" third-party challenge plugin's multi-round flow "is not driven" — that the implementation fixed. And three narrower ones: - The binary routing rules omitted two shipped normative behaviours: the 10-round challenge cap, which is a real v4 behaviour change (an unusually long GSSAPI negotiation that used to complete now fails), and the dropping of a non-refresh challenge that arrives mid-round. - "Both HTTP transports off-load the whole v4 header composition" is true of the lookup client only; BaseResource still composes on the calling thread. The known gap had become invisible to a PIP reader. - The perf tools were said to "gain --jsse-provider / --jca-provider". They gained neither. The provider matrix also now states plainly what ships: the JSSE column everywhere except pulsar-perf and the WebSocket proxy's outbound leg, and the JCA column in the SPI and default factory but with no configuration key anywhere — reachable today only through the v5 builder. That is the open decision, so the column stays and the note describes the gap rather than hiding it. Assisted-by: Claude Code (Opus 5), Claude Fable
…he WebSocket proxy's outbound providers A poisoned SASL role token was never invalidated. isRoleTokenExpired only recognised an explicit rejection — a Kerberos SASL response whose State reports the token expired — which is all AuthenticationSasl ever checked. A broker whose signer secret has rotated, or a peer broker holding a divergent secret, instead rejects a replayed token with a bare response carrying no SASL headers at all. That case fell through, so the cached token was replayed on every request, burned the whole round budget, and failed, for the life of the client — and the cross-request cache exists precisely so it is replayed. An unadorned response to a round that replayed the token is now treated as a rejection: the token is dropped from both the conversation and the cache, and the exchange restarts. The cost when a server merely answers oddly is one extra negotiation; the cost of the previous behaviour was every request. The WebSocket proxy gains brokerClientSslProvider and brokerClientJsseProvider for its own outbound proxy-to-broker client, wired independently of brokerClientTlsFactoryClassName and documented in websocket.conf. It was the last component whose outbound leg could not be pinned — the broker, the proxy and the functions worker all could — so a FIPS deployment had one process that silently ran on the platform default. The listener-side tlsProvider/jsseProvider keep governing its web server alone. Assisted-by: Claude Code (Opus 5)
The PIP specifies a two-axis provider model — a JSSE provider for the SSLContext and a JCA provider for the KeyStore/CertificateFactory/KeyFactory engines that parse the material — and its FIPS argument (Motivation #4, Goal #5) rests on having both, since BCJSSE is only half of a FIPS deployment without BCFIPS underneath it. The axis itself was already implemented and honoured end to end: TlsPolicy.jcaProvider() is threaded through TlsMaterialSource into every material load — keystores, PEM certificates, private keys, trust certs and the auth-provided material source — and TlsContexts resolves both axes together on the JDK and Netty trust paths. What was missing was the configuration surface, so the axis was reachable only programmatically through the v5 builder and no operator could pin it from a config file. That is what made the PIP's provider matrix read as an overclaim. Adds jcaProvider and, where the component has an outbound leg, brokerClientJcaProvider to ServiceConfiguration, ProxyConfiguration, WebSocketProxyConfiguration and WorkerConfig, plus jcaProvider on ClientConfigurationData. Each is wired into the TlsPolicy at the same site as its jsseProvider counterpart — all nine of them, including the v5 builder's material fold, where dropping half a pinned pair is as bad as dropping both, and PulsarClientTool's client.conf mapping. The embedded functions worker inherits both new keys from the broker on the same terms as the JSSE ones, and all six shipped conf files document them. Note the asymmetry with jsseProvider, which is deliberate: the JSSE axis has a web-listener default (Conscrypt when usable) resolved by the caller, so it is threaded through resolveWebJsseProvider / resolveJsseProvider. The JCA axis has no such default — unset means the JVM search order everywhere — so it is read straight from the configuration on every purpose. Assisted-by: Claude Code (Opus 5)
… tools The PIP specified both flags and neither was implemented, so pulsar-perf could reach neither provider axis — leaving no way to run a FIPS validation against a cluster whose brokers are pinned, which is exactly the scenario the flags exist for. Both are format-independent: they apply to PEM and keystore material alike, and both fall back to client.conf through the existing default-value provider. The PIP's provider matrix now states that both columns ship on every row, in place of the note added a few commits ago recording the JCA column as unreachable from configuration. Assisted-by: Claude Code (Opus 5)
AuthenticationInitContext.scheduler() returned null when no client bound framework services, the same defect blockingExecutor() had: the SPI invites a plugin to schedule its credential refresh there, so null makes every third-party plugin either NPE or roll its own pool. It now returns a shared process-lifetime daemon scheduler, sized at one thread because the contract is that scheduled work hands the blocking off to blockingExecutor(); core threads time out so an unused pool costs nothing. httpClientFactory() deliberately stays null and now says why. An HTTP client factory cannot be built without the client's TLS configuration and lifecycle, and handing back a bare one would give a plugin an HTTP client that quietly ignores the deployment's trust settings — worse than an honest absence. It is the one accessor a plugin must null-check. Assisted-by: Claude Code (Opus 5)
… sentinel delta Two review follow-ups that are contract statements rather than behaviour changes. `TlsFactoryInitContext` presents `scheduler()` and `blockingExecutor()` as if they were distinct pools, but every wiring in the repository — broker binary and web, proxy inbound, broker-client and web, websocket, functions worker, and the client — passes one general-purpose scheduled pool for both. That is a deliberate choice rather than an oversight: neither role is ever an event loop, so an occasional blocking material load is tolerable, and eight dedicated pools would cost more than they buy. The contract now says so, and states the obligation that makes sharing safe — work triggered on a scheduler thread must be dispatched to the blocking executor rather than run in place, which is what `FileBasedTlsFactory`'s poll already does. `pip-478.md` specified that a broker-pushed REFRESH starts a fresh exchange (binary routing rule 2) but never drew the consequence for a bridged v4 plugin: its refresh call arrives as `authenticate(INIT_AUTH_DATA)` where the v4 client passed `REFRESH_AUTH_DATA`, so the sentinel never reaches such a plugin at all. Invisible for every plugin that ignores the argument on its credential path — all the built-ins — but observable for a third-party plugin that branches on it, which makes it a behaviour change worth stating rather than leaving to be discovered.
lhotari
force-pushed
the
lh-pip-478-followups
branch
from
August 14, 2026 14:57
d10c458 to
c1bd3e6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Main Issue: #25890
PIP: #25890
Motivation
This is the last part of the PIP-478 series. Two things were deferred while the migration was in flight, and both come due now that it has landed: the follow-ups accumulated across the earlier PRs, and the design document, which had drifted from the code in both directions.
The interesting half of that drift is where the code was wrong, not the document — four cases of the implementation not doing what the PIP says it does:
jsseProviderandbrokerClientJsseProvidersettings.DefaultBrokerTlsFactorypassednullas the explicit JSSE provider at all three sites, so a key shipped inbroker.conforstandalone.confdid nothing — while the proxy, websocket service and functions worker all honour theirs. The wiring was lost in the re-land, not designed away.PulsarTlsFactorywas never initialized on a plaintext client.needsClientTlsFactory()omitted the adopted-factory arm, so a factory handed in through the v5 builder on apulsar://URL with no policy map was closed on shutdown having never been initialized. The admin path has that arm and cites the PIP for it.AuthenticationInitContexthanded plugins a null blocking executor when no client bound services. The SPI instructs a plugin to off-load its blocking work onto that executor, so null makes every third-party plugin either fail with an NPE or defensively do the one thing the contract forbids. The built-ins were only safe because they funnel through a helper that null-checks for them.TlsHandle#get()'s javadoc was weaker than the PIP on the safety-relevant half: the PIP says an implementation must not return a released native context, the javadoc said only that the value afterdispose()is "unspecified". A consumer handed a releasedReferenceCountedOpenSslContexthas a use-after-free.Modifications
The four defects above are fixed, each where it belongs: the three provider keys wired through, the adopted-factory arm added to
needsClientTlsFactory(),AuthenticationInitContextreturning the shared pool instead of null, and theTlsHandle#get()contract strengthened to match the PIP.The follow-ups cleared:
AsyncAuthenticationDriverleaves the public API module. It lived inpulsar-client-apibecause it existed for a carve-out — a v4Authenticationcould additionally implement it andClientCnxwould then drive that plugin asynchronously instead of synchronously. With the client driving the v5 model natively there is no such choice: the client resolves the authentication it drives and wraps it itself, and nothing outsidepulsar-clientimplements or observes the type. Moved toorg.apache.pulsar.client.impl.auth.v5and renamedBinaryAuthenticationDriver. The package it left behind,org.apache.pulsar.client.api.internal, is deleted — it was empty in a shipped public artifact, with a package javadoc still linking the type that left.SynchronousQueuewith the default abort policy, so the seventeenth concurrent credential call failed outright. Every caller is a connection attempt or an authenticated request, so a reconnect storm against a slow identity provider produced failed connections rather than slow ones. This is the same fix the shared fallback pool already received, and it is whatpip-478.mdalready claimed the executor did.V5ToV4AuthenticationAdapterare re-homed. A plugin exposing noBinaryAuthDataProvidercannot authenticate a binary connection (PIP-478 binary routing rule 1); the adapter refused it instart(), so a misconfiguration failed the client build. Without that, the client builds fine and every connection attempt fails for its lifetime with the reason buried in a connection error. The check is now made where the adapter made it, on the path the adapter served. Separately, serializing a configuration carrying a v5 plugin used to fail with an actionableNotSerializableExceptionnamingauthPluginClassName; the v5 slot is transient, so it had become a silent drop — and a configuration whose authentication was dropped authenticates as nobody, discovered as a broker rejection far from its cause.ClientConfigurationDatarefuses again, unless the string form is also configured.BrokerServicecarried an orphaned javadoc block forresolveBrokerClientTlsFactorystacked in front of a different method, so that helper's real javadoc was preceded by a description of something else. AndPulsarAdminImpl's binding claimed the scheduler and blocking executor are "unused on the HTTP-only admin path" — untrue since SASL gained a v5 body, whose SASL-over-HTTP challenge rounds off-load their GSSAPI work through exactly that executor. Leaving it unbound is still right (an unbound executor now falls back to the shared pool rather than the caller thread, and lending the admin's own request threads would let a slow KDC consume them), but the comment should say so rather than claim nothing uses it.pip/pip-478.mdreconciled across four axes — bridging direction, API inventory, threading, and the TLS SPI — as 46 edits derived from reading each passage against the code. The document still specifiedV5ToV4AuthenticationAdapteras permanent machinery, described the built-in shims as exposing a v5 body only for the credential-heavy plugins with the rest staying on a verbatim synchronous path, and listed a framework-services marker as new public API. None of that survives the inversion. Beyond the direction rewrite:ClientCnx's protected-surface change — the authentication field replaced by a driver whose type is not in a public API module, plus a new overridableauthMethodName(). An out-of-tree subclass notices both.Passages that describe removed machinery as history are deliberately untouched: the PIP-337 critique in Motivation, the
JettySslContextFactoryandTrustManagerProxyanalyses in the appendices, and the migration tables telling operators what to move away from.Findings from a whole-series cross review
After the above, the cumulative final state of every file the series touched (320 paths, 286 surviving) was reviewed against
pip/pip-478.mdas the spec — rather than any one PR's diff — with Codexgpt-5.6-solas an independent reviewer. Nine further defects, each verified against the code before being accepted. Three would reach an operator:tlsFactory(...)forced transport TLS on a plaintext broker.PulsarClientBuilderV5.tlsFactorycalledsetUseTls(true), so adopting a factory to serveCLIENT_OAUTH2— an HTTPS identity provider behind a private CA, the case the SPI exists for — made apulsar://client attempt a TLS handshake against the plaintext broker port. The PIP states the rule for this method by name, and the siblingtlsPolicy(...)already implements it with a comment describing this exact failure;tlsFactorywas missed. Composition is unaffected —needsClientTlsFactory()has the adopted-factory arm added earlier in this PR.warnOnStalePip337ClusterFactoryattached the wholebrokerClientSslFactoryPluginParamsvalue as a log attribute; PIP-337 defined it as an opaque plugin-interpreted string, so it routinely carries keystore passwords or KMS credentials. It now reports only whether the value was set.brokerClient*provider keys onto its ownPulsarClientandPulsarAdminindependently ofbrokerClientTlsFactoryClassName, and that an embedded worker inherits them.WorkerConfigdeclared only the listener-sidetlsProvider/jsseProvider, and it ignores unknown YAML — so an operator pinningBCJSSEfor FIPS got silence rather than an error. AddsbrokerClientSslProviderandbrokerClientJsseProvider, wires both into the client and admin builders, and inherits them (plus the two web-listener keys) for an embedded worker.And six more: a failed post-connect auth challenge left the channel open (failing an already-completed
connectionFutureis a no-op on aREFRESH, so no response was sent and the client never reconnected); a failed TLS probe leaked the initialized factory with its metrics registration, file watcher and refresh task;TlsHandle.get()could return a released native context afterdispose()on both handle kinds; disabling hostname verification produced none of the WARN the PIP promises for insecure settings; three javadocs still described the deletedV5ToV4AuthenticationAdapterdesign as current, one of them also claiming an unbound driver runs credential work inline on the caller thread; and two comments described code as it is not.One thing this PR deliberately does not do
The PIP specifies a two-axis provider model — a JSSE provider and a JCA provider — and the JCA axis has no configuration keys, so a server-side or v4-client operator cannot reach it.
The axis itself is implemented:
TlsPolicy.jcaProvider()is a real field with a builder setter,TlsMaterialSourceresolves it and threads it through every material load (keystores, PEM certificates, private keys, trust certs, and the auth-provided material source), andTlsContextsresolves both axes together on the JDK and Netty trust paths. A v5-builder caller can pin it today withTlsPolicy.builder().jsseProvider("BCJSSE").jcaProvider("BCFIPS").What is missing is only the configuration surface — no
jcaProvider/brokerClientJcaProviderkey onServiceConfiguration,ProxyConfiguration,WebSocketProxyConfiguration,WorkerConfigorClientConfigurationData. Adding those keys is the same shape as thejsseProviderwiring fixed above, at the same call sites; the alternative is to document the axis as v5-builder-only for 5.0. Either way it is a deliberate choice rather than a cleanup, sopip-478.mdis left untouched on this point — including the sentence promising the worker propagates "three" provider keys, of which two now exist.Verifying this change
This change added tests and can be verified as follows:
ClientAuthenticationFailsLoudlyTest— the two re-homed invariants: a plugin that cannot serve the binary transport fails the client build, and serializing a configuration carrying a v5 plugin is refused. Includes the case that must keep working: a configuration usingauthPluginClassName+authParamsstill serializes.PulsarClientBuilderV5Test.testTlsFactoryDoesNotEnableTransportTls/testTlsFactoryLeavesAnSslUrlEnabled— adopting a factory does not switch the transport to TLS, andpulsar+ssl://still does. Mutation-verified: restoring thesetUseTls(true)fails exactly this test and nothing else.FileBasedTlsFactoryTest.getAfterDisposeThrowsRatherThanServingAReleasedContext— both handle kinds refuseget()afterdispose(), anddispose()stays idempotent. Mutation-verified the same way.WorkerConfigProviderInheritanceTest— an embedded worker inherits the broker's four provider pins, and inherits nothing when the broker leaves them unset.The remainder is covered by existing tests, which the moved type's rename runs through:
:pulsar-client-original:test,:pulsar-client-v5:test,:pulsar-common:test,:pulsar-broker-common:test,quickCheckandsanityCheck(all modules, main and test sources, forced rerun) pass locally.Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
The public API:
AsyncAuthenticationDriveris removed frompulsar-client-api(moved topulsar-clientasBinaryAuthenticationDriver), and the now-empty packageorg.apache.pulsar.client.api.internalis deleted. Both are new in this release cycle and were internal by contract; no released API is affected.The threading model: the client's blocking auth executor queues rather than rejects when saturated, and
AuthenticationInitContext#blockingExecutor()returns the shared pool instead of null when no client bound services — so a plugin's blocking work has somewhere to go on every path.Deployment: three changes an operator can observe.
jsseProviderandbrokerClientJsseProviderinbroker.conf/standalone.conftake effect for the first time. A deployment that set either key and silently got the platform default now gets the provider it asked for — the documented intent of the key, but a behaviour change on upgrade for anyone who set it and adapted to it being ignored.functions_worker.ymlgainsbrokerClientSslProviderandbrokerClientJsseProvider, and an embedded worker now inherits the broker's provider pins instead of silently running on the default.tlsFactory(...)on apulsar://URL now stays plaintext instead of attempting TLS. Anyone who relied on the old behaviour to enable TLS should use apulsar+ssl://service URL, which is how the transport has always been selected.Documentation
doc-requireddoc-not-neededdocdoc-completeThe operator-facing consequences are documented in
pip-478.md, which this PR reconciles with the implementation.Matching PR in forked repository
PR in forked repository: lhotari#255
Prepared with the assistance of Claude Code (Opus 5).