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.
