michaeljmarshall opened a new pull request, #20067: URL: https://github.com/apache/pulsar/pull/20067
Fixes: https://github.com/apache/pulsar/issues/10816 PIP: #19771 Supersedes: https://github.com/apache/pulsar/pull/19026 Depends on: https://github.com/apache/pulsar/pull/20062 ### Motivation The Pulsar Proxy does not properly handle authentication data refresh when in state `ProxyLookupRequests`. The consequence is described in https://github.com/apache/pulsar/issues/10816. Essentially, the problem is that the proxy caches stale authentication data and sends it to the broker leading to connection failures. https://github.com/apache/pulsar/pull/17831 attempted to fix the underlying problem, but it missed an important edge cases. Specifically, it missed the case that the `ConnectionPool` will have multiple connections when a lookup gets redirected. As such, the following problem exists (and is fixed by this PR): 1. Client opens connection to perform lookups. 2. Proxy connects to broker 1 to get the topic ownership info. 3. Time passes. 4. Client does an additional lookup, and this topic is on a newly created broker 2. In this case, the proxy opens a new connection with the stale client auth data. 5. Broker 2 rejects the connection because it fails with expired authentication. ### Modifications * Remove some of the implementation from https://github.com/apache/pulsar/pull/17831. This new implementation still allows a broker to challenge the client through the proxy, but notably, it limits the number of challenges sent to the client. Further, the proxy does not challenge the client when the auth data is not expired. * Introduce authentication refresh in the proxy so that the proxy challenges the client any time the auth data is expired. * Update the `ProxyClientCnx` to get the `clientAuthData` from the `ProxyConnection` to ensure that it gets new authentication data. ### Verifying this change The `ProxyRefreshAuthTest` covers the existing behavior. I don't yet have a test to cover the redirect scenario. It will be much easier to test once we implement #19624. ### Documentation - [x] `doc-not-needed` ### Matching PR in forked repository PR in forked repository: the relevant tests pass locally, so I am going to skip the forked tests. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
