apache / arrow

Apache Arrow is a multi-language toolbox for accelerated data interchange and in-memory processing
https://arrow.apache.org/
Apache License 2.0
13.9k stars 3.38k forks source link

[FlightRPC][Java] Provide standard way to get client IP address #31924

Open asfimport opened 2 years ago

asfimport commented 2 years ago

Obtaining the IP address of the client can be accomplished via gRPC, but the gRPC interface exposing this information is considered unstable.  It would be good to expose this through a standard interface in Flight, both to buffer any future changes/instability in gRPC, but also in support of alternative transports.

Reporter: Todd Farmer / @toddfarmer

Note: This issue was originally created as ARROW-16565. Please see the migration documentation for further details.

asfimport commented 2 years ago

David Li / @lidavidm: Marking as Java since C++/Python support this.

aiguofer commented 1 year ago

It seems like this could be helpful in various situations. Although the gRPC interface for it is considered unstable, it's been around for quite a while and would likely not break unexpectedly (there would be a deprecation and migration path).

An easy way to expose this could be creating some sort of RequestInfo object that gets added to the gRPC Context and later passed into the Flight CallContext so it's easily accessible in request handlers. What other types of data are exposed in the C++/Python interface that we might want to add?

scruz-denodo commented 1 week ago

Hi, I have done some changes about this issue. Before creating a pull request I would like to discuss them here.

This is the change: https://github.com/apache/arrow/compare/main...scruz-denodo:arrow:issue/31924

About the experimetal status from the io.grpc.ServerCall#getAttributes method. There is some recent movement here. https://github.com/grpc/grpc-java/issues/1779

I think it is also useful if the following method receives the new org.apache.arrow.flight.RequestInfo object. org.apache.arrow.flight.auth2.CallHeaderAuthenticator#authenticate

The information about the client ip could be interesting for deciding about the authentication.

Updated: The movement on the grpc issue was before the comment of @aiguofer

aiguofer commented 1 week ago

This is great! I would love to have something like this. Another nice thing to add to the RequestInfo would be the authority.. we've been doing this https://github.com/apache/arrow/issues/38911#issuecomment-1868017220 but that interceptor happens after authenticate is called.

Side note: I would also love to have the RequestContext passed to authenticate since it's likely users might want to inject values that are looked up during auth into the request context.

scruz-denodo commented 6 days ago

I've just added changes for having the RequestInfo available at authentication process with org.apache.arrow.flight.auth2.CallHeaderAuthenticator

Maybe I add tests and create a PR, for checking if they are valid changes for contributing.