Sorry for the late response. Why do we need to make the broker aware of the proxy when, by normal software design, we should avoid coupling the concepts in the proxy and the broker? The previous authentication was for historical reasons, but we should not continue to introduce this coupling.
The proxy should be a separate component. Instead of continuing to couple the relevant proxy concepts into the broker, everyone should be a client to the broker. Best, Mattison On Feb 25, 2023, 01:12 +0800, Michael Marshall <mmarsh...@apache.org>, wrote: > Great suggestions, Enrico. > > > It would be interesting to reject connections on the Proxy if the > > client tries to set that field. > > I support making the proxy reject invalid input. We could also have > the client reject connections where the client connect command > includes `original_principal`, `original_auth_data`, or > `original_auth_method`, since those are also only meant to be sent by > the proxy. > > > On the broker there is no way to distinguish a proxy from a client, that's > > fair. > > We can reject these connections when authentication and authorization > are enabled. My draft PR includes such logic. > > Thanks, > Michael > > On Fri, Feb 24, 2023 at 7:29 AM Enrico Olivelli <eolive...@gmail.com> wrote: > > > > Makes sense. > > > > It would be interesting to reject connections on the Proxy if the > > client tries to set that field. > > On the broker there is no way to distinguish a proxy from a client, that's > > fair. > > But on the proxy it is not expected to see a connection from another proxy. > > > > +1 > > > > Enrico > > > > Il giorno ven 24 feb 2023 alle ore 10:00 Zike Yang <z...@apache.org> ha > > scritto: > > > > > > > > Hi, Michael > > > > > > > > Thanks for initiating this PIP. > > > > > > > > +1 > > > > > > > > BR, > > > > Zike Yang > > > > > > > > > > > > Zike Yang > > > > > > > > On Fri, Feb 24, 2023 at 12:16 PM Michael Marshall > > > > <mmarsh...@apache.org> wrote: > > > > > > > > > > > > Hi Pulsar Community, > > > > > > > > > > > > In talking with Zike Yang on > > > > > > https://github.com/apache/pulsar/pull/19540, we identified that it > > > > > > would be helpful for the proxy to forward its own version when > > > > > > connecting to the broker. Here is a related PIP to improve the > > > > > > connection information available to operators. > > > > > > > > > > > > Issue: https://github.com/apache/pulsar/issues/19623 > > > > > > Implementation: https://github.com/apache/pulsar/pull/19618 > > > > > > > > > > > > I look forward to your feedback! > > > > > > > > > > > > Thanks, > > > > > > Michael > > > > > > > > > > > > Text of PIP copied below: > > > > > > > > > > > > ### Motivation > > > > > > > > > > > > When clients connect through the proxy, it is valuable to know which > > > > > > version of the proxy connected to the broker. That information isn't > > > > > > currently logged or reported in any easily identifiable way. The > > > > > > only > > > > > > way to get information about the connection is to infer which proxy > > > > > > forwarded a connection based on matching up the IP address in the > > > > > > logs. > > > > > > > > > > > > An additional change proposed in the implementation is to log this > > > > > > new > > > > > > information along with the `clientVersion`, `clientProtocolVersion`, > > > > > > and relevant authentication role information. This information will > > > > > > improve debug-ability and could also serve as a form of audit > > > > > > logging. > > > > > > > > > > > > ### Goal > > > > > > > > > > > > Improve the value of the broker's logs and metrics about connections > > > > > > to simplify debugging and to make it easier for Pulsar operators to > > > > > > understand how clients are connecting to their clusters. > > > > > > > > > > > > ### API Changes > > > > > > > > > > > > Add the following: > > > > > > > > > > > > ```proto > > > > > > message CommandConnect { > > > > > > // Other fields omitted > > > > > > optional string proxy_version = 11; // Version of the proxy. > > > > > > Should only be forwarded by a proxy. > > > > > > } > > > > > > ``` > > > > > > > > > > > > ### Implementation > > > > > > > > > > > > Initial implementation: https://github.com/apache/pulsar/pull/19618 > > > > > > > > > > > > ### Alternatives > > > > > > > > > > > > The `CommandAuthResponse` has a `client_version` field. It's > > > > > > possible > > > > > > that someone would want this `proxy_version` field on that message. > > > > > > However, I think we should not continue down the path of duplicating > > > > > > fields in the connection handshake protocol. > > > > > > > > > > > > ### Anything else? > > > > > > > > > > > > Future work could include exposing this `proxyVersion` and > > > > > > `clientVersion` information via Prometheus metrics.
