Flight-SQL JDBC driver ignores JVM proxy settings

[scruz-denodo]

Describe the enhancement requested

Hi, I was making some tests for checking the support of proxies with the Flight-SQL JDBC driver.

GRPC supports the JVM properties for indicating the default http proxy to use. Nevertheless, I tested with Flight-SQL JDBC driver without luck. The driver is ignoring them.
I mean these ones:
-Dhttps.proxyHost=<host> -Dhttps.proxyPort=<port>

Checking the reason, I arrived to these lines where the Netty channel is opened:

arrow-java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightClient.java

Lines 854 to 862 in bd2173c

	{
	builder = NettyChannelBuilder.forAddress(location.toSocketAddress());
	break;
	}
	case LocationSchemes.GRPC_DOMAIN_SOCKET:
	{
	// The implementation is platform-specific, so we have to find the classes at runtime
	builder = NettyChannelBuilder.forAddress(location.toSocketAddress());
	try {

Using method io.grpc.netty.NettyChannelBuilder#forAddress(java.net.SocketAddress) makes GRPC to create the channel using a direct connection to the host:port specified at connection uri, ignoring the proxy.
This happens on this code from ManagedChannelImplBuilder

  public ManagedChannelImplBuilder(SocketAddress directServerAddress, String authority,
      @Nullable ChannelCredentials channelCreds, @Nullable CallCredentials callCreds,
      ClientTransportFactoryBuilder clientTransportFactoryBuilder,
      @Nullable ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider) {
    this.target = makeTargetStringForDirectAddress(directServerAddress);
    this.channelCredentials = channelCreds;
    this.callCredentials = callCreds;

I am not sure if there is a special reason for not opening the connection with the host and port values, instead of the java.net.SocketAddress:

builder = NettyChannelBuilder.forAddress(
                    location.getUri().getHost(), location.getUri().getPort());

But opening the channel in the previous way makes GRPC to follow the specified proxy at JVM properties.
-Dhttps.proxyHost=<host> -Dhttps.proxyPort=<port>

Could this be a valid enhancement request?

[jbonofre]

General comment: as JDBC driver is shading and relocating the dependency (netty and gRPC especially), the properties name can be different (org.apache.arrow.io.grpc... instead of io.grpc... for instance).
Specifically for the HTTP proxy, I think a different "issue" in the way we create the gRPC/Netty channel. I will fix that.

[scruz-denodo]

Hi, I saw that also and I though the same.

Nevertheless, I did the test of modifying how the channel is created with the suggested code, generated a driver with the change and validated the proxy works.

builder = NettyChannelBuilder.forAddress(
                    location.getUri().getHost(), location.getUri().getPort());

So, maybe only with this change is enough.

[jbonofre]

Actually, I don't think it's a bug because:

1. location.toSocketAddress is actually using the URI schema.
2. If the schema is GRPC (e.g. grpc), GRPC_TLS (e.g. grpc+tls), GRPC_INSECURE (e.g. grpc+tcp), then uri host and port are used (and so the HTTP proxy properties can be used)
3. If the schema is GRPC_DOMAIN_SOCKET (e.g. grpc+unix), we are not using TCP but io.netty.channel.unix.DomainSocketAddress so only the URI path is considered (not host and port)

What's the JDBC URL used ?

[scruz-denodo]

Hi, I used the following for the test.
jdbc:arrow-flight-sql://localhost:3128?&useEncryption=0

I agree that proxy should apply to case 2, when the connection use http. I did not test case 3, but I think it should not be a reason for breaking case 2 ignoring JVM http proxy properties.

[jbonofre]

@scruz-denodo thanks. I suspect either URL parsing issue or env. Let me write a quick test case to try to reproduce.

[nicornk]

hi folks, was there any progress on proxy support for the Arrow Flight SQL JDBC Driver?

[ennuite]

@nicornk Hello, I will look into this one in the coming days.

[ennuite]

@nicornk I checked out the relevant part of this codebase. There was a refactor that moved the NettyChannelBuilder call to NettyClientBuilder, see https://github.com/apache/arrow-java/pull/660/changes, but the lines weren't changed.

I also confirmed that grpc-java itself recommends against using the current API for HTTP/TCP sockets, see https://github.com/grpc/grpc-java/blob/73abb48547df4cbaadddfa5bd6a82ef387fac800/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java#L125 This is not a URL parsing error, it's an incorrect usage of the gRPC NettyChannelBuilder.

I'm working on a PR to fix this. The fix itself is trivial: I'm writing unit tests for it, which is a bit more involved.

@jbonofre you're the current assignee. Is it ok if I take this one?